From 8d916f6ff192968944f9bb6a328aa5e136d346bc Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 25 Jul 2023 09:32:57 -0700 Subject: [PATCH 1/2] [Impeller] Use basis of effect transform in MatrixFilter. (#43990) Follow-up/reland of https://github.com/flutter/engine/pull/43943. Issue https://github.com/flutter/flutter/issues/130355. I've updated the golden to better reproduce the problem at hand; the second circle is a scaled down copy of the backdrop. If the matrix filter is handling the transform correctly, the circle is centered over the bottom-right-most edge (4:30) of the circle: ![image](https://github.com/flutter/engine/assets/919017/3f2e099d-9d43-4ee6-8efe-3203edad47ad) (cherry picked from commit 3e5ae6a4f65195e96505b8f047d16e1fa4b2b7dc) --- impeller/aiks/aiks_unittests.cc | 28 +++++++++++++++ .../filters/matrix_filter_contents.cc | 19 ++++++++++- impeller/entity/entity_pass.cc | 34 ++++++++++--------- 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index e91c705f68a12..297bb7db23e45 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2837,5 +2837,33 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, MatrixBackdropFilter) { + Canvas canvas; + canvas.DrawPaint({.color = Color::Black()}); + canvas.SaveLayer({}, std::nullopt); + { + canvas.DrawCircle(Point(200, 200), 100, + {.color = Color::Green().WithAlpha(0.5), + .blend_mode = BlendMode::kPlus}); + // Should render a second circle, centered on the bottom-right-most edge of + // the circle. + canvas.SaveLayer({}, std::nullopt, + [](const FilterInput::Ref& input, + const Matrix& effect_transform, bool is_subpass) { + Matrix matrix = + Matrix::MakeTranslation(Vector2(1, 1) * + (100 + 100 * k1OverSqrt2)) * + Matrix::MakeScale(Vector2(1, 1) * 0.2) * + Matrix::MakeTranslation(Vector2(-100, -100)); + return FilterContents::MakeMatrixFilter( + input, matrix, {}, Matrix(), true); + }); + canvas.Restore(); + } + canvas.Restore(); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index b9c9c2424df7d..81d1b51beaec9 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -34,11 +34,28 @@ std::optional MatrixFilterContents::RenderFilter( return std::nullopt; } - auto& transform = is_subpass_ ? effect_transform : entity.GetTransformation(); + // 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. + + auto transform = is_subpass_ ? effect_transform.Basis() + : entity.GetTransformation().Basis(); snapshot->transform = transform * // matrix_ * // transform.Invert() * // snapshot->transform; + snapshot->sampler_descriptor = sampler_descriptor_; return Entity::FromSnapshot(snapshot, entity.GetBlendMode(), entity.GetStencilDepth()); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 8b1aac582550e..cac4d5f4d8a67 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -434,12 +434,12 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( return EntityPass::EntityResult::Skip(); } - std::shared_ptr backdrop_filter_contents = nullptr; + std::shared_ptr 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); @@ -476,9 +476,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(); } @@ -501,17 +502,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(); From 55621cae3d581241c641e8c37d654932624be6fd Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 25 Jul 2023 22:17:08 -0700 Subject: [PATCH 2/2] [Impeller] Only strip the translation from backdrop filter effect transforms. (#44029) Resolves https://github.com/flutter/flutter/issues/131305. (cherry picked from commit a4a54395c8c6a7ceac5aa759a8ad3d8e0441dbd2) --- impeller/aiks/aiks_unittests.cc | 32 +++++++++++++++++++ .../filters/matrix_filter_contents.cc | 3 +- impeller/entity/entity_pass.cc | 6 ++-- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 297bb7db23e45..46a732bb7d6f9 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2837,6 +2837,38 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, MatrixSaveLayerFilter) { + Canvas canvas; + canvas.DrawPaint({.color = Color::Black()}); + canvas.SaveLayer({}, std::nullopt); + { + canvas.DrawCircle(Point(200, 200), 100, + {.color = Color::Green().WithAlpha(0.5), + .blend_mode = BlendMode::kPlus}); + // Should render a second circle, centered on the bottom-right-most edge of + // the circle. + canvas.SaveLayer({.image_filter = + [](const FilterInput::Ref& input, + const Matrix& effect_transform, bool is_subpass) { + Matrix matrix = + Matrix::MakeTranslation( + Vector2(1, 1) * (100 + 100 * k1OverSqrt2)) * + Matrix::MakeScale(Vector2(1, 1) * 0.2) * + Matrix::MakeTranslation(Vector2(-100, -100)); + return FilterContents::MakeMatrixFilter( + input, matrix, {}, Matrix(), true); + }}, + std::nullopt); + canvas.DrawCircle(Point(200, 200), 100, + {.color = Color::Green().WithAlpha(0.5), + .blend_mode = BlendMode::kPlus}); + canvas.Restore(); + } + canvas.Restore(); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, MatrixBackdropFilter) { Canvas canvas; canvas.DrawPaint({.color = Color::Black()}); diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 81d1b51beaec9..55bc05f28a099 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -49,8 +49,7 @@ std::optional MatrixFilterContents::RenderFilter( // mentioned above). And so we sneak the subpass's captured CTM in through the // effect transform. - auto transform = is_subpass_ ? effect_transform.Basis() - : entity.GetTransformation().Basis(); + auto transform = is_subpass_ ? effect_transform : entity.GetTransformation(); snapshot->transform = transform * // matrix_ * // transform.Invert() * // diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index cac4d5f4d8a67..93d706bdfd39e 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -439,9 +439,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( auto texture = pass_context.GetTexture(); // Render the backdrop texture before any of the pass elements. const auto& proc = subpass->backdrop_filter_proc_; - subpass_backdrop_filter_contents = - proc(FilterInput::Make(std::move(texture)), subpass->xformation_, - /*is_subpass*/ true); + subpass_backdrop_filter_contents = proc( + FilterInput::Make(std::move(texture)), subpass->xformation_.Basis(), + /*is_subpass*/ true); // The subpass will need to read from the current pass texture when // rendering the backdrop, so if there's an active pass, end it prior to