Revert "Revert "[Impeller] Fix MatrixFilter multiplication ordering for subpasses." (flutter#43978)"

This reverts commit 176457d.

Author: bdero

impeller/aiks/aiks_unittests.cc

@@ -2908,15 +2908,22 @@ TEST_P(AiksTest, DrawPictureWithText) {
 
 TEST_P(AiksTest, MatrixBackdropFilter) {
   Canvas canvas;
-  canvas.SaveLayer({}, std::nullopt,
-                   [](const FilterInput::Ref& input,
-                      const Matrix& effect_transform, bool is_subpass) {
-                     return FilterContents::MakeMatrixFilter(
-                         input, Matrix::MakeTranslation(Vector2(100, 100)), {},
-                         Matrix(), true);
-                   });
-  canvas.DrawCircle(Point(100, 100), 100,
-                    {.color = Color::Green(), .blend_mode = BlendMode::kPlus});
+  canvas.DrawPaint({.color = Color::Black()});
+  canvas.SaveLayer({}, std::nullopt);
+  {
+    canvas.DrawCircle(
+        Point(200, 200), 100,
+        {.color = Color::Green(), .blend_mode = BlendMode::kPlus});
+    // Should render a second intersecting circle, offset by 100, 100.
+    canvas.SaveLayer({}, std::nullopt,
+                     [](const FilterInput::Ref& input,
+                        const Matrix& effect_transform, bool is_subpass) {
+                       return FilterContents::MakeMatrixFilter(
+                           input, Matrix::MakeTranslation(Vector2(100, 100)),
+                           {}, Matrix(), true);
+                     });
+    canvas.Restore();
+  }
   canvas.Restore();
 
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));

impeller/entity/contents/filters/matrix_filter_contents.cc

@@ -34,11 +34,45 @@ std::optional<Entity> MatrixFilterContents::RenderFilter(
     return std::nullopt;
   }
 
-  auto& transform = is_subpass_ ? effect_transform : entity.GetTransformation();
-  snapshot->transform = transform *           //
-                        matrix_ *             //
-                        transform.Invert() *  //
-                        snapshot->transform;
+  if (is_subpass_) {
+    // There are two special quirks with how Matrix filters behave when used as
+    // subpass backdrop filters:
+    //
+    // 1. For subpass backdrop filters, the snapshot transform is always just a
+    //    translation that positions the parent pass texture correctly relative
+    //    to the subpass texture. However, this translation always needs to be
+    //    applied in screen space.
+    //
+    //    Since we know the snapshot transform will always have an identity
+    //    basis in this case, we safely reverse the order and apply the filter's
+    //    matrix within the snapshot transform space.
+    //
+    // 2. The filter's matrix needs to be applied within the space defined by
+    //    the scene's current transformation matrix (CTM). For example: If the
+    //    CTM is scaled up, then translations applied by the matrix should be
+    //    magnified accordingly.
+    //
+    //    To accomplish this, we sandwitch the filter's matrix within the CTM in
+    //    both cases. But notice that for the subpass backdrop filter case, we
+    //    use the "effect transform" instead of the Entity's transform!
+    //
+    //    That's because in the subpass backdrop filter case, the Entity's
+    //    transform isn't actually the captured CTM of the scene like it usually
+    //    is; instead, it's just a screen space translation that offsets the
+    //    backdrop texture (as mentioned above). And so we sneak the subpass's
+    //    captured CTM in through the effect transform.
+    //
+
+    snapshot->transform = snapshot->transform *  //
+                          effect_transform *     //
+                          matrix_ *              //
+                          effect_transform.Invert();
+  } else {
+    snapshot->transform = entity.GetTransformation() *           //
+                          matrix_ *                              //
+                          entity.GetTransformation().Invert() *  //
+                          snapshot->transform;
+  }
   snapshot->sampler_descriptor = sampler_descriptor_;
   return Entity::FromSnapshot(snapshot, entity.GetBlendMode(),
                               entity.GetStencilDepth());

impeller/entity/entity_pass.cc

@@ -465,12 +465,12 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
       return EntityPass::EntityResult::Skip();
     }
 
-    std::shared_ptr<Contents> backdrop_filter_contents = nullptr;
+    std::shared_ptr<Contents> subpass_backdrop_filter_contents = nullptr;
     if (subpass->backdrop_filter_proc_) {
       auto texture = pass_context.GetTexture();
       // Render the backdrop texture before any of the pass elements.
       const auto& proc = subpass->backdrop_filter_proc_;
-      backdrop_filter_contents =
+      subpass_backdrop_filter_contents =
           proc(FilterInput::Make(std::move(texture)), subpass->xformation_,
                /*is_subpass*/ true);
 
@@ -507,9 +507,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
       return EntityPass::EntityResult::Skip();
     }
 
-    auto subpass_coverage = (subpass->flood_clip_ || backdrop_filter_contents)
-                                ? coverage_limit
-                                : GetSubpassCoverage(*subpass, coverage_limit);
+    auto subpass_coverage =
+        (subpass->flood_clip_ || subpass_backdrop_filter_contents)
+            ? coverage_limit
+            : GetSubpassCoverage(*subpass, coverage_limit);
     if (!subpass_coverage.has_value()) {
       return EntityPass::EntityResult::Skip();
     }
@@ -532,17 +533,18 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
 
     // Stencil textures aren't shared between EntityPasses (as much of the
     // time they are transient).
-    if (!subpass->OnRender(renderer,                  // renderer
-                           root_pass_size,            // root_pass_size
-                           subpass_target,            // pass_target
-                           subpass_coverage->origin,  // global_pass_position
-                           subpass_coverage->origin -
-                               global_pass_position,  // local_pass_position
-                           ++pass_depth,              // pass_depth
-                           stencil_coverage_stack,    // stencil_coverage_stack
-                           subpass->stencil_depth_,   // stencil_depth_floor
-                           backdrop_filter_contents  // backdrop_filter_contents
-                           )) {
+    if (!subpass->OnRender(
+            renderer,                  // renderer
+            root_pass_size,            // root_pass_size
+            subpass_target,            // pass_target
+            subpass_coverage->origin,  // global_pass_position
+            subpass_coverage->origin -
+                global_pass_position,         // local_pass_position
+            ++pass_depth,                     // pass_depth
+            stencil_coverage_stack,           // stencil_coverage_stack
+            subpass->stencil_depth_,          // stencil_depth_floor
+            subpass_backdrop_filter_contents  // backdrop_filter_contents
+            )) {
       // Validation error messages are triggered for all `OnRender()` failure
       // cases.
       return EntityPass::EntityResult::Failure();