Skip to content

Commit

Permalink
Revert "[Impeller] Fix MatrixFilter multiplication ordering for subpa…
Browse files Browse the repository at this point in the history 
…sses." (flutter#43978)

Reverts flutter#43943

Closes flutter/flutter#131182.
Opens flutter/flutter#130355.

Regression spotted in flutter/flutter#131182
  • Loading branch information
@bdero
bdero authored Jul 24, 2023
1 parent 4fded78 commit 176457d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 73 deletions.
25 changes: 9 additions & 16 deletions impeller/aiks/aiks_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2908,22 +2908,15 @@ TEST_P(AiksTest, DrawPictureWithText) {

TEST_P(AiksTest, MatrixBackdropFilter) {
Canvas canvas;
canvas.DrawPaint({.color = Color::Black()});
canvas.SaveLayer({}, std::nullopt);
{
canvas.DrawCircle(
Point(200, 200), 100,
{.color = Color::Green(), .blend_mode = BlendMode::kPlus});
// Should render a second intersecting circle, offset by 100, 100.
canvas.SaveLayer({}, std::nullopt,
[](const FilterInput::Ref& input,
const Matrix& effect_transform, bool is_subpass) {
return FilterContents::MakeMatrixFilter(
input, Matrix::MakeTranslation(Vector2(100, 100)),
{}, Matrix(), true);
});
canvas.Restore();
}
canvas.SaveLayer({}, std::nullopt,
[](const FilterInput::Ref& input,
const Matrix& effect_transform, bool is_subpass) {
return FilterContents::MakeMatrixFilter(
input, Matrix::MakeTranslation(Vector2(100, 100)), {},
Matrix(), true);
});
canvas.DrawCircle(Point(100, 100), 100,
{.color = Color::Green(), .blend_mode = BlendMode::kPlus});
canvas.Restore();

ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
Expand Down
44 changes: 5 additions & 39 deletions impeller/entity/contents/filters/matrix_filter_contents.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,45 +34,11 @@ std::optional<Entity> MatrixFilterContents::RenderFilter(
return std::nullopt;
}

if (is_subpass_) {
// 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.GetTransformation() * //
matrix_ * //
entity.GetTransformation().Invert() * //
snapshot->transform;
}
auto& transform = is_subpass_ ? effect_transform : entity.GetTransformation();
snapshot->transform = transform * //
matrix_ * //
transform.Invert() * //
snapshot->transform;
snapshot->sampler_descriptor = sampler_descriptor_;
return Entity::FromSnapshot(snapshot, entity.GetBlendMode(),
entity.GetStencilDepth());
Expand Down
34 changes: 16 additions & 18 deletions impeller/entity/entity_pass.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,12 +465,12 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
return EntityPass::EntityResult::Skip();
}

std::shared_ptr<Contents> subpass_backdrop_filter_contents = nullptr;
std::shared_ptr<Contents> backdrop_filter_contents = nullptr;
if (subpass->backdrop_filter_proc_) {
auto texture = pass_context.GetTexture();
// Render the backdrop texture before any of the pass elements.
const auto& proc = subpass->backdrop_filter_proc_;
subpass_backdrop_filter_contents =
backdrop_filter_contents =
proc(FilterInput::Make(std::move(texture)), subpass->xformation_,
/*is_subpass*/ true);

Expand Down Expand Up @@ -507,10 +507,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
return EntityPass::EntityResult::Skip();
}

auto subpass_coverage =
(subpass->flood_clip_ || subpass_backdrop_filter_contents)
? coverage_limit
: GetSubpassCoverage(*subpass, coverage_limit);
auto subpass_coverage = (subpass->flood_clip_ || backdrop_filter_contents)
? coverage_limit
: GetSubpassCoverage(*subpass, coverage_limit);
if (!subpass_coverage.has_value()) {
return EntityPass::EntityResult::Skip();
}
Expand All @@ -533,18 +532,17 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(

// Stencil textures aren't shared between EntityPasses (as much of the
// time they are transient).
if (!subpass->OnRender(
renderer, // renderer
root_pass_size, // root_pass_size
subpass_target, // pass_target
subpass_coverage->origin, // global_pass_position
subpass_coverage->origin -
global_pass_position, // local_pass_position
++pass_depth, // pass_depth
stencil_coverage_stack, // stencil_coverage_stack
subpass->stencil_depth_, // stencil_depth_floor
subpass_backdrop_filter_contents // backdrop_filter_contents
)) {
if (!subpass->OnRender(renderer, // renderer
root_pass_size, // root_pass_size
subpass_target, // pass_target
subpass_coverage->origin, // global_pass_position
subpass_coverage->origin -
global_pass_position, // local_pass_position
++pass_depth, // pass_depth
stencil_coverage_stack, // stencil_coverage_stack
subpass->stencil_depth_, // stencil_depth_floor
backdrop_filter_contents // backdrop_filter_contents
)) {
// Validation error messages are triggered for all `OnRender()` failure
// cases.
return EntityPass::EntityResult::Failure();
Expand Down

0 comments on commit 176457d

Please sign in to comment.