Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2810503002:
Only store previous clip rects for PaintLayers that support subsequences. (Closed)
DescriptionOnly store previous clip rects for PaintLayers that support subsequences.
To do this:
2. When updating paint properties, only invalidate painting optimizations like
subsequence for PaintLayers that support it (stacking context, SVG root), or
have clip-related properties
BUG=692614, 711413
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Review-Url: https://codereview.chromium.org/2810503002
Cr-Commit-Position: refs/heads/master@{#464670}
Committed: https://chromium.googlesource.com/chromium/src/+/d1cf31c84ed81fce3a5033e601afafd402fee87f
Patch Set 1 #Patch Set 2 : none #Patch Set 3 : none #Patch Set 4 : none #Patch Set 5 : none #Patch Set 6 : none #Patch Set 7 : none #Patch Set 8 : none #Patch Set 9 : none #Patch Set 10 : none #Patch Set 11 : none #
Total comments: 6
Patch Set 12 : none #Patch Set 13 : none #Patch Set 14 : none #
Total comments: 1
Patch Set 15 : none #Patch Set 16 : none #
Total comments: 5
Patch Set 17 : none #
Total comments: 3
Patch Set 18 : none #
Messages
Total messages: 71 (50 generated)
Description was changed from ========== none none BUG= ========== to ========== none none BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Description was changed from ========== none none BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this, mark all such PaintLayers as needing a paint property tree update when creating a subsequence or invalidating. BUG=692614 ==========
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this, mark all such PaintLayers as needing a paint property tree update when creating a subsequence or invalidating. BUG=692614 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this, mark all such PaintLayers as needing a paint property tree update when creating a subsequence or invalidating. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this, mark all such PaintLayers as needing a paint property tree update when creating a subsequence or invalidating. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. When painting a subsequnce, mark it as needing a paint property update the next time a frame is generated. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root). 3. When updating paint properties, save off the list of PaintLayers checked in step 2. In subsequent paint property updates, make sure to traverse down into them by marking them as needing a paint property update. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. When painting a subsequnce, mark it as needing a paint property update the next time a frame is generated. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root). 3. When updating paint properties, save off the list of PaintLayers checked in step 2. In subsequent paint property updates, make sure to traverse down into them by marking them as needing a paint property update. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. When painting a subsequnce, mark it as needing a paint property update the next time a frame is generated. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root). 3. When updating paint properties, save off the list of PaintLayers checked in step 2. In subsequent paint property updates, make sure to traverse down into them by marking them as needing a paint property update. In local testing, this yields up to a 10% improvement in pre-paint time for each frame of PerformanceTests/Paint/containment-resize.html. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
chrishtr@chromium.org changed reviewers: + pdr@chromium.org
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. When painting a subsequnce, mark it as needing a paint property update the next time a frame is generated. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root). 3. When updating paint properties, save off the list of PaintLayers checked in step 2. In subsequent paint property updates, make sure to traverse down into them by marking them as needing a paint property update. In local testing, this yields up to a 10% improvement in pre-paint time for each frame of PerformanceTests/Paint/containment-resize.html. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. Add a descendant-dependent bit on PaintLayer for whether there is a descendant that supports subsequence caching. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root). 3. Don't early-out the pre-paint tree walk if there is a descendant that supports subsequence caching. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
The CQ bit was unchecked by chrishtr@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
New version of the patch, PTAL. This one uses the approach we discussed offline.
https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right): https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:105: WTF::WrapUnique(new PaintPropertyTreeBuilderContext); This is a little weird and awkward, let's discuss offline ways to improve this code.
https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right): https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintLayer.cpp:2758: bool PaintLayer::SupportsSubsequenceCaching() const { Can you add a DCHECK in SubsequenceRecorder that SupportsSubsequenceCaching is true? Similarly, can you DCHECK that the parent has HasDescendantThatSupportsSubsequenceCaching()? https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right): https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:271: (parent_context.enclosing_paint_layer && Can enclosing_paint_layer ever be null here? Do we actually need to track the enclosing paint layer or can we just track a bool for HasDescendantThatSupportsSubsequenceCaching? https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h (right): https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h:22: class CORE_EXPORT PrePaintTreeWalk { Is this only needed for tests?
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right): https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintLayer.cpp:2758: bool PaintLayer::SupportsSubsequenceCaching() const { On 2017/04/12 at 18:40:06, pdr. wrote: > Can you add a DCHECK in SubsequenceRecorder that SupportsSubsequenceCaching is true? > > Similarly, can you DCHECK that the parent has HasDescendantThatSupportsSubsequenceCaching()? Done. https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h (right): https://codereview.chromium.org/2810503002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalk.h:22: class CORE_EXPORT PrePaintTreeWalk { On 2017/04/12 at 18:40:06, pdr. wrote: > Is this only needed for tests? Yes. https://codereview.chromium.org/2810503002/diff/260001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): https://codereview.chromium.org/2810503002/diff/260001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:506: bool usesCompositedScrolling = false; FYI this code re-adds the logic removed in https://codereview.chromium.org/2702883002/ which was removed expressly because it was made redundant by the code in PrePaintTreeWalk. Now that that code has been optimized, this has to come back.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. Add a descendant-dependent bit on PaintLayer for whether there is a descendant that supports subsequence caching. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root). 3. Don't early-out the pre-paint tree walk if there is a descendant that supports subsequence caching. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. Add a descendant-dependent bit on PaintLayer for whether there is a descendant that supports subsequence caching. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clips and contain subsequence-supporting PaintLayers. 3. Re-add support to force paint invalidation in subtrees if visual rects change. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Now uses latest "hybrid" algorithm described offline.
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #17 (id:320001) has been deleted
Overall looks good https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:507: if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && Why do we need !RuntimeEnabledFeatures::slimmingPaintV2Enabled() here and on line 511? https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right): https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintLayer.cpp:2768: // SVG is also painted atomically. Nit: Remove the line "SVG is also painted atomically." (SVG atomic comment was moved above). https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp (right): https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp:314: " .heightClass { height: 100px !important }" !important is a little confusing and there's an extra div in the example. Please rewrite as: SetBodyInnerHTML( "<style>" " #parent { height: 75px; width: 100px; position: relative; }" "</style>" "<div id='parent' style='height: 100px'>" " <div id='child' style='overflow: hidden; width: 100%; height: 100%; " " position: relative;'>" " <div id='grandchild' style='width: 50px; height: 200px;'></div>" " </div>" "</div>"); auto* grandchild = GetLayoutObjectByElementId("grandchild"); // Remove the 100px height and fall back to a height of 75px. GetDocument().GetElementById("parent")->removeAttribute("style"); ...
https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:507: if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && On 2017/04/13 at 16:21:45, pdr. wrote: > Why do we need !RuntimeEnabledFeatures::slimmingPaintV2Enabled() here and on line 511? Because if SPv2 is on, we don't map visual rects through ancestor clips. That happens later in PAC. https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp (right): https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp:314: " .heightClass { height: 100px !important }" On 2017/04/13 at 16:21:45, pdr. wrote: > !important is a little confusing and there's an extra div in the example. Please rewrite as: > SetBodyInnerHTML( > "<style>" > " #parent { height: 75px; width: 100px; position: relative; }" > "</style>" > "<div id='parent' style='height: 100px'>" > " <div id='child' style='overflow: hidden; width: 100%; height: 100%; " > " position: relative;'>" > " <div id='grandchild' style='width: 50px; height: 200px;'></div>" > " </div>" > "</div>"); > > auto* grandchild = GetLayoutObjectByElementId("grandchild"); > > // Remove the 100px height and fall back to a height of 75px. > GetDocument().GetElementById("parent")->removeAttribute("style"); > ... Done.
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2810503002/diff/340001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right): https://codereview.chromium.org/2810503002/diff/340001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:158: (RuntimeEnabledFeatures::slimmingPaintV2Enabled() || I also added this for symmetry with PaintInvalidator.
The CQ bit was unchecked by commit-bot@chromium.org
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_...)
On 2017/04/13 at 17:15:38, chrishtr wrote: > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:507: if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && > On 2017/04/13 at 16:21:45, pdr. wrote: > > Why do we need !RuntimeEnabledFeatures::slimmingPaintV2Enabled() here and on line 511? > > Because if SPv2 is on, we don't map visual rects through ancestor clips. That happens > later in PAC. > When I run paint/invalidation/clip-unclip-and-change.html with --additional-driver-flag=--enable-slimming-paint-v2, the invalidations go from "rect: [8, 120, 100, 300]" (before change) to "rect: infinite" (with change). Is that expected or are we failing to re-compute the clip rects in pac?
On 2017/04/13 at 19:20:11, pdr wrote: > On 2017/04/13 at 17:15:38, chrishtr wrote: > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > > File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): > > > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > > third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:507: if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && > > On 2017/04/13 at 16:21:45, pdr. wrote: > > > Why do we need !RuntimeEnabledFeatures::slimmingPaintV2Enabled() here and on line 511? > > > > Because if SPv2 is on, we don't map visual rects through ancestor clips. That happens > > later in PAC. > > > > When I run paint/invalidation/clip-unclip-and-change.html with --additional-driver-flag=--enable-slimming-paint-v2, the invalidations go from "rect: [8, 120, 100, 300]" (before change) to "rect: infinite" (with change). Is that expected or are we failing to re-compute the clip rects in pac? That sounds like an SPv2 bug in invalidation. But the clips rects are not applied to the invalidations by design.
On 2017/04/13 at 19:24:42, chrishtr wrote: > On 2017/04/13 at 19:20:11, pdr wrote: > > On 2017/04/13 at 17:15:38, chrishtr wrote: > > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > > > File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): > > > > > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > > > third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:507: if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && > > > On 2017/04/13 at 16:21:45, pdr. wrote: > > > > Why do we need !RuntimeEnabledFeatures::slimmingPaintV2Enabled() here and on line 511? > > > > > > Because if SPv2 is on, we don't map visual rects through ancestor clips. That happens > > > later in PAC. > > > > > > > When I run paint/invalidation/clip-unclip-and-change.html with --additional-driver-flag=--enable-slimming-paint-v2, the invalidations go from "rect: [8, 120, 100, 300]" (before change) to "rect: infinite" (with change). Is that expected or are we failing to re-compute the clip rects in pac? > > That sounds like an SPv2 bug in invalidation. But the clips rects are not applied to the invalidations by design. OK can you file that since it changes with this patch? LGTM because this patch is primarily for SPv1.
https://codereview.chromium.org/2810503002/diff/340001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp (right): https://codereview.chromium.org/2810503002/diff/340001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp:319: " <div>" Still have an extra div here.
On 2017/04/13 at 19:24:42, chrishtr wrote: > On 2017/04/13 at 19:20:11, pdr wrote: > > On 2017/04/13 at 17:15:38, chrishtr wrote: > > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > > > File third_party/WebKit/Source/core/paint/PaintInvalidator.cpp (right): > > > > > > https://codereview.chromium.org/2810503002/diff/300001/third_party/WebKit/Sou... > > > third_party/WebKit/Source/core/paint/PaintInvalidator.cpp:507: if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled() && > > > On 2017/04/13 at 16:21:45, pdr. wrote: > > > > Why do we need !RuntimeEnabledFeatures::slimmingPaintV2Enabled() here and on line 511? > > > > > > Because if SPv2 is on, we don't map visual rects through ancestor clips. That happens > > > later in PAC. > > > > > > > When I run paint/invalidation/clip-unclip-and-change.html with --additional-driver-flag=--enable-slimming-paint-v2, the invalidations go from "rect: [8, 120, 100, 300]" (before change) to "rect: infinite" (with change). Is that expected or are we failing to re-compute the clip rects in pac? > > That sounds like an SPv2 bug in invalidation. But the clips rects are not applied to the invalidations by design. Done: https://bugs.chromium.org/p/chromium/issues/detail?id=711413
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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_...) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 1. Add a descendant-dependent bit on PaintLayer for whether there is a descendant that supports subsequence caching. 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clips and contain subsequence-supporting PaintLayers. 3. Re-add support to force paint invalidation in subtrees if visual rects change. BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clip-related properties BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Updated to simplify the CL further. I realized that checking for changed visual rects during the paint invalidation walk is not necessary if we allow all PaintLayers that have clip-related properties to force subtree walks in PrePaintTreeWalk, and the code is simpler, more clear, more-or-less as fast and more definitely correct. I also realized that you were right that we do in fact need to do the pre-paint tree walk check regardless of the SPv2 flag, because ancestor clip updates are still needed to detect whether we should invalidate subsequences. https://codereview.chromium.org/2810503002/diff/340001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp (right): https://codereview.chromium.org/2810503002/diff/340001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp:319: " <div>" On 2017/04/13 at 19:29:36, pdr. wrote: > Still have an extra div here. This div is necessary for the repro. Otherwise layout will set paint invalidation needed.
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clip-related properties BUG=692614 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clip-related properties BUG=692614,711413 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Also, crbug.com/711413 now works as it did before in SPv2 mode.
On 2017/04/14 at 02:04:28, chrishtr wrote: > Also, crbug.com/711413 now works as it did before in SPv2 mode. LGTM!
The CQ bit was unchecked by chrishtr@chromium.org
The CQ bit was checked by chrishtr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 360001, "attempt_start_ts": 1492139288158700,
"parent_rev": "440edf53f31507418a80bda58e2da143619b7ddb", "commit_rev":
"d1cf31c84ed81fce3a5033e601afafd402fee87f"}
Message was sent while issue was closed.
Description was changed from ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clip-related properties BUG=692614,711413 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Only store previous clip rects for PaintLayers that support subsequences. To do this: 2. When updating paint properties, only invalidate painting optimizations like subsequence for PaintLayers that support it (stacking context, SVG root), or have clip-related properties BUG=692614,711413 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2810503002 Cr-Commit-Position: refs/heads/master@{#464670} Committed: https://chromium.googlesource.com/chromium/src/+/d1cf31c84ed81fce3a5033e601af... ==========
Message was sent while issue was closed.
Committed patchset #18 (id:360001) as https://chromium.googlesource.com/chromium/src/+/d1cf31c84ed81fce3a5033e601af... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
