Snap paint offset paint property to pixel boundaries

Snap paint offset paint property to pixel boundaries

Transformed content is snapped to pixel boundaries to reduce aliasing
so we need to pixel snap the paint offset transform created before
transform paint property nodes. Any leftover subpixel value is analagous
to the subpixel accumulation on PaintLayer.h and is passed through to
the transformed content.

Committed: https://crrev.com/5a41d4879655e6e726b84f83b72a8bf52ccc8e07
Cr-Commit-Position: refs/heads/master@{#403322}

[pdr., 2016-06-29 23:31:46]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org, trchen@chromium.org

[pdr., 2016-06-29 23:32:33]

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

[commit-bot: I haz the power, 2016-06-29 23:33:35]

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

[chrishtr, 2016-06-29 23:46:07]

Please add a test for position: fixed, which is also a trigger for
paintsWithTransform.

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-layer.html
(right):

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-layer.html:7:
<div style="width: 70px; height: 70px; left: 0.7px; top: 0.7px;">
Please indent the divs to indicate containment.

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-layer.html:9:
<div style="background: white; width: 40px; height: 40px; left: 0.7px; top:
0.7px;">
Add a comment that this is testing behavior to round down the left side, which
causes it to obscure the red div.

[chrishtr, 2016-06-29 23:46:30]

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

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp:938:
EXPECT_EQ(TransformationMatrix().translate(1, 1),
bProperties->transform()->parent()->matrix());
Is there already a unittest for not snapping if there is no transform yet
fractional paintOffset?

[commit-bot: I haz the power, 2016-06-30 00:49:33]

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

[commit-bot: I haz the power, 2016-06-30 00:49:34]

Dry run: This issue passed the CQ dry run.

[pdr., 2016-06-30 06:01:25]

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

[commit-bot: I haz the power, 2016-06-30 06:01:53]

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, 2016-06-30 07:04:43]

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

[commit-bot: I haz the power, 2016-06-30 07:04:44]

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

[pdr., 2016-06-30 16:03:12]

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-layer.html
(right):

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-layer.html:7:
<div style="width: 70px; height: 70px; left: 0.7px; top: 0.7px;">
On 2016/06/29 at 23:46:07, chrishtr wrote:
> Please indent the divs to indicate containment.

So wasteful of bytes... Done.

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-layer.html:9:
<div style="background: white; width: 40px; height: 40px; left: 0.7px; top:
0.7px;">
On 2016/06/29 at 23:46:07, chrishtr wrote:
> Add a comment that this is testing behavior to round down the left side, which
causes it to obscure the red div.

Done.

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

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp:938:
EXPECT_EQ(TransformationMatrix().translate(1, 1),
bProperties->transform()->parent()->matrix());
On 2016/06/29 at 23:46:30, chrishtr wrote:
> Is there already a unittest for not snapping if there is no transform yet
fractional paintOffset?

Added one in PaintOffsetWithBasicPixelSnapping.

[pdr., 2016-06-30 16:53:04]

On 2016/06/29 at 23:46:30, chrishtr wrote:
>
https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
(right):
> 
>
https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp:938:
EXPECT_EQ(TransformationMatrix().translate(1, 1),
bProperties->transform()->parent()->matrix());
> Is there already a unittest for not snapping if there is no transform yet
fractional paintOffset?

Added a layouttest because https://crbug.com/624667 showed that we have
little-no coverage of this area. The layouttest works around the issue in
https://crbug.com/624667.

[chrishtr, 2016-06-30 16:57:17]

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

https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp:938:
EXPECT_EQ(TransformationMatrix().translate(1, 1),
bProperties->transform()->parent()->matrix());
On 2016/06/30 at 16:03:12, pdr. wrote:
> On 2016/06/29 at 23:46:30, chrishtr wrote:
> > Is there already a unittest for not snapping if there is no transform yet
fractional paintOffset?
> 
> Added one in PaintOffsetWithBasicPixelSnapping.

That test still uses a transform. I was asking for one that has no transform yet
has fractional pixel offsets, to demonstrate
that they simply get added together, including integral parts.

https://codereview.chromium.org/2112753002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-fixedpos.html
(right):

https://codereview.chromium.org/2112753002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-fixedpos.html:3:
<div style="background: white; width: 100px; height: 100px; position: relative;
left: 0.7px;">
Unittest with fixed-pos also?

[pdr., 2016-06-30 18:54:41]

On 2016/06/30 at 16:57:17, chrishtr wrote:
>
https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
(right):
> 
>
https://codereview.chromium.org/2112753002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp:938:
EXPECT_EQ(TransformationMatrix().translate(1, 1),
bProperties->transform()->parent()->matrix());
> On 2016/06/30 at 16:03:12, pdr. wrote:
> > On 2016/06/29 at 23:46:30, chrishtr wrote:
> > > Is there already a unittest for not snapping if there is no transform yet
fractional paintOffset?
> > 
> > Added one in PaintOffsetWithBasicPixelSnapping.
> 
> That test still uses a transform. I was asking for one that has no transform
yet has fractional pixel offsets, to demonstrate
> that they simply get added together, including integral parts.

Added PaintPropertyTreeBuilderTest.FractionalPaintOffset

[pdr., 2016-06-30 18:54:48]

PTAL

https://codereview.chromium.org/2112753002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-fixedpos.html
(right):

https://codereview.chromium.org/2112753002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/paint/subpixel/subpixel-accumulation-with-fixedpos.html:3:
<div style="background: white; width: 100px; height: 100px; position: relative;
left: 0.7px;">
On 2016/06/30 at 16:57:17, chrishtr wrote:
> Unittest with fixed-pos also?

Added PaintPropertyTreeBuilderTest.PaintOffsetWithPixelSnappingWithFixedPos

[pdr., 2016-06-30 19:41:31]

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

[commit-bot: I haz the power, 2016-06-30 19:42:08]

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

[chrishtr, 2016-06-30 20:02:18]

lgtm

[pdr., 2016-06-30 20:03:43]

On 2016/06/30 at 20:02:18, chrishtr wrote:
> lgtm

@trchen, lgty 2?

[commit-bot: I haz the power, 2016-06-30 21:14:35]

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

[commit-bot: I haz the power, 2016-06-30 21:14:36]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[trchen, 2016-06-30 21:56:42]

lgtm

[pdr., 2016-06-30 21:56:56]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-06-30 21:57:43]

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, 2016-06-30 23:05:11]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-30 23:05:20]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 23:07:11]

Description was changed from

==========
Snap paint offset paint property to pixel boundaries

Transformed content is snapped to pixel boundaries to reduce aliasing
so we need to pixel snap the paint offset transform created before
transform paint property nodes. Any leftover subpixel value is analagous
to the subpixel accumulation on PaintLayer.h and is passed through to
the transformed content.
==========

to

==========
Snap paint offset paint property to pixel boundaries

Transformed content is snapped to pixel boundaries to reduce aliasing
so we need to pixel snap the paint offset transform created before
transform paint property nodes. Any leftover subpixel value is analagous
to the subpixel accumulation on PaintLayer.h and is passed through to
the transformed content.

Committed: https://crrev.com/5a41d4879655e6e726b84f83b72a8bf52ccc8e07
Cr-Commit-Position: refs/heads/master@{#403322}
==========

[commit-bot: I haz the power, 2016-06-30 23:07:12]

Patchset 3 (id:??) landed as
https://crrev.com/5a41d4879655e6e726b84f83b72a8bf52ccc8e07
Cr-Commit-Position: refs/heads/master@{#403322}