[Impeller] Remove removal of save layer from clip. (flutter#46113)

Fixes flutter/flutter#134705 The save layer is observable so it isn't safe to optimize out.
geerpm · Sep 20, 2023 · 991f529 · 991f529
1 parent 0fbd990
commit 991f529
Show file tree

Hide file tree

Showing 5 changed files with 5 additions and 64 deletions.
diff --git a/flow/layers/clip_rrect_layer_unittests.cc b/flow/layers/clip_rrect_layer_unittests.cc
@@ -595,51 +595,6 @@ TEST_F(ClipRRectLayerTest, EmptyClipDoesNotCullPlatformView) {
   EXPECT_EQ(embedder.painted_views(), std::vector<int64_t>({view_id}));
 }
 
-TEST_F(ClipRRectLayerTest, AntiAliasWithSaveLayerIgnoresSaveLayerImpeller) {
-  enable_impeller();
-
-  auto path1 = SkPath().addRect({10, 10, 30, 30});
-  auto mock1 = MockLayer::MakeOpacityCompatible(path1);
-  auto path2 = SkPath().addRect({20, 20, 40, 40});
-  auto mock2 = MockLayer::MakeOpacityCompatible(path2);
-  auto children_bounds = path1.getBounds();
-  children_bounds.join(path2.getBounds());
-  SkRect clip_rect = SkRect::MakeWH(500, 500);
-  SkRRect clip_rrect = SkRRect::MakeRectXY(clip_rect, 20, 20);
-  auto clip_rrect_layer = std::make_shared<ClipRRectLayer>(
-      clip_rrect, Clip::antiAliasWithSaveLayer);
-  clip_rrect_layer->Add(mock1);
-  clip_rrect_layer->Add(mock2);
-
-  // ClipRectLayer will pass through compatibility from multiple
-  // non-overlapping compatible children
-  PrerollContext* context = preroll_context();
-  clip_rrect_layer->Preroll(context);
-  EXPECT_EQ(context->renderable_state_flags, 0);
-
-  DisplayListBuilder expected_builder;
-  /* OpacityLayer::Paint() */ {
-    expected_builder.Save();
-    {
-      /* ClipRectLayer::Paint() */ {
-        expected_builder.Save();
-        expected_builder.ClipRRect(clip_rrect, ClipOp::kIntersect, true);
-        /* child layer1 paint */ {
-          expected_builder.DrawPath(path1, DlPaint());
-        }
-        /* child layer2 paint */ {  //
-          expected_builder.DrawPath(path2, DlPaint());
-        }
-        // expected_builder.Restore();
-      }
-    }
-    expected_builder.Restore();
-  }
-
-  clip_rrect_layer->Paint(display_list_paint_context());
-  EXPECT_TRUE(DisplayListsEQ_Verbose(expected_builder.Build(), display_list()));
-}
-
 }  // namespace testing
 }  // namespace flutter
 

diff --git a/flow/layers/clip_shape_layer.h b/flow/layers/clip_shape_layer.h
@@ -32,8 +32,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
         context->MarkSubtreeDirty(context->GetOldLayerPaintRegion(old_layer));
       }
     }
-    if (UsesSaveLayer(context->impeller_enabled()) &&
-        context->has_raster_cache()) {
+    if (UsesSaveLayer() && context->has_raster_cache()) {
       context->WillPaintWithIntegralTransform();
     }
     if (context->PushCullRect(clip_shape_bounds())) {
@@ -43,7 +42,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
   }
 
   void Preroll(PrerollContext* context) override {
-    bool uses_save_layer = UsesSaveLayer(context->impeller_enabled);
+    bool uses_save_layer = UsesSaveLayer();
 
     // We can use the raster_cache for children only when the use_save_layer is
     // true so if use_save_layer is false we pass the layer_raster_item is
@@ -53,8 +52,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
                   context, context->state_stack.transform_3x3());
 
     Layer::AutoPrerollSaveLayerState save =
-        Layer::AutoPrerollSaveLayerState::Create(
-            context, UsesSaveLayer(context->impeller_enabled));
+        Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
 
     auto mutator = context->state_stack.save();
     ApplyClip(mutator);
@@ -80,7 +78,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
     auto mutator = context.state_stack.save();
     ApplyClip(mutator);
 
-    if (!UsesSaveLayer(context.impeller_enabled)) {
+    if (!UsesSaveLayer()) {
       PaintChildren(context);
       return;
     }
@@ -101,10 +99,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
     PaintChildren(context);
   }
 
-  bool UsesSaveLayer(bool enable_impeller) const {
-    if (enable_impeller) {
-      return false;
-    }
+  bool UsesSaveLayer() const {
     return clip_behavior_ == Clip::antiAliasWithSaveLayer;
   }
 

diff --git a/flow/layers/layer.h b/flow/layers/layer.h
@@ -71,8 +71,6 @@ struct PrerollContext {
   // presence of a texture layer during Preroll.
   bool has_texture_layer = false;
 
-  bool impeller_enabled = false;
-
   // The list of flags that describe which rendering state attributes
   // (such as opacity, ColorFilter, ImageFilter) a given layer can
   // render itself without requiring the parent to perform a protective

diff --git a/flow/layers/layer_tree.cc b/flow/layers/layer_tree.cc
@@ -60,7 +60,6 @@ bool LayerTree::Preroll(CompositorContext::ScopedFrame& frame,
       .raster_time                   = frame.context().raster_time(),
       .ui_time                       = frame.context().ui_time(),
       .texture_registry              = frame.context().texture_registry(),
-      .impeller_enabled              = !frame.gr_context(),
       .raster_cached_entries         = &raster_cache_items_,
       // clang-format on
   };

diff --git a/flow/testing/layer_test.h b/flow/testing/layer_test.h
@@ -196,12 +196,6 @@ class LayerTestBase : public CanvasTestBase<BaseT> {
     paint_context_.layer_snapshot_store = nullptr;
   }
 
-  void enable_impeller() {
-    preroll_context_.impeller_enabled = true;
-    paint_context_.impeller_enabled = true;
-    display_list_paint_context_.impeller_enabled = true;
-  }
-
  private:
   void set_raster_cache_(std::unique_ptr<RasterCache> raster_cache) {
     raster_cache_ = std::move(raster_cache);