Address comments

bdero · bdero · commit d09eba180d61 · 2023-10-05T14:59:04.000-07:00
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
@@ -3639,7 +3639,26 @@ TEST_P(AiksTest, MatrixImageFilterDoesntCullWhenTranslatedFromOffscreen) {
       .image_filter = std::make_shared<MatrixImageFilter>(
           Matrix::MakeTranslation({300, 0}), SamplerDescriptor{}),
   });
-  canvas.DrawCircle({-300, 0}, 100, {.color = Color::White()});
+  canvas.DrawCircle({-300, 0}, 100, {.color = Color::Green()});
+  canvas.Restore();
+
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
+// Render a white circle at the top left corner of the screen.
+TEST_P(AiksTest,
+       MatrixImageFilterDoesntCullWhenScaledAndTranslatedFromOffscreen) {
+  Canvas canvas;
+  canvas.Scale(GetContentScale());
+  canvas.Translate({100, 100});
+  // Draw a circle in a SaveLayer at -300, but move it back on-screen with a
+  // +300 translation applied by a SaveLayer image filter.
+  canvas.SaveLayer({
+      .image_filter = std::make_shared<MatrixImageFilter>(
+          Matrix::MakeTranslation({300, 0}) * Matrix::MakeScale({2, 2, 2}),
+          SamplerDescriptor{}),
+  });
+  canvas.DrawCircle({-150, 0}, 50, {.color = Color::Green()});
   canvas.Restore();
 
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc
@@ -641,9 +641,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
 
     // 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 its rendered, causing
-    // parent clip coverage to get misaligned with the actual area that the
-    // subpass will affect in the parent pass.
+    // save layers may transform the subpass texture after it's rendered,
+    // causing parent clip coverage to get misaligned with the actual area that
+    // the subpass will affect in the parent pass.
     ClipCoverageStack subpass_clip_coverage_stack = {ClipCoverageLayer{
         .coverage = subpass_coverage, .clip_depth = subpass->clip_depth_}};
 
diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h
@@ -142,7 +142,16 @@ class EntityPass {
   void SetEnableOffscreenCheckerboard(bool enabled);
 
   //----------------------------------------------------------------------------
-  /// @brief  Get the coverage of an unfiltered subpass.
+  /// @brief  Computes the coverage of a given subpass. This is used to
+  ///         determine the texture size of a given subpass before it's rendered
+  ///         to and passed through the subpass ImageFilter, if any.
+  ///
+  /// @param[in]  subpass         The EntityPass for which to compute
+  ///                             pre-filteredcoverage.
+  /// @param[in]  coverage_limit  Confines coverage to a specified area. This
+  ///                             hint is used to trim coverage to the root
+  ///                             framebuffer area. `std::nullopt` means no
+  ///                             coverage.
   ///
   std::optional<Rect> GetSubpassCoverage(
       const EntityPass& subpass,
