Revert "[Impeller] Avoid inserting additional save layers based on clip configuration. (#43759)"

This reverts commit 0ec9cc8.

Author: Jonah Williams

flow/compositor_context.cc

@@ -13,14 +13,13 @@ namespace flutter {
 
 std::optional<SkRect> FrameDamage::ComputeClipRect(
     flutter::LayerTree& layer_tree,
-    bool has_raster_cache,
-    bool impeller_enabled) {
+    bool has_raster_cache) {
   if (layer_tree.root_layer()) {
     PaintRegionMap empty_paint_region_map;
     DiffContext context(layer_tree.frame_size(), layer_tree.paint_region_map(),
                         prev_layer_tree_ ? prev_layer_tree_->paint_region_map()
                                          : empty_paint_region_map,
-                        has_raster_cache, impeller_enabled);
+                        has_raster_cache);
     context.PushCullRect(SkRect::MakeIWH(layer_tree.frame_size().width(),
                                          layer_tree.frame_size().height()));
     {
@@ -122,8 +121,7 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
 
   std::optional<SkRect> clip_rect;
   if (frame_damage) {
-    clip_rect = frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache,
-                                              !gr_context_);
+    clip_rect = frame_damage->ComputeClipRect(layer_tree, !ignore_raster_cache);
 
     if (aiks_context_ &&
         !ShouldPerformPartialRepaint(clip_rect, layer_tree.frame_size())) {

flow/compositor_context.h

@@ -84,8 +84,7 @@ class FrameDamage {
   // but the paint region of layer_tree will be calculated so that it can be
   // used for diffing of subsequent frames.
   std::optional<SkRect> ComputeClipRect(flutter::LayerTree& layer_tree,
-                                        bool has_raster_cache,
-                                        bool impeller_enabled);
+                                        bool has_raster_cache);
 
   // See Damage::frame_damage.
   std::optional<SkIRect> GetFrameDamage() const {

flow/diff_context.cc

@@ -10,15 +10,13 @@ namespace flutter {
 DiffContext::DiffContext(SkISize frame_size,
                          PaintRegionMap& this_frame_paint_region_map,
                          const PaintRegionMap& last_frame_paint_region_map,
-                         bool has_raster_cache,
-                         bool impeller_enabled)
+                         bool has_raster_cache)
     : clip_tracker_(DisplayListMatrixClipTracker(kGiantRect, SkMatrix::I())),
       rects_(std::make_shared<std::vector<SkRect>>()),
       frame_size_(frame_size),
       this_frame_paint_region_map_(this_frame_paint_region_map),
       last_frame_paint_region_map_(last_frame_paint_region_map),
-      has_raster_cache_(has_raster_cache),
-      impeller_enabled_(impeller_enabled) {}
+      has_raster_cache_(has_raster_cache) {}
 
 void DiffContext::BeginSubtree() {
   state_stack_.push_back(state_);

flow/diff_context.h

@@ -46,8 +46,7 @@ class DiffContext {
   explicit DiffContext(SkISize frame_size,
                        PaintRegionMap& this_frame_paint_region_map,
                        const PaintRegionMap& last_frame_paint_region_map,
-                       bool has_raster_cache,
-                       bool impeller_enabled);
+                       bool has_raster_cache);
 
   // Starts a new subtree.
   void BeginSubtree();
@@ -162,8 +161,6 @@ class DiffContext {
   // cached.
   bool has_raster_cache() const { return has_raster_cache_; }
 
-  bool impeller_enabled() const { return impeller_enabled_; }
-
   class Statistics {
    public:
     // Picture replaced by different picture
@@ -248,7 +245,6 @@ class DiffContext {
   PaintRegionMap& this_frame_paint_region_map_;
   const PaintRegionMap& last_frame_paint_region_map_;
   bool has_raster_cache_;
-  bool impeller_enabled_;
 
   void AddDamage(const SkRect& rect);
 

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
 

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;
   }
 

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
@@ -119,7 +117,6 @@ struct PaintContext {
   // only when leaf layer tracing is enabled.
   LayerSnapshotStore* layer_snapshot_store = nullptr;
   bool enable_leaf_layer_tracing = false;
-  bool impeller_enabled = false;
   impeller::AiksContext* aiks_context;
 };
 

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
   };
@@ -140,7 +139,6 @@ void LayerTree::Paint(CompositorContext::ScopedFrame& frame,
       .raster_cache                  = cache,
       .layer_snapshot_store          = snapshot_store,
       .enable_leaf_layer_tracing     = enable_leaf_layer_tracing_,
-      .impeller_enabled              = !!frame.aiks_context(),
       .aiks_context                  = frame.aiks_context(),
       // clang-format on
   };

flow/testing/diff_context_test.cc

@@ -17,13 +17,11 @@ Damage DiffContextTest::DiffLayerTree(MockLayerTree& layer_tree,
                                       const SkIRect& additional_damage,
                                       int horizontal_clip_alignment,
                                       int vertical_clip_alignment,
-                                      bool use_raster_cache,
-                                      bool impeller_enabled) {
+                                      bool use_raster_cache) {
   FML_CHECK(layer_tree.size() == old_layer_tree.size());
 
   DiffContext dc(layer_tree.size(), layer_tree.paint_region_map(),
-                 old_layer_tree.paint_region_map(), use_raster_cache,
-                 impeller_enabled);
+                 old_layer_tree.paint_region_map(), use_raster_cache);
   dc.PushCullRect(
       SkRect::MakeIWH(layer_tree.size().width(), layer_tree.size().height()));
   layer_tree.root()->Diff(&dc, old_layer_tree.root());

flow/testing/diff_context_test.h

@@ -41,8 +41,7 @@ class DiffContextTest : public LayerTest {
                        const SkIRect& additional_damage = SkIRect::MakeEmpty(),
                        int horizontal_clip_alignment = 0,
                        int vertical_alignment = 0,
-                       bool use_raster_cache = true,
-                       bool impeller_enabled = false);
+                       bool use_raster_cache = true);
 
   // Create display list consisting of filled rect with given color; Being able
   // to specify different color is useful to test deep comparison of pictures