[Impeller] Avoid inserting additional save layers based on clip configuration. (flutter/engine#43759)

Fixes https://github.com/flutter/flutter/issues/130775

On the Skia backend, antiAliasWithSaveLayer is the highest fidelity clipping option. In the Impeller backend, there isn't any difference in how we clip, since the stencil buffer is always used. Nevertheless we were still inserting the save layer, which results in an extra offscreen texture and is wasteful.

Track if impeller is enabled in the diff/preroll/paint context and avoid inserting a save layer.

engine/src/flutter/flow/compositor_context.cc

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

engine/src/flutter/flow/compositor_context.h

@@ -82,7 +82,8 @@ 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 has_raster_cache,
+                                        bool impeller_enabled);
 
   // See Damage::frame_damage.
   std::optional<SkIRect> GetFrameDamage() const {

engine/src/flutter/flow/diff_context.cc

@@ -10,13 +10,15 @@ 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 has_raster_cache,
+                         bool impeller_enabled)
     : 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) {}
+      has_raster_cache_(has_raster_cache),
+      impeller_enabled_(impeller_enabled) {}
 
 void DiffContext::BeginSubtree() {
   state_stack_.push_back(state_);

engine/src/flutter/flow/diff_context.h

@@ -46,7 +46,8 @@ 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 has_raster_cache,
+                       bool impeller_enabled);
 
   // Starts a new subtree.
   void BeginSubtree();
@@ -161,6 +162,8 @@ 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
@@ -245,6 +248,7 @@ 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);
 

engine/src/flutter/flow/layers/clip_rrect_layer_unittests.cc

@@ -595,6 +595,51 @@ 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
 

engine/src/flutter/flow/layers/clip_shape_layer.h

@@ -32,7 +32,8 @@ class ClipShapeLayer : public CacheableContainerLayer {
         context->MarkSubtreeDirty(context->GetOldLayerPaintRegion(old_layer));
       }
     }
-    if (UsesSaveLayer() && context->has_raster_cache()) {
+    if (UsesSaveLayer(context->impeller_enabled()) &&
+        context->has_raster_cache()) {
       context->WillPaintWithIntegralTransform();
     }
     if (context->PushCullRect(clip_shape_bounds())) {
@@ -42,7 +43,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
   }
 
   void Preroll(PrerollContext* context) override {
-    bool uses_save_layer = UsesSaveLayer();
+    bool uses_save_layer = UsesSaveLayer(context->impeller_enabled);
 
     // 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
@@ -52,7 +53,8 @@ class ClipShapeLayer : public CacheableContainerLayer {
                   context, context->state_stack.transform_3x3());
 
     Layer::AutoPrerollSaveLayerState save =
-        Layer::AutoPrerollSaveLayerState::Create(context, UsesSaveLayer());
+        Layer::AutoPrerollSaveLayerState::Create(
+            context, UsesSaveLayer(context->impeller_enabled));
 
     auto mutator = context->state_stack.save();
     ApplyClip(mutator);
@@ -78,7 +80,7 @@ class ClipShapeLayer : public CacheableContainerLayer {
     auto mutator = context.state_stack.save();
     ApplyClip(mutator);
 
-    if (!UsesSaveLayer()) {
+    if (!UsesSaveLayer(context.impeller_enabled)) {
       PaintChildren(context);
       return;
     }
@@ -99,7 +101,10 @@ class ClipShapeLayer : public CacheableContainerLayer {
     PaintChildren(context);
   }
 
-  bool UsesSaveLayer() const {
+  bool UsesSaveLayer(bool enable_impeller) const {
+    if (enable_impeller) {
+      return false;
+    }
     return clip_behavior_ == Clip::antiAliasWithSaveLayer;
   }
 

engine/src/flutter/flow/layers/layer.h

@@ -71,6 +71,8 @@ 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
@@ -117,6 +119,7 @@ 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;
 };
 

engine/src/flutter/flow/layers/layer_tree.cc

@@ -60,6 +60,7 @@ 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
   };
@@ -139,6 +140,7 @@ 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
   };

engine/src/flutter/flow/testing/diff_context_test.cc

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

engine/src/flutter/flow/testing/diff_context_test.h

@@ -41,7 +41,8 @@ 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 use_raster_cache = true,
+                       bool impeller_enabled = false);
 
   // Create display list consisting of filled rect with given color; Being able
   // to specify different color is useful to test deep comparison of pictures

engine/src/flutter/flow/testing/layer_test.h

@@ -195,6 +195,12 @@ 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);