Reland of Don't pass subpixel offsets through non-translation transforms

Reland of Don't pass subpixel offsets through non-translation transforms (patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms (patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed: https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de764076cf60394
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed: https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4aa52638ec6e4

# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=717882
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2862053002
Cr-Commit-Position: refs/heads/master@{#469772}
Committed: https://chromium.googlesource.com/chromium/src/+/34718d4879fbba5182af5611438da97f17058142

[Xianzhu, 2017-05-04 18:33:38]

Created Reland of Don't pass subpixel offsets through non-translation transforms

[chrishtr, 2017-05-04 18:39:02]

https://codereview.chromium.org/2862053002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right):

https://codereview.chromium.org/2862053002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:847: delta +=
painting_info.sub_pixel_accumulation;
Perhaps the bug here that caused issue 716163 is that
we failed to pixel-snap this post-translate. It does seem
incorrect to anchor the transform at a non-pixel-snapped
position.
I think that should fix it, could you confirm?

[Xianzhu, 2017-05-04 23:32:00]

Description was changed from

==========
Reland of Don't pass subpixel offsets through non-translation transforms
(patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms
(patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> >
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed:
https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de7...
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed:
https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4...

TBR=chrishtr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=717882
==========

to

==========
Reland of Don't pass subpixel offsets through non-translation transforms
(patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms
(patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> >
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed:
https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de7...
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed:
https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4...

TBR=chrishtr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=717882
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-05-04 23:32:10]

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

[commit-bot: I haz the power, 2017-05-04 23:33:58]

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-05-04 23:34:15]

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

[commit-bot: I haz the power, 2017-05-04 23:34:16]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2017-05-04 23:57:27]

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

[commit-bot: I haz the power, 2017-05-04 23:57:57]

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-05-04 23:58:00]

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

[commit-bot: I haz the power, 2017-05-04 23:58:02]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2017-05-04 23:59:21]

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

[commit-bot: I haz the power, 2017-05-05 00:00:04]

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-05-05 00:00:17]

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

[commit-bot: I haz the power, 2017-05-05 00:00:17]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2017-05-05 00:01:20]

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

[Xianzhu, 2017-05-05 00:02:18]

Description was changed from

==========
Reland of Don't pass subpixel offsets through non-translation transforms
(patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms
(patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> >
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed:
https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de7...
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed:
https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4...

TBR=chrishtr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=717882
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Reland of Don't pass subpixel offsets through non-translation transforms
(patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms
(patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> >
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed:
https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de7...
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed:
https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4...

# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=717882
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2017-05-05 00:02:26]

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

[commit-bot: I haz the power, 2017-05-05 00:03:06]

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-05-05 01:24:20]

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

[commit-bot: I haz the power, 2017-05-05 01:24:21]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2017-05-05 06:14:39]

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

[commit-bot: I haz the power, 2017-05-05 06:14:56]

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

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

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

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2017-05-05 15:51:57]

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

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

Dry run: CQ is trying da patch.

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

[Xianzhu, 2017-05-05 15:54:09]

Now snap transform to pixels, and discard remainder for non-translate
transforms.

PTAL.

[chrishtr, 2017-05-05 16:04:44]

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1213: if
(!StyleRef().Preserves3D()) {
Why remove the SPv2 condition here?

[Xianzhu, 2017-05-05 16:14:51]

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1213: if
(!StyleRef().Preserves3D()) {
On 2017/05/05 16:04:44, chrishtr wrote:
> Why remove the SPv2 condition here?

MapToVisualRectInAncestorSpace() is still used on SPv2 for non-painting
purposes. Without EnclosingBoundingBox() the result will mismatch where we
paint.

For example,
  <div style="transform: scale(50)">
    <div style="position: relative; left: 0.7px; width: 100px; height:
100px"></div>
  </div>
we'll paint the inner div at (50,0 100x100) which is snapped to pixels in the
transformed space. Without EnclosingBoundingBox() here we'll get visual rect
(35,0 100x100) which mismatches where we paint. With EnclosingBoundingBox()
we'll get (0,0 200x100) which covers the painted area.

In some cases PaintPropertyTreeBulderTest also expects this behavior with
slop_factor.

[Xianzhu, 2017-05-05 16:16:03]

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1213: if
(!StyleRef().Preserves3D()) {
On 2017/05/05 16:14:51, Xianzhu wrote:
> On 2017/05/05 16:04:44, chrishtr wrote:
> > Why remove the SPv2 condition here?
> 
> MapToVisualRectInAncestorSpace() is still used on SPv2 for non-painting
> purposes. Without EnclosingBoundingBox() the result will mismatch where we
> paint.
> 
> For example,
>   <div style="transform: scale(50)">
>     <div style="position: relative; left: 0.7px; width: 100px; height:
> 100px"></div>
>   </div>
> we'll paint the inner div at (50,0 100x100) which is snapped to pixels in the
> transformed space. Without EnclosingBoundingBox() here we'll get visual rect
> (35,0 100x100) which mismatches where we paint. With EnclosingBoundingBox()
> we'll get (0,0 200x100) which covers the painted area.
> 
> In some cases PaintPropertyTreeBulderTest also expects this behavior with
> slop_factor.

Correction: with width and height of the inner div should be 2px instead of
100px.

[chrishtr, 2017-05-05 16:17:19]

lgtm

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/2862053002/diff/340001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1213: if
(!StyleRef().Preserves3D()) {
On 2017/05/05 at 16:14:51, Xianzhu wrote:
> On 2017/05/05 16:04:44, chrishtr wrote:
> > Why remove the SPv2 condition here?
> 
> MapToVisualRectInAncestorSpace() is still used on SPv2 for non-painting
purposes. Without EnclosingBoundingBox() the result will mismatch where we
paint.
> 
> For example,
>   <div style="transform: scale(50)">
>     <div style="position: relative; left: 0.7px; width: 100px; height:
100px"></div>
>   </div>
> we'll paint the inner div at (50,0 100x100) which is snapped to pixels in the
transformed space. Without EnclosingBoundingBox() here we'll get visual rect
(35,0 100x100) which mismatches where we paint. With EnclosingBoundingBox()
we'll get (0,0 200x100) which covers the painted area.
> 
> In some cases PaintPropertyTreeBulderTest also expects this behavior with
slop_factor.

Ah, right. Ok.

[commit-bot: I haz the power, 2017-05-05 17:24:33]

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

[commit-bot: I haz the power, 2017-05-05 17:24:34]

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

[Xianzhu, 2017-05-05 20:51:26]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2017-05-05 20:52:33]

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-05-05 21:29:39]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1494017486835350,
"parent_rev": "6779a04e21ebe7424ed2edae270f3303894104dc", "commit_rev":
"34718d4879fbba5182af5611438da97f17058142"}

[commit-bot: I haz the power, 2017-05-05 21:30:25]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1494017486835350,
"parent_rev": "6779a04e21ebe7424ed2edae270f3303894104dc", "commit_rev":
"34718d4879fbba5182af5611438da97f17058142"}

[commit-bot: I haz the power, 2017-05-05 21:30:28]

Description was changed from

==========
Reland of Don't pass subpixel offsets through non-translation transforms
(patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms
(patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> >
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed:
https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de7...
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed:
https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4...

# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=717882
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Reland of Don't pass subpixel offsets through non-translation transforms
(patchset #1 id:1 of https://codereview.chromium.org/2859483004/ )

Reason for revert:
Just to create a CL for the base of relanding.

Original issue's description:
> Revert of Don't pass subpixel offsets through non-translation transforms
(patchset #7 id:120001 of https://codereview.chromium.org/2847873002/ )
> 
> Reason for revert:
> BUG=717882
> 
> Original issue's description:
> > Don't pass subpixel offsets through non-translation transforms
> > 
> > Non-translation transforms will change direction and/or scale, etc.
> > of offsets thus making subpixel accumulation through the transform
> > meaningless.
> > 
> > This CL addresses the issue in PaintLayerPainter and
> > PaintPropertyTreeBuilder. We still need to address the issue in
> > CompositedLayerMapping (crbug.com/716163).
> > 
> > BUG=710665
> >
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> > 
> > Review-Url: https://codereview.chromium.org/2847873002
> > Cr-Commit-Position: refs/heads/master@{#468516}
> > Committed:
https://chromium.googlesource.com/chromium/src/+/098430b33423191b06dc32f13de7...
> 
> TBR=chrishtr@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=710665
> 
> Review-Url: https://codereview.chromium.org/2859483004
> Cr-Commit-Position: refs/heads/master@{#469054}
> Committed:
https://chromium.googlesource.com/chromium/src/+/763e91015ba203feaa3b27c77ce4...

# Skipping CQ checks because original CL landed less than 1 days ago.
BUG=717882
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2862053002
Cr-Commit-Position: refs/heads/master@{#469772}
Committed:
https://chromium.googlesource.com/chromium/src/+/34718d4879fbba5182af5611438d...
==========

[commit-bot: I haz the power, 2017-05-05 21:30:28]

Committed patchset #4 (id:340001) as
https://chromium.googlesource.com/chromium/src/+/34718d4879fbba5182af5611438d...