From a3ba22889b3f2565f6e430eca4a827e0e52e0301 Mon Sep 17 00:00:00 2001 From: Chris Harrelson Date: Wed, 19 Apr 2017 11:15:35 -0700 Subject: [PATCH] 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} (cherry picked from commit d1cf31c84ed81fce3a5033e601afafd402fee87f) Review-Url: https://codereview.chromium.org/2832603002 . Cr-Commit-Position: refs/branch-heads/3071@{#57} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} --- .../WebKit/Source/core/paint/PaintLayer.cpp | 18 ++++++ .../WebKit/Source/core/paint/PaintLayer.h | 5 ++ .../Source/core/paint/PaintLayerPainter.cpp | 18 ++---- .../Source/core/paint/PaintLayerTest.cpp | 43 +++++++++++++++ .../Source/core/paint/PrePaintTreeWalk.cpp | 8 +++ .../core/paint/PrePaintTreeWalkTest.cpp | 55 +++++++++---------- 6 files changed, 104 insertions(+), 43 deletions(-) diff --git a/third_party/WebKit/Source/core/paint/PaintLayer.cpp b/third_party/WebKit/Source/core/paint/PaintLayer.cpp index 2736ebb03cb8..7100c617d45d 100644 --- a/third_party/WebKit/Source/core/paint/PaintLayer.cpp +++ b/third_party/WebKit/Source/core/paint/PaintLayer.cpp @@ -2748,6 +2748,24 @@ bool PaintLayer::PaintsWithTransform( GetCompositingState() != kPaintsIntoOwnBacking); } +bool PaintLayer::SupportsSubsequenceCaching() const { + // SVG paints atomically. + if (GetLayoutObject().IsSVGRoot()) + return true; + + // Create subsequence for only stacking contexts whose painting are atomic. + if (!StackingNode()->IsStackingContext()) + return false; + + // The layer doesn't have children. Subsequence caching is not worth it, + // because normally the actual painting will be cheap. + // SVG is also painted atomically. + if (!PaintLayerStackingNodeIterator(*StackingNode(), kAllChildren).Next()) + return false; + + return true; +} + ScrollingCoordinator* PaintLayer::GetScrollingCoordinator() { Page* page = GetLayoutObject().GetFrame()->GetPage(); return (!page) ? nullptr : page->GetScrollingCoordinator(); diff --git a/third_party/WebKit/Source/core/paint/PaintLayer.h b/third_party/WebKit/Source/core/paint/PaintLayer.h index c5ef7b905860..83691b8b2a5b 100644 --- a/third_party/WebKit/Source/core/paint/PaintLayer.h +++ b/third_party/WebKit/Source/core/paint/PaintLayer.h @@ -584,6 +584,8 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient { bool PaintsWithTransform(GlobalPaintFlags) const; + bool SupportsSubsequenceCaching() const; + // Returns true if background phase is painted opaque in the given rect. // The query rect is given in local coordinates. bool BackgroundIsKnownToBeOpaqueInRect(const LayoutRect&) const; @@ -928,6 +930,9 @@ class CORE_EXPORT PaintLayer : public DisplayItemClient { void SetPreviousPaintingClipRects(ClipRects& clip_rects) { previous_painting_clip_rects_ = &clip_rects; } + void ClearPreviousPaintingClipRects() { + previous_painting_clip_rects_.Clear(); + } LayoutRect PreviousPaintDirtyRect() const { return previous_paint_dirty_rect_; diff --git a/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp b/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp index 039a63448a6a..0a6cd48300b7 100644 --- a/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp +++ b/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp @@ -167,6 +167,9 @@ static bool ShouldCreateSubsequence(const PaintLayer& paint_layer, if (context.Printing()) return false; + if (!paint_layer.SupportsSubsequenceCaching()) + return false; + // Don't create subsequence for a composited layer because if it can be // cached, we can skip the whole painting in GraphicsLayer::paint() with // CachedDisplayItemList. This also avoids conflict of @@ -185,20 +188,6 @@ static bool ShouldCreateSubsequence(const PaintLayer& paint_layer, kPaintLayerPaintingOverlayScrollbars | kPaintLayerUncachedClipRects)) return false; - // Create subsequence for only stacking contexts whose painting are atomic. - // SVG is also painted atomically. - if (!paint_layer.StackingNode()->IsStackingContext() && - !paint_layer.GetLayoutObject().IsSVGRoot()) - return false; - - // The layer doesn't have children. Subsequence caching is not worth because - // normally the actual painting will be cheap. - // SVG is also painted atomically. - if (!PaintLayerStackingNodeIterator(*paint_layer.StackingNode(), kAllChildren) - .Next() && - !paint_layer.GetLayoutObject().IsSVGRoot()) - return false; - // When in FOUC-avoidance mode, don't cache any subsequences, to avoid having // to invalidate all of them when leaving this mode. There is an early-out in // BlockPainter::paintContents that may result in nothing getting painted in @@ -346,6 +335,7 @@ PaintResult PaintLayerPainter::PaintLayerContents( SubsequenceRecorder::UseCachedSubsequenceIfPossible(context, paint_layer_)) return result; + DCHECK(paint_layer_.SupportsSubsequenceCaching()); subsequence_recorder.emplace(context, paint_layer_); } else { should_clear_empty_paint_phase_flags = true; diff --git a/third_party/WebKit/Source/core/paint/PaintLayerTest.cpp b/third_party/WebKit/Source/core/paint/PaintLayerTest.cpp index 08eeaba0fb6e..34b833947d9b 100644 --- a/third_party/WebKit/Source/core/paint/PaintLayerTest.cpp +++ b/third_party/WebKit/Source/core/paint/PaintLayerTest.cpp @@ -286,6 +286,49 @@ TEST_P(PaintLayerTest, HasNonIsolatedDescendantWithBlendMode) { EXPECT_TRUE(parent->HasVisibleDescendant()); } +TEST_P(PaintLayerTest, SubsequenceCachingStackingContexts) { + SetBodyInnerHTML( + "
" + "
" + "
" + "
" + "
" + "
" + "
" + "
" + "
" + "
"); + PaintLayer* parent = GetPaintLayerByElementId("parent"); + PaintLayer* child1 = GetPaintLayerByElementId("child1"); + PaintLayer* child2 = GetPaintLayerByElementId("child2"); + PaintLayer* grandchild1 = GetPaintLayerByElementId("grandchild1"); + + EXPECT_FALSE(parent->SupportsSubsequenceCaching()); + EXPECT_FALSE(child1->SupportsSubsequenceCaching()); + EXPECT_TRUE(child2->SupportsSubsequenceCaching()); + EXPECT_FALSE(grandchild1->SupportsSubsequenceCaching()); + + GetDocument() + .GetElementById("grandchild1") + ->setAttribute(HTMLNames::styleAttr, "isolation: isolate"); + GetDocument().View()->UpdateAllLifecyclePhases(); + + EXPECT_FALSE(parent->SupportsSubsequenceCaching()); + EXPECT_FALSE(child1->SupportsSubsequenceCaching()); + EXPECT_TRUE(child2->SupportsSubsequenceCaching()); + EXPECT_TRUE(grandchild1->SupportsSubsequenceCaching()); +} + +TEST_P(PaintLayerTest, SubsequenceCachingSVGRoot) { + SetBodyInnerHTML( + "
" + " " + "
"); + + PaintLayer* svgroot = GetPaintLayerByElementId("svgroot"); + EXPECT_TRUE(svgroot->SupportsSubsequenceCaching()); +} + TEST_P(PaintLayerTest, HasDescendantWithClipPath) { SetBodyInnerHTML( "
" diff --git a/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp b/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp index a66c093bc1c4..e4944c4f5c42 100644 --- a/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp +++ b/third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp @@ -157,6 +157,14 @@ void PrePaintTreeWalk::InvalidatePaintLayerOptimizationsIfNeeded( context.ancestor_transformed_or_root_paint_layer = &paint_layer; } + // This code below checks whether any clips have changed that might: + // (a) invalidate optimizations made for a PaintLayer that supports + // subsequence caching, or + // (b) impact clipping of descendant visual rects. + if (!paint_layer.SupportsSubsequenceCaching() && + !paint_layer.GetLayoutObject().HasClipRelatedProperty()) + return; + const auto& ancestor = context.ancestor_transformed_or_root_paint_layer->GetLayoutObject(); PropertyTreeState ancestor_state = *ancestor.LocalBorderBoxProperties(); diff --git a/third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp b/third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp index 5ffe916f7b3e..f5fcf4f2fb7c 100644 --- a/third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp +++ b/third_party/WebKit/Source/core/paint/PrePaintTreeWalkTest.cpp @@ -8,6 +8,7 @@ #include "core/paint/ObjectPaintProperties.h" #include "core/paint/PaintLayer.h" #include "core/paint/PaintPropertyTreePrinter.h" +#include "core/paint/PrePaintTreeWalk.h" #include "platform/graphics/paint/GeometryMapper.h" #include "platform/graphics/paint/ScrollPaintPropertyNode.h" #include "platform/graphics/paint/TransformPaintPropertyNode.h" @@ -20,16 +21,18 @@ namespace blink { -typedef bool TestParamRootLayerScrolling; +typedef std::pair SlimmingPaintAndRootLayerScrolling; class PrePaintTreeWalkTest - : public ::testing::WithParamInterface, + : public ::testing::WithParamInterface, private ScopedSlimmingPaintV2ForTest, + private ScopedSlimmingPaintInvalidationForTest, private ScopedRootLayerScrollingForTest, public RenderingTest { public: PrePaintTreeWalkTest() - : ScopedSlimmingPaintV2ForTest(true), - ScopedRootLayerScrollingForTest(GetParam()), + : ScopedSlimmingPaintV2ForTest(GetParam().second), + ScopedSlimmingPaintInvalidationForTest(true), + ScopedRootLayerScrollingForTest(GetParam().first), RenderingTest(EmptyLocalFrameClient::Create()) {} const TransformPaintPropertyNode* FramePreTranslation() { @@ -52,6 +55,11 @@ class PrePaintTreeWalkTest return frame_view->ScrollTranslation(); } + protected: + PaintLayer* GetPaintLayerByElementId(const char* id) { + return ToLayoutBoxModelObject(GetLayoutObjectByElementId(id))->Layer(); + } + private: void SetUp() override { Settings::SetMockScrollbarsEnabled(true); @@ -67,7 +75,15 @@ class PrePaintTreeWalkTest } }; -INSTANTIATE_TEST_CASE_P(All, PrePaintTreeWalkTest, ::testing::Bool()); +SlimmingPaintAndRootLayerScrolling g_prepaint_foo[] = { + SlimmingPaintAndRootLayerScrolling(false, false), + SlimmingPaintAndRootLayerScrolling(true, false), + SlimmingPaintAndRootLayerScrolling(false, true), + SlimmingPaintAndRootLayerScrolling(true, true)}; + +INSTANTIATE_TEST_CASE_P(All, + PrePaintTreeWalkTest, + ::testing::ValuesIn(g_prepaint_foo)); TEST_P(PrePaintTreeWalkTest, PropertyTreesRebuiltWithBorderInvalidation) { SetBodyInnerHTML( @@ -135,6 +151,9 @@ TEST_P(PrePaintTreeWalkTest, PropertyTreesRebuiltWithCSSTransformInvalidation) { } TEST_P(PrePaintTreeWalkTest, PropertyTreesRebuiltWithOpacityInvalidation) { + // In SPv1 mode, we don't need or store property tree nodes for effects. + if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()) + return; SetBodyInnerHTML( "" "
" - "
" + "
" " content" "
" "
"); @@ -261,26 +280,4 @@ TEST_P(PrePaintTreeWalkTest, ClearSubsequenceCachingClipChangePosFixed) { EXPECT_TRUE(child_paint_layer->NeedsRepaint()); } -TEST_P(PrePaintTreeWalkTest, ClipChangeHasRadius) { - SetBodyInnerHTML( - "" - "
"); - - auto* target = GetDocument().GetElementById("target"); - auto* target_object = ToLayoutBoxModelObject(target->GetLayoutObject()); - target->setAttribute(HTMLNames::styleAttr, "border-radius: 5px"); - GetDocument().View()->UpdateAllLifecyclePhasesExceptPaint(); - EXPECT_TRUE(target_object->Layer()->NeedsRepaint()); - // And should not trigger any assert failure. - GetDocument().View()->UpdateAllLifecyclePhases(); -} - } // namespace blink