Skip to content

Commit

Permalink
Reverts "Fixes MatrixFilterContents rendering/coverage (#52880)" (#52918
Browse files Browse the repository at this point in the history 
)

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
  • Loading branch information
@auto-submit
auto-submit[bot] authored May 18, 2024
1 parent 2f80d28 commit 552a965
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 387 deletions.
1 change: 0 additions & 1 deletion ci/licenses_golden/excluded_files
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@
../../../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
Expand Down
34 changes: 12 additions & 22 deletions impeller/aiks/aiks_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2772,29 +2772,19 @@ TEST_P(AiksTest, VerticesGeometryColorUVPositionDataAdvancedBlend) {
}

TEST_P(AiksTest, MatrixImageFilterMagnify) {
Scalar scale = 2.0;
auto callback = [&](AiksContext& renderer) -> std::optional<Picture> {
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<Image>(CreateTextureForFixture("airplane.jpg"));
canvas.Translate({600, -200});
canvas.SaveLayer({
.image_filter = std::make_shared<MatrixImageFilter>(
Matrix::MakeScale({scale, scale, 1}), SamplerDescriptor{}),
});
canvas.DrawImage(image, {0, 0},
Paint{.color = Color::White().WithAlpha(0.5)});
canvas.Restore();
return canvas.EndRecordingAsPicture();
};
Canvas canvas;
canvas.Scale(GetContentScale());
auto image = std::make_shared<Image>(CreateTextureForFixture("airplane.jpg"));
canvas.Translate({600, -200});
canvas.SaveLayer({
.image_filter = std::make_shared<MatrixImageFilter>(
Matrix::MakeScale({2, 2, 2}), SamplerDescriptor{}),
});
canvas.DrawImage(image, {0, 0},
Paint{.color = Color::White().WithAlpha(0.5)});
canvas.Restore();

ASSERT_TRUE(OpenPlaygroundHere(callback));
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

// Render a white circle at the top left corner of the screen.
Expand Down
6 changes: 2 additions & 4 deletions impeller/aiks/canvas.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::kBackdropSubpass;
entry.rendering_mode = Entity::RenderingMode::kSubpass;
auto subpass = std::make_unique<EntityPass>();
if (backdrop_filter) {
EntityPass::BackdropFilterProc backdrop_filter_proc =
Expand Down Expand Up @@ -261,9 +261,7 @@ bool Canvas::Restore() {
current_pass_->PopClips(num_clips, current_depth_);

if (transform_stack_.back().rendering_mode ==
Entity::RenderingMode::kBackdropSubpass ||
transform_stack_.back().rendering_mode ==
Entity::RenderingMode::kImageFilterSubpass) {
Entity::RenderingMode::kSubpass) {
current_pass_->SetClipDepth(++current_depth_);
current_pass_ = GetCurrentPass().GetSuperpass();
FML_DCHECK(current_pass_);
Expand Down
6 changes: 2 additions & 4 deletions impeller/aiks/experimental_canvas.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::kBackdropSubpass;
entry.rendering_mode = Entity::RenderingMode::kSubpass;
transform_stack_.emplace_back(entry);

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

if (transform_stack_.back().rendering_mode ==
Entity::RenderingMode::kBackdropSubpass ||
transform_stack_.back().rendering_mode ==
Entity::RenderingMode::kImageFilterSubpass) {
Entity::RenderingMode::kSubpass) {
auto inline_pass = std::move(inline_pass_contexts_.back());

SaveLayerState save_layer_state = save_layer_state_.back();
Expand Down
4 changes: 2 additions & 2 deletions impeller/aiks/paint.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ std::shared_ptr<Contents> Paint::WithFilters(
std::shared_ptr<Contents> Paint::WithFiltersForSubpassTarget(
std::shared_ptr<Contents> input,
const Matrix& effect_transform) const {
auto image_filter = WithImageFilter(
input, effect_transform, Entity::RenderingMode::kImageFilterSubpass);
auto image_filter =
WithImageFilter(input, effect_transform, Entity::RenderingMode::kSubpass);
if (image_filter) {
input = image_filter;
}
Expand Down
4 changes: 2 additions & 2 deletions impeller/aiks/paint_pass_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ std::shared_ptr<FilterContents> PaintPassDelegate::WithImageFilter(
const FilterInput::Variant& input,
const Matrix& effect_transform) const {
return paint_.WithImageFilter(input, effect_transform,
Entity::RenderingMode::kImageFilterSubpass);
Entity::RenderingMode::kSubpass);
}

/// OpacityPeepholePassDelegate
Expand Down Expand Up @@ -153,7 +153,7 @@ std::shared_ptr<FilterContents> OpacityPeepholePassDelegate::WithImageFilter(
const FilterInput::Variant& input,
const Matrix& effect_transform) const {
return paint_.WithImageFilter(input, effect_transform,
Entity::RenderingMode::kImageFilterSubpass);
Entity::RenderingMode::kSubpass);
}

} // namespace impeller
52 changes: 0 additions & 52 deletions impeller/display_list/dl_golden_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ namespace testing {

using impeller::PlaygroundBackend;
using impeller::PlaygroundTest;
using impeller::Point;

INSTANTIATE_PLAYGROUND_SUITE(DlGoldenTest);

Expand Down Expand Up @@ -49,56 +48,5 @@ 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,
const std::vector<sk_sp<DlImage>>& 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);
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->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, -280, //
0, 3, -920, //
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);
}
canvas->Restore();
}
canvas->Restore();
};

DisplayListBuilder builder;
std::vector<sk_sp<DlImage>> images;
draw(&builder, images);

ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}

} // namespace testing
} // namespace flutter
1 change: 0 additions & 1 deletion impeller/entity/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,6 @@ impeller_component("entity_unittests") {
"contents/clip_contents_unittests.cc",
"contents/filters/gaussian_blur_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",
Expand Down
103 changes: 31 additions & 72 deletions impeller/entity/contents/filters/matrix_filter_contents.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,6 @@ void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) {
sampler_descriptor_ = std::move(desc);
}

namespace {
Matrix CalculateSubpassTransform(const Matrix& snapshot_transform,
const Matrix& effect_transform,
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() * //
snapshot_transform;
}
}
} // namespace

std::optional<Entity> MatrixFilterContents::RenderFilter(
const FilterInput::Vector& inputs,
const ContentContext& renderer,
Expand All @@ -60,43 +40,29 @@ std::optional<Entity> MatrixFilterContents::RenderFilter(
return std::nullopt;
}

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:
//
// 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 = CalculateSubpassTransform(
snapshot->transform, effect_transform, matrix_, rendering_mode_);
} else {
snapshot->transform = entity.GetTransform() * //
matrix_ * //
entity.GetTransform().Invert() * //
snapshot->transform;
}
// 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;

snapshot->sampler_descriptor = sampler_descriptor_;
if (!snapshot.has_value()) {
return std::nullopt;
Expand Down Expand Up @@ -125,24 +91,17 @@ std::optional<Rect> MatrixFilterContents::GetFilterCoverage(
return std::nullopt;
}

std::optional<Rect> coverage = inputs[0]->GetCoverage(entity);
auto coverage = inputs[0]->GetCoverage(entity);
if (!coverage.has_value()) {
return std::nullopt;
}

Matrix input_transform = inputs[0]->GetTransform(entity);
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_, rendering_mode_);
return coverage_bounds.TransformBounds(transform);
} else {
Matrix transform = input_transform * //
matrix_ * //
input_transform.Invert(); //
return coverage->TransformBounds(transform);
}
auto& m = rendering_mode_ == Entity::RenderingMode::kSubpass
? effect_transform
: inputs[0]->GetTransform(entity);
auto transform = m * //
matrix_ * //
m.Invert(); //
return coverage->TransformBounds(transform);
}

} // namespace impeller
Loading

0 comments on commit 552a965

Please sign in to comment.