Replace inverse transform with Cross-product computation.

Replace inverse transform with Cross-product computation.

This changes how we compute the 3d normals for quads moved into a
3d context.

The inverse transform approach seems to suffer numeric
instability around small Y rotations that invert the normal, causing
artifacts during rotation animations.

BUG=565758
TEST=observe 565758#2 html, and see no flicker at the start of
     the twist.
R=vollick@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5240189cba88ed90ef654310c064d588c022f837
Cr-Commit-Position: refs/heads/master@{#364367}

[Peter Mayo, 2015-12-04 04:22:52]

Description was changed from

==========
Replace inverse transform with Cross-product computation.

This changes how we compute the 3d normals for quads moved into a
3d context.

The inverse transform approach seems to suffer numeric
instability around small Y rotations that invert the normal, causing
artifacts during rotation animations.

BUG=565758
TEST=observe 565758#2 html, and see no flicker at the start of
     the twist.
R=vollick@chromium.org
==========

to

==========
Replace inverse transform with Cross-product computation.

This changes how we compute the 3d normals for quads moved into a
3d context.

The inverse transform approach seems to suffer numeric
instability around small Y rotations that invert the normal, causing
artifacts during rotation animations.

BUG=565758
TEST=observe 565758#2 html, and see no flicker at the start of
     the twist.
R=vollick@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Peter Mayo, 2015-12-04 04:33:56]

PTAL

Still not enough to fix Marcofolio's 3d example, but it fixes the related repro
case.

https://codereview.chromium.org/1497153002/diff/1/cc/quads/draw_polygon_unitt...
File cc/quads/draw_polygon_unittest.cc (right):

https://codereview.chromium.org/1497153002/diff/1/cc/quads/draw_polygon_unitt...
cc/quads/draw_polygon_unittest.cc:94: EXPECT_NORMAL(polygon, 0.0, 0.0, 1.0);
Perhaps one that isn't axis-aligned?

[Peter Mayo, 2015-12-04 05:19:03]

Fix expectations of test that transforms a pointless polygon.

[Peter Mayo, 2015-12-04 05:21:34]

https://codereview.chromium.org/1497153002/diff/1/cc/quads/draw_polygon_unitt...
File cc/quads/draw_polygon_unittest.cc (right):

https://codereview.chromium.org/1497153002/diff/1/cc/quads/draw_polygon_unitt...
cc/quads/draw_polygon_unittest.cc:94: EXPECT_NORMAL(polygon, 0.0, 0.0, 1.0);
On 2015/12/04 04:33:56, Peter Mayo wrote:
> Perhaps one that isn't axis-aligned?

Added in the transform test.

[Peter Mayo, 2015-12-04 16:40:41]

https://codereview.chromium.org/1497153002/diff/40001/cc/quads/draw_polygon.cc
File cc/quads/draw_polygon.cc (right):

https://codereview.chromium.org/1497153002/diff/40001/cc/quads/draw_polygon.c...
cc/quads/draw_polygon.cc:101: normal_.set_z(0.0f);
What about saving the old normal and adding a fallback of inverting the
transformation?

[Peter Mayo, 2015-12-04 16:43:52]

rebase + more windows

[Ian Vollick, 2015-12-05 01:09:58]

lgtm with a few nits (and thanks for the nice tests).

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

https://codereview.chromium.org/1497153002/diff/60001/cc/quads/draw_polygon_u...
cc/quads/draw_polygon_unittest.cc:60: vertices.push_back(gfx::Point3F(10.0f,
10.0f, 0.0f));
looks like |vertices| aren't used here?

https://codereview.chromium.org/1497153002/diff/60001/cc/quads/draw_polygon_u...
cc/quads/draw_polygon_unittest.cc:65: }
nit: would be nice to check that putting the vertices in reverse order will flip
the normal, too.

[Peter Mayo, 2015-12-07 15:02:36]

Remove dead code

[Peter Mayo, 2015-12-07 15:45:21]

Add inverse normal test early

[Peter Mayo, 2015-12-07 22:27:23]

Rebased

[Peter Mayo, 2015-12-07 22:28:03]

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

https://codereview.chromium.org/1497153002/diff/60001/cc/quads/draw_polygon_u...
cc/quads/draw_polygon_unittest.cc:60: vertices.push_back(gfx::Point3F(10.0f,
10.0f, 0.0f));
On 2015/12/05 01:09:58, vollick wrote:
> looks like |vertices| aren't used here?

Correct, this mimics the direct renderer use case.
Removed.

https://codereview.chromium.org/1497153002/diff/60001/cc/quads/draw_polygon_u...
cc/quads/draw_polygon_unittest.cc:65: }
On 2015/12/05 01:09:58, vollick wrote:
> nit: would be nice to check that putting the vertices in reverse order will
flip
> the normal, too.

Done.

[Peter Mayo, 2015-12-07 22:28:22]

The CQ bit was checked by petermayo@chromium.org

[Peter Mayo, 2015-12-07 22:28:22]

The patchset sent to the CQ was uploaded after l-g-t-m from vollick@chromium.org
Link to the patchset: https://codereview.chromium.org/1497153002/#ps120001
(title: "Rebased")

[commit-bot: I haz the power, 2015-12-07 22:29:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1497153002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1497153002/120001

[commit-bot: I haz the power, 2015-12-08 00:16:18]

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

[commit-bot: I haz the power, 2015-12-08 00:16:18]

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

[Peter Mayo, 2015-12-08 00:50:58]

Another Windows fix

[Peter Mayo, 2015-12-08 01:32:38]

Another Windows fix

[Peter Mayo, 2015-12-08 03:30:32]

The CQ bit was checked by petermayo@chromium.org

[Peter Mayo, 2015-12-08 03:30:32]

The patchset sent to the CQ was uploaded after l-g-t-m from vollick@chromium.org
Link to the patchset: https://codereview.chromium.org/1497153002/#ps160001
(title: "Another Windows fix")

[commit-bot: I haz the power, 2015-12-08 03:33:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1497153002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1497153002/160001

[commit-bot: I haz the power, 2015-12-08 05:33:49]

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

[commit-bot: I haz the power, 2015-12-08 05:33:50]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[Peter Mayo, 2015-12-09 15:22:14]

Rebased

[Peter Mayo, 2015-12-09 15:22:39]

PTAL at test changes.

https://codereview.chromium.org/1497153002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/1497153002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/TestExpectations:1359: crbug.com/565758
transforms/3d/point-mapping/3d-point-mapping-preserve-3d.html [ NeedsRebaseline
]
This rebaseline will include the undesired artifact of 567940.

[Peter Mayo, 2015-12-09 20:45:53]

Moved section and rebased again

[Peter Mayo, 2015-12-09 20:46:04]

The CQ bit was checked by petermayo@chromium.org

[Peter Mayo, 2015-12-09 20:46:05]

The patchset sent to the CQ was uploaded after l-g-t-m from vollick@chromium.org
Link to the patchset: https://codereview.chromium.org/1497153002/#ps220001
(title: "Moved section and rebased again")

[commit-bot: I haz the power, 2015-12-09 20:48:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1497153002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1497153002/220001

[commit-bot: I haz the power, 2015-12-10 00:38:53]

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

[commit-bot: I haz the power, 2015-12-10 00:38:54]

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

[Peter Mayo, 2015-12-10 15:35:39]

The CQ bit was checked by petermayo@chromium.org

[commit-bot: I haz the power, 2015-12-10 15:37:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1497153002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1497153002/220001

[commit-bot: I haz the power, 2015-12-10 15:42:15]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-12-10 15:43:05]

Description was changed from

==========
Replace inverse transform with Cross-product computation.

This changes how we compute the 3d normals for quads moved into a
3d context.

The inverse transform approach seems to suffer numeric
instability around small Y rotations that invert the normal, causing
artifacts during rotation animations.

BUG=565758
TEST=observe 565758#2 html, and see no flicker at the start of
     the twist.
R=vollick@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Replace inverse transform with Cross-product computation.

This changes how we compute the 3d normals for quads moved into a
3d context.

The inverse transform approach seems to suffer numeric
instability around small Y rotations that invert the normal, causing
artifacts during rotation animations.

BUG=565758
TEST=observe 565758#2 html, and see no flicker at the start of
     the twist.
R=vollick@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5240189cba88ed90ef654310c064d588c022f837
Cr-Commit-Position: refs/heads/master@{#364367}
==========

[commit-bot: I haz the power, 2015-12-10 15:43:06]

Patchset 12 (id:??) landed as
https://crrev.com/5240189cba88ed90ef654310c064d588c022f837
Cr-Commit-Position: refs/heads/master@{#364367}