[Impeller] fix clip culling with exp canvas. (#54701)

Fixes performance problem where image filters break clip culling, and lack of clip culling stops the clear color optimization from firing.

on the current canvas the cull rect computation is slightly incorrect, as we drop it as soon as we get a image filter. With the new canvas, we have the actual render target sizes, so we can correctly cull without it.

After switching to experimental canvas, I will remove the cull rect field from the canvas stack entry - as the clip coverage stack already performs basically the same culling.

This fixes the performance issue on the uncached zoom page transition where we lose the clear color optimization too early.

Author: jonahwilliams

impeller/aiks/experimental_canvas.cc

@@ -11,6 +11,7 @@
 #include "impeller/base/validation.h"
 #include "impeller/core/allocator.h"
 #include "impeller/core/formats.h"
+#include "impeller/entity/contents/clip_contents.h"
 #include "impeller/entity/contents/framebuffer_blend_contents.h"
 #include "impeller/entity/contents/text_contents.h"
 #include "impeller/entity/entity.h"
@@ -442,6 +443,12 @@ void ExperimentalCanvas::SaveLayer(
   entry.rendering_mode = Entity::RenderingMode::kSubpassAppendSnapshotTransform;
   transform_stack_.emplace_back(entry);
 
+  // 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;
+  }
+
   // Start non-collapsed subpasses with a fresh clip coverage stack limited by
   // the subpass coverage. This is important because image filters applied to
   // save layers may transform the subpass texture after it's rendered,
@@ -764,6 +771,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,

impeller/entity/contents/clip_contents.cc

@@ -53,8 +53,11 @@ Contents::ClipCoverage ClipContents::GetClipCoverage(
     case Entity::ClipOperation::kDifference:
       // This can be optimized further by considering cases when the bounds of
       // the current stencil will shrink.
-      return {.type = ClipCoverage::Type::kAppend,
-              .coverage = current_clip_coverage};
+      return {
+          .type = ClipCoverage::Type::kAppend,  //
+          .is_difference_or_non_square = true,  //
+          .coverage = current_clip_coverage     //
+      };
     case Entity::ClipOperation::kIntersect:
       if (!geometry_) {
         return {.type = ClipCoverage::Type::kAppend, .coverage = std::nullopt};
@@ -64,8 +67,9 @@ Contents::ClipCoverage ClipContents::GetClipCoverage(
         return {.type = ClipCoverage::Type::kAppend, .coverage = std::nullopt};
       }
       return {
-          .type = ClipCoverage::Type::kAppend,
-          .coverage = current_clip_coverage->Intersection(coverage.value()),
+          .type = ClipCoverage::Type::kAppend,                                //
+          .is_difference_or_non_square = !geometry_->IsAxisAlignedRect(),     //
+          .coverage = current_clip_coverage->Intersection(coverage.value()),  //
       };
   }
   FML_UNREACHABLE();
@@ -91,19 +95,6 @@ bool ClipContents::Render(const ContentContext& renderer,
 
   using VS = ClipPipeline::VertexShader;
 
-  if (clip_op_ == Entity::ClipOperation::kIntersect &&
-      geometry_->IsAxisAlignedRect() &&
-      entity.GetTransform().IsTranslationScaleOnly()) {
-    std::optional<Rect> coverage =
-        geometry_->GetCoverage(entity.GetTransform());
-    if (coverage.has_value() &&
-        coverage->Contains(Rect::MakeSize(pass.GetRenderTargetSize()))) {
-      // Skip axis-aligned intersect clips that cover the whole render target
-      // since they won't draw anything to the depth buffer.
-      return true;
-    }
-  }
-
   VS::FrameInfo info;
   info.depth = GetShaderClipDepth(entity);
 

impeller/entity/contents/contents.h

@@ -38,6 +38,17 @@ class Contents {
     enum class Type { kNoChange, kAppend, kRestore };
 
     Type type = Type::kNoChange;
+    // TODO(jonahwilliams): this should probably use the Entity::ClipOperation
+    // enum, but that has transitive import errors.
+    bool is_difference_or_non_square = false;
+
+    /// @brief This coverage is the outer coverage of the clip.
+    ///
+    /// For example, if the clip is a circular clip, this is the rectangle that
+    /// contains the circle and not the rectangle that is contained within the
+    /// circle. This means that we cannot use the coverage alone to determine if
+    /// a clip can be culled, and instead also use the somewhat hacky
+    /// "is_difference_or_non_square" field.
     std::optional<Rect> coverage = std::nullopt;
   };
 

impeller/entity/entity_pass_clip_stack.cc

@@ -64,7 +64,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;
@@ -76,6 +76,24 @@ 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 for
+      // intersect clips, we do not need to change the clip region.
+      if (!global_clip_coverage.is_difference_or_non_square &&
+          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});
@@ -85,11 +103,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,115 @@ 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);
+}
+
+// Since clip entities return the outer coverage we can only cull axis aligned
+// rectangles and intersect clips.
+TEST(EntityPassClipStackTest,
+     AppendLargerClipCoverageWithDifferenceOrNonSquare) {
+  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,
+          .is_difference_or_non_square = true,
+          .coverage = Rect::MakeLTRB(0, 0, 100, 100),
+      },
+      entity, 0, Point(0, 0));
+
+  EXPECT_TRUE(result.should_render);
+  EXPECT_TRUE(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));