Opt in to CompositeSVG based on descendants rather than paint

This patch updates GraphicsLayer::ShouldCreateLayersAfterPaint during
the CompositedLayerMapping update based on
LayoutSVGRoot::HasDescendantWithCompositingReason, rather than doing
this after paint based on the painted output. This is both simpler and
avoids unnecessary compositing.

Bug: 1132598
Change-Id: I11b6d2a32b7dc3b7f9a1b068a4806382d2cd6443
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461429Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815236}

third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc

@@ -2056,10 +2056,6 @@ bool CompositedLayerMapping::IsUnderSVGHiddenContainer() const {
   return owning_layer_.IsUnderSVGHiddenContainer();
 }
 
-bool CompositedLayerMapping::IsSVGRoot() const {
-  return GetLayoutObject().IsSVGRoot();
-}
-
 bool CompositedLayerMapping::IsTrackingRasterInvalidations() const {
   return GetLayoutObject().GetFrameView()->IsTrackingRasterInvalidations();
 }

third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h

@@ -179,7 +179,6 @@ class CORE_EXPORT CompositedLayerMapping final : public GraphicsLayerClient {
                      const IntRect& interest_rect) const override;
   bool ShouldThrottleRendering() const override;
   bool IsUnderSVGHiddenContainer() const override;
-  bool IsSVGRoot() const override;
   bool IsTrackingRasterInvalidations() const override;
   void GraphicsLayersDidChange() override;
   bool PaintBlockedByDisplayLockIncludingAncestors() const override;

third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc

@@ -1844,4 +1844,65 @@ TEST_F(CompositedLayerMappingTest,
   EXPECT_FALSE(GetSquashedLayerInScrollingContents(*mapping, *squashed));
 }
 
+// Unlike CompositingTest.WillChangeTransformHintInSVG, will-change hints on the
+// SVG element itself should not opt into creating layers after paint.
+TEST_F(CompositedLayerMappingTest, WillChangeTransformHintOnSVG) {
+  ScopedCompositeSVGForTest enable_feature(true);
+  SetBodyInnerHTML(R"HTML(
+    <svg width="99" height="99" id="willChange" style="will-change: transform;">
+      <rect width="100%" height="100%" fill="blue"></rect>
+    </svg>
+  )HTML");
+
+  Element* svg = GetDocument().getElementById("willChange");
+  PaintLayer* paint_layer =
+      ToLayoutBoxModelObject(svg->GetLayoutObject())->Layer();
+  GraphicsLayer* graphics_layer = paint_layer->GraphicsLayerBacking();
+  EXPECT_FALSE(graphics_layer->ShouldCreateLayersAfterPaint());
+}
+
+// Test that will-change changes inside SVG correctly update whether the
+// graphics layer should create layers after paint.
+TEST_F(CompositedLayerMappingTest, WillChangeTransformHintInSVGChanged) {
+  ScopedCompositeSVGForTest enable_feature(true);
+  SetBodyInnerHTML(R"HTML(
+    <svg width="99" height="99" id="svg" style="will-change: transform;">
+      <rect id="rect" width="100%" height="100%" fill="blue"></rect>
+    </svg>
+  )HTML");
+
+  Element* svg = GetDocument().getElementById("svg");
+  PaintLayer* paint_layer =
+      ToLayoutBoxModelObject(svg->GetLayoutObject())->Layer();
+  EXPECT_FALSE(
+      paint_layer->GraphicsLayerBacking()->ShouldCreateLayersAfterPaint());
+
+  Element* rect = GetDocument().getElementById("rect");
+  rect->setAttribute(html_names::kStyleAttr, "will-change: transform;");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_TRUE(
+      paint_layer->GraphicsLayerBacking()->ShouldCreateLayersAfterPaint());
+
+  rect->removeAttribute(html_names::kStyleAttr);
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_FALSE(
+      paint_layer->GraphicsLayerBacking()->ShouldCreateLayersAfterPaint());
+
+  // Remove will-change from the svg element and perform the same tests. The
+  // z-index just ensures a paint layer exists so the test is similar.
+  svg->setAttribute(html_names::kStyleAttr, "z-index: 5;");
+  UpdateAllLifecyclePhasesForTest();
+  paint_layer = ToLayoutBoxModelObject(svg->GetLayoutObject())->Layer();
+  EXPECT_FALSE(paint_layer->GraphicsLayerBacking());
+
+  rect->setAttribute(html_names::kStyleAttr, "will-change: transform;");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_TRUE(
+      paint_layer->GraphicsLayerBacking()->ShouldCreateLayersAfterPaint());
+
+  rect->removeAttribute(html_names::kStyleAttr);
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_FALSE(paint_layer->GraphicsLayerBacking());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater.cc

@@ -7,6 +7,7 @@
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/frame/visual_viewport.h"
 #include "third_party/blink/renderer/core/layout/layout_box_model_object.h"
+#include "third_party/blink/renderer/core/layout/svg/layout_svg_root.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h"
 #include "third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.h"
@@ -196,6 +197,14 @@ void CompositingLayerPropertyUpdater::Update(const LayoutObject& object) {
     mask_layer->SetLayerState(
         state, snapped_paint_offset + mask_layer->OffsetFromLayoutObject());
   }
+
+  if (RuntimeEnabledFeatures::CompositeSVGEnabled()) {
+    if (object.IsSVGRoot()) {
+      main_graphics_layer->SetShouldCreateLayersAfterPaint(
+          ToLayoutSVGRoot(object).HasDescendantCompositingReasons() &&
+          main_graphics_layer->PaintsContentOrHitTest());
+    }
+  }
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/graphics/graphics_layer.cc

@@ -306,7 +306,6 @@ void GraphicsLayer::Paint() {
 
   if (PaintWithoutCommit()) {
     GetPaintController().CommitNewDisplayItems();
-    UpdateShouldCreateLayersAfterPaint();
   } else if (!needs_check_raster_invalidation_ &&
              GetPaintController().GetBenchmarkMode() !=
                  PaintBenchmarkMode::kForceRasterInvalidationAndConvert) {
@@ -355,10 +354,11 @@ void GraphicsLayer::Paint() {
   repainted_ = true;
 }
 
-void GraphicsLayer::UpdateShouldCreateLayersAfterPaint() {
-  bool new_create_layers_after_paint = ComputeShouldCreateLayersAfterPaint();
-  if (new_create_layers_after_paint != should_create_layers_after_paint_) {
-    should_create_layers_after_paint_ = new_create_layers_after_paint;
+void GraphicsLayer::SetShouldCreateLayersAfterPaint(
+    bool should_create_layers_after_paint) {
+  DCHECK(RuntimeEnabledFeatures::CompositeSVGEnabled());
+  if (should_create_layers_after_paint != should_create_layers_after_paint_) {
+    should_create_layers_after_paint_ = should_create_layers_after_paint;
     // Depending on |should_create_layers_after_paint_|, raster invalidation
     // will happen in via two different code paths. When it changes we need to
     // fully invalidate because the incremental raster invalidations of these
@@ -374,27 +374,6 @@ void GraphicsLayer::UpdateShouldCreateLayersAfterPaint() {
   }
 }
 
-bool GraphicsLayer::ComputeShouldCreateLayersAfterPaint() const {
-  if (!RuntimeEnabledFeatures::CompositeSVGEnabled())
-    return false;
-  if (!PaintsContentOrHitTest())
-    return false;
-  // Only layerize content under SVG for now. This requires that the SVG root
-  // has a GraphicsLayer.
-  if (!client_.IsSVGRoot())
-    return false;
-  for (const auto& paint_chunk : GetPaintController().PaintChunks()) {
-    const auto& chunk_state = paint_chunk.properties;
-    if (chunk_state.GetPropertyTreeState() == GetPropertyTreeState())
-      continue;
-    // TODO(pdr): Check for direct compositing reasons along the chain from
-    // this state to the layer state.
-    if (chunk_state.HasDirectCompositingReasons())
-      return true;
-  }
-  return false;
-}
-
 bool GraphicsLayer::PaintWithoutCommitForTesting(
     const base::Optional<IntRect>& interest_rect) {
   return PaintWithoutCommit(base::OptionalOrNullptr(interest_rect));

third_party/blink/renderer/platform/graphics/graphics_layer.h

@@ -228,6 +228,7 @@ class PLATFORM_EXPORT GraphicsLayer : public DisplayItemClient,
   bool PaintWithoutCommitForTesting(
       const base::Optional<IntRect>& interest_rect = base::nullopt);
 
+  void SetShouldCreateLayersAfterPaint(bool);
   bool ShouldCreateLayersAfterPaint() const {
     return should_create_layers_after_paint_;
   }
@@ -248,9 +249,6 @@ class PLATFORM_EXPORT GraphicsLayer : public DisplayItemClient,
   bool FillsBoundsCompletely() const override { return false; }
   size_t GetApproximateUnsharedMemoryUsage() const final;
 
-  void UpdateShouldCreateLayersAfterPaint();
-  bool ComputeShouldCreateLayersAfterPaint() const;
-
   // Returns true if PaintController::PaintArtifact() changed and needs commit.
   bool PaintWithoutCommit(const IntRect* interest_rect = nullptr);
   void Paint();

third_party/blink/renderer/platform/graphics/graphics_layer_client.h

@@ -74,8 +74,6 @@ class PLATFORM_EXPORT GraphicsLayerClient {
   // painting and hit testing.
   virtual bool IsUnderSVGHiddenContainer() const { return false; }
 
-  virtual bool IsSVGRoot() const { return false; }
-
   virtual bool IsTrackingRasterInvalidations() const { return false; }
 
   virtual void GraphicsLayersDidChange() {}

third_party/blink/renderer/platform/graphics/graphics_layer_test.cc

@@ -151,48 +151,4 @@ TEST_F(GraphicsLayerTest, ContentsLayer) {
   EXPECT_EQ(nullptr, graphics_layer.ContentsLayer());
 }
 
-TEST_F(GraphicsLayerTest, ShouldCreateLayersAfterPaint) {
-  FakeGraphicsLayerClient svg_root_client;
-  svg_root_client.SetIsSVGRoot(true);
-  auto svg_root_layer = std::make_unique<FakeGraphicsLayer>(svg_root_client);
-  svg_root_layer->SetDrawsContent(true);
-  svg_root_layer->SetHitTestable(true);
-  svg_root_layer->SetLayerState(PropertyTreeState::Root(), IntPoint());
-  layers_.graphics_layer().AddChild(svg_root_layer.get());
-  layers_.graphics_layer().SetDrawsContent(false);
-
-  auto effect = CreateOpacityEffect(EffectPaintPropertyNode::Root(), 1,
-                                    CompositingReason::kWillChangeOpacity);
-  svg_root_client.SetPainter([&](const GraphicsLayer* layer,
-                                 GraphicsContext& context,
-                                 GraphicsLayerPaintingPhase, const IntRect&) {
-    {
-      ScopedPaintChunkProperties properties(context.GetPaintController(),
-                                            *effect, *svg_root_layer,
-                                            kBackgroundType);
-      PaintControllerTestBase::DrawRect(context, *layer, kBackgroundType,
-                                        IntRect(0, 0, 100, 100));
-    }
-  });
-
-  svg_root_client.SetNeedsRepaint(true);
-  EXPECT_TRUE(layers_.graphics_layer().PaintRecursively());
-  EXPECT_TRUE(svg_root_layer->Repainted());
-  EXPECT_TRUE(svg_root_layer->ShouldCreateLayersAfterPaint());
-  svg_root_client.SetNeedsRepaint(false);
-  svg_root_layer->GetPaintController().FinishCycle();
-
-  svg_root_layer->SetDrawsContent(false);
-  layers_.graphics_layer().PaintRecursively();
-  EXPECT_FALSE(svg_root_layer->Repainted());
-  EXPECT_FALSE(svg_root_layer->ShouldCreateLayersAfterPaint());
-
-  svg_root_client.SetNeedsRepaint(true);
-  svg_root_layer->SetPaintsHitTest(true);
-  layers_.graphics_layer().PaintRecursively();
-  EXPECT_TRUE(svg_root_layer->Repainted());
-  EXPECT_TRUE(svg_root_layer->ShouldCreateLayersAfterPaint());
-  svg_root_layer->GetPaintController().FinishCycle();
-}
-
 }  // namespace blink

third_party/blink/renderer/platform/graphics/paint/ref_counted_property_tree_state.h

@@ -26,11 +26,6 @@ class PLATFORM_EXPORT RefCountedPropertyTreeState {
     return *this = RefCountedPropertyTreeState(property_tree_state);
   }
 
-  bool HasDirectCompositingReasons() const {
-    return Transform().Unalias().HasDirectCompositingReasons() ||
-           Effect().Unalias().HasDirectCompositingReasons();
-  }
-
   const TransformPaintPropertyNodeOrAlias& Transform() const {
     return *transform_;
   }

third_party/blink/renderer/platform/testing/fake_graphics_layer_client.h

@@ -32,7 +32,6 @@ class FakeGraphicsLayerClient : public GraphicsLayerClient {
     if (painter_)
       painter_(layer, context, phase, rect);
   }
-  bool IsSVGRoot() const override { return is_svg_root_; }
 
   void SetIsTrackingRasterInvalidations(bool is_tracking_raster_invalidations) {
     is_tracking_raster_invalidations_ = is_tracking_raster_invalidations;
@@ -46,13 +45,10 @@ class FakeGraphicsLayerClient : public GraphicsLayerClient {
                                      const IntRect&)>;
   void SetPainter(const Painter& painter) { painter_ = painter; }
 
-  void SetIsSVGRoot(bool is_svg_root) { is_svg_root_ = is_svg_root; }
-
  private:
   Painter painter_ = nullptr;
   bool is_tracking_raster_invalidations_ = false;
   bool needs_repaint_ = false;
-  bool is_svg_root_ = false;
 };
 
 }  // namespace blink

third_party/blink/web_tests/paint/invalidation/svg/resize-svg-invalidate-children-2-expected.txt

@@ -8,8 +8,11 @@
     },
     {
       "name": "LayoutSVGRoot svg",
-      "position": [50, 0],
-      "bounds": [500, 400],
+      "bounds": [600, 400],
+      "invalidations": [
+        [50, 0, 500, 400],
+        [0, 120, 200, 160]
+      ],
       "transform": 1
     }
   ],