Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 552a965

Browse files
auto-submit[bot]auto-submit[bot]
authored
Reverts "Fixes MatrixFilterContents rendering/coverage (#52880)" (#52918)
Reverts: #52880 Initiated by: jonahwilliams Reason for reverting: unexpected framework golden change Original PR Author: gaaclarke Reviewed By: {bdero} This change reverts the following previous change: fixes: flutter/flutter#147807 relands #43943 (with fixes that hopefully avoid it being reverted again) [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 2f80d28 commit 552a965

File tree

13 files changed

+56
-387
lines changed

13 files changed

+56
-387
lines changed

‎ci/licenses_golden/excluded_files

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,6 @@
147147
../../../flutter/impeller/entity/contents/clip_contents_unittests.cc
148148
../../../flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc
149149
../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc
150-
../../../flutter/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc
151150
../../../flutter/impeller/entity/contents/host_buffer_unittests.cc
152151
../../../flutter/impeller/entity/contents/test
153152
../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc

‎impeller/aiks/aiks_unittests.cc

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,29 +2772,19 @@ TEST_P(AiksTest, VerticesGeometryColorUVPositionDataAdvancedBlend) {
27722772
}
27732773

27742774
TEST_P(AiksTest, MatrixImageFilterMagnify) {
2775-
Scalar scale = 2.0;
2776-
auto callback = [&](AiksContext& renderer) -> std::optional<Picture> {
2777-
if (AiksTest::ImGuiBegin("Controls", nullptr,
2778-
ImGuiWindowFlags_AlwaysAutoResize)) {
2779-
ImGui::SliderFloat("Scale", &scale, 1, 2);
2780-
ImGui::End();
2781-
}
2782-
Canvas canvas;
2783-
canvas.Scale(GetContentScale());
2784-
auto image =
2785-
std::make_shared<Image>(CreateTextureForFixture("airplane.jpg"));
2786-
canvas.Translate({600, -200});
2787-
canvas.SaveLayer({
2788-
.image_filter = std::make_shared<MatrixImageFilter>(
2789-
Matrix::MakeScale({scale, scale, 1}), SamplerDescriptor{}),
2790-
});
2791-
canvas.DrawImage(image, {0, 0},
2792-
Paint{.color = Color::White().WithAlpha(0.5)});
2793-
canvas.Restore();
2794-
return canvas.EndRecordingAsPicture();
2795-
};
2775+
Canvas canvas;
2776+
canvas.Scale(GetContentScale());
2777+
auto image = std::make_shared<Image>(CreateTextureForFixture("airplane.jpg"));
2778+
canvas.Translate({600, -200});
2779+
canvas.SaveLayer({
2780+
.image_filter = std::make_shared<MatrixImageFilter>(
2781+
Matrix::MakeScale({2, 2, 2}), SamplerDescriptor{}),
2782+
});
2783+
canvas.DrawImage(image, {0, 0},
2784+
Paint{.color = Color::White().WithAlpha(0.5)});
2785+
canvas.Restore();
27962786

2797-
ASSERT_TRUE(OpenPlaygroundHere(callback));
2787+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
27982788
}
27992789

28002790
// Render a white circle at the top left corner of the screen.

‎impeller/aiks/canvas.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ void Canvas::Save(bool create_subpass,
225225
entry.cull_rect = transform_stack_.back().cull_rect;
226226
entry.clip_height = transform_stack_.back().clip_height;
227227
if (create_subpass) {
228-
entry.rendering_mode = Entity::RenderingMode::kBackdropSubpass;
228+
entry.rendering_mode = Entity::RenderingMode::kSubpass;
229229
auto subpass = std::make_unique<EntityPass>();
230230
if (backdrop_filter) {
231231
EntityPass::BackdropFilterProc backdrop_filter_proc =
@@ -261,9 +261,7 @@ bool Canvas::Restore() {
261261
current_pass_->PopClips(num_clips, current_depth_);
262262

263263
if (transform_stack_.back().rendering_mode ==
264-
Entity::RenderingMode::kBackdropSubpass ||
265-
transform_stack_.back().rendering_mode ==
266-
Entity::RenderingMode::kImageFilterSubpass) {
264+
Entity::RenderingMode::kSubpass) {
267265
current_pass_->SetClipDepth(++current_depth_);
268266
current_pass_ = GetCurrentPass().GetSuperpass();
269267
FML_DCHECK(current_pass_);

‎impeller/aiks/experimental_canvas.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ void ExperimentalCanvas::SaveLayer(
230230
<< entry.clip_depth << " <=? " << transform_stack_.back().clip_depth
231231
<< " after allocating " << total_content_depth;
232232
entry.clip_height = transform_stack_.back().clip_height;
233-
entry.rendering_mode = Entity::RenderingMode::kBackdropSubpass;
233+
entry.rendering_mode = Entity::RenderingMode::kSubpass;
234234
transform_stack_.emplace_back(entry);
235235

236236
auto inline_pass = std::make_unique<InlinePassContext>(
@@ -272,9 +272,7 @@ bool ExperimentalCanvas::Restore() {
272272
current_depth_ = transform_stack_.back().clip_depth;
273273

274274
if (transform_stack_.back().rendering_mode ==
275-
Entity::RenderingMode::kBackdropSubpass ||
276-
transform_stack_.back().rendering_mode ==
277-
Entity::RenderingMode::kImageFilterSubpass) {
275+
Entity::RenderingMode::kSubpass) {
278276
auto inline_pass = std::move(inline_pass_contexts_.back());
279277

280278
SaveLayerState save_layer_state = save_layer_state_.back();

‎impeller/aiks/paint.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ std::shared_ptr<Contents> Paint::WithFilters(
6767
std::shared_ptr<Contents> Paint::WithFiltersForSubpassTarget(
6868
std::shared_ptr<Contents> input,
6969
const Matrix& effect_transform) const {
70-
auto image_filter = WithImageFilter(
71-
input, effect_transform, Entity::RenderingMode::kImageFilterSubpass);
70+
auto image_filter =
71+
WithImageFilter(input, effect_transform, Entity::RenderingMode::kSubpass);
7272
if (image_filter) {
7373
input = image_filter;
7474
}

‎impeller/aiks/paint_pass_delegate.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ std::shared_ptr<FilterContents> PaintPassDelegate::WithImageFilter(
5151
const FilterInput::Variant& input,
5252
const Matrix& effect_transform) const {
5353
return paint_.WithImageFilter(input, effect_transform,
54-
Entity::RenderingMode::kImageFilterSubpass);
54+
Entity::RenderingMode::kSubpass);
5555
}
5656

5757
/// OpacityPeepholePassDelegate
@@ -153,7 +153,7 @@ std::shared_ptr<FilterContents> OpacityPeepholePassDelegate::WithImageFilter(
153153
const FilterInput::Variant& input,
154154
const Matrix& effect_transform) const {
155155
return paint_.WithImageFilter(input, effect_transform,
156-
Entity::RenderingMode::kImageFilterSubpass);
156+
Entity::RenderingMode::kSubpass);
157157
}
158158

159159
} // namespace impeller

‎impeller/display_list/dl_golden_unittests.cc

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ namespace testing {
1313

1414
using impeller::PlaygroundBackend;
1515
using impeller::PlaygroundTest;
16-
using impeller::Point;
1716

1817
INSTANTIATE_PLAYGROUND_SUITE(DlGoldenTest);
1918

@@ -49,56 +48,5 @@ TEST_P(DlGoldenTest, CanRenderImage) {
4948
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
5049
}
5150

52-
// Asserts that subpass rendering of MatrixImageFilters works.
53-
// https://github.com/flutter/flutter/issues/147807
54-
TEST_P(DlGoldenTest, Bug147807) {
55-
Point content_scale = GetContentScale();
56-
auto draw = [content_scale](DlCanvas* canvas,
57-
const std::vector<sk_sp<DlImage>>& images) {
58-
canvas->Transform2DAffine(content_scale.x, 0, 0, 0, content_scale.y, 0);
59-
DlPaint paint;
60-
paint.setColor(DlColor(0xfffef7ff));
61-
canvas->DrawRect(SkRect::MakeLTRB(0, 0, 375, 667), paint);
62-
paint.setColor(DlColor(0xffff9800));
63-
canvas->DrawRect(SkRect::MakeLTRB(0, 0, 187.5, 333.5), paint);
64-
paint.setColor(DlColor(0xff9c27b0));
65-
canvas->DrawRect(SkRect::MakeLTRB(187.5, 0, 375, 333.5), paint);
66-
paint.setColor(DlColor(0xff4caf50));
67-
canvas->DrawRect(SkRect::MakeLTRB(0, 333.5, 187.5, 667), paint);
68-
paint.setColor(DlColor(0xfff44336));
69-
canvas->DrawRect(SkRect::MakeLTRB(187.5, 333.5, 375, 667), paint);
70-
71-
canvas->Save();
72-
{
73-
canvas->ClipRRect(
74-
SkRRect::MakeOval(SkRect::MakeLTRB(201.25, 10, 361.25, 170)),
75-
DlCanvas::ClipOp::kIntersect, true);
76-
SkRect save_layer_bounds = SkRect::MakeLTRB(201.25, 10, 361.25, 170);
77-
DlMatrixImageFilter backdrop(SkMatrix::MakeAll(3, 0, -280, //
78-
0, 3, -920, //
79-
0, 0, 1),
80-
DlImageSampling::kLinear);
81-
canvas->SaveLayer(&save_layer_bounds, /*paint=*/nullptr, &backdrop);
82-
{
83-
canvas->Translate(201.25, 10);
84-
auto paint = DlPaint()
85-
.setAntiAlias(true)
86-
.setColor(DlColor(0xff2196f3))
87-
.setStrokeWidth(5)
88-
.setDrawStyle(DlDrawStyle::kStroke);
89-
canvas->DrawCircle(SkPoint::Make(80, 80), 80, paint);
90-
}
91-
canvas->Restore();
92-
}
93-
canvas->Restore();
94-
};
95-
96-
DisplayListBuilder builder;
97-
std::vector<sk_sp<DlImage>> images;
98-
draw(&builder, images);
99-
100-
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
101-
}
102-
10351
} // namespace testing
10452
} // namespace flutter

‎impeller/entity/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ impeller_component("entity_unittests") {
241241
"contents/clip_contents_unittests.cc",
242242
"contents/filters/gaussian_blur_filter_contents_unittests.cc",
243243
"contents/filters/inputs/filter_input_unittests.cc",
244-
"contents/filters/matrix_filter_contents_unittests.cc",
245244
"contents/host_buffer_unittests.cc",
246245
"contents/tiled_texture_contents_unittests.cc",
247246
"entity_pass_target_unittests.cc",

‎impeller/entity/contents/filters/matrix_filter_contents.cc

Lines changed: 31 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,6 @@ void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) {
2828
sampler_descriptor_ = std::move(desc);
2929
}
3030

31-
namespace {
32-
Matrix CalculateSubpassTransform(const Matrix& snapshot_transform,
33-
const Matrix& effect_transform,
34-
const Matrix& matrix,
35-
Entity::RenderingMode rendering_mode) {
36-
if (rendering_mode == Entity::RenderingMode::kBackdropSubpass) {
37-
return snapshot_transform * //
38-
effect_transform * //
39-
matrix * //
40-
effect_transform.Invert();
41-
} else {
42-
FML_DCHECK(rendering_mode == Entity::RenderingMode::kImageFilterSubpass);
43-
return effect_transform * //
44-
matrix * //
45-
effect_transform.Invert() * //
46-
snapshot_transform;
47-
}
48-
}
49-
} // namespace
50-
5131
std::optional<Entity> MatrixFilterContents::RenderFilter(
5232
const FilterInput::Vector& inputs,
5333
const ContentContext& renderer,
@@ -60,43 +40,29 @@ std::optional<Entity> MatrixFilterContents::RenderFilter(
6040
return std::nullopt;
6141
}
6242

63-
if (rendering_mode_ == Entity::RenderingMode::kImageFilterSubpass ||
64-
rendering_mode_ == Entity::RenderingMode::kBackdropSubpass) {
65-
// There are two special quirks with how Matrix filters behave when used as
66-
// subpass backdrop filters:
67-
//
68-
// 1. For subpass backdrop filters, the snapshot transform is always just a
69-
// translation that positions the parent pass texture correctly relative
70-
// to the subpass texture. However, this translation always needs to be
71-
// applied in screen space.
72-
//
73-
// Since we know the snapshot transform will always have an identity
74-
// basis in this case, we safely reverse the order and apply the filter's
75-
// matrix within the snapshot transform space.
76-
//
77-
// 2. The filter's matrix needs to be applied within the space defined by
78-
// the scene's current transformation matrix (CTM). For example: If the
79-
// CTM is scaled up, then translations applied by the matrix should be
80-
// magnified accordingly.
81-
//
82-
// To accomplish this, we sandwitch the filter's matrix within the CTM in
83-
// both cases. But notice that for the subpass backdrop filter case, we
84-
// use the "effect transform" instead of the Entity's transform!
85-
//
86-
// That's because in the subpass backdrop filter case, the Entity's
87-
// transform isn't actually the captured CTM of the scene like it usually
88-
// is; instead, it's just a screen space translation that offsets the
89-
// backdrop texture (as mentioned above). And so we sneak the subpass's
90-
// captured CTM in through the effect transform.
91-
//
92-
snapshot->transform = CalculateSubpassTransform(
93-
snapshot->transform, effect_transform, matrix_, rendering_mode_);
94-
} else {
95-
snapshot->transform = entity.GetTransform() * //
96-
matrix_ * //
97-
entity.GetTransform().Invert() * //
98-
snapshot->transform;
99-
}
43+
// The filter's matrix needs to be applied within the space defined by the
44+
// scene's current transform matrix (CTM). For example: If the CTM is
45+
// scaled up, then translations applied by the matrix should be magnified
46+
// accordingly.
47+
//
48+
// To accomplish this, we sandwich the filter's matrix within the CTM in both
49+
// cases. But notice that for the subpass backdrop filter case, we use the
50+
// "effect transform" instead of the Entity's transform!
51+
//
52+
// That's because in the subpass backdrop filter case, the Entity's transform
53+
// isn't actually the captured CTM of the scene like it usually is; instead,
54+
// it's just a screen space translation that offsets the backdrop texture (as
55+
// mentioned above). And so we sneak the subpass's captured CTM in through the
56+
// effect transform.
57+
58+
auto transform = rendering_mode_ == Entity::RenderingMode::kSubpass
59+
? effect_transform
60+
: entity.GetTransform();
61+
snapshot->transform = transform * //
62+
matrix_ * //
63+
transform.Invert() * //
64+
snapshot->transform;
65+
10066
snapshot->sampler_descriptor = sampler_descriptor_;
10167
if (!snapshot.has_value()) {
10268
return std::nullopt;
@@ -125,24 +91,17 @@ std::optional<Rect> MatrixFilterContents::GetFilterCoverage(
12591
return std::nullopt;
12692
}
12793

128-
std::optional<Rect> coverage = inputs[0]->GetCoverage(entity);
94+
auto coverage = inputs[0]->GetCoverage(entity);
12995
if (!coverage.has_value()) {
13096
return std::nullopt;
13197
}
132-
133-
Matrix input_transform = inputs[0]->GetTransform(entity);
134-
if (rendering_mode_ == Entity::RenderingMode::kImageFilterSubpass ||
135-
rendering_mode_ == Entity::RenderingMode::kBackdropSubpass) {
136-
Rect coverage_bounds = coverage->TransformBounds(input_transform.Invert());
137-
Matrix transform = CalculateSubpassTransform(
138-
input_transform, effect_transform, matrix_, rendering_mode_);
139-
return coverage_bounds.TransformBounds(transform);
140-
} else {
141-
Matrix transform = input_transform * //
142-
matrix_ * //
143-
input_transform.Invert(); //
144-
return coverage->TransformBounds(transform);
145-
}
98+
auto& m = rendering_mode_ == Entity::RenderingMode::kSubpass
99+
? effect_transform
100+
: inputs[0]->GetTransform(entity);
101+
auto transform = m * //
102+
matrix_ * //
103+
m.Invert(); //
104+
return coverage->TransformBounds(transform);
146105
}
147106

148107
} // namespace impeller

0 commit comments

Comments
 (0)