From 2f9ad87f84932548a15c5adc2f15f959cba22c11 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 14 May 2024 13:34:06 -0700 Subject: [PATCH 01/23] [Impeller] fixed matrix transform --- impeller/display_list/dl_golden_unittests.cc | 91 ++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index 81f9e70b33d04..16caffdbe5b15 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -48,5 +48,96 @@ TEST_P(DlGoldenTest, CanRenderImage) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +TEST_P(DlGoldenTest, Bug147807) { + auto draw = [](DlCanvas* canvas, const std::vector>& images) { + canvas->Transform2DAffine(2, 0, 0, 0, 2, 0); + DlPaint paint; + paint.setColor(DlColor(0xfffef7ff)); + canvas->DrawRect(SkRect::MakeLTRB(0, 0, 375, 667), paint); + paint.setColor(DlColor(0xffff9800)); + canvas->DrawRect(SkRect::MakeLTRB(0, 0, 187.5, 333.5), paint); + paint.setColor(DlColor(0xff9c27b0)); + canvas->DrawRect(SkRect::MakeLTRB(187.5, 0, 375, 333.5), paint); + paint.setColor(DlColor(0xff4caf50)); + canvas->DrawRect(SkRect::MakeLTRB(0, 333.5, 187.5, 667), paint); + paint.setColor(DlColor(0xfff44336)); + canvas->DrawRect(SkRect::MakeLTRB(187.5, 333.5, 375, 667), paint); + canvas->Save(); + { + canvas->Transform2DAffine(1, 2.449293705170336e-16, -1.70530256582424e-13, + -2.449293705170336e-16, 1, + 1.13686837721616e-13); + canvas->DrawShadow(SkPath().addRect(SkRect::MakeLTRB(303, 595, 359, 651)), + DlColor(0xffebddff), 6, false, 2); + canvas->DrawPath(SkPath().addRect(SkRect::MakeLTRB(303, 595, 359, 651)), + DlPaint().setColor(DlColor(0xffebddff))); + canvas->Save(); + { + canvas->Translate(303, 595); + canvas->ClipRRect(SkRRect::MakeRect(SkRect::MakeLTRB(0, 0, 56, 56)), + DlCanvas::ClipOp::kIntersect, true); + canvas->Save(); + { + canvas->ClipRRect(SkRRect::MakeOval(SkRect::MakeLTRB(0, 0, 56, 56)), + DlCanvas::ClipOp::kIntersect, true); + canvas->DrawCircle( + SkPoint::Make(21.55413055419922, 25.02498245239258), + 12.15186500549316, DlPaint().setColor(DlColor(0x6230f46))); + } + canvas->Restore(); + canvas->Save(); + { + canvas->ClipRRect(SkRRect::MakeOval(SkRect::MakeLTRB(0, 0, 56, 56)), + DlCanvas::ClipOp::kIntersect, true); + canvas->DrawRect(SkRect::MakeLTRB(0, 0, 56, 56), + DlPaint().setColor(DlColor(0x9bcbcbc))); + } + canvas->Restore(); + } + canvas->Restore(); + //drawTextFrame + } + canvas->Restore(); + + canvas->Save(); + { + canvas->ClipRRect( + SkRRect::MakeOval(SkRect::MakeLTRB(201.25, 10, 361.25, 170)), + DlCanvas::ClipOp::kIntersect, true); + SkRect save_layer_bounds = SkRect::MakeLTRB(201.25, 10, 361.25, 170); + DlMatrixImageFilter backdrop( + SkMatrix::MakeAll(3, 0, -299.25, 0, 3, -949, 0, 0, 1), + DlImageSampling::kLinear); + canvas->SaveLayer(&save_layer_bounds, /*paint=*/nullptr, &backdrop); + { + canvas->Translate(201.25, 10); + auto paint = DlPaint() + .setAntiAlias(true) + .setColor(DlColor(0xff2196f3)) + .setStrokeWidth(5) + .setDrawStyle(DlDrawStyle::kStroke); + canvas->DrawCircle(SkPoint::Make(80, 80), 80, paint); + paint.setColor(DlColor(0xfff44336)); + paint.setStrokeWidth(1.666666626930237); + canvas->DrawRect(SkRect::MakeLTRB(70, 70, 90, 90), paint); + paint.setColor(DlColor(0xff000000)); + canvas->DrawLine(SkPoint::Make(75.19999694824219, 80), + SkPoint::Make(84.80000305175781, 80), paint); + canvas->DrawLine(SkPoint::Make(80, 75.19999694824219), + SkPoint::Make(80, 84.80000305175781), paint); + } + canvas->Restore(); + } + canvas->Restore(); + }; + + DisplayListBuilder builder; + std::vector> images; + images.emplace_back(CreateDlImageForFixture("kalimba.jpg")); + draw(&builder, images); + + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + } // namespace testing } // namespace flutter From d127c47fd126f47778036c58ce6c1ac93ebeceb9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 14 May 2024 13:58:12 -0700 Subject: [PATCH 02/23] cleaned up test --- impeller/display_list/dl_golden_unittests.cc | 59 +++----------------- 1 file changed, 9 insertions(+), 50 deletions(-) diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index 16caffdbe5b15..d0cded0e1bb89 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -13,6 +13,7 @@ namespace testing { using impeller::PlaygroundBackend; using impeller::PlaygroundTest; +using impeller::Point; INSTANTIATE_PLAYGROUND_SUITE(DlGoldenTest); @@ -49,8 +50,10 @@ TEST_P(DlGoldenTest, CanRenderImage) { } TEST_P(DlGoldenTest, Bug147807) { - auto draw = [](DlCanvas* canvas, const std::vector>& images) { - canvas->Transform2DAffine(2, 0, 0, 0, 2, 0); + Point content_scale = GetContentScale(); + auto draw = [content_scale](DlCanvas* canvas, + const std::vector>& images) { + canvas->Transform2DAffine(content_scale.x, 0, 0, 0, content_scale.y, 0); DlPaint paint; paint.setColor(DlColor(0xfffef7ff)); canvas->DrawRect(SkRect::MakeLTRB(0, 0, 375, 667), paint); @@ -62,42 +65,6 @@ TEST_P(DlGoldenTest, Bug147807) { canvas->DrawRect(SkRect::MakeLTRB(0, 333.5, 187.5, 667), paint); paint.setColor(DlColor(0xfff44336)); canvas->DrawRect(SkRect::MakeLTRB(187.5, 333.5, 375, 667), paint); - canvas->Save(); - { - canvas->Transform2DAffine(1, 2.449293705170336e-16, -1.70530256582424e-13, - -2.449293705170336e-16, 1, - 1.13686837721616e-13); - canvas->DrawShadow(SkPath().addRect(SkRect::MakeLTRB(303, 595, 359, 651)), - DlColor(0xffebddff), 6, false, 2); - canvas->DrawPath(SkPath().addRect(SkRect::MakeLTRB(303, 595, 359, 651)), - DlPaint().setColor(DlColor(0xffebddff))); - canvas->Save(); - { - canvas->Translate(303, 595); - canvas->ClipRRect(SkRRect::MakeRect(SkRect::MakeLTRB(0, 0, 56, 56)), - DlCanvas::ClipOp::kIntersect, true); - canvas->Save(); - { - canvas->ClipRRect(SkRRect::MakeOval(SkRect::MakeLTRB(0, 0, 56, 56)), - DlCanvas::ClipOp::kIntersect, true); - canvas->DrawCircle( - SkPoint::Make(21.55413055419922, 25.02498245239258), - 12.15186500549316, DlPaint().setColor(DlColor(0x6230f46))); - } - canvas->Restore(); - canvas->Save(); - { - canvas->ClipRRect(SkRRect::MakeOval(SkRect::MakeLTRB(0, 0, 56, 56)), - DlCanvas::ClipOp::kIntersect, true); - canvas->DrawRect(SkRect::MakeLTRB(0, 0, 56, 56), - DlPaint().setColor(DlColor(0x9bcbcbc))); - } - canvas->Restore(); - } - canvas->Restore(); - //drawTextFrame - } - canvas->Restore(); canvas->Save(); { @@ -105,9 +72,10 @@ TEST_P(DlGoldenTest, Bug147807) { SkRRect::MakeOval(SkRect::MakeLTRB(201.25, 10, 361.25, 170)), DlCanvas::ClipOp::kIntersect, true); SkRect save_layer_bounds = SkRect::MakeLTRB(201.25, 10, 361.25, 170); - DlMatrixImageFilter backdrop( - SkMatrix::MakeAll(3, 0, -299.25, 0, 3, -949, 0, 0, 1), - DlImageSampling::kLinear); + DlMatrixImageFilter backdrop(SkMatrix::MakeAll(3, 0, -299.25, // + 0, 3, -949, // + 0, 0, 1), + DlImageSampling::kLinear); canvas->SaveLayer(&save_layer_bounds, /*paint=*/nullptr, &backdrop); { canvas->Translate(201.25, 10); @@ -117,14 +85,6 @@ TEST_P(DlGoldenTest, Bug147807) { .setStrokeWidth(5) .setDrawStyle(DlDrawStyle::kStroke); canvas->DrawCircle(SkPoint::Make(80, 80), 80, paint); - paint.setColor(DlColor(0xfff44336)); - paint.setStrokeWidth(1.666666626930237); - canvas->DrawRect(SkRect::MakeLTRB(70, 70, 90, 90), paint); - paint.setColor(DlColor(0xff000000)); - canvas->DrawLine(SkPoint::Make(75.19999694824219, 80), - SkPoint::Make(84.80000305175781, 80), paint); - canvas->DrawLine(SkPoint::Make(80, 75.19999694824219), - SkPoint::Make(80, 84.80000305175781), paint); } canvas->Restore(); } @@ -133,7 +93,6 @@ TEST_P(DlGoldenTest, Bug147807) { DisplayListBuilder builder; std::vector> images; - images.emplace_back(CreateDlImageForFixture("kalimba.jpg")); draw(&builder, images); ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); From 464a8c6c1143e8ff4c22ce22845fc58d6d3627e4 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 24 Jul 2023 14:07:43 -0700 Subject: [PATCH 03/23] [Impeller] Fix MatrixFilter multiplication ordering for subpasses. (#43943) Resolves https://github.com/flutter/flutter/issues/130355. When drawing a backdrop filter, we offset the backdrop entity with `-local_pass_position` in order to align the backdrop texture with the position of the pass it's being drawn to. When rendering the filter, this transform propagates to the filter snapshot. Prior to this change, that offset was being applied within the space of the backdrop filter's matrix, which is wrong! It needs to be applied in screen space. We didn't notice this prior to the coverage hint optimization because we were previously forcing subpass sizes to match the parent whenever a backdrop filter was present. This one was very difficult to reason through at first, and so I added a detailed comment to explain the rationale behind the behavioral differences between the backdrop filter and non-backdrop filter cases of the matrix filter. --- .../filters/matrix_filter_contents.cc | 62 ++++++++++++------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 9681e8a6d6ad5..634fea3db9286 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -40,29 +40,45 @@ std::optional MatrixFilterContents::RenderFilter( return std::nullopt; } - // The filter's matrix needs to be applied within the space defined by the - // scene's current transform matrix (CTM). For example: If the CTM is - // scaled up, then translations applied by the matrix should be magnified - // accordingly. - // - // To accomplish this, we sandwich 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 = rendering_mode_ == Entity::RenderingMode::kSubpass - ? effect_transform - : entity.GetTransform(); - snapshot->transform = transform * // - matrix_ * // - transform.Invert() * // - snapshot->transform; - + if (rendering_mode_ == Entity::RenderingMode::kSubpass) { + // 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.GetTransform() * // + matrix_ * // + entity.GetTransform().Invert() * // + snapshot->transform; + } snapshot->sampler_descriptor = sampler_descriptor_; if (!snapshot.has_value()) { return std::nullopt; From a6fddde0d28a89735033fd94e9af2fcee139e2b1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 15 May 2024 14:59:03 -0700 Subject: [PATCH 04/23] started adding coverage tests --- impeller/entity/BUILD.gn | 1 + .../contents/filters/filter_contents.cc | 1 + .../entity/contents/filters/filter_contents.h | 2 ++ .../filters/matrix_filter_contents.cc | 23 +++++++++++++++---- .../contents/filters/matrix_filter_contents.h | 4 ++++ .../matrix_filter_contents_unittests.cc | 23 +++++++++++++++++++ 6 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 impeller/entity/contents/filters/matrix_filter_contents_unittests.cc diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index c529d46684393..00b9a6dce0006 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -240,6 +240,7 @@ impeller_component("entity_unittests") { sources = [ "contents/clip_contents_unittests.cc", "contents/filters/gaussian_blur_filter_contents_unittests.cc", + "contents/filters/matrix_filter_contents_unittests.cc", "contents/filters/inputs/filter_input_unittests.cc", "contents/host_buffer_unittests.cc", "contents/tiled_texture_contents_unittests.cc", diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index 46cfab8c874d9..397a9e9e314d6 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -153,6 +153,7 @@ std::optional FilterContents::GetLocalCoverage( auto coverage = GetFilterCoverage(inputs_, local_entity, effect_transform_); auto coverage_hint = GetCoverageHint(); if (coverage_hint.has_value() && coverage.has_value()) { + FML_DLOG(ERROR) << coverage_hint.value() << " " << coverage.value(); coverage = coverage->Intersection(coverage_hint.value()); } diff --git a/impeller/entity/contents/filters/filter_contents.h b/impeller/entity/contents/filters/filter_contents.h index 27ecd6c520ed2..4e88d7a0f8432 100644 --- a/impeller/entity/contents/filters/filter_contents.h +++ b/impeller/entity/contents/filters/filter_contents.h @@ -192,6 +192,8 @@ class FilterContents : public Contents { /// is used in this case. virtual void SetRenderingMode(Entity::RenderingMode rendering_mode); + //virtual const Matrix& GetCoverageHintTransform() const { return Matrix(); }; + private: /// @brief Internal utility method for |GetLocalCoverage| that computes /// the output coverage of this filter across the specified inputs, diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 634fea3db9286..a04f75d614d3e 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -75,7 +75,7 @@ std::optional MatrixFilterContents::RenderFilter( effect_transform.Invert(); } else { snapshot->transform = entity.GetTransform() * // - matrix_ * // + matrix_ * // entity.GetTransform().Invert() * // snapshot->transform; } @@ -111,12 +111,25 @@ std::optional MatrixFilterContents::GetFilterCoverage( if (!coverage.has_value()) { return std::nullopt; } + +// Matrix transform; +// if (rendering_mode_ == Entity::RenderingMode::kSubpass) { +// transform = inputs[0]->GetTransform(entity) * // +// effect_transform * // +// matrix_ * // +// effect_transform.Invert(); +// } else { +// transform = entity.GetTransform() * // +// matrix_ * // +// entity.GetTransform().Invert() * // +// inputs[0]->GetTransform(entity); +// } auto& m = rendering_mode_ == Entity::RenderingMode::kSubpass - ? effect_transform - : inputs[0]->GetTransform(entity); + ? effect_transform + : inputs[0]->GetTransform(entity); auto transform = m * // - matrix_ * // - m.Invert(); // + matrix_ * // + m.Invert(); // return coverage->TransformBounds(transform); } diff --git a/impeller/entity/contents/filters/matrix_filter_contents.h b/impeller/entity/contents/filters/matrix_filter_contents.h index dd6334e4169c8..a232e971b2747 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.h +++ b/impeller/entity/contents/filters/matrix_filter_contents.h @@ -32,6 +32,10 @@ class MatrixFilterContents final : public FilterContents { const Entity& entity, const Matrix& effect_transform) const override; +// const Matrix& GetCoverageHintTransform() const override { +// return matrix_; +// } + private: // |FilterContents| std::optional RenderFilter( diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc new file mode 100644 index 0000000000000..393c1383b7704 --- /dev/null +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -0,0 +1,23 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/testing.h" +#include "gmock/gmock.h" +#include "impeller/entity/contents/filters/matrix_filter_contents.h" + +#if FML_OS_MACOSX +#define IMPELLER_RAND arc4random +#else +#define IMPELLER_RAND rand +#endif + +namespace impeller { +namespace testing { +TEST(MatrixFilterContentsTest, Create) { + MatrixFilterContents contents; + EXPECT_TRUE(contents.IsTranslationOnly()); +} + +} // namespace testing +} // namespace impeller From 7100e5c43c6004a2ea44439fbd3914d8004b1d64 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 15 May 2024 15:51:12 -0700 Subject: [PATCH 05/23] expanded tests --- .../matrix_filter_contents_unittests.cc | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index 393c1383b7704..7dc1d8c0cb119 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -6,12 +6,6 @@ #include "gmock/gmock.h" #include "impeller/entity/contents/filters/matrix_filter_contents.h" -#if FML_OS_MACOSX -#define IMPELLER_RAND arc4random -#else -#define IMPELLER_RAND rand -#endif - namespace impeller { namespace testing { TEST(MatrixFilterContentsTest, Create) { @@ -19,5 +13,37 @@ TEST(MatrixFilterContentsTest, Create) { EXPECT_TRUE(contents.IsTranslationOnly()); } +TEST(MatrixFilterContentsTest, CoverageEmpty) { + MatrixFilterContents contents; + FilterInput::Vector inputs = {}; + Entity entity; + std::optional coverage = + contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix()); + ASSERT_FALSE(coverage.has_value()); +} + +TEST(MatrixFilterContentsTest, CoverageSimple) { + MatrixFilterContents contents; + FilterInput::Vector inputs = { + FilterInput::Make(Rect::MakeLTRB(10, 10, 110, 110))}; + Entity entity; + std::optional coverage = + contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix()); + + ASSERT_EQ(coverage, Rect::MakeLTRB(10, 10, 110, 110)); +} + +TEST(MatrixFilterContentsTest, Coverage2x) { + MatrixFilterContents contents; + contents.SetMatrix(Matrix::MakeScale({2.0, 2.0, 1.0})); + FilterInput::Vector inputs = { + FilterInput::Make(Rect::MakeXYWH(10, 10, 100, 100))}; + Entity entity; + std::optional coverage = + contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix()); + + ASSERT_EQ(coverage, Rect::MakeXYWH(20, 20, 200, 200)); +} + } // namespace testing } // namespace impeller From 0f18297b81f345bdae075812bc864eda09f8fd74 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 08:52:33 -0700 Subject: [PATCH 06/23] added test --- .../filters/matrix_filter_contents_unittests.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index 7dc1d8c0cb119..784ecc22f7347 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -45,5 +45,16 @@ TEST(MatrixFilterContentsTest, Coverage2x) { ASSERT_EQ(coverage, Rect::MakeXYWH(20, 20, 200, 200)); } +TEST(MatrixFilterContentsTest, Coverage2xEffect) { + MatrixFilterContents contents; + FilterInput::Vector inputs = { + FilterInput::Make(Rect::MakeXYWH(10, 10, 100, 100))}; + Entity entity; + std::optional coverage = contents.GetFilterCoverage( + inputs, entity, /*effect_transform=*/Matrix::MakeScale({2.0, 2.0, 1.0})); + + ASSERT_EQ(coverage, Rect::MakeXYWH(10, 10, 100, 100)); +} + } // namespace testing } // namespace impeller From a63c263473039f721b643a7150a4fc62337544a6 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 09:45:06 -0700 Subject: [PATCH 07/23] added more tests --- .../matrix_filter_contents_unittests.cc | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index 784ecc22f7347..42cd89a6b2e1a 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -5,9 +5,43 @@ #include "flutter/testing/testing.h" #include "gmock/gmock.h" #include "impeller/entity/contents/filters/matrix_filter_contents.h" +#include "impeller/entity/entity_playground.h" +#include "impeller/geometry/geometry_asserts.h" namespace impeller { namespace testing { + +class MatrixFilterContentsTest : public EntityPlayground { + public: + /// Create a texture that has been cleared to transparent black. + std::shared_ptr MakeTexture(ISize size) { + std::shared_ptr command_buffer = + GetContentContext()->GetContext()->CreateCommandBuffer(); + if (!command_buffer) { + return nullptr; + } + + auto render_target = GetContentContext()->MakeSubpass( + "Clear Subpass", size, command_buffer, + [](const ContentContext&, RenderPass&) { return true; }); + + if (!GetContentContext() + ->GetContext() + ->GetCommandQueue() + ->Submit(/*buffers=*/{command_buffer}) + .ok()) { + return nullptr; + } + + if (render_target.ok()) { + return render_target.value().GetRenderTargetTexture(); + } + return nullptr; + } +}; + +INSTANTIATE_PLAYGROUND_SUITE(MatrixFilterContentsTest); + TEST(MatrixFilterContentsTest, Create) { MatrixFilterContents contents; EXPECT_TRUE(contents.IsTranslationOnly()); @@ -56,5 +90,60 @@ TEST(MatrixFilterContentsTest, Coverage2xEffect) { ASSERT_EQ(coverage, Rect::MakeXYWH(10, 10, 100, 100)); } +TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageIdentity) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + MatrixFilterContents contents; + contents.SetInputs({FilterInput::Make(texture)}); + + Entity entity; + entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); + + std::shared_ptr renderer = GetContentContext(); + std::optional result = + contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); + EXPECT_TRUE(result.has_value()); + if (result.has_value()) { + EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); + std::optional result_coverage = result.value().GetCoverage(); + std::optional contents_coverage = contents.GetCoverage(entity); + EXPECT_TRUE(result_coverage.has_value()); + EXPECT_TRUE(contents_coverage.has_value()); + if (result_coverage.has_value() && contents_coverage.has_value()) { + EXPECT_TRUE(RectNear(contents_coverage.value(), + Rect::MakeXYWH(100, 200, 100, 100))); + EXPECT_TRUE(RectNear(result_coverage.value(), + Rect::MakeXYWH(100, 200, 100, 100))); + } + } +} + +TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + MatrixFilterContents contents; + contents.SetInputs({FilterInput::Make(texture)}); + contents.SetMatrix(Matrix::MakeTranslation({50, 100, 0})); + + Entity entity; + entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); + + std::shared_ptr renderer = GetContentContext(); + std::optional result = + contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); + EXPECT_TRUE(result.has_value()); + if (result.has_value()) { + EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); + std::optional result_coverage = result.value().GetCoverage(); + std::optional contents_coverage = contents.GetCoverage(entity); + EXPECT_TRUE(result_coverage.has_value()); + EXPECT_TRUE(contents_coverage.has_value()); + if (result_coverage.has_value() && contents_coverage.has_value()) { + EXPECT_TRUE(RectNear(contents_coverage.value(), + Rect::MakeXYWH(150, 300, 100, 100))); + EXPECT_TRUE(RectNear(result_coverage.value(), + Rect::MakeXYWH(150, 300, 100, 100))); + } + } +} + } // namespace testing } // namespace impeller From e22fa1b0480c4c063da3d115d1e54550d1b72237 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 10:18:21 -0700 Subject: [PATCH 08/23] expanded tests --- .../matrix_filter_contents_unittests.cc | 70 +++++++++++-------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index 42cd89a6b2e1a..c55bc5bb0b9ff 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -90,6 +90,24 @@ TEST(MatrixFilterContentsTest, Coverage2xEffect) { ASSERT_EQ(coverage, Rect::MakeXYWH(10, 10, 100, 100)); } +namespace { +void expectRenderCoverageEqual(const std::optional& result, + const std::optional contents_coverage, + const Rect& expected) { + EXPECT_TRUE(result.has_value()); + if (result.has_value()) { + EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); + std::optional result_coverage = result.value().GetCoverage(); + EXPECT_TRUE(result_coverage.has_value()); + EXPECT_TRUE(contents_coverage.has_value()); + if (result_coverage.has_value() && contents_coverage.has_value()) { + EXPECT_TRUE(RectNear(contents_coverage.value(), expected)); + EXPECT_TRUE(RectNear(result_coverage.value(), expected)); + } + } +} +} // namespace + TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageIdentity) { std::shared_ptr texture = MakeTexture(ISize(100, 100)); MatrixFilterContents contents; @@ -101,20 +119,8 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageIdentity) { std::shared_ptr renderer = GetContentContext(); std::optional result = contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); - EXPECT_TRUE(result.has_value()); - if (result.has_value()) { - EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); - std::optional result_coverage = result.value().GetCoverage(); - std::optional contents_coverage = contents.GetCoverage(entity); - EXPECT_TRUE(result_coverage.has_value()); - EXPECT_TRUE(contents_coverage.has_value()); - if (result_coverage.has_value() && contents_coverage.has_value()) { - EXPECT_TRUE(RectNear(contents_coverage.value(), - Rect::MakeXYWH(100, 200, 100, 100))); - EXPECT_TRUE(RectNear(result_coverage.value(), - Rect::MakeXYWH(100, 200, 100, 100))); - } - } + expectRenderCoverageEqual(result, contents.GetCoverage(entity), + Rect::MakeXYWH(100, 200, 100, 100)); } TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { @@ -122,6 +128,7 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { MatrixFilterContents contents; contents.SetInputs({FilterInput::Make(texture)}); contents.SetMatrix(Matrix::MakeTranslation({50, 100, 0})); + contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); Entity entity; entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); @@ -129,20 +136,27 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { std::shared_ptr renderer = GetContentContext(); std::optional result = contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); - EXPECT_TRUE(result.has_value()); - if (result.has_value()) { - EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); - std::optional result_coverage = result.value().GetCoverage(); - std::optional contents_coverage = contents.GetCoverage(entity); - EXPECT_TRUE(result_coverage.has_value()); - EXPECT_TRUE(contents_coverage.has_value()); - if (result_coverage.has_value() && contents_coverage.has_value()) { - EXPECT_TRUE(RectNear(contents_coverage.value(), - Rect::MakeXYWH(150, 300, 100, 100))); - EXPECT_TRUE(RectNear(result_coverage.value(), - Rect::MakeXYWH(150, 300, 100, 100))); - } - } + expectRenderCoverageEqual(result, contents.GetCoverage(entity), + Rect::MakeXYWH(150, 300, 100, 100)); +} + +TEST_P(MatrixFilterContentsTest, + RenderCoverageMatchesGetCoverageSubpassTranslate) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + MatrixFilterContents contents; + contents.SetInputs({FilterInput::Make(texture)}); + contents.SetMatrix(Matrix::MakeTranslation({50, 100, 0})); + contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); + contents.SetRenderingMode(Entity::RenderingMode::kSubpass); + + Entity entity; + entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); + + std::shared_ptr renderer = GetContentContext(); + std::optional result = + contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); + expectRenderCoverageEqual(result, contents.GetCoverage(entity), + Rect::MakeXYWH(200, 400, 100, 100)); } } // namespace testing From 05fcaf0e9cae04acca0f878b9b24caaffe610fc6 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 10:42:25 -0700 Subject: [PATCH 09/23] added test that breaks --- .../matrix_filter_contents_unittests.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index c55bc5bb0b9ff..4eaede3914b91 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -159,5 +159,24 @@ TEST_P(MatrixFilterContentsTest, Rect::MakeXYWH(200, 400, 100, 100)); } +TEST_P(MatrixFilterContentsTest, + RenderCoverageMatchesGetCoverageSubpassScale) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + MatrixFilterContents contents; + contents.SetInputs({FilterInput::Make(texture)}); + contents.SetMatrix(Matrix::MakeScale({3, 3, 1})); + contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); + contents.SetRenderingMode(Entity::RenderingMode::kSubpass); + + Entity entity; + entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); + + std::shared_ptr renderer = GetContentContext(); + std::optional result = + contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); + expectRenderCoverageEqual(result, contents.GetCoverage(entity), + Rect::MakeXYWH(300, 600, 300, 300)); +} + } // namespace testing } // namespace impeller From c2aecb26d4e2bec74ec1d48c8a92599bbdb91981 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 10:43:01 -0700 Subject: [PATCH 10/23] format --- impeller/entity/BUILD.gn | 2 +- .../entity/contents/filters/filter_contents.h | 3 +- .../filters/matrix_filter_contents.cc | 32 +++++++++---------- .../contents/filters/matrix_filter_contents.h | 6 ++-- .../matrix_filter_contents_unittests.cc | 3 +- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index 00b9a6dce0006..fbf419da1abf5 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -240,8 +240,8 @@ impeller_component("entity_unittests") { sources = [ "contents/clip_contents_unittests.cc", "contents/filters/gaussian_blur_filter_contents_unittests.cc", - "contents/filters/matrix_filter_contents_unittests.cc", "contents/filters/inputs/filter_input_unittests.cc", + "contents/filters/matrix_filter_contents_unittests.cc", "contents/host_buffer_unittests.cc", "contents/tiled_texture_contents_unittests.cc", "entity_pass_target_unittests.cc", diff --git a/impeller/entity/contents/filters/filter_contents.h b/impeller/entity/contents/filters/filter_contents.h index 4e88d7a0f8432..0750bf4335cdc 100644 --- a/impeller/entity/contents/filters/filter_contents.h +++ b/impeller/entity/contents/filters/filter_contents.h @@ -192,7 +192,8 @@ class FilterContents : public Contents { /// is used in this case. virtual void SetRenderingMode(Entity::RenderingMode rendering_mode); - //virtual const Matrix& GetCoverageHintTransform() const { return Matrix(); }; + // virtual const Matrix& GetCoverageHintTransform() const { return Matrix(); + // }; private: /// @brief Internal utility method for |GetLocalCoverage| that computes diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index a04f75d614d3e..95c5d0f461d9c 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -112,24 +112,24 @@ std::optional MatrixFilterContents::GetFilterCoverage( return std::nullopt; } -// Matrix transform; -// if (rendering_mode_ == Entity::RenderingMode::kSubpass) { -// transform = inputs[0]->GetTransform(entity) * // -// effect_transform * // -// matrix_ * // -// effect_transform.Invert(); -// } else { -// transform = entity.GetTransform() * // -// matrix_ * // -// entity.GetTransform().Invert() * // -// inputs[0]->GetTransform(entity); -// } + // Matrix transform; + // if (rendering_mode_ == Entity::RenderingMode::kSubpass) { + // transform = inputs[0]->GetTransform(entity) * // + // effect_transform * // + // matrix_ * // + // effect_transform.Invert(); + // } else { + // transform = entity.GetTransform() * // + // matrix_ * // + // entity.GetTransform().Invert() * // + // inputs[0]->GetTransform(entity); + // } auto& m = rendering_mode_ == Entity::RenderingMode::kSubpass - ? effect_transform - : inputs[0]->GetTransform(entity); + ? effect_transform + : inputs[0]->GetTransform(entity); auto transform = m * // - matrix_ * // - m.Invert(); // + matrix_ * // + m.Invert(); // return coverage->TransformBounds(transform); } diff --git a/impeller/entity/contents/filters/matrix_filter_contents.h b/impeller/entity/contents/filters/matrix_filter_contents.h index a232e971b2747..4917fcb51677b 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.h +++ b/impeller/entity/contents/filters/matrix_filter_contents.h @@ -32,9 +32,9 @@ class MatrixFilterContents final : public FilterContents { const Entity& entity, const Matrix& effect_transform) const override; -// const Matrix& GetCoverageHintTransform() const override { -// return matrix_; -// } + // const Matrix& GetCoverageHintTransform() const override { + // return matrix_; + // } private: // |FilterContents| diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index 4eaede3914b91..6d781a3544c52 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -159,8 +159,7 @@ TEST_P(MatrixFilterContentsTest, Rect::MakeXYWH(200, 400, 100, 100)); } -TEST_P(MatrixFilterContentsTest, - RenderCoverageMatchesGetCoverageSubpassScale) { +TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageSubpassScale) { std::shared_ptr texture = MakeTexture(ISize(100, 100)); MatrixFilterContents contents; contents.SetInputs({FilterInput::Make(texture)}); From 62c200a0e15afa3e1d65b4d0304eefffec4c1dd2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 10:44:10 -0700 Subject: [PATCH 11/23] removed stray comments --- impeller/entity/contents/filters/filter_contents.h | 3 --- .../contents/filters/matrix_filter_contents.cc | 12 ------------ 2 files changed, 15 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.h b/impeller/entity/contents/filters/filter_contents.h index 0750bf4335cdc..27ecd6c520ed2 100644 --- a/impeller/entity/contents/filters/filter_contents.h +++ b/impeller/entity/contents/filters/filter_contents.h @@ -192,9 +192,6 @@ class FilterContents : public Contents { /// is used in this case. virtual void SetRenderingMode(Entity::RenderingMode rendering_mode); - // virtual const Matrix& GetCoverageHintTransform() const { return Matrix(); - // }; - private: /// @brief Internal utility method for |GetLocalCoverage| that computes /// the output coverage of this filter across the specified inputs, diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 95c5d0f461d9c..bcf355c10737a 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -112,18 +112,6 @@ std::optional MatrixFilterContents::GetFilterCoverage( return std::nullopt; } - // Matrix transform; - // if (rendering_mode_ == Entity::RenderingMode::kSubpass) { - // transform = inputs[0]->GetTransform(entity) * // - // effect_transform * // - // matrix_ * // - // effect_transform.Invert(); - // } else { - // transform = entity.GetTransform() * // - // matrix_ * // - // entity.GetTransform().Invert() * // - // inputs[0]->GetTransform(entity); - // } auto& m = rendering_mode_ == Entity::RenderingMode::kSubpass ? effect_transform : inputs[0]->GetTransform(entity); From 93043dc9ffb9c51b93f43246db87e250af80bf7e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 12:42:04 -0700 Subject: [PATCH 12/23] added scale test for non subpass --- .../matrix_filter_contents_unittests.cc | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index 6d781a3544c52..e93e529e53eef 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -159,6 +159,23 @@ TEST_P(MatrixFilterContentsTest, Rect::MakeXYWH(200, 400, 100, 100)); } +TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageScale) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + MatrixFilterContents contents; + contents.SetInputs({FilterInput::Make(texture)}); + contents.SetMatrix(Matrix::MakeScale({3, 3, 1})); + contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); + + Entity entity; + entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); + + std::shared_ptr renderer = GetContentContext(); + std::optional result = + contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); + expectRenderCoverageEqual(result, contents.GetCoverage(entity), + Rect::MakeXYWH(100, 200, 300, 300)); +} + TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageSubpassScale) { std::shared_ptr texture = MakeTexture(ISize(100, 100)); MatrixFilterContents contents; @@ -174,7 +191,7 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageSubpassScale) { std::optional result = contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); expectRenderCoverageEqual(result, contents.GetCoverage(entity), - Rect::MakeXYWH(300, 600, 300, 300)); + Rect::MakeXYWH(100, 200, 300, 300)); } } // namespace testing From c44f4cbd9494323602a0bbf4d59d2fff154576cf Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 13:58:35 -0700 Subject: [PATCH 13/23] hackfix --- .../filters/matrix_filter_contents.cc | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index bcf355c10737a..834da5bd177d3 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -112,13 +112,20 @@ std::optional MatrixFilterContents::GetFilterCoverage( return std::nullopt; } - auto& m = rendering_mode_ == Entity::RenderingMode::kSubpass - ? effect_transform - : inputs[0]->GetTransform(entity); - auto transform = m * // - matrix_ * // - m.Invert(); // - return coverage->TransformBounds(transform); + auto m = inputs[0]->GetTransform(entity); + if (rendering_mode_ == Entity::RenderingMode::kSubpass) { + auto foo = Rect::MakeXYWH(0, 0, coverage->GetWidth(), coverage->GetHeight()); + auto transform = m * + effect_transform * // + matrix_ * // + effect_transform.Invert(); // + return foo.TransformBounds(transform); + } else { + auto transform = m * // + matrix_ * // + m.Invert(); // + return coverage->TransformBounds(transform); + } } } // namespace impeller From a6c8b2a8edbfb0f2b16f08d9b71ec26518ecbff4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 14:05:17 -0700 Subject: [PATCH 14/23] removed some test code --- impeller/entity/contents/filters/filter_contents.cc | 1 - impeller/entity/contents/filters/matrix_filter_contents.h | 4 ---- 2 files changed, 5 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index 397a9e9e314d6..46cfab8c874d9 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -153,7 +153,6 @@ std::optional FilterContents::GetLocalCoverage( auto coverage = GetFilterCoverage(inputs_, local_entity, effect_transform_); auto coverage_hint = GetCoverageHint(); if (coverage_hint.has_value() && coverage.has_value()) { - FML_DLOG(ERROR) << coverage_hint.value() << " " << coverage.value(); coverage = coverage->Intersection(coverage_hint.value()); } diff --git a/impeller/entity/contents/filters/matrix_filter_contents.h b/impeller/entity/contents/filters/matrix_filter_contents.h index 4917fcb51677b..dd6334e4169c8 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.h +++ b/impeller/entity/contents/filters/matrix_filter_contents.h @@ -32,10 +32,6 @@ class MatrixFilterContents final : public FilterContents { const Entity& entity, const Matrix& effect_transform) const override; - // const Matrix& GetCoverageHintTransform() const override { - // return matrix_; - // } - private: // |FilterContents| std::optional RenderFilter( From 3b53a74f519287ec9a97470bac45fd7da3aeaad4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 14:15:46 -0700 Subject: [PATCH 15/23] cleaned up hack --- .../contents/filters/matrix_filter_contents.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 834da5bd177d3..9b344a37711a5 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -112,18 +112,18 @@ std::optional MatrixFilterContents::GetFilterCoverage( return std::nullopt; } - auto m = inputs[0]->GetTransform(entity); + auto input_transform = inputs[0]->GetTransform(entity); if (rendering_mode_ == Entity::RenderingMode::kSubpass) { - auto foo = Rect::MakeXYWH(0, 0, coverage->GetWidth(), coverage->GetHeight()); - auto transform = m * + auto coverage_bounds = coverage->TransformBounds(input_transform.Invert()); + auto transform = input_transform * // effect_transform * // matrix_ * // effect_transform.Invert(); // - return foo.TransformBounds(transform); + return coverage_bounds.TransformBounds(transform); } else { - auto transform = m * // - matrix_ * // - m.Invert(); // + auto transform = input_transform * // + matrix_ * // + input_transform.Invert(); // return coverage->TransformBounds(transform); } } From a5d4d18e58ecd79369c5e571e72448fddbe7d622 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 14:24:23 -0700 Subject: [PATCH 16/23] tweaked golden --- impeller/display_list/dl_golden_unittests.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index d0cded0e1bb89..1b2cd139d7172 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -72,8 +72,8 @@ TEST_P(DlGoldenTest, Bug147807) { SkRRect::MakeOval(SkRect::MakeLTRB(201.25, 10, 361.25, 170)), DlCanvas::ClipOp::kIntersect, true); SkRect save_layer_bounds = SkRect::MakeLTRB(201.25, 10, 361.25, 170); - DlMatrixImageFilter backdrop(SkMatrix::MakeAll(3, 0, -299.25, // - 0, 3, -949, // + DlMatrixImageFilter backdrop(SkMatrix::MakeAll(3, 0, -280, // + 0, 3, -920, // 0, 0, 1), DlImageSampling::kLinear); canvas->SaveLayer(&save_layer_bounds, /*paint=*/nullptr, &backdrop); From aaae45e3429b324c8e173f5ba894214383e30ffb Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 14:29:13 -0700 Subject: [PATCH 17/23] cleanup --- .../filters/matrix_filter_contents.cc | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 9b344a37711a5..1d14104479492 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -28,6 +28,17 @@ void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) { sampler_descriptor_ = std::move(desc); } +namespace { +Matrix CalculateSubpassTransform(const Matrix& entity_transform, + const Matrix& effect_transform, + const Matrix& matrix) { + return entity_transform * // + effect_transform * // + matrix * // + effect_transform.Invert(); +} +} // namespace + std::optional MatrixFilterContents::RenderFilter( const FilterInput::Vector& inputs, const ContentContext& renderer, @@ -68,11 +79,8 @@ std::optional MatrixFilterContents::RenderFilter( // 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(); + snapshot->transform = CalculateSubpassTransform(snapshot->transform, + effect_transform, matrix_); } else { snapshot->transform = entity.GetTransform() * // matrix_ * // @@ -107,23 +115,21 @@ std::optional MatrixFilterContents::GetFilterCoverage( return std::nullopt; } - auto coverage = inputs[0]->GetCoverage(entity); + std::optional coverage = inputs[0]->GetCoverage(entity); if (!coverage.has_value()) { return std::nullopt; } - auto input_transform = inputs[0]->GetTransform(entity); + Matrix input_transform = inputs[0]->GetTransform(entity); if (rendering_mode_ == Entity::RenderingMode::kSubpass) { - auto coverage_bounds = coverage->TransformBounds(input_transform.Invert()); - auto transform = input_transform * // - effect_transform * // - matrix_ * // - effect_transform.Invert(); // + Rect coverage_bounds = coverage->TransformBounds(input_transform.Invert()); + Matrix transform = + CalculateSubpassTransform(input_transform, effect_transform, matrix_); return coverage_bounds.TransformBounds(transform); } else { - auto transform = input_transform * // - matrix_ * // - input_transform.Invert(); // + Matrix transform = input_transform * // + matrix_ * // + input_transform.Invert(); // return coverage->TransformBounds(transform); } } From 228b5a6c045af07763514bdf2816192c1f677846 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 14:35:12 -0700 Subject: [PATCH 18/23] license --- ci/licenses_golden/excluded_files | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index d3b20a4a90ce7..8859f1c43eac1 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -147,6 +147,7 @@ ../../../flutter/impeller/entity/contents/clip_contents_unittests.cc ../../../flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc ../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc +../../../flutter/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc ../../../flutter/impeller/entity/contents/host_buffer_unittests.cc ../../../flutter/impeller/entity/contents/test ../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc From 4097c9260528057ee7316c9f07201f573e0dd213 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 16 May 2024 15:08:15 -0700 Subject: [PATCH 19/23] updated golden --- impeller/display_list/dl_golden_unittests.cc | 2 ++ testing/impeller_golden_tests_output.txt | 3 +++ 2 files changed, 5 insertions(+) diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index 1b2cd139d7172..89f04aa4b55ea 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -49,6 +49,8 @@ TEST_P(DlGoldenTest, CanRenderImage) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +// Asserts that subpass rendering of MatrixImageFilters works. +// https://github.com/flutter/flutter/issues/147807 TEST_P(DlGoldenTest, Bug147807) { Point content_scale = GetContentScale(); auto draw = [content_scale](DlCanvas* canvas, diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index 1ec4f8151e373..c291136019a77 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -777,6 +777,9 @@ impeller_Play_AiksTest_VerticesGeometryUVPositionDataWithTranslate_Vulkan.png impeller_Play_AiksTest_VerticesGeometryUVPositionData_Metal.png impeller_Play_AiksTest_VerticesGeometryUVPositionData_OpenGLES.png impeller_Play_AiksTest_VerticesGeometryUVPositionData_Vulkan.png +impeller_Play_DlGoldenTest_Bug147807_Metal.png +impeller_Play_DlGoldenTest_Bug147807_OpenGLES.png +impeller_Play_DlGoldenTest_Bug147807_Vulkan.png impeller_Play_DlGoldenTest_CanDrawPaint_Metal.png impeller_Play_DlGoldenTest_CanDrawPaint_OpenGLES.png impeller_Play_DlGoldenTest_CanDrawPaint_Vulkan.png From 4f4845bce2b7199d0794999a35666034d1803324 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 17 May 2024 10:49:54 -0700 Subject: [PATCH 20/23] fixes MatrixImageFilterMagnify --- impeller/aiks/aiks_unittests.cc | 34 ++++++++++++------- .../filters/matrix_filter_contents.cc | 5 +-- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 12c5414ae1b70..a99bff89484b6 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2772,19 +2772,29 @@ TEST_P(AiksTest, VerticesGeometryColorUVPositionDataAdvancedBlend) { } TEST_P(AiksTest, MatrixImageFilterMagnify) { - Canvas canvas; - canvas.Scale(GetContentScale()); - auto image = std::make_shared(CreateTextureForFixture("airplane.jpg")); - canvas.Translate({600, -200}); - canvas.SaveLayer({ - .image_filter = std::make_shared( - Matrix::MakeScale({2, 2, 2}), SamplerDescriptor{}), - }); - canvas.DrawImage(image, {0, 0}, - Paint{.color = Color::White().WithAlpha(0.5)}); - canvas.Restore(); + Scalar scale = 2.0; + auto callback = [&](AiksContext& renderer) -> std::optional { + if (AiksTest::ImGuiBegin("Controls", nullptr, + ImGuiWindowFlags_AlwaysAutoResize)) { + ImGui::SliderFloat("Scale", &scale, 1, 2); + ImGui::End(); + } + Canvas canvas; + canvas.Scale(GetContentScale()); + auto image = + std::make_shared(CreateTextureForFixture("airplane.jpg")); + canvas.Translate({600, -200}); + canvas.SaveLayer({ + .image_filter = std::make_shared( + Matrix::MakeScale({scale, scale, 1}), SamplerDescriptor{}), + }); + canvas.DrawImage(image, {0, 0}, + Paint{.color = Color::White().WithAlpha(0.5)}); + canvas.Restore(); + return canvas.EndRecordingAsPicture(); + }; - ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); + ASSERT_TRUE(OpenPlaygroundHere(callback)); } // Render a white circle at the top left corner of the screen. diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 1d14104479492..1377fc714f641 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -32,10 +32,11 @@ namespace { Matrix CalculateSubpassTransform(const Matrix& entity_transform, const Matrix& effect_transform, const Matrix& matrix) { + Matrix effect_basis = effect_transform.Basis(); return entity_transform * // - effect_transform * // + effect_basis * // matrix * // - effect_transform.Invert(); + effect_basis.Invert(); } } // namespace From 49b23cb5862c06ff7e039501ce3644c4c866f49a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 17 May 2024 14:02:07 -0700 Subject: [PATCH 21/23] fix attempt 2 --- .../filters/matrix_filter_contents.cc | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index 1377fc714f641..ea04d36d6edd4 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -29,14 +29,22 @@ void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) { } namespace { -Matrix CalculateSubpassTransform(const Matrix& entity_transform, +Matrix CalculateSubpassTransform(const Matrix& snapshot_transform, const Matrix& effect_transform, const Matrix& matrix) { - Matrix effect_basis = effect_transform.Basis(); - return entity_transform * // - effect_basis * // - matrix * // - effect_basis.Invert(); + Scalar x = effect_transform.m[12]; + Scalar y = effect_transform.m[13]; + if (Point(x, y) == Point()) { + return snapshot_transform * // + effect_transform * // + matrix * // + effect_transform.Invert(); + } else { + return effect_transform * // + matrix * // + effect_transform.Invert() * // + snapshot_transform; + } } } // namespace From 8cc1df63d0da9c8cf0d5625462ec420440574452 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 17 May 2024 16:46:24 -0700 Subject: [PATCH 22/23] introduced the new render modes for the matrix filter --- impeller/aiks/canvas.cc | 6 +++-- impeller/aiks/experimental_canvas.cc | 6 +++-- impeller/aiks/paint.cc | 4 ++-- impeller/aiks/paint_pass_delegate.cc | 4 ++-- .../filters/matrix_filter_contents.cc | 22 ++++++++++--------- .../matrix_filter_contents_unittests.cc | 4 ++-- impeller/entity/entity.h | 3 ++- impeller/entity/entity_pass.cc | 8 +++---- 8 files changed, 32 insertions(+), 25 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 4692d1109884d..ace34951073a8 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -225,7 +225,7 @@ void Canvas::Save(bool create_subpass, entry.cull_rect = transform_stack_.back().cull_rect; entry.clip_height = transform_stack_.back().clip_height; if (create_subpass) { - entry.rendering_mode = Entity::RenderingMode::kSubpass; + entry.rendering_mode = Entity::RenderingMode::kBackdropSubpass; auto subpass = std::make_unique(); if (backdrop_filter) { EntityPass::BackdropFilterProc backdrop_filter_proc = @@ -261,7 +261,9 @@ bool Canvas::Restore() { current_pass_->PopClips(num_clips, current_depth_); if (transform_stack_.back().rendering_mode == - Entity::RenderingMode::kSubpass) { + Entity::RenderingMode::kBackdropSubpass || + transform_stack_.back().rendering_mode == + Entity::RenderingMode::kImageFilterSubpass) { current_pass_->SetClipDepth(++current_depth_); current_pass_ = GetCurrentPass().GetSuperpass(); FML_DCHECK(current_pass_); diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 7837b0ca9fb90..b8df394423dfc 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -230,7 +230,7 @@ void ExperimentalCanvas::SaveLayer( << entry.clip_depth << " <=? " << transform_stack_.back().clip_depth << " after allocating " << total_content_depth; entry.clip_height = transform_stack_.back().clip_height; - entry.rendering_mode = Entity::RenderingMode::kSubpass; + entry.rendering_mode = Entity::RenderingMode::kBackdropSubpass; transform_stack_.emplace_back(entry); auto inline_pass = std::make_unique( @@ -272,7 +272,9 @@ bool ExperimentalCanvas::Restore() { current_depth_ = transform_stack_.back().clip_depth; if (transform_stack_.back().rendering_mode == - Entity::RenderingMode::kSubpass) { + Entity::RenderingMode::kBackdropSubpass || + transform_stack_.back().rendering_mode == + Entity::RenderingMode::kImageFilterSubpass) { auto inline_pass = std::move(inline_pass_contexts_.back()); SaveLayerState save_layer_state = save_layer_state_.back(); diff --git a/impeller/aiks/paint.cc b/impeller/aiks/paint.cc index 1efb587601e0d..32252b386a5b9 100644 --- a/impeller/aiks/paint.cc +++ b/impeller/aiks/paint.cc @@ -67,8 +67,8 @@ std::shared_ptr Paint::WithFilters( std::shared_ptr Paint::WithFiltersForSubpassTarget( std::shared_ptr input, const Matrix& effect_transform) const { - auto image_filter = - WithImageFilter(input, effect_transform, Entity::RenderingMode::kSubpass); + auto image_filter = WithImageFilter( + input, effect_transform, Entity::RenderingMode::kImageFilterSubpass); if (image_filter) { input = image_filter; } diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc index 34610488f5a24..2879075e73063 100644 --- a/impeller/aiks/paint_pass_delegate.cc +++ b/impeller/aiks/paint_pass_delegate.cc @@ -51,7 +51,7 @@ std::shared_ptr PaintPassDelegate::WithImageFilter( const FilterInput::Variant& input, const Matrix& effect_transform) const { return paint_.WithImageFilter(input, effect_transform, - Entity::RenderingMode::kSubpass); + Entity::RenderingMode::kImageFilterSubpass); } /// OpacityPeepholePassDelegate @@ -153,7 +153,7 @@ std::shared_ptr OpacityPeepholePassDelegate::WithImageFilter( const FilterInput::Variant& input, const Matrix& effect_transform) const { return paint_.WithImageFilter(input, effect_transform, - Entity::RenderingMode::kSubpass); + Entity::RenderingMode::kImageFilterSubpass); } } // namespace impeller diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index ea04d36d6edd4..1c73cd37d4dbd 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -31,15 +31,15 @@ void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) { namespace { Matrix CalculateSubpassTransform(const Matrix& snapshot_transform, const Matrix& effect_transform, - const Matrix& matrix) { - Scalar x = effect_transform.m[12]; - Scalar y = effect_transform.m[13]; - if (Point(x, y) == Point()) { + const Matrix& matrix, + Entity::RenderingMode rendering_mode) { + if (rendering_mode == Entity::RenderingMode::kBackdropSubpass) { return snapshot_transform * // effect_transform * // matrix * // effect_transform.Invert(); } else { + FML_DCHECK(rendering_mode == Entity::RenderingMode::kImageFilterSubpass); return effect_transform * // matrix * // effect_transform.Invert() * // @@ -60,7 +60,8 @@ std::optional MatrixFilterContents::RenderFilter( return std::nullopt; } - if (rendering_mode_ == Entity::RenderingMode::kSubpass) { + if (rendering_mode_ == Entity::RenderingMode::kImageFilterSubpass || + rendering_mode_ == Entity::RenderingMode::kBackdropSubpass) { // There are two special quirks with how Matrix filters behave when used as // subpass backdrop filters: // @@ -88,8 +89,8 @@ std::optional MatrixFilterContents::RenderFilter( // backdrop texture (as mentioned above). And so we sneak the subpass's // captured CTM in through the effect transform. // - snapshot->transform = CalculateSubpassTransform(snapshot->transform, - effect_transform, matrix_); + snapshot->transform = CalculateSubpassTransform( + snapshot->transform, effect_transform, matrix_, rendering_mode_); } else { snapshot->transform = entity.GetTransform() * // matrix_ * // @@ -130,10 +131,11 @@ std::optional MatrixFilterContents::GetFilterCoverage( } Matrix input_transform = inputs[0]->GetTransform(entity); - if (rendering_mode_ == Entity::RenderingMode::kSubpass) { + if (rendering_mode_ == Entity::RenderingMode::kImageFilterSubpass || + rendering_mode_ == Entity::RenderingMode::kBackdropSubpass) { Rect coverage_bounds = coverage->TransformBounds(input_transform.Invert()); - Matrix transform = - CalculateSubpassTransform(input_transform, effect_transform, matrix_); + Matrix transform = CalculateSubpassTransform( + input_transform, effect_transform, matrix_, rendering_mode_); return coverage_bounds.TransformBounds(transform); } else { Matrix transform = input_transform * // diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index e93e529e53eef..f5c2cc67cb3a8 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -147,7 +147,7 @@ TEST_P(MatrixFilterContentsTest, contents.SetInputs({FilterInput::Make(texture)}); contents.SetMatrix(Matrix::MakeTranslation({50, 100, 0})); contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); - contents.SetRenderingMode(Entity::RenderingMode::kSubpass); + contents.SetRenderingMode(Entity::RenderingMode::kBackdropSubpass); Entity entity; entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); @@ -182,7 +182,7 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageSubpassScale) { contents.SetInputs({FilterInput::Make(texture)}); contents.SetMatrix(Matrix::MakeScale({3, 3, 1})); contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); - contents.SetRenderingMode(Entity::RenderingMode::kSubpass); + contents.SetRenderingMode(Entity::RenderingMode::kBackdropSubpass); Entity entity; entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); diff --git a/impeller/entity/entity.h b/impeller/entity/entity.h index d2d4eb9596a64..293a17867661e 100644 --- a/impeller/entity/entity.h +++ b/impeller/entity/entity.h @@ -33,7 +33,8 @@ class Entity { /// rather than local space, and so some filters (namely, /// MatrixFilterContents) need to interpret the given EffectTransform as the /// current transform matrix. - kSubpass, + kBackdropSubpass, + kImageFilterSubpass, }; /// An enum to define how to repeat, fold, or omit colors outside of the diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 596f4412daf84..ee28a0dd4f68e 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -185,7 +185,7 @@ std::optional EntityPass::GetElementsCoverage( std::shared_ptr backdrop_filter = subpass.backdrop_filter_proc_( FilterInput::Make(accumulated_coverage.value()), - subpass.transform_, Entity::RenderingMode::kSubpass); + subpass.transform_, Entity::RenderingMode::kBackdropSubpass); if (backdrop_filter) { auto backdrop_coverage = backdrop_filter->GetCoverage({}); unfiltered_coverage = @@ -585,9 +585,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->transform_.Basis(), Entity::RenderingMode::kSubpass); + subpass_backdrop_filter_contents = proc( + FilterInput::Make(std::move(texture)), subpass->transform_.Basis(), + Entity::RenderingMode::kBackdropSubpass); // If the very first thing we render in this EntityPass is a subpass that // happens to have a backdrop filter, than that backdrop filter will end From 4960576774880a0ca27f991acad2e3c91b54698e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 17 May 2024 16:49:23 -0700 Subject: [PATCH 23/23] updated tests --- .../matrix_filter_contents_unittests.cc | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc index f5c2cc67cb3a8..3c2a3afd41ccd 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc @@ -141,7 +141,7 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { } TEST_P(MatrixFilterContentsTest, - RenderCoverageMatchesGetCoverageSubpassTranslate) { + RenderCoverageMatchesGetCoverageBackdropSubpassTranslate) { std::shared_ptr texture = MakeTexture(ISize(100, 100)); MatrixFilterContents contents; contents.SetInputs({FilterInput::Make(texture)}); @@ -176,7 +176,8 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageScale) { Rect::MakeXYWH(100, 200, 300, 300)); } -TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageSubpassScale) { +TEST_P(MatrixFilterContentsTest, + RenderCoverageMatchesGetCoverageBackdropSubpassScale) { std::shared_ptr texture = MakeTexture(ISize(100, 100)); MatrixFilterContents contents; contents.SetInputs({FilterInput::Make(texture)}); @@ -194,5 +195,24 @@ TEST_P(MatrixFilterContentsTest, RenderCoverageMatchesGetCoverageSubpassScale) { Rect::MakeXYWH(100, 200, 300, 300)); } +TEST_P(MatrixFilterContentsTest, + RenderCoverageMatchesGetCoverageImageFilterSubpassScale) { + std::shared_ptr texture = MakeTexture(ISize(100, 100)); + MatrixFilterContents contents; + contents.SetInputs({FilterInput::Make(texture)}); + contents.SetMatrix(Matrix::MakeScale({3, 3, 1})); + contents.SetEffectTransform(Matrix::MakeScale({2, 2, 1})); + contents.SetRenderingMode(Entity::RenderingMode::kImageFilterSubpass); + + Entity entity; + entity.SetTransform(Matrix::MakeTranslation({100, 200, 0})); + + std::shared_ptr renderer = GetContentContext(); + std::optional result = + contents.GetEntity(*renderer, entity, /*coverage_hint=*/{}); + expectRenderCoverageEqual(result, contents.GetCoverage(entity), + Rect::MakeXYWH(300, 600, 300, 300)); +} + } // namespace testing } // namespace impeller