move clip skipping logic into clip stack.

impeller/aiks/experimental_canvas.cc

@@ -432,10 +432,8 @@ void ExperimentalCanvas::SaveLayer(
   entry.rendering_mode = Entity::RenderingMode::kSubpassAppendSnapshotTransform;
   transform_stack_.emplace_back(entry);
 
-  // The DisplayList bounds/rtree doesn't account for filters applied to parent
-  // layers, and so sub-DisplayLists are getting culled as if no filters are
-  // applied.
-  // See also: https://github.com/flutter/flutter/issues/139294
+  // The current clip aiks clip culling can not handle image filters.
+  // Remove this once we've migrated to exp canvas and removed it.
   if (paint.image_filter) {
     transform_stack_.back().cull_rect = std::nullopt;
   }
@@ -761,6 +759,7 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) {
   auto transform = entity.GetTransform();
   entity.SetTransform(
       Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * transform);
+
   // Ideally the clip depth would be greater than the current rendering
   // depth because any rendering calls that follow this clip operation will
   // pre-increment the depth and then be rendering above our clip depth,
@@ -783,14 +782,6 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) {
         current_clip_coverage->Shift(-GetGlobalPassPosition());
   }
 
-  // Skip rendering the clip if it is fully contained by the current render
-  // target.
-  if (entity.GetContents()->CanSkip(
-          render_passes_.back().inline_pass_context->GetTexture()->GetSize(),
-          entity.GetTransform())) {
-    return;
-  }
-
   auto clip_coverage = entity.GetClipCoverage(current_clip_coverage);
   if (clip_coverage.coverage.has_value()) {
     clip_coverage.coverage =

impeller/entity/contents/clip_contents.cc

@@ -82,20 +82,6 @@ bool ClipContents::CanInheritOpacity(const Entity& entity) const {
 
 void ClipContents::SetInheritedOpacity(Scalar opacity) {}
 
-bool ClipContents::CanSkip(ISize render_pass_size, const Matrix& ctm) const {
-  if (clip_op_ == Entity::ClipOperation::kIntersect &&
-      geometry_->IsAxisAlignedRect() && ctm.IsTranslationScaleOnly()) {
-    std::optional<Rect> coverage = geometry_->GetCoverage(ctm);
-    if (coverage.has_value() &&
-        coverage->Contains(Rect::MakeSize(render_pass_size))) {
-      // Skip axis-aligned intersect clips that cover the whole render target
-      // since they won't draw anything to the depth buffer.
-      return true;
-    }
-  }
-  return false;
-}
-
 bool ClipContents::Render(const ContentContext& renderer,
                           const Entity& entity,
                           RenderPass& pass) const {
@@ -105,10 +91,6 @@ bool ClipContents::Render(const ContentContext& renderer,
 
   using VS = ClipPipeline::VertexShader;
 
-  if (CanSkip(pass.GetRenderTargetSize(), entity.GetTransform())) {
-    return true;
-  }
-
   VS::FrameInfo info;
   info.depth = GetShaderClipDepth(entity);
 

impeller/entity/contents/clip_contents.h

@@ -25,8 +25,6 @@ class ClipContents final : public Contents {
 
   void SetClipOperation(Entity::ClipOperation clip_op);
 
-  bool CanSkip(ISize render_pass_size, const Matrix& transform) const override;
-
   // |Contents|
   std::optional<Rect> GetCoverage(const Entity& entity) const override;
 

impeller/entity/contents/contents.cc

@@ -182,8 +182,4 @@ void Contents::SetColorSourceSize(Size size) {
   color_source_size_ = size;
 }
 
-bool Contents::CanSkip(ISize render_pass_size, const Matrix& transform) const {
-  return false;
-}
-
 }  // namespace impeller

impeller/entity/contents/contents.h

@@ -83,13 +83,6 @@ class Contents {
   ///
   void SetCoverageHint(std::optional<Rect> coverage_hint);
 
-  //----------------------------------------------------------------------------
-  /// @brief  Whether this entity can be entirely skipped during rendering.
-  ///
-  /// TODO(jonahwilliams): remove this method which was only added for clipping
-  /// once experimental canvas lands.
-  virtual bool CanSkip(ISize render_pass_size, const Matrix& transform) const;
-
   const std::optional<Rect>& GetCoverageHint() const;
 
   //----------------------------------------------------------------------------

impeller/entity/entity_pass_clip_stack.cc

@@ -60,7 +60,7 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
     case Contents::ClipCoverage::Type::kNoChange:
       break;
     case Contents::ClipCoverage::Type::kAppend: {
-      auto op = CurrentClipCoverage();
+      auto maybe_coverage = CurrentClipCoverage();
 
       // Compute the previous clip height.
       size_t previous_clip_height = 0;
@@ -72,6 +72,23 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
         previous_clip_height = clip_height_floor;
       }
 
+      if (!maybe_coverage.has_value()) {
+        // Running this append op won't impact the clip buffer because the
+        // whole screen is already being clipped, so skip it.
+        return result;
+      }
+      auto op = maybe_coverage.value();
+
+      // If the new clip coverage is bigger than the existing coverage, we
+      // do not need to change the clip region.
+      if (global_clip_coverage.coverage.has_value() &&
+          global_clip_coverage.coverage.value().Contains(op)) {
+        subpass_state.clip_coverage.push_back(ClipCoverageLayer{
+            .coverage = op, .clip_height = previous_clip_height + 1});
+
+        return result;
+      }
+
       subpass_state.clip_coverage.push_back(
           ClipCoverageLayer{.coverage = global_clip_coverage.coverage,
                             .clip_height = previous_clip_height + 1});
@@ -81,11 +98,6 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
                  subpass_state.clip_coverage.front().clip_height +
                      subpass_state.clip_coverage.size() - 1);
 
-      if (!op.has_value()) {
-        // Running this append op won't impact the clip buffer because the
-        // whole screen is already being clipped, so skip it.
-        return result;
-      }
     } break;
     case Contents::ClipCoverage::Type::kRestore: {
       ClipRestoreContents* restore_contents =

impeller/entity/entity_pass_unittests.cc

@@ -130,6 +130,82 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) {
   EXPECT_EQ(recorder.GetReplayEntities().size(), 0u);
 }
 
+// Append two clip coverages, the second is larger the first. This
+// should result in the second clip not requiring any update.
+TEST(EntityPassClipStackTest, AppendLargerClipCoverage) {
+  EntityPassClipStack recorder =
+      EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+
+  // Push a clip.
+  Entity entity;
+  EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
+      Contents::ClipCoverage{
+          .type = Contents::ClipCoverage::Type::kAppend,
+          .coverage = Rect::MakeLTRB(50, 50, 55, 55),
+      },
+      entity, 0, Point(0, 0));
+  EXPECT_TRUE(result.should_render);
+  EXPECT_TRUE(result.clip_did_change);
+
+  // Push a clip with larger coverage than the previous state.
+  result = recorder.ApplyClipState(
+      Contents::ClipCoverage{
+          .type = Contents::ClipCoverage::Type::kAppend,
+          .coverage = Rect::MakeLTRB(0, 0, 100, 100),
+      },
+      entity, 0, Point(0, 0));
+
+  EXPECT_FALSE(result.should_render);
+  EXPECT_FALSE(result.clip_did_change);
+}
+
+TEST(EntityPassClipStackTest, AppendDecreasingSizeClipCoverage) {
+  EntityPassClipStack recorder =
+      EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+
+  // Push Clips that shrink in size. All should be applied.
+  Entity entity;
+
+  for (auto i = 1; i < 20; i++) {
+    EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
+        Contents::ClipCoverage{
+            .type = Contents::ClipCoverage::Type::kAppend,
+            .coverage = Rect::MakeLTRB(i, i, 100 - i, 100 - i),
+        },
+        entity, 0, Point(0, 0));
+    EXPECT_TRUE(result.should_render);
+    EXPECT_TRUE(result.clip_did_change);
+    EXPECT_EQ(recorder.CurrentClipCoverage(),
+              Rect::MakeLTRB(i, i, 100 - i, 100 - i));
+  }
+}
+
+TEST(EntityPassClipStackTest, AppendIncreasingSizeClipCoverage) {
+  EntityPassClipStack recorder =
+      EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+
+  // Push Clips that grow in size. All should be skipped.
+  Entity entity;
+
+  for (auto i = 1; i < 20; i++) {
+    EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
+        Contents::ClipCoverage{
+            .type = Contents::ClipCoverage::Type::kAppend,
+            .coverage = Rect::MakeLTRB(0 - i, 0 - i, 100 + i, 100 + i),
+        },
+        entity, 0, Point(0, 0));
+    EXPECT_FALSE(result.should_render);
+    EXPECT_FALSE(result.clip_did_change);
+    EXPECT_EQ(recorder.CurrentClipCoverage(), Rect::MakeLTRB(0, 0, 100, 100));
+  }
+}
+
 TEST(EntityPassClipStackTest, UnbalancedRestore) {
   EntityPassClipStack recorder =
       EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));

impeller/entity/entity_unittests.cc

@@ -1532,35 +1532,6 @@ TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) {
   }
 }
 
-TEST_P(EntityTest, ClipContentsCanSkip) {
-  // Intersect Clips can be skipped if the render target coverage is fully
-  // inside of them.
-
-  auto clip = std::make_shared<ClipContents>();
-  clip->SetGeometry(Geometry::MakeRect(Rect::MakeLTRB(0, 0, 100, 100)));
-  clip->SetClipOperation(Entity::ClipOperation::kIntersect);
-
-  // Fully Contained
-  EXPECT_TRUE(clip->CanSkip(ISize::MakeWH(50, 50), {}));
-
-  // Not Intersect
-  clip->SetClipOperation(Entity::ClipOperation::kDifference);
-  EXPECT_FALSE(clip->CanSkip(ISize::MakeWH(50, 50), {}));
-
-  // Not Contained
-  clip->SetClipOperation(Entity::ClipOperation::kIntersect);
-  EXPECT_FALSE(clip->CanSkip(ISize::MakeWH(200, 200), {}));
-
-  // Not Scale Translate
-  clip->SetClipOperation(Entity::ClipOperation::kIntersect);
-  EXPECT_FALSE(
-      clip->CanSkip(ISize::MakeWH(50, 50), Matrix::MakeRotationX(Radians(2))));
-
-  // Not Axis Aligned Rectangle.
-  clip->SetGeometry(Geometry::MakeOval(Rect::MakeLTRB(0, 0, 50, 50)));
-  EXPECT_FALSE(clip->CanSkip(ISize::MakeWH(50, 50), {}));
-}
-
 TEST_P(EntityTest, ClipContentsGetClipCoverageIsCorrect) {
   // Intersection: No stencil coverage, no geometry.
   {