[SPInvalidation] Handle pixel-snapping of paint invalidation rects

[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

Committed: https://crrev.com/8f9c94471ef61f9bc6ddd7ddb3b485c3b76b30cd
Cr-Commit-Position: refs/heads/master@{#427827}

[Xianzhu, 2016-10-25 19:44:58]

Description was changed from

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can do snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

BUG=648769
==========

to

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can do snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

[Xianzhu, 2016-10-25 19:45:02]

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

[commit-bot: I haz the power, 2016-10-25 19:45:33]

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

[Xianzhu, 2016-10-25 19:46:30]

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

[commit-bot: I haz the power, 2016-10-25 22:11:31]

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

[commit-bot: I haz the power, 2016-10-25 22:11:32]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2016-10-25 23:22:35]

Description was changed from

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can do snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

to

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

[pdr., 2016-10-26 05:25:12]

Overall looks good.

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObject.h:1603: bool
previousPaintInvalidationRectCoversExtraPixels() const {
I wonder if "*ExpandedForSnapping" would be more descriptive than
"*CoversExtraPixels"?

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintInvalidator.h (right):

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.h:94: LayoutRect
computePaintInvalidationRectInBacking(
Returning through both a return value and an out-param is a little confusing.
WDYT of making them both out-params, or returning something like:
struct PaintInvalidationRectInBacking {
    LayoutRect rect;
    bool coversExtraPixels;
}

[Xianzhu, 2016-10-26 18:32:31]

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObject.h:1603: bool
previousPaintInvalidationRectCoversExtraPixels() const {
On 2016/10/26 05:25:12, pdr. wrote:
> I wonder if "*ExpandedForSnapping" would be more descriptive than
> "*CoversExtraPixels"?

In next steps, the field will be also used for transforms with rotate, skew etc.
which also causes the rect to cover extra pixels. See TODO(crbug.com/426111) in
BoxPaintInvalidator.cpp.

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintInvalidator.h (right):

https://codereview.chromium.org/2449953005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintInvalidator.h:94: LayoutRect
computePaintInvalidationRectInBacking(
On 2016/10/26 05:25:12, pdr. wrote:
> Returning through both a return value and an out-param is a little confusing.
> WDYT of making them both out-params, or returning something like:
> struct PaintInvalidationRectInBacking {
>     LayoutRect rect;
>     bool coversExtraPixels;
> }

Done.

[pdr., 2016-10-26 18:36:22]

Did you forget to upload a new patch?

[Xianzhu, 2016-10-26 19:10:10]

On 2016/10/26 18:36:22, pdr. wrote:
> Did you forget to upload a new patch?

Sorry. Uploaded.

[Xianzhu, 2016-10-26 19:10:15]

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

[commit-bot: I haz the power, 2016-10-26 19:10:53]

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

[pdr., 2016-10-26 20:59:29]

LGTM

https://codereview.chromium.org/2449953005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/2449953005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:1714: void
setPreviousPaintInvalidationRect(const LayoutRect& r,
Idea: Could we refactor this to take a PaintInvalidationRectInBacking?
(If it's a good idea and simple, lets include it in this patch. If not, just
ignore or we can do it in a followup).

[Xianzhu, 2016-10-26 21:05:21]

https://codereview.chromium.org/2449953005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/2449953005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.h:1714: void
setPreviousPaintInvalidationRect(const LayoutRect& r,
On 2016/10/26 20:59:29, pdr. wrote:
> Idea: Could we refactor this to take a PaintInvalidationRectInBacking?
> (If it's a good idea and simple, lets include it in this patch. If not, just
> ignore or we can do it in a followup).

I didn't want to include PaintInvalidator.h from LayoutObject.h or vice versa,
so kept this to make them independent.

[pdr., 2016-10-26 21:06:49]

On 2016/10/26 at 21:05:21, wangxianzhu wrote:
>
https://codereview.chromium.org/2449953005/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutObject.h (right):
> 
>
https://codereview.chromium.org/2449953005/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutObject.h:1714: void
setPreviousPaintInvalidationRect(const LayoutRect& r,
> On 2016/10/26 20:59:29, pdr. wrote:
> > Idea: Could we refactor this to take a PaintInvalidationRectInBacking?
> > (If it's a good idea and simple, lets include it in this patch. If not, just
> > ignore or we can do it in a followup).
> 
> I didn't want to include PaintInvalidator.h from LayoutObject.h or vice versa,
so kept this to make them independent.

sgtm

[commit-bot: I haz the power, 2016-10-26 21:36:34]

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

[commit-bot: I haz the power, 2016-10-26 21:36:35]

Dry run: This issue passed the CQ dry run.

[Xianzhu, 2016-10-26 21:37:23]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-10-26 21:38:41]

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

[Xianzhu, 2016-10-26 21:50:20]

The CQ bit was unchecked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-10-26 21:51:07]

Description was changed from

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

to

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

[commit-bot: I haz the power, 2016-10-26 21:51:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-26 21:54:07]

Description was changed from

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

to

==========
[SPInvalidation] Handle pixel-snapping of paint invalidation rects

In SlimmingPaintInvalidation mode we can snap a paint invalidation
rect to whole pixels more accurately by applying paint offset to
the local paint invalidation rect and expanding the rect to whole
pixels before mapping it to backing, as discussed in
https://bugs.chromium.org/p/chromium/issues/detail?id=648769#c6.

This causes a new issue that the paint invalidation rect may cover
extra pixels causing incremental invalidation not applicable. To avoid
the issue, now force full paint invalidation in the case.

Also confirmed that our current handling of SVG pixel snapping is
already correct. In PaintPropertyTreeBuilder, we bake the current
paint offset into transformToPixelSnappedBorderBox() of SVGRoot which
correctly handles pixel snapping. For SVG foreign objects, pixel
snapping of geometries is done during layout.

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

Committed: https://crrev.com/8f9c94471ef61f9bc6ddd7ddb3b485c3b76b30cd
Cr-Commit-Position: refs/heads/master@{#427827}
==========

[commit-bot: I haz the power, 2016-10-26 21:54:09]

Patchset 2 (id:??) landed as
https://crrev.com/8f9c94471ef61f9bc6ddd7ddb3b485c3b76b30cd
Cr-Commit-Position: refs/heads/master@{#427827}