Account for perspective and preserve-3d in mapToVisualRectInAncestorSpace

Account for perspective and preserve-3d in mapToVisualRectInAncestorSpace.

BUG=590095
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2727093002
Cr-Commit-Position: refs/heads/master@{#454522}
Committed: https://chromium.googlesource.com/chromium/src/+/3a2c4663e5be244f263af1a7824bae449a927524

[chrishtr, 2017-03-02 01:04:44]

Description was changed from

==========
none


none


none


none


none

BUG=
==========

to

==========
none


none


none


none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-03-02 18:25:43]

Description was changed from

==========
none


none


none


none


none

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Account for perspective and preserve-3d in mapToVisualRectInAncestorSpace.

BUG=590095
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-03-02 23:01:25]

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

[chrishtr, 2017-03-02 23:01:32]

chrishtr@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-03-02 23:02:13]

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

[chrishtr, 2017-03-02 23:24:09]

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

[commit-bot: I haz the power, 2017-03-02 23:24:44]

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-03 01:08:09]

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

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

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

[chrishtr, 2017-03-03 01:14:45]

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

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

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

[Xianzhu, 2017-03-03 01:24:42]

lgtm with some minor comments.

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:2421:
(style()->preserves3D() && !isText()));
I think !container->isText() and !isText() are both always true so can be
omitted.

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:2439: // Use
enclosingIntRect because we cannot properly compute pixel snapping
enclosingIntRect -> enclosingBoundingBox().

Not a problem of this CL, but the name of FloatQuad::enclosingBoundingBox()
doesn't look good because the enclosingIntRect() semantics is not obvious from
the name. This can be addressed with a follow-up.

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutInline.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutInline.cpp:1210:
(style()->preserves3D() && !isText());
Ditto about !isText().

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutInline.cpp:1235: LayoutRect
rect(transformState.lastPlanarQuad().boundingBox());
Do we need enclosingBoundingBox here?

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1312: bool preserve3D =
(parent->style()->preserves3D() && !parent->isText()) ||
Ditto about !parent->isText().

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:43: return
LayoutRect(transformState.lastPlanarQuad().boundingBox());
Can you wrap the above code into LayoutView::mapToVisualRectInAncestorSpace()?

[chrishtr, 2017-03-03 02:45:14]

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:2421:
(style()->preserves3D() && !isText()));
On 2017/03/03 at 01:24:42, Xianzhu wrote:
> I think !container->isText() and !isText() are both always true so can be
omitted.

Done.

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:2439: // Use
enclosingIntRect because we cannot properly compute pixel snapping
On 2017/03/03 at 01:24:42, Xianzhu wrote:
> enclosingIntRect -> enclosingBoundingBox().
> 
> Not a problem of this CL, but the name of FloatQuad::enclosingBoundingBox()
doesn't look good because the enclosingIntRect() semantics is not obvious from
the name. This can be addressed with a follow-up.

Will fix in a followup. Changed comment.

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutInline.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutInline.cpp:1210:
(style()->preserves3D() && !isText());
On 2017/03/03 at 01:24:42, Xianzhu wrote:
> Ditto about !isText().

Done.

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1312: bool preserve3D =
(parent->style()->preserves3D() && !parent->isText()) ||
On 2017/03/03 at 01:24:42, Xianzhu wrote:
> Ditto about !parent->isText().

I copied this conditional from:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...

This is meant to cover inlines. Added here:

https://codereview.chromium.org/1516003003

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right):

https://codereview.chromium.org/2727093002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:43: return
LayoutRect(transformState.lastPlanarQuad().boundingBox());
On 2017/03/03 at 01:24:42, Xianzhu wrote:
> Can you wrap the above code into LayoutView::mapToVisualRectInAncestorSpace()?

Done.

[chrishtr, 2017-03-03 02:46:29]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-03 02:46:30]

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

[commit-bot: I haz the power, 2017-03-03 02:47:03]

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

[chrishtr, 2017-03-03 02:49:14]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-03 02:49:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2727093002/#ps160001
(title: "none")

[commit-bot: I haz the power, 2017-03-03 02:49:46]

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

[chrishtr, 2017-03-03 04:14:06]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-03-03 04:14:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2727093002/#ps180001
(title: "none")

[commit-bot: I haz the power, 2017-03-03 04:14:18]

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-03 05:51:51]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1488514446959620,
"parent_rev": "1e2e898c6c8c2e603085654a6256dc581c74c67c", "commit_rev":
"3a2c4663e5be244f263af1a7824bae449a927524"}

[commit-bot: I haz the power, 2017-03-03 05:52:36]

Description was changed from

==========
Account for perspective and preserve-3d in mapToVisualRectInAncestorSpace.

BUG=590095
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Account for perspective and preserve-3d in mapToVisualRectInAncestorSpace.

BUG=590095
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2727093002
Cr-Commit-Position: refs/heads/master@{#454522}
Committed:
https://chromium.googlesource.com/chromium/src/+/3a2c4663e5be244f263af1a7824b...
==========

[commit-bot: I haz the power, 2017-03-03 05:52:37]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/3a2c4663e5be244f263af1a7824b...