Refactor to only need to check cull rect changes up to containing transform. [chromium/src : master]
Chris harrelson (Gerrit)
Chris harrelson uploaded patch set #3 to this change.
Refactor to only need to check cull rect changes up to containing transform.
This allows us to
(a) simplify the PrePaintTree walk code, and make it faster
and
(b) makes cull rects for transforms SPv2-compatible, because we can't cull them
out if they might end up composited
Details:
1. Stop culling out CSS transformed elements and passing along cull rects from above
the transform. This change means that we don't need to worry about any cull rects
of a transform or from outside of it.
2. Simplify cull rect change detection logic in PrePaintTreeWalk and
PaintPropertyTreeBuilder to check only for whether a clip rect changed, or was
added/removed, between self and containing transform. This is made possible by #1.
3. Stop optimizing out paint invalidations for empty visual rects. This avoids
paint under-invalidation failures in cases when a transform was previously clipped
out.
Bug:
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic4d8a382c9991c51d4b25c03cb71b406b1ee98ac
---
M third_party/WebKit/LayoutTests/paint/invalidation/repaint-subsequence-on-ancestor-clip-change-complex.html
M third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h
8 files changed, 75 insertions(+), 176 deletions(-)
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 3:Commit-Queue +1
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 4:Commit-Queue +1
Chris harrelson (Gerrit)
Chris harrelson would like Philip Rogers and Xianzhu Wang to review this change.
Refactor to only need to check cull rect changes up to containing transform.
This allows us to
(a) simplify the PrePaintTree walk code, and make it faster
and
(b) makes cull rects for transforms SPv2-compatible, because we can't cull them
out if they might end up composited
Details:
1. Stop culling out CSS transformed elements and passing along cull rects from above
the transform. This change means that we don't need to worry about any cull rects
of a transform or from outside of it.
2. Simplify cull rect change detection logic in PrePaintTreeWalk and
PaintPropertyTreeBuilder to check only for whether a clip rect changed, or was
added/removed, between self and containing transform. This is made possible by #1.
3. Stop optimizing out paint invalidations for empty visual rects. This avoids
paint under-invalidation failures in cases when a transform was previously clipped
out.
Bug:
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic4d8a382c9991c51d4b25c03cb71b406b1ee98ac
---
M third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidationTest.cpp
M third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h
9 files changed, 80 insertions(+), 208 deletions(-)
Chris harrelson (Gerrit)
(2 comments)
File third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp:
I removed this optimization, because otherwise we'd have to somehow either
(a) invalidate invisible things only when transformed, or (b) change
the definition of Visual Rect to skip clips above transforms.(b) would lead to negative performance impacts on RTRees, or the need
to compute two kinds of rects for every LayoutObject.Note, however, that SPv2 will not have a significant amount of empty visual
rects, because the Visual Rect in SPv2 is defined as the unclipped bounds
of painting of the object in the space of the containing transform node.
Therefore SPv2 will have to pay the cost of invalidating these objects,
and so we should pay that cost now in order to gather any important
performance feedback.
File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:
This codes stops culling transformed content. based on clips above it.
It will have a negative performance impact of some kind on some web pages which
had hidden content under a transform, or when transformed offscrreen (a
more likely scenario).However, it seems unavoidable in the SPv2 architecture, so I think we should bite
the bullet and accept it now, so we can gather any performance feedback earlier.
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 6:Commit-Queue +1
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch set 6:
(2 comments)
Can you give an example for the under-invalidation? Does this apply to SPv2 as don't clip visual rects for SPv2?
File third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h:
Patch Set #6, Line 231:
return !(a.Rect() == b.Rect() && a.GetRadii() == b.GetRadii());return !(a == b);
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 6:
(1 comment)
Can you give an example for the under-invalidation? Does this apply to SPv2
An example is paint/invalidation/percentage-transform-paint-offset.html.
In this test, the div with id container-outer is transformed and has an empty
visual rect before and after the style change, due to being clipped out by its
container's clip, but nevertheless changes width from 700px to 600px. Without
the change to stop optimizing out such invalidations, we wouldn't paint the
contents of that transform correctly. This is not user-visibile for SPv1,
so it is technically not a bug. For SPv2 there will be no empty visual rect
in this case.
To view, visit change 544024. To unsubscribe, visit settings.
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch set 6:
(2 comments)
File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),What if the bounds of subtree are very big (much bigger than the interest rect)?
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:
Patch Set #4, Line 572:
FloatRoundedRect(mask_clip));Do we need to check change of the clip rect of mask clip?
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),What if the bounds of subtree are very big (much bigger than the interest r
Hmm. Good point. Currently, painting of composited transforms is limited
to on-screen region + 4000px. I could re-add that with some more work..
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:
Patch Set #4, Line 572:
FloatRoundedRect(mask_clip));Do we need to check change of the clip rect of mask clip?
Yes. Will fix that.
To view, visit change 544024. To unsubscribe, visit settings.
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),
Hmm. Good point. Currently, painting of composited transforms is limited
Can add a RuntimeEnabledFeatures::SlimmingPaintV2Enabled() condition here to avoid affecting SPv1, and a TODO?
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:
Patch Set #4, Line 572:
FloatRoundedRect(mask_clip));
Yes. Will fix that.
I think based on https://chromium-review.googlesource.com/c/537813/7/third_party/WebKit/Source/platform/graphics/paint/ClipPaintPropertyNode.h#50, we can let XXXPaintPropertyNode::Update() return a bool to indicate whether the node value changed, and let ObjectPaintProperties::UpdateXXX() return an enum { kNoChange, kValueChanged, kNodeExistenceChanged }. If this method looks good to you, I can split out the Update() method part from the CL.
To view, visit change 544024. To unsubscribe, visit settings.
Philip Rogers (Gerrit)
Philip Rogers posted comments on this change.
Patch set 6:
(5 comments)
An example is paint/invalidation/percentage-transform-paint-offset.html.
We talked about this patch possibly fixing an existing spv1 bug where transformed content is animated up from the bottom of the html5 single page spec. Is that a real bug fixed by this patch?
File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),
Can add a RuntimeEnabledFeatures::SlimmingPaintV2Enabled() condition here t
Can you think of a real scenario where the subtree bounds would be huge and we'd need to avoid painting the content? I couldn't think of one which is why I think it's okay to skip the interest rect which is primarily used for very long documents.
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h:
Patch Set #6, Line 106:
bool clip_changed = false;Can you add a comment here describing why we need to track this and how it is used? It would be useful to mention that we're only tracking up to the nearest transform. Maybe clip_to_transform_changed?
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:
Patch Set #6, Line 813:
local_clip_changed |= properties.UpdateInnerBorderRadiusClip(+1 to Xianzhu's idea of returning an 3-state enum from the Update functions.
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:
Patch Set #6, Line 145:
context.tree_builder_context->clip_changed = false;Do you also need to do this for the svg local transform?
To view, visit change 544024. To unsubscribe, visit settings.
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch set 6:
(1 comment)
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),
Can you think of a real scenario where the subtree bounds would be huge and
I'm not sure how rare cases like the following are:
<div style="transform: translateX(0)"><!-- or any other property triggering this code path-->
main contents of the page
</div>
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 7:
(6 comments)
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),
I'm not sure how rare cases like the following are:
Not sure what to do. Shall we keep it as-is and refine later? Otherwise I could
compute a transform of the interest rect from CLM::DoPaintTask into local space.
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h:
Patch Set #6, Line 106:
// Whether a clip paint property node appeared, disappeared, or changed
Can you add a comment here describing why we need to track this and how it
PaintPropertyTreeBuilder doesn't actually track transforms, which is why I didn't mention
this here. But added a comment.
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:
Patch Set #4, Line 572:
rounded_mask_clip != properties.MaskClip()->ClipRect())
I think based on https://chromium-review.googlesource.com/c/537813/7/third_
Sure, SGTM
Patch Set #4, Line 572:
rounded_mask_clip != properties.MaskClip()->ClipRect())
I think based on https://chromium-review.googlesource.com/c/537813/7/third_
Patch Set #6, Line 145:
context.tree_builder_context->clip_changed = false;Do you also need to do this for the svg local transform?
I don't think so, because those can't be composited right now.
File third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h:
Patch Set #6, Line 231:
return !(a == b);return !(a == b);
Done
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 7:Commit-Queue +1
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch set 7:
(1 comment)
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),
Not sure what to do. Shall we keep it as-is and refine later? Otherwise I c
SGTM if you mean keeping the old code as-is. If we do need the bigger rect for SPv2, I think we should add a condition here to avoid the change from affecting SPv1 non-composited layers.
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson uploaded patch set #9 to this change.
Refactor to only need to check cull rect changes up to containing transform.
This allows us to
(a) simplify the PrePaintTree walk code, and make it faster
and
(b) makes cull rects for transforms SPv2-compatible, because we can't cull them
out if they might end up composited
Details:
1. Stop culling out CSS transformed elements and passing along cull rects from above
the transform. This change means that we don't need to worry about any cull rects
of a transform or from outside of it.
2. Simplify cull rect change detection logic in PrePaintTreeWalk and
PaintPropertyTreeBuilder to check only for whether a clip rect changed, or was
added/removed, between self and containing transform. This is made possible by #1.
3. Stop optimizing out paint invalidations for empty visual rects. This avoids
paint under-invalidation failures in cases when a transform was previously clipped
out.
Bug: 729514
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic4d8a382c9991c51d4b25c03cb71b406b1ee98ac
---
M third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidationTest.cpp
M third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h
9 files changed, 94 insertions(+), 207 deletions(-)
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 10:Commit-Queue +1
Chris harrelson (Gerrit)
Added two layout tests to test changes to css clip and mask clip.
Chris harrelson (Gerrit)
(1 comment)
Patch Set #4, Line 856:
&paint_layer_, LayoutRect(LayoutRect::InfiniteIntRect()),
SGTM if you mean keeping the old code as-is. If we do need the bigger rect
Just discussed with pdr in person. Sounds like we are all ok with leaving the code as-is
in the patch?Regarding the suggestion for SPv2 differences: it's imperative that we stop culling transforms
with non-interest-rect clips in SPv1, because the pre-paint tree walk strategy I implemented
depends upon it not being culled.
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
We talked about this patch possibly fixing an existing spv1 bug where trans
Turns out there is no such bug. In the composited case, setting the transform
moves the composited layer onto the screen via the usual compositor drawing mechanics,
and blink doesn't have to do anything. In the non-composited case, setting the transform
is detected by repaint-after-layout.
To view, visit change 544024. To unsubscribe, visit settings.
Philip Rogers (Gerrit)
Philip Rogers posted comments on this change.
Patch set 11:Code-Review +1
LGTM here but please wait for Xianzhu to review too.
Philip Rogers (Gerrit)
just fyi, you will need to rebase on top of Xianzhu's change: https://chromium-review.googlesource.com/c/545241/
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch set 11:Code-Review +1
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 13:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 13:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"none" https://chromium-review.googlesource.com/c/544024/13
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/544024/13
Bot data: {"action": "start", "triggered_at": "2017-06-24T00:09:40.0Z", "cq_cfg_revision": "e12d437dc7f395d72995b548c9dacf21b0b1526e", "revision": "7958297cce4588442f5e973e389ed23d35425861"}
Commit Bot (Gerrit)
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_rel_ng/builds/487139)
Bot data: {"action": "cancel", "triggered_at": "2017-06-24T00:09:40.0Z", "cq_cfg_revision": "e12d437dc7f395d72995b548c9dacf21b0b1526e", "revision": "7958297cce4588442f5e973e389ed23d35425861"}
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 16:Commit-Queue +1
Chris harrelson (Gerrit)
Chris harrelson uploaded patch set #18 to this change.
Refactor to only need to check cull rect changes up to containing transform.
This allows us to
(a) simplify the PrePaintTree walk code, and make it faster
and
(b) makes cull rects for transforms SPv2-compatible, because we can't cull them
out if they might end up composited
Details:
1. Stop culling out CSS transformed elements and passing along cull rects from above
the transform. This change means that we don't need to worry about any cull rects
of a transform or from outside of it.
2. Simplify cull rect change detection logic in PrePaintTreeWalk and
PaintPropertyTreeBuilder to check only for whether a clip rect changed, or was
added/removed, between self and containing transform. This is made possible by #1.
3. Stop optimizing out paint invalidations for empty visual rects under transform.
This avoids paint under-invalidation failures in cases when a transform was
previously clipped out.
Bug: 729514
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic4d8a382c9991c51d4b25c03cb71b406b1ee98ac
---
A third_party/WebKit/LayoutTests/paint/invalidation/css-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/css-clip-change-stacking-child.html
A third_party/WebKit/LayoutTests/paint/invalidation/mask-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/mask-clip-change-stacking-child.html
M third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidationTest.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidator.h
M third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h
M third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
15 files changed, 212 insertions(+), 222 deletions(-)
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 18:Commit-Queue +1
Chris harrelson (Gerrit)
Chris harrelson uploaded patch set #19 to this change.
Refactor to only need to check cull rect changes up to containing transform.
This allows us to
(a) simplify the PrePaintTree walk code, and make it faster
and
(b) makes cull rects for transforms SPv2-compatible, because we can't cull them
out if they might end up composited
Details:
1. Stop culling out CSS transformed elements and passing along cull rects from above
the transform. This change means that we don't need to worry about any cull rects
of a transform or from outside of it.
2. Simplify cull rect change detection logic in PrePaintTreeWalk and
PaintPropertyTreeBuilder to check only for whether a clip rect changed, or was
added/removed, between self and containing transform. This is made possible by #1.
3. Stop optimizing out paint invalidations for empty visual rects under transform.
This avoids paint under-invalidation failures in cases when a transform was
previously clipped out.
4. Remove ancestor_transformed_or_root_paint_layer from the pre-paint tree
walk context.
Bug: 729514
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic4d8a382c9991c51d4b25c03cb71b406b1ee98ac
---
A third_party/WebKit/LayoutTests/paint/invalidation/css-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/css-clip-change-stacking-child.html
A third_party/WebKit/LayoutTests/paint/invalidation/mask-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/mask-clip-change-stacking-child.html
M third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidationTest.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidator.h
M third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h
M third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
15 files changed, 212 insertions(+), 222 deletions(-)
To view, visit change 544024. To unsubscribe, visit settings.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 20:
Updated the patch to no longer skip empty visual rects in
ObjectPaintInvalidator except under transform. I made this change because
there were a lot of affected paint invalidation tests, and I was concerned
about unnecessarily worsening performance for cases that really are
clipped out and not under transforms.
Running a rebaseline-cl job now, but a run yesterday yielded only a few
changed tests, and those were a couple of additional invalidations.
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 21:Commit-Queue +1
Chris harrelson (Gerrit)
Chris harrelson posted comments on this change.
Patch set 22:Commit-Queue +1
Philip Rogers (Gerrit)
Philip Rogers posted comments on this change.
Patch set 22:
Still LGTM but please wait for Xianzhu's review too
(3 comments)
File third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/svg/window-expected.txt:
Do you know what these new <g> invalidations come from?
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:
Patch Set #22, Line 205:
&object ==Can you pull this object == invalidation container bit into a bool? I think that'll make it more clear that this is doing:
if (hasTransform || paintInvalidationContainer) {
if (!paintInvalidationContainer)
...Patch Set #22, Line 212:
context.paint_invalidator_context->subtree_flags &=This line was not obvious to me at first. Can you add a comment here that says we're removing the kInvalidateEmptyVisualRect bit?
To view, visit change 544024. To unsubscribe, visit settings.
Xianzhu Wang (Gerrit)
Xianzhu Wang posted comments on this change.
Patch set 22:
LGTM
(1 comment)
Do you know what these new <g> invalidations come from?
Perhaps because of the removal of empty rect optimization? If yes, this looks good because object invalidation is fast, and we still won't invalidate empty rects.
To view, visit change 544024. To unsubscribe, visit settings.
Chris Harrelson (Gerrit)
Chris Harrelson posted comments on this change.
Patch set 24:
Also moved the code to PaintInvalidator, which is cleaner.
(3 comments)
Perhaps because of the removal of empty rect optimization? If yes, this loo
Yes it's because of that. These things are underneath an SVG element
which has a transform set on it in window.svg.
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:
Patch Set #22, Line 205:
*context.paint_invalidator_context);
Can you pull this object == invalidation container bit into a bool? I think
Done
This line was not obvious to me at first. Can you add a comment here that s
Done
To view, visit change 544024. To unsubscribe, visit settings.
Chris Harrelson (Gerrit)
Chris Harrelson posted comments on this change.
Patch set 24:Commit-Queue +1
Chris Harrelson (Gerrit)
Chris Harrelson posted comments on this change.
Patch set 25:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 25:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"none" https://chromium-review.googlesource.com/c/544024/25
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/544024/25
Bot data: {"action": "start", "triggered_at": "2017-06-27T21:04:17.0Z", "cq_cfg_revision": "e12d437dc7f395d72995b548c9dacf21b0b1526e", "revision": "f959b31967e314aed53aa9fc0699ba10eaffa71c"}
Commit Bot (Gerrit)
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_ng/builds/487468)
Bot data: {"action": "cancel", "triggered_at": "2017-06-27T21:04:17.0Z", "cq_cfg_revision": "e12d437dc7f395d72995b548c9dacf21b0b1526e", "revision": "f959b31967e314aed53aa9fc0699ba10eaffa71c"}
Chris Harrelson (Gerrit)
Chris Harrelson posted comments on this change.
Patch set 26:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 26:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"none" https://chromium-review.googlesource.com/c/544024/26
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/544024/26
Bot data: {"action": "start", "triggered_at": "2017-06-28T00:08:11.0Z", "cq_cfg_revision": "e12d437dc7f395d72995b548c9dacf21b0b1526e", "revision": "13d56ebac39d1ed090c84d2a101c6474c6da3b74"}
Commit Bot (Gerrit)
Commit Bot merged this change.
Refactor to only need to check cull rect changes up to containing transform.
This allows us to
(a) simplify the PrePaintTree walk code, and make it faster
and
(b) makes cull rects for transforms SPv2-compatible, because we can't cull them
out if they might end up composited
Details:
1. Stop culling out CSS transformed elements and passing along cull rects from above
the transform. This change means that we don't need to worry about any cull rects
of a transform or from outside of it.
2. Simplify cull rect change detection logic in PrePaintTreeWalk and
PaintPropertyTreeBuilder to check only for whether a clip rect changed, or was
added/removed, between self and containing transform. This is made possible by #1.
3. Stop optimizing out paint invalidations for empty visual rects under transform.
This avoids paint under-invalidation failures in cases when a transform was
previously clipped out.
4. Remove ancestor_transformed_or_root_paint_layer from the pre-paint tree
walk context.
Bug: 729514
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ic4d8a382c9991c51d4b25c03cb71b406b1ee98ac
Reviewed-on: https://chromium-review.googlesource.com/544024
Commit-Queue: Chris Harrelson <chri...@chromium.org>
Reviewed-by: Philip Rogers <p...@chromium.org>
Reviewed-by: Xianzhu Wang <wangx...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482854}
---
M third_party/WebKit/LayoutTests/TestExpectations
A third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/clip-unclip-and-change-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/css-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/mask-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/css-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/css-clip-change-stacking-child.html
A third_party/WebKit/LayoutTests/paint/invalidation/mask-clip-change-stacking-child-expected.txt
A third_party/WebKit/LayoutTests/paint/invalidation/mask-clip-change-stacking-child.html
M third_party/WebKit/LayoutTests/platform/linux/fast/borders/border-antialiasing-expected.png
A third_party/WebKit/LayoutTests/platform/linux/http/tests/security/xssAuditor/object-src-inject-expected.txt
M third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/svg/animated-path-inside-transformed-html-expected.txt
M third_party/WebKit/LayoutTests/platform/linux/paint/invalidation/svg/window-expected.txt
M third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/animated-path-inside-transformed-html-expected.txt
M third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/window-expected.txt
A third_party/WebKit/LayoutTests/platform/win/http/tests/security/xssAuditor/object-src-inject-expected.txt
M third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/animated-path-inside-transformed-html-expected.txt
M third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/window-expected.txt
A third_party/WebKit/LayoutTests/platform/win7/http/tests/security/xssAuditor/object-src-inject-expected.txt
M third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidationTest.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
M third_party/WebKit/Source/core/paint/PaintInvalidator.h
M third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
M third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp
M third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h
M third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp
M third_party/WebKit/Source/platform/geometry/FloatRoundedRect.h
M third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
30 files changed, 385 insertions(+), 222 deletions(-)