Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverts "Fixes MatrixFilterContents rendering/coverage (#52880)" #52918

Merged
merged 1 commit into from
May 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ci/licenses_golden/excluded_files
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
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
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
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
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
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
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
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
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