Only update the scissor when the clip stack changes.

impeller/entity/entity_pass.cc

@@ -848,14 +848,17 @@ bool EntityPass::RenderElement(Entity& element_entity,
   element_entity.GetContents()->SetCoverageHint(
       Rect::Intersection(element_coverage_hint, current_clip_coverage));
 
-  bool should_render = clip_coverage_stack.ApplyClipState(
-      clip_coverage, element_entity, clip_depth_floor, global_pass_position);
-
-  SetClipScissor(clip_coverage_stack.CurrentClipCoverage(), *result.pass,
-                 global_pass_position);
+  EntityPassClipStack::ClipStateResult clip_state_result =
+      clip_coverage_stack.ApplyClipState(clip_coverage, element_entity,
+                                         clip_depth_floor,
+                                         global_pass_position);
+
+  if (clip_state_result.clip_did_change) {
+    SetClipScissor(clip_coverage_stack.CurrentClipCoverage(), *result.pass,
+                   global_pass_position);
+  }
 
-  if (!should_render) {
-    // `ApplyClipState` returns false if the Entity can be safely skipped.
+  if (!clip_state_result.should_render) {
     return true;
   }
 

impeller/entity/entity_pass_clip_stack.cc

@@ -49,11 +49,13 @@ EntityPassClipStack::GetClipCoverageLayers() const {
   return subpass_state_.back().clip_coverage;
 }
 
-bool EntityPassClipStack::ApplyClipState(
+EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
     Contents::ClipCoverage global_clip_coverage,
     Entity& entity,
     size_t clip_depth_floor,
     Point global_pass_position) {
+  ClipStateResult result = {.should_render = false, .clip_did_change = false};
+
   auto& subpass_state = GetCurrentSubpassState();
   switch (global_clip_coverage.type) {
     case Contents::ClipCoverage::Type::kNoChange:
@@ -63,6 +65,7 @@ bool EntityPassClipStack::ApplyClipState(
       subpass_state.clip_coverage.push_back(
           ClipCoverageLayer{.coverage = global_clip_coverage.coverage,
                             .clip_depth = entity.GetClipDepth() + 1});
+      result.clip_did_change = true;
 
       FML_DCHECK(subpass_state.clip_coverage.back().clip_depth ==
                  subpass_state.clip_coverage.front().clip_depth +
@@ -71,14 +74,14 @@ bool EntityPassClipStack::ApplyClipState(
       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 false;
+        return result;
       }
     } break;
     case Contents::ClipCoverage::Type::kRestore: {
       if (subpass_state.clip_coverage.back().clip_depth <=
           entity.GetClipDepth()) {
         // Drop clip restores that will do nothing.
-        return false;
+        return result;
       }
 
       auto restoration_index = entity.GetClipDepth() -
@@ -96,18 +99,19 @@ bool EntityPassClipStack::ApplyClipState(
         restore_coverage = restore_coverage->Shift(-global_pass_position);
       }
       subpass_state.clip_coverage.resize(restoration_index + 1);
+      result.clip_did_change = true;
 
       if constexpr (ContentContext::kEnableStencilThenCover) {
         // Skip all clip restores when stencil-then-cover is enabled.
         if (subpass_state.clip_coverage.back().coverage.has_value()) {
           RecordEntity(entity, global_clip_coverage.type, Rect());
         }
-        return false;
+        return result;
       }
 
       if (!subpass_state.clip_coverage.back().coverage.has_value()) {
         // Running this restore op won't make anything renderable, so skip it.
-        return false;
+        return result;
       }
 
       auto restore_contents =
@@ -133,7 +137,8 @@ bool EntityPassClipStack::ApplyClipState(
   RecordEntity(entity, global_clip_coverage.type,
                subpass_state.clip_coverage.back().coverage);
 
-  return true;
+  result.should_render = true;
+  return result;
 }
 
 void EntityPassClipStack::RecordEntity(const Entity& entity,

impeller/entity/entity_pass_clip_stack.h

@@ -28,6 +28,11 @@ class EntityPassClipStack {
     std::optional<Rect> clip_coverage;
   };
 
+  struct ClipStateResult {
+    bool should_render = false;
+    bool clip_did_change = false;
+  };
+
   /// Create a new [EntityPassClipStack] with an initialized coverage rect.
   explicit EntityPassClipStack(const Rect& initial_coverage_rect);
 
@@ -43,12 +48,10 @@ class EntityPassClipStack {
 
   /// @brief  Applies the current clip state to an Entity. If the given Entity
   ///         is a clip operation, then the clip state is updated accordingly.
-  ///
-  /// @return true if the Entity should be rendered.
-  bool ApplyClipState(Contents::ClipCoverage global_clip_coverage,
-                      Entity& entity,
-                      size_t clip_depth_floor,
-                      Point global_pass_position);
+  ClipStateResult ApplyClipState(Contents::ClipCoverage global_clip_coverage,
+                                 Entity& entity,
+                                 size_t clip_depth_floor,
+                                 Point global_pass_position);
 
   // Visible for testing.
   void RecordEntity(const Entity& entity,

impeller/entity/entity_pass_unittests.cc

@@ -70,12 +70,14 @@ TEST(EntityPassClipStackTest, AppendCoverageNoChange) {
   EXPECT_EQ(recorder.GetClipCoverageLayers()[0].clip_depth, 0u);
 
   Entity entity;
-  recorder.ApplyClipState(
+  EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
       Contents::ClipCoverage{
           .type = Contents::ClipCoverage::Type::kNoChange,
           .coverage = std::nullopt,
       },
       entity, 0, Point(0, 0));
+  EXPECT_TRUE(result.should_render);
+  EXPECT_FALSE(result.clip_did_change);
 
   EXPECT_EQ(recorder.GetClipCoverageLayers()[0].coverage,
             Rect::MakeSize(Size::MakeWH(100, 100)));
@@ -91,12 +93,14 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) {
   // Push a clip.
   Entity entity;
   entity.SetClipDepth(0);
-  recorder.ApplyClipState(
+  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);
 
   ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 2u);
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
@@ -129,12 +133,14 @@ TEST(EntityPassClipStackTest, UnbalancedRestore) {
   // Restore the clip.
   Entity entity;
   entity.SetClipDepth(0);
-  recorder.ApplyClipState(
+  EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
       Contents::ClipCoverage{
           .type = Contents::ClipCoverage::Type::kRestore,
           .coverage = Rect::MakeLTRB(50, 50, 55, 55),
       },
       entity, 0, Point(0, 0));
+  EXPECT_FALSE(result.should_render);
+  EXPECT_FALSE(result.clip_did_change);
 
   ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
   EXPECT_EQ(recorder.GetClipCoverageLayers()[0].coverage,
@@ -152,12 +158,16 @@ TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpasses) {
   // Push a clip.
   Entity entity;
   entity.SetClipDepth(0u);
-  recorder.ApplyClipState(
-      Contents::ClipCoverage{
-          .type = Contents::ClipCoverage::Type::kAppend,
-          .coverage = Rect::MakeLTRB(50, 50, 55, 55),
-      },
-      entity, 0, Point(0, 0));
+  {
+    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);
+  }
 
   ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 2u);
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
@@ -172,12 +182,16 @@ TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpasses) {
             Rect::MakeLTRB(50, 50, 55, 55));
 
   entity.SetClipDepth(1);
-  recorder.ApplyClipState(
-      Contents::ClipCoverage{
-          .type = Contents::ClipCoverage::Type::kAppend,
-          .coverage = Rect::MakeLTRB(54, 54, 55, 55),
-      },
-      entity, 0, Point(0, 0));
+  {
+    EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
+        Contents::ClipCoverage{
+            .type = Contents::ClipCoverage::Type::kAppend,
+            .coverage = Rect::MakeLTRB(54, 54, 55, 55),
+        },
+        entity, 0, Point(0, 0));
+    EXPECT_TRUE(result.should_render);
+    EXPECT_TRUE(result.clip_did_change);
+  }
 
   EXPECT_EQ(recorder.GetClipCoverageLayers()[1].coverage,
             Rect::MakeLTRB(54, 54, 55, 55));