[Impeller] optimize clip rects by checking if integral coverage is sufficient.

impeller/display_list/canvas.cc

@@ -551,7 +551,8 @@ void Canvas::DrawCircle(const Point& center,
 }
 
 void Canvas::ClipGeometry(const Geometry& geometry,
-                          Entity::ClipOperation clip_op) {
+                          Entity::ClipOperation clip_op,
+                          bool is_aa) {
   if (IsSkipping()) {
     return;
   }
@@ -587,12 +588,13 @@ void Canvas::ClipGeometry(const Geometry& geometry,
   clip_contents.SetClipOperation(clip_op);
 
   EntityPassClipStack::ClipStateResult clip_state_result =
-      clip_coverage_stack_.RecordClip(clip_contents,            //
-                                      clip_transform,           //
-                                      GetGlobalPassPosition(),  //
-                                      clip_depth,               //
-                                      GetClipHeightFloor()      //
-      );
+      clip_coverage_stack_.RecordClip(
+          clip_contents,                                     //
+          /*transform=*/clip_transform,                      //
+          /*global_pass_position=*/GetGlobalPassPosition(),  //
+          /*clip_depth=*/clip_depth,                         //
+          /*clip_height_floor=*/GetClipHeightFloor(),        //
+          /*is_aa=*/is_aa);
 
   if (clip_state_result.clip_did_change) {
     // We only need to update the pass scissor if the clip state has changed.

impeller/display_list/canvas.h

@@ -226,7 +226,9 @@ class Canvas {
   void DrawAtlas(const std::shared_ptr<AtlasContents>& atlas_contents,
                  const Paint& paint);
 
-  void ClipGeometry(const Geometry& geometry, Entity::ClipOperation clip_op);
+  void ClipGeometry(const Geometry& geometry,
+                    Entity::ClipOperation clip_op,
+                    bool is_aa = true);
 
   void EndReplay();
 

impeller/display_list/dl_dispatcher.cc

@@ -439,7 +439,7 @@ void DlDispatcherBase::clipRect(const DlRect& rect,
   AUTO_DEPTH_WATCHER(0u);
 
   RectGeometry geom(rect);
-  GetCanvas().ClipGeometry(geom, ToClipOperation(clip_op));
+  GetCanvas().ClipGeometry(geom, ToClipOperation(clip_op), /*is_aa=*/is_aa);
 }
 
 // |flutter::DlOpReceiver|
@@ -461,7 +461,7 @@ void DlDispatcherBase::clipRoundRect(const DlRoundRect& rrect,
   auto clip_op = ToClipOperation(sk_op);
   if (rrect.IsRect()) {
     RectGeometry geom(rrect.GetBounds());
-    GetCanvas().ClipGeometry(geom, clip_op);
+    GetCanvas().ClipGeometry(geom, clip_op, /*is_aa=*/is_aa);
   } else if (rrect.IsOval()) {
     EllipseGeometry geom(rrect.GetBounds());
     GetCanvas().ClipGeometry(geom, clip_op);
@@ -483,7 +483,7 @@ void DlDispatcherBase::clipPath(const DlPath& path, ClipOp sk_op, bool is_aa) {
   DlRect rect;
   if (path.IsRect(&rect)) {
     RectGeometry geom(rect);
-    GetCanvas().ClipGeometry(geom, clip_op);
+    GetCanvas().ClipGeometry(geom, clip_op, /*is_aa=*/is_aa);
   } else if (path.IsOval(&rect)) {
     EllipseGeometry geom(rect);
     GetCanvas().ClipGeometry(geom, clip_op);

impeller/entity/clip_stack_unittests.cc

@@ -19,13 +19,13 @@ TEST(EntityPassClipStackTest, CanPushAndPopEntities) {
 
   recorder.RecordClip(ClipContents(Rect::MakeLTRB(0, 0, 100, 100),
                                    /*is_axis_aligned_rect=*/false),
-                      Matrix(), {0, 0}, 0, 100);
+                      Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
   EXPECT_EQ(recorder.GetReplayEntities().size(), 1u);
 
-  recorder.RecordClip(
-      ClipContents(Rect::MakeLTRB(0, 0, 50, 50), /*is_axis_aligned_rect=*/true),
-      Matrix(), {0, 0}, 2, 100);
+  recorder.RecordClip(ClipContents(Rect::MakeLTRB(0, 0, 50.5, 50.5),
+                                   /*is_axis_aligned_rect=*/true),
+                      Matrix(), {0, 0}, 2, 100, /*is_aa=*/true);
 
   EXPECT_EQ(recorder.GetReplayEntities().size(), 2u);
   ASSERT_TRUE(recorder.GetReplayEntities()[0].clip_coverage.has_value());
@@ -35,7 +35,7 @@ TEST(EntityPassClipStackTest, CanPushAndPopEntities) {
   EXPECT_EQ(recorder.GetReplayEntities()[0].clip_coverage.value(),
             Rect::MakeLTRB(0, 0, 100, 100));
   EXPECT_EQ(recorder.GetReplayEntities()[1].clip_coverage.value(),
-            Rect::MakeLTRB(0, 0, 50, 50));
+            Rect::MakeLTRB(0, 0, 50.5, 50.5));
   // NOLINTEND(bugprone-unchecked-optional-access)
 
   recorder.RecordRestore({0, 0}, 1);
@@ -62,14 +62,43 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) {
   ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
 
   // Push a clip.
-  Entity entity;
   EntityPassClipStack::ClipStateResult result =
-      recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55, 55),
+      recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55.5, 55.5),
                                        /*is_axis_aligned_rect=*/true),
-                          Matrix(), {0, 0}, 0, 100);
+                          Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
   EXPECT_TRUE(result.should_render);
   EXPECT_TRUE(result.clip_did_change);
 
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 2u);
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
+            Rect::MakeLTRB(50, 50, 55.5, 55.5));
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[1].clip_height, 1u);
+  EXPECT_EQ(recorder.GetReplayEntities().size(), 1u);
+
+  // Restore the clip.
+  recorder.RecordRestore({0, 0}, 0);
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[0].coverage,
+            Rect::MakeSize(Size::MakeWH(100, 100)));
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[0].clip_height, 0u);
+  EXPECT_EQ(recorder.GetReplayEntities().size(), 0u);
+}
+
+TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverageNonAA) {
+  EntityPassClipStack recorder =
+      EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+
+  // Push a clip.
+  EntityPassClipStack::ClipStateResult result =
+      recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55.4, 55.4),
+                                       /*is_axis_aligned_rect=*/true),
+                          Matrix(), {0, 0}, 0, 100, /*is_aa=*/false);
+  EXPECT_FALSE(result.should_render);
+  EXPECT_TRUE(result.clip_did_change);
+
   ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 2u);
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
             Rect::MakeLTRB(50, 50, 55, 55));
@@ -96,17 +125,17 @@ TEST(EntityPassClipStackTest, AppendLargerClipCoverage) {
 
   // Push a clip.
   EntityPassClipStack::ClipStateResult result =
-      recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55, 55),
+      recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55.5, 55.5),
                                        /*is_axis_aligned_rect=*/true),
-                          Matrix(), {0, 0}, 0, 100);
+                          Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
   EXPECT_TRUE(result.should_render);
   EXPECT_TRUE(result.clip_did_change);
 
   // Push a clip with larger coverage than the previous state.
-  result = recorder.RecordClip(ClipContents(Rect::MakeLTRB(0, 0, 100, 100),
+  result = recorder.RecordClip(ClipContents(Rect::MakeLTRB(0, 0, 100.5, 100.5),
                                             /*is_axis_aligned_rect=*/true),
-                               Matrix(), {0, 0}, 1, 100);
+                               Matrix(), {0, 0}, 1, 100, /*is_aa=*/true);
 
   EXPECT_FALSE(result.should_render);
   EXPECT_FALSE(result.clip_did_change);
@@ -125,15 +154,15 @@ TEST(EntityPassClipStackTest,
   EntityPassClipStack::ClipStateResult result =
       recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55, 55),
                                        /*is_axis_aligned_rect=*/true),
-                          Matrix(), {0, 0}, 0, 100);
+                          Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
-  EXPECT_TRUE(result.should_render);
+  EXPECT_FALSE(result.should_render);
   EXPECT_TRUE(result.clip_did_change);
 
   // Push a clip with larger coverage than the previous state.
   result = recorder.RecordClip(ClipContents(Rect::MakeLTRB(0, 0, 100, 100),
                                             /*is_axis_aligned_rect=*/false),
-                               Matrix(), {0, 0}, 0, 100);
+                               Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
   EXPECT_TRUE(result.should_render);
   EXPECT_TRUE(result.clip_did_change);
@@ -149,15 +178,15 @@ TEST(EntityPassClipStackTest, AppendDecreasingSizeClipCoverage) {
   Entity entity;
 
   for (auto i = 1; i < 20; i++) {
-    EntityPassClipStack::ClipStateResult result =
-        recorder.RecordClip(ClipContents(Rect::MakeLTRB(i, i, 100 - i, 100 - i),
-                                         /*is_axis_aligned_rect=*/true),
-                            Matrix(), {0, 0}, 0, 100);
+    EntityPassClipStack::ClipStateResult result = recorder.RecordClip(
+        ClipContents(Rect::MakeLTRB(i, i, 99.6 - i, 99.6 - i),
+                     /*is_axis_aligned_rect=*/true),
+        Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
     EXPECT_TRUE(result.should_render);
     EXPECT_TRUE(result.clip_did_change);
     EXPECT_EQ(recorder.CurrentClipCoverage(),
-              Rect::MakeLTRB(i, i, 100 - i, 100 - i));
+              Rect::MakeLTRB(i, i, 99.6 - i, 99.6 - i));
   }
 }
 
@@ -173,7 +202,7 @@ TEST(EntityPassClipStackTest, AppendIncreasingSizeClipCoverage) {
     EntityPassClipStack::ClipStateResult result = recorder.RecordClip(
         ClipContents(Rect::MakeLTRB(0 - i, 0 - i, 100 + i, 100 + i),
                      /*is_axis_aligned_rect=*/true),
-        Matrix(), {0, 0}, 0, 100);
+        Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
     EXPECT_FALSE(result.should_render);
     EXPECT_FALSE(result.clip_did_change);
@@ -209,17 +238,17 @@ TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpasses) {
   // Push a clip.
   {
     EntityPassClipStack::ClipStateResult result =
-        recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55, 55),
+        recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55.5, 55.5),
                                          /*is_axis_aligned_rect=*/true),
-                            Matrix(), {0, 0}, 0, 100);
+                            Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
     EXPECT_TRUE(result.should_render);
     EXPECT_TRUE(result.clip_did_change);
   }
 
   ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 2u);
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
-            Rect::MakeLTRB(50, 50, 55, 55));
+            Rect::MakeLTRB(50, 50, 55.5, 55.5));
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].clip_height, 1u);
   EXPECT_EQ(recorder.GetReplayEntities().size(), 1u);
 
@@ -231,16 +260,65 @@ TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpasses) {
 
   {
     EntityPassClipStack::ClipStateResult result =
-        recorder.RecordClip(ClipContents(Rect::MakeLTRB(54, 54, 55, 55),
+        recorder.RecordClip(ClipContents(Rect::MakeLTRB(54, 54, 54.5, 54.5),
                                          /*is_axis_aligned_rect=*/true),
-                            Matrix(), {0, 0}, 0, 100);
+                            Matrix(), {0, 0}, 0, 100, /*is_aa=*/true);
 
     EXPECT_TRUE(result.should_render);
     EXPECT_TRUE(result.clip_did_change);
   }
 
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
-            Rect::MakeLTRB(54, 54, 55, 55));
+            Rect::MakeLTRB(54, 54, 54.5, 54.5));
+
+  // End subpass.
+  recorder.PopSubpass();
+
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
+            Rect::MakeLTRB(50, 50, 55.5, 55.5));
+}
+
+TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpassesNonAA) {
+  EntityPassClipStack recorder =
+      EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+
+  // Push a clip.
+  {
+    EntityPassClipStack::ClipStateResult result =
+        recorder.RecordClip(ClipContents(Rect::MakeLTRB(50, 50, 55.4, 55.4),
+                                         /*is_axis_aligned_rect=*/true),
+                            Matrix(), {0, 0}, 0, 100, /*is_aa=*/false);
+
+    EXPECT_FALSE(result.should_render);
+    EXPECT_TRUE(result.clip_did_change);
+  }
+
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 2u);
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
+            Rect::MakeLTRB(50, 50, 55.0, 55.0));
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[1].clip_height, 1u);
+  EXPECT_EQ(recorder.GetReplayEntities().size(), 1u);
+
+  // Begin a subpass.
+  recorder.PushSubpass(Rect::MakeLTRB(50, 50, 55, 55), 1);
+  ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[0].coverage,
+            Rect::MakeLTRB(50, 50, 55, 55));
+
+  {
+    EntityPassClipStack::ClipStateResult result =
+        recorder.RecordClip(ClipContents(Rect::MakeLTRB(54, 54, 55.4, 55.4),
+                                         /*is_axis_aligned_rect=*/true),
+                            Matrix(), {0, 0}, 0, 100, /*is_aa=*/false);
+
+    EXPECT_FALSE(result.should_render);
+    EXPECT_TRUE(result.clip_did_change);
+  }
+
+  EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
+            Rect::MakeLTRB(54, 54, 55.0, 55.0));
 
   // End subpass.
   recorder.PopSubpass();

impeller/entity/entity_pass_clip_stack.cc

@@ -99,7 +99,8 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::RecordClip(
     Matrix transform,
     Point global_pass_position,
     uint32_t clip_depth,
-    size_t clip_height_floor) {
+    size_t clip_height_floor,
+    bool is_aa) {
   ClipStateResult result = {.should_render = false, .clip_did_change = false};
 
   std::optional<Rect> maybe_clip_coverage = CurrentClipCoverage();
@@ -120,7 +121,7 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::RecordClip(
         clip_coverage.coverage->Shift(global_pass_position);
   }
 
-  auto& subpass_state = GetCurrentSubpassState();
+  SubpassState& subpass_state = GetCurrentSubpassState();
 
   // Compute the previous clip height.
   size_t previous_clip_height = 0;
@@ -145,13 +146,38 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::RecordClip(
     return result;
   }
 
+  // If the clip is an axis aligned rect and either is_aa is false or
+  // the clip is very nearly integral, then the depth write can be
+  // skipped for intersect clips. Since we use 4x MSAA, anything within
+  // < ~0.125 of an integral value in either axis can be treated as
+  // approximately the same as an integral value.
+  bool should_render = true;
+  std::optional<Rect> coverage_value = clip_coverage.coverage;
+  if (!clip_coverage.is_difference_or_non_square &&
+      coverage_value.has_value()) {
+    const Rect& coverage = coverage_value.value();
+    constexpr Scalar threshold = 0.124;
+    if (!is_aa ||
+        (std::abs(std::round(coverage.GetLeft()) - coverage.GetLeft()) <=
+             threshold &&
+         std::abs(std::round(coverage.GetTop()) - coverage.GetTop()) <=
+             threshold &&
+         std::abs(std::round(coverage.GetRight()) - coverage.GetRight()) <=
+             threshold &&
+         std::abs(std::round(coverage.GetBottom()) - coverage.GetBottom()) <=
+             threshold)) {
+      coverage_value = Rect::Round(clip_coverage.coverage.value());
+      should_render = false;
+    }
+  }
+
   subpass_state.clip_coverage.push_back(ClipCoverageLayer{
-      .coverage = clip_coverage.coverage,      //
+      .coverage = coverage_value,              //
       .clip_height = previous_clip_height + 1  //
 
   });
   result.clip_did_change = true;
-  result.should_render = true;
+  result.should_render = should_render;
 
   FML_DCHECK(subpass_state.clip_coverage.back().clip_height ==
              subpass_state.clip_coverage.front().clip_height +
@@ -161,10 +187,10 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::RecordClip(
       << "Not all clips have been replayed before appending new clip.";
 
   subpass_state.rendered_clip_entities.push_back(ReplayResult{
-      .clip_contents = clip_contents,           //
-      .transform = transform,                   //
-      .clip_coverage = clip_coverage.coverage,  //
-      .clip_depth = clip_depth                  //
+      .clip_contents = clip_contents,   //
+      .transform = transform,           //
+      .clip_coverage = coverage_value,  //
+      .clip_depth = clip_depth          //
   });
   next_replay_index_++;
 

impeller/entity/entity_pass_clip_stack.h

@@ -55,7 +55,8 @@ class EntityPassClipStack {
                              Matrix transform,
                              Point global_pass_position,
                              uint32_t clip_depth,
-                             size_t clip_height_floor);
+                             size_t clip_height_floor,
+                             bool is_aa);
 
   ReplayResult& GetLastReplayResult() {
     return GetCurrentSubpassState().rendered_clip_entities.back();