[Impeller] Use the scissor to limit all draws by clip coverage.

impeller/entity/entity_pass.cc

@@ -25,6 +25,7 @@
 #include "impeller/entity/inline_pass_context.h"
 #include "impeller/geometry/color.h"
 #include "impeller/geometry/rect.h"
+#include "impeller/geometry/size.h"
 #include "impeller/renderer/command_buffer.h"
 
 #ifdef IMPELLER_DEBUG
@@ -752,6 +753,25 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
   FML_UNREACHABLE();
 }
 
+static void SetClipScissor(std::optional<Rect> clip_coverage,
+                           RenderPass& pass,
+                           Point global_pass_position) {
+  if constexpr (!ContentContext::kEnableStencilThenCover) {
+    return;
+  }
+  // Set the scissor to the clip coverage area. We do this prior to rendering
+  // the clip itself and all its contents.
+  IRect scissor;
+  if (clip_coverage.has_value()) {
+    clip_coverage = clip_coverage->Shift(-global_pass_position);
+    scissor = IRect::RoundOut(clip_coverage.value());
+    // The scissor rect must not exceed the size of the render target.
+    scissor = scissor.Intersection(IRect::MakeSize(pass.GetRenderTargetSize()))
+                  .value_or(IRect());
+  }
+  pass.SetScissor(scissor);
+}
+
 bool EntityPass::RenderElement(Entity& element_entity,
                                size_t clip_depth_floor,
                                InlinePassContext& pass_context,
@@ -771,8 +791,10 @@ bool EntityPass::RenderElement(Entity& element_entity,
     // Restore any clips that were recorded before the backdrop filter was
     // applied.
     auto& replay_entities = clip_coverage_stack.GetReplayEntities();
-    for (const auto& entity : replay_entities) {
-      if (!entity.Render(renderer, *result.pass)) {
+    for (const auto& replay : replay_entities) {
+      SetClipScissor(clip_coverage_stack.CurrentClipCoverage(), *result.pass,
+                     global_pass_position);
+      if (!replay.entity.Render(renderer, *result.pass)) {
         VALIDATION_LOG << "Failed to render entity for clip restore.";
       }
     }
@@ -826,11 +848,18 @@ bool EntityPass::RenderElement(Entity& element_entity,
   element_entity.GetContents()->SetCoverageHint(
       Rect::Intersection(element_coverage_hint, current_clip_coverage));
 
-  if (!clip_coverage_stack.AppendClipCoverage(clip_coverage, element_entity,
-                                              clip_depth_floor,
-                                              global_pass_position)) {
-    // If the entity's coverage change did not change the clip coverage, we
-    // don't need to render it.
+  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) {
+    // We only need to update the pass scissor if the clip state has changed.
+    SetClipScissor(clip_coverage_stack.CurrentClipCoverage(), *result.pass,
+                   global_pass_position);
+  }
+
+  if (!clip_state_result.should_render) {
     return true;
   }
 

impeller/entity/entity_pass_clip_stack.cc

@@ -49,20 +49,23 @@ EntityPassClipStack::GetClipCoverageLayers() const {
   return subpass_state_.back().clip_coverage;
 }
 
-bool EntityPassClipStack::AppendClipCoverage(
-    Contents::ClipCoverage clip_coverage,
+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 (clip_coverage.type) {
+  switch (global_clip_coverage.type) {
     case Contents::ClipCoverage::Type::kNoChange:
       break;
     case Contents::ClipCoverage::Type::kAppend: {
       auto op = CurrentClipCoverage();
       subpass_state.clip_coverage.push_back(
-          ClipCoverageLayer{.coverage = clip_coverage.coverage,
+          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::AppendClipCoverage(
       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::AppendClipCoverage(
         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, clip_coverage.type);
+          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 =
@@ -130,19 +134,23 @@ bool EntityPassClipStack::AppendClipCoverage(
 #endif
 
   entity.SetClipDepth(entity.GetClipDepth() - clip_depth_floor);
-  RecordEntity(entity, clip_coverage.type);
+  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,
-                                       Contents::ClipCoverage::Type type) {
+                                       Contents::ClipCoverage::Type type,
+                                       std::optional<Rect> clip_coverage) {
   auto& subpass_state = GetCurrentSubpassState();
   switch (type) {
     case Contents::ClipCoverage::Type::kNoChange:
       return;
     case Contents::ClipCoverage::Type::kAppend:
-      subpass_state.rendered_clip_entities.push_back(entity.Clone());
+      subpass_state.rendered_clip_entities.push_back(
+          {.entity = entity.Clone(), .clip_coverage = clip_coverage});
       break;
     case Contents::ClipCoverage::Type::kRestore:
       if (!subpass_state.rendered_clip_entities.empty()) {
@@ -157,7 +165,8 @@ EntityPassClipStack::GetCurrentSubpassState() {
   return subpass_state_.back();
 }
 
-const std::vector<Entity>& EntityPassClipStack::GetReplayEntities() const {
+const std::vector<EntityPassClipStack::ReplayResult>&
+EntityPassClipStack::GetReplayEntities() const {
   return subpass_state_.back().rendered_clip_entities;
 }
 

impeller/entity/entity_pass_clip_stack.h

@@ -6,6 +6,8 @@
 #define FLUTTER_IMPELLER_ENTITY_ENTITY_PASS_CLIP_STACK_H_
 
 #include "impeller/entity/contents/contents.h"
+#include "impeller/entity/entity.h"
+#include "impeller/geometry/rect.h"
 
 namespace impeller {
 
@@ -21,6 +23,20 @@ struct ClipCoverageLayer {
 ///        stencil buffer is left in an identical state.
 class EntityPassClipStack {
  public:
+  struct ReplayResult {
+    Entity entity;
+    std::optional<Rect> clip_coverage;
+  };
+
+  struct ClipStateResult {
+    /// Whether or not the Entity should be rendered. If false, the Entity may
+    /// be safely skipped.
+    bool should_render = false;
+    /// Whether or not the current clip coverage changed during the call to
+    /// `ApplyClipState`.
+    bool clip_did_change = false;
+  };
+
   /// Create a new [EntityPassClipStack] with an initialized coverage rect.
   explicit EntityPassClipStack(const Rect& initial_coverage_rect);
 
@@ -34,24 +50,27 @@ class EntityPassClipStack {
 
   bool HasCoverage() const;
 
-  /// Returns true if entity should be rendered.
-  bool AppendClipCoverage(Contents::ClipCoverage clip_coverage,
-                          Entity& entity,
-                          size_t clip_depth_floor,
-                          Point global_pass_position);
+  /// @brief  Applies the current clip state to an Entity. If the given Entity
+  ///         is a clip operation, then the clip state is updated accordingly.
+  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, Contents::ClipCoverage::Type type);
+  void RecordEntity(const Entity& entity,
+                    Contents::ClipCoverage::Type type,
+                    std::optional<Rect> clip_coverage);
 
   // Visible for testing.
-  const std::vector<Entity>& GetReplayEntities() const;
+  const std::vector<ReplayResult>& GetReplayEntities() const;
 
   // Visible for testing.
   const std::vector<ClipCoverageLayer> GetClipCoverageLayers() const;
 
  private:
   struct SubpassState {
-    std::vector<Entity> rendered_clip_entities;
+    std::vector<ReplayResult> rendered_clip_entities;
     std::vector<ClipCoverageLayer> clip_coverage;
   };
 

impeller/entity/entity_pass_unittests.cc

@@ -17,16 +17,26 @@ TEST(EntityPassClipStackTest, CanPushAndPopEntities) {
   EXPECT_TRUE(recorder.GetReplayEntities().empty());
 
   Entity entity;
-  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kAppend);
+  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kAppend,
+                        Rect::MakeLTRB(0, 0, 100, 100));
   EXPECT_EQ(recorder.GetReplayEntities().size(), 1u);
 
-  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kAppend);
+  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kAppend,
+                        Rect::MakeLTRB(0, 0, 50, 50));
   EXPECT_EQ(recorder.GetReplayEntities().size(), 2u);
-
-  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore);
+  ASSERT_TRUE(recorder.GetReplayEntities()[0].clip_coverage.has_value());
+  ASSERT_TRUE(recorder.GetReplayEntities()[1].clip_coverage.has_value());
+  // NOLINTBEGIN(bugprone-unchecked-optional-access)
+  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));
+  // NOLINTEND(bugprone-unchecked-optional-access)
+
+  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore, Rect());
   EXPECT_EQ(recorder.GetReplayEntities().size(), 1u);
 
-  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore);
+  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore, Rect());
   EXPECT_TRUE(recorder.GetReplayEntities().empty());
 }
 
@@ -37,7 +47,7 @@ TEST(EntityPassClipStackTest, CanPopEntitiesSafely) {
   EXPECT_TRUE(recorder.GetReplayEntities().empty());
 
   Entity entity;
-  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore);
+  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kRestore, Rect());
   EXPECT_TRUE(recorder.GetReplayEntities().empty());
 }
 
@@ -48,7 +58,8 @@ TEST(EntityPassClipStackTest, CanAppendNoChange) {
   EXPECT_TRUE(recorder.GetReplayEntities().empty());
 
   Entity entity;
-  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kNoChange);
+  recorder.RecordEntity(entity, Contents::ClipCoverage::Type::kNoChange,
+                        Rect());
   EXPECT_TRUE(recorder.GetReplayEntities().empty());
 }
 
@@ -61,12 +72,14 @@ TEST(EntityPassClipStackTest, AppendCoverageNoChange) {
   EXPECT_EQ(recorder.GetClipCoverageLayers()[0].clip_depth, 0u);
 
   Entity entity;
-  recorder.AppendClipCoverage(
+  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)));
@@ -82,12 +95,14 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) {
   // Push a clip.
   Entity entity;
   entity.SetClipDepth(0);
-  recorder.AppendClipCoverage(
+  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,
@@ -97,7 +112,7 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) {
 
   // Restore the clip.
   entity.SetClipDepth(0);
-  recorder.AppendClipCoverage(
+  recorder.ApplyClipState(
       Contents::ClipCoverage{
           .type = Contents::ClipCoverage::Type::kRestore,
           .coverage = Rect::MakeLTRB(50, 50, 55, 55),
@@ -120,12 +135,14 @@ TEST(EntityPassClipStackTest, UnbalancedRestore) {
   // Restore the clip.
   Entity entity;
   entity.SetClipDepth(0);
-  recorder.AppendClipCoverage(
+  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,
@@ -143,12 +160,16 @@ TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpasses) {
   // Push a clip.
   Entity entity;
   entity.SetClipDepth(0u);
-  recorder.AppendClipCoverage(
-      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,
@@ -163,12 +184,16 @@ TEST(EntityPassClipStackTest, ClipAndRestoreWithSubpasses) {
             Rect::MakeLTRB(50, 50, 55, 55));
 
   entity.SetClipDepth(1);
-  recorder.AppendClipCoverage(
-      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));