Correct polygon splitting in an almost-coplanar case.

Correct polygon splitting in an almost-coplanar case.

In the case described in the associated bug, the signs of the vertices
of the quad to be split are [ 0 0 + - ], which means that floating
point rounding has resulted in a polygon that is not flat. Originally
one of the two points that lies on the splitting plane was not being
included in either output polygon, which caused the result to be
incorrect.

This corrects the case when two consecutive points are on the splitting
plane.

BUG=693826
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2714243002
Cr-Commit-Position: refs/heads/master@{#453888}
Committed: https://chromium.googlesource.com/chromium/src/+/02a3371e8aa5f5ec354509e3625371cbf6f81707

[Tobias Sargeant, 2017-02-24 20:59:55]

Description was changed from

==========
Correct polygon splitting in an almost-coplanar case.

In the case described in the associated bug, the signs of the vertices
of the quad to be split are [ 0 0 + - ], which means that floating
point rounding has resulted in a polygon that is not flat. Originally
one of the two points that lies on the splitting plane was not being
included in either output polygon, which caused the result to be
incorrect.

This corrects the case when two consecutive points are on the splitting
plane.

BUG=693826
==========

to

==========
Correct polygon splitting in an almost-coplanar case.

In the case described in the associated bug, the signs of the vertices
of the quad to be split are [ 0 0 + - ], which means that floating
point rounding has resulted in a polygon that is not flat. Originally
one of the two points that lies on the splitting plane was not being
included in either output polygon, which caused the result to be
incorrect.

This corrects the case when two consecutive points are on the splitting
plane.

BUG=693826
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Tobias Sargeant, 2017-02-24 21:04:06]

tobiasjs@chromium.org changed reviewers:
+ enne@chromium.org

[Tobias Sargeant, 2017-02-24 21:04:07]

PTAL. In the linked bug I found a small repro for this case. Should it be added
as a pixel test?

[enne (OOO), 2017-02-24 21:12:50]

On 2017/02/24 at 21:04:07, tobiasjs wrote:
> PTAL. In the linked bug I found a small repro for this case. Should it be
added as a pixel test?

Could it be a unit test?

[Tobias Sargeant, 2017-02-27 16:44:21]

On 2017/02/24 21:12:50, enne wrote:
> On 2017/02/24 at 21:04:07, tobiasjs wrote:
> > PTAL. In the linked bug I found a small repro for this case. Should it be
> added as a pixel test?
> 
> Could it be a unit test?

Yes. I've added one.

[enne (OOO), 2017-02-27 17:35:33]

lgtm

https://codereview.chromium.org/2714243002/diff/20001/cc/quads/draw_polygon_u...
File cc/quads/draw_polygon_unittest.cc (right):

https://codereview.chromium.org/2714243002/diff/20001/cc/quads/draw_polygon_u...
cc/quads/draw_polygon_unittest.cc:494: // polygon to become twisted. Splitting
should still give a valid
I wish more unit tests explained themselves this well.  :)

[Tobias Sargeant, 2017-02-28 12:26:39]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2017-02-28 12:26:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 13:02:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 13:02:15]

Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Tobias Sargeant, 2017-02-28 16:26:10]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 16:26:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tobias Sargeant, 2017-02-28 16:38:13]

On 2017/02/28 13:02:15, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Two problems caused failures.

* I didn't change the normal when I cut and pasted the new data from an existing
test.
* An existing test started failing. It was asserting that cutting a pentagon
produced two triangles, so I'm pretty sure that it was actually testing the
presence of a bug in the original splitting code. I've updated it to match the
output post my change, but I thought I should flag it. I added the "all input
vertices should appear in output polygons" assertion to the DoubleSplit test,
and it did indeed fail prior to this change.

[enne (OOO), 2017-02-28 17:37:53]

lgtm 2

Thanks for the explanation.  :)

[Tobias Sargeant, 2017-02-28 18:43:00]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2017-02-28 18:43:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 18:53:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 18:53:31]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Tobias Sargeant, 2017-02-28 19:01:24]

Patchset #5 (id:80001) has been deleted

[Tobias Sargeant, 2017-02-28 19:01:34]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2017-02-28 19:01:34]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2714243002/#ps100001
(title: "Add additional check to DoubleSplit unittest")

[commit-bot: I haz the power, 2017-02-28 19:02:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 19:43:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 19:43:52]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Tobias Sargeant, 2017-02-28 21:58:08]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 21:59:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 22:28:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 22:28:35]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Tobias Sargeant, 2017-03-01 07:51:53]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 07:52:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 08:26:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-01 08:26:16]

Dry run: This issue passed the CQ dry run.

[Tobias Sargeant, 2017-03-01 08:55:55]

The CQ bit was checked by tobiasjs@chromium.org

[Tobias Sargeant, 2017-03-01 08:55:55]

The patchset sent to the CQ was uploaded after l-g-t-m from enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2714243002/#ps120001
(title: "Make constants float, for Windows.")

[commit-bot: I haz the power, 2017-03-01 08:56:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 09:01:41]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1488358555433570,
"parent_rev": "da0deb9b9f7e997f597b2d7c8dd1f3c38f4ebbea", "commit_rev":
"02a3371e8aa5f5ec354509e3625371cbf6f81707"}

[commit-bot: I haz the power, 2017-03-01 09:02:44]

Description was changed from

==========
Correct polygon splitting in an almost-coplanar case.

In the case described in the associated bug, the signs of the vertices
of the quad to be split are [ 0 0 + - ], which means that floating
point rounding has resulted in a polygon that is not flat. Originally
one of the two points that lies on the splitting plane was not being
included in either output polygon, which caused the result to be
incorrect.

This corrects the case when two consecutive points are on the splitting
plane.

BUG=693826
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Correct polygon splitting in an almost-coplanar case.

In the case described in the associated bug, the signs of the vertices
of the quad to be split are [ 0 0 + - ], which means that floating
point rounding has resulted in a polygon that is not flat. Originally
one of the two points that lies on the splitting plane was not being
included in either output polygon, which caused the result to be
incorrect.

This corrects the case when two consecutive points are on the splitting
plane.

BUG=693826
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2714243002
Cr-Commit-Position: refs/heads/master@{#453888}
Committed:
https://chromium.googlesource.com/chromium/src/+/02a3371e8aa5f5ec354509e36253...
==========

[commit-bot: I haz the power, 2017-03-01 09:02:45]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/02a3371e8aa5f5ec354509e36253...