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.

Sign up for GitHub

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

[Impeller] Make matrix image filter in saving layer work as expected #40171

Merged
merged 2 commits into from
Mar 15, 2023
Merged

[Impeller] Make matrix image filter in saving layer work as expected #40171

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a15f97b
[Impeller] Make matrix image filter in saving layer work as expected
ColdPaleLight Mar 9, 2023
ad21ac5
Clean code
ColdPaleLight Mar 13, 2023
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
2 changes: 1 addition & 1 deletion impeller/aiks/aiks_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1931,7 +1931,7 @@ TEST_P(AiksTest, PaintWithFilters) {
ASSERT_TRUE(paint.HasColorFilter());

paint.image_filter = [](const FilterInput::Ref& input,
const Matrix& effect_transform) {
const Matrix& effect_transform, bool is_subpass) {
return FilterContents::MakeGaussianBlur(
input, Sigma(1.0), Sigma(1.0), FilterContents::BlurStyle::kNormal,
Entity::TileMode::kClamp, effect_transform);
Expand Down
11 changes: 6 additions & 5 deletions impeller/aiks/paint.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ std::shared_ptr<Contents> Paint::WithFilters(
input = WithColorFilter(input);
input = WithInvertFilter(input);
input = WithMaskBlur(input, is_solid_color_val, effect_transform);
input = WithImageFilter(input, effect_transform);
input = WithImageFilter(input, effect_transform, /*is_subpass=*/false);
return input;
}

std::shared_ptr<Contents> Paint::WithFiltersForSubpassTarget(
std::shared_ptr<Contents> input,
const Matrix& effect_transform) const {
input = WithImageFilter(input, effect_transform);
input = WithColorFilter(input, /**absorb_opacity=*/true);
input = WithImageFilter(input, effect_transform, /*is_subpass=*/true);
input = WithColorFilter(input, /*absorb_opacity=*/true);
return input;
}

Expand All @@ -88,10 +88,11 @@ std::shared_ptr<Contents> Paint::WithMaskBlur(

std::shared_ptr<Contents> Paint::WithImageFilter(
std::shared_ptr<Contents> input,
const Matrix& effect_transform) const {
const Matrix& effect_transform,
bool is_subpass) const {
if (image_filter.has_value()) {
const ImageFilterProc& filter = image_filter.value();
input = filter(FilterInput::Make(input), effect_transform);
input = filter(FilterInput::Make(input), effect_transform, is_subpass);
}
return input;
}
Expand Down
9 changes: 5 additions & 4 deletions impeller/aiks/paint.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ namespace impeller {
struct Paint {
using ImageFilterProc = std::function<std::shared_ptr<FilterContents>(
FilterInput::Ref,
const Matrix& effect_transform)>;
const Matrix& effect_transform,
bool is_subpass)>;
using ColorFilterProc =
std::function<std::shared_ptr<ColorFilterContents>(FilterInput::Ref)>;
using MaskFilterProc = std::function<std::shared_ptr<FilterContents>(
Expand Down Expand Up @@ -118,9 +119,9 @@ struct Paint {
bool is_solid_color,
const Matrix& effect_transform) const;

std::shared_ptr<Contents> WithImageFilter(
std::shared_ptr<Contents> input,
const Matrix& effect_transform) const;
std::shared_ptr<Contents> WithImageFilter(std::shared_ptr<Contents> input,
const Matrix& effect_transform,
bool is_subpass) const;

std::shared_ptr<Contents> WithColorFilter(std::shared_ptr<Contents> input,
bool absorb_opacity = false) const;
Expand Down
35 changes: 21 additions & 14 deletions impeller/display_list/display_list_dispatcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
auto tile_mode = ToTileMode(blur->tile_mode());

return [sigma_x, sigma_y, tile_mode](const FilterInput::Ref& input,
const Matrix& effect_transform) {
const Matrix& effect_transform,
bool is_subpass) {
return FilterContents::MakeGaussianBlur(
input, sigma_x, sigma_y, FilterContents::BlurStyle::kNormal,
tile_mode, effect_transform);
Expand All @@ -670,7 +671,8 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
auto radius_x = Radius(dilate->radius_x());
auto radius_y = Radius(dilate->radius_y());
return [radius_x, radius_y](FilterInput::Ref input,
const Matrix& effect_transform) {
const Matrix& effect_transform,
bool is_subpass) {
return FilterContents::MakeMorphology(
std::move(input), radius_x, radius_y,
FilterContents::MorphType::kDilate, effect_transform);
Expand All @@ -686,7 +688,8 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
auto radius_x = Radius(erode->radius_x());
auto radius_y = Radius(erode->radius_y());
return [radius_x, radius_y](FilterInput::Ref input,
const Matrix& effect_transform) {
const Matrix& effect_transform,
bool is_subpass) {
return FilterContents::MakeMorphology(
std::move(input), radius_x, radius_y,
FilterContents::MorphType::kErode, effect_transform);
Expand All @@ -699,8 +702,9 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
auto matrix = ToMatrix(matrix_filter->matrix());
auto desc = ToSamplerDescriptor(matrix_filter->sampling());
return [matrix, desc](FilterInput::Ref input,
const Matrix& effect_transform) {
return FilterContents::MakeMatrixFilter(std::move(input), matrix, desc);
const Matrix& effect_transform, bool is_subpass) {
return FilterContents::MakeMatrixFilter(std::move(input), matrix, desc,
effect_transform, is_subpass);
};
break;
}
Expand All @@ -719,10 +723,13 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
}
FML_DCHECK(outer_proc.has_value() && inner_proc.has_value());
return [outer_filter = outer_proc.value(),
inner_filter = inner_proc.value()](
FilterInput::Ref input, const Matrix& effect_transform) {
auto contents = inner_filter(std::move(input), effect_transform);
contents = outer_filter(FilterInput::Make(contents), effect_transform);
inner_filter = inner_proc.value()](FilterInput::Ref input,
const Matrix& effect_transform,
bool is_subpass) {
auto contents =
inner_filter(std::move(input), effect_transform, is_subpass);
contents = outer_filter(FilterInput::Make(contents), effect_transform,
is_subpass);
return contents;
};
break;
Expand All @@ -736,9 +743,8 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
return std::nullopt;
}
return [color_filter = color_filter_proc.value()](
FilterInput::Ref input, const Matrix& effect_transform) {
return color_filter(std::move(input));
};
FilterInput::Ref input, const Matrix& effect_transform,
bool is_subpass) { return color_filter(std::move(input)); };
break;
}
case flutter::DlImageFilterType::kLocalMatrixFilter: {
Expand All @@ -755,9 +761,10 @@ static std::optional<Paint::ImageFilterProc> ToImageFilterProc(
auto matrix = ToMatrix(local_matrix_filter->matrix());

return [matrix, filter_proc = image_filter_proc.value()](
FilterInput::Ref input, const Matrix& effect_transform) {
FilterInput::Ref input, const Matrix& effect_transform,
bool is_subpass) {
std::shared_ptr<FilterContents> filter =
filter_proc(std::move(input), effect_transform);
filter_proc(std::move(input), effect_transform, is_subpass);
return FilterContents::MakeLocalMatrixFilter(FilterInput::Make(filter),
matrix);
};
Expand Down
33 changes: 33 additions & 0 deletions impeller/display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,39 @@ TEST_P(DisplayListTest, CanDrawWithMatrixFilter) {
ASSERT_TRUE(OpenPlaygroundHere(callback));
}

TEST_P(DisplayListTest, CanDrawWithMatrixFilterWhenSavingLayer) {
flutter::DisplayListBuilder builder;
builder.Save();
builder.Scale(2.0, 2.0);
flutter::DlPaint paint;
paint.setColor(flutter::DlColor::kYellow());
builder.DrawRect(SkRect::MakeWH(300, 300), paint);
paint.setStrokeWidth(1.0);
paint.setDrawStyle(flutter::DlDrawStyle::kStroke);
paint.setColor(flutter::DlColor::kBlack().withAlpha(0x80));
builder.DrawLine(SkPoint::Make(150, 0), SkPoint::Make(150, 300), paint);
builder.DrawLine(SkPoint::Make(0, 150), SkPoint::Make(300, 150), paint);

SkRect bounds = SkRect::MakeXYWH(100, 100, 100, 100);
flutter::DlPaint save_paint;
SkMatrix filter_matrix = SkMatrix::I();
filter_matrix.postTranslate(-150, -150);
filter_matrix.postScale(0.2f, 0.2f);
filter_matrix.postTranslate(150, 150);

auto filter = flutter::DlMatrixImageFilter(
filter_matrix, flutter::DlImageSampling::kNearestNeighbor);
save_paint.setImageFilter(filter.shared());

builder.SaveLayer(&bounds, &save_paint);
flutter::DlPaint paint2;
paint2.setColor(flutter::DlColor::kBlue());
builder.DrawRect(bounds, paint2);
builder.Restore();

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

TEST_P(DisplayListTest, CanDrawRectWithLinearToSrgbColorFilter) {
flutter::DlPaint paint;
paint.setColor(flutter::DlColor(0xFF2196F3).withAlpha(128));
Expand Down
8 changes: 6 additions & 2 deletions impeller/entity/contents/filters/filter_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,15 @@ std::shared_ptr<FilterContents> FilterContents::MakeMorphology(
std::shared_ptr<FilterContents> FilterContents::MakeMatrixFilter(
FilterInput::Ref input,
const Matrix& matrix,
const SamplerDescriptor& desc) {
const SamplerDescriptor& desc,
const Matrix& effect_transform,
bool is_subpass) {
auto filter = std::make_shared<MatrixFilterContents>();
filter->SetInputs({std::move(input)});
filter->SetMatrix(matrix);
filter->SetSamplerDescriptor(desc);
filter->SetEffectTransform(effect_transform);
filter->SetSubpass(is_subpass);
return filter;
}

Expand Down Expand Up @@ -154,7 +158,7 @@ void FilterContents::SetCoverageCrop(std::optional<Rect> coverage_crop) {
}

void FilterContents::SetEffectTransform(Matrix effect_transform) {
effect_transform_ = effect_transform.Basis();
effect_transform_ = effect_transform;
}

bool FilterContents::Render(const ContentContext& renderer,
Expand Down
4 changes: 3 additions & 1 deletion impeller/entity/contents/filters/filter_contents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ class FilterContents : public Contents {
static std::shared_ptr<FilterContents> MakeMatrixFilter(
FilterInput::Ref input,
const Matrix& matrix,
const SamplerDescriptor& desc);
const SamplerDescriptor& desc,
const Matrix& effect_transform,
bool is_subpass);

static std::shared_ptr<FilterContents> MakeLocalMatrixFilter(
FilterInput::Ref input,
Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ std::optional<Entity> DirectionalGaussianBlurFilterContents::RenderFilter(

auto radius = Radius{blur_sigma_}.radius;

auto transform = entity.GetTransformation() * effect_transform;
auto transform = entity.GetTransformation() * effect_transform.Basis();
auto transformed_blur_radius =
transform.TransformDirection(blur_direction_ * radius);

Expand Down Expand Up @@ -311,7 +311,7 @@ std::optional<Rect> DirectionalGaussianBlurFilterContents::GetFilterCoverage(
return std::nullopt;
}

auto transform = inputs[0]->GetTransform(entity) * effect_transform;
auto transform = inputs[0]->GetTransform(entity) * effect_transform.Basis();
auto transformed_blur_vector =
transform.TransformDirection(blur_direction_* Radius{blur_sigma_}.radius)
.Abs();
Expand Down
19 changes: 12 additions & 7 deletions impeller/entity/contents/filters/matrix_filter_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ void MatrixFilterContents::SetMatrix(Matrix matrix) {
matrix_ = matrix;
}

void MatrixFilterContents::SetSubpass(bool is_subpass) {
is_subpass_ = is_subpass;
}

void MatrixFilterContents::SetSamplerDescriptor(SamplerDescriptor desc) {
sampler_descriptor_ = std::move(desc);
}
Expand All @@ -29,9 +33,10 @@ std::optional<Entity> MatrixFilterContents::RenderFilter(
return std::nullopt;
}

snapshot->transform = entity.GetTransformation() * //
matrix_ * //
entity.GetTransformation().Invert() * //
auto& transform = is_subpass_ ? effect_transform : entity.GetTransformation();
snapshot->transform = transform * //
matrix_ * //
transform.Invert() * //
snapshot->transform;
snapshot->sampler_descriptor = sampler_descriptor_;
return Contents::EntityFromSnapshot(snapshot, entity.GetBlendMode(),
Expand All @@ -50,10 +55,10 @@ std::optional<Rect> MatrixFilterContents::GetFilterCoverage(
if (!coverage.has_value()) {
return std::nullopt;
}

auto transform = inputs[0]->GetTransform(entity) * //
matrix_ * //
inputs[0]->GetTransform(entity).Invert();
auto& m = is_subpass_ ? effect_transform : inputs[0]->GetTransform(entity);
auto transform = m * //
matrix_ * //
m.Invert(); //
return coverage->TransformBounds(transform);
}

Expand Down
3 changes: 3 additions & 0 deletions impeller/entity/contents/filters/matrix_filter_contents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class MatrixFilterContents final : public FilterContents {

void SetMatrix(Matrix matrix);

void SetSubpass(bool is_subpass);
ColdPaleLight marked this conversation as resolved.
Show resolved Hide resolved

void SetSamplerDescriptor(SamplerDescriptor desc);

// |FilterContents|
Expand All @@ -35,6 +37,7 @@ class MatrixFilterContents final : public FilterContents {

Matrix matrix_;
SamplerDescriptor sampler_descriptor_ = {};
bool is_subpass_;
ColdPaleLight marked this conversation as resolved.
Show resolved Hide resolved

FML_DISALLOW_COPY_AND_ASSIGN(MatrixFilterContents);
};
Expand Down
4 changes: 2 additions & 2 deletions impeller/entity/contents/filters/morphology_filter_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ std::optional<Entity> DirectionalMorphologyFilterContents::RenderFilter(
frame_info.texture_sampler_y_coord_scale =
input_snapshot->texture->GetYCoordScale();

auto transform = entity.GetTransformation() * effect_transform;
auto transform = entity.GetTransformation() * effect_transform.Basis();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The given effect transform should already have its translation/projective coords stripped. Can we remove the Basis() calls here and in the gaussian blur?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The given effect transform should already have its translation/projective coords stripped. Can we remove the Basis() calls here and in the gaussian blur?

I modified the implementation of FilterContents::SetEffectTransform, because MatrixImageFilter needs an original matrix instead of a stripped one, otherwise the result will be incorrect (such as when nesting Transformed widget). So I added Basis() to blur and morphology.

void FilterContents::SetEffectTransform(Matrix effect_transform) {
-  effect_transform_ = effect_transform.Basis();
+  effect_transform_ = effect_transform;
}

auto transformed_radius =
transform.TransformDirection(direction_ * radius_.radius);
auto transformed_texture_vertices =
Expand Down Expand Up @@ -162,7 +162,7 @@ std::optional<Rect> DirectionalMorphologyFilterContents::GetFilterCoverage(
if (!coverage.has_value()) {
return std::nullopt;
}
auto transform = inputs[0]->GetTransform(entity) * effect_transform;
auto transform = inputs[0]->GetTransform(entity) * effect_transform.Basis();
auto transformed_vector =
transform.TransformDirection(direction_ * radius_.radius).Abs();

Expand Down
3 changes: 2 additions & 1 deletion impeller/entity/entity_pass.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,8 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
// Render the backdrop texture before any of the pass elements.
const auto& proc = subpass->backdrop_filter_proc_.value();
backdrop_filter_contents =
proc(FilterInput::Make(std::move(texture)), subpass->xformation_);
proc(FilterInput::Make(std::move(texture)), subpass->xformation_,
/*is_subpass*/ true);

// The subpass will need to read from the current pass texture when
// rendering the backdrop, so if there's an active pass, end it prior to
Expand Down
3 changes: 2 additions & 1 deletion impeller/entity/entity_pass.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class EntityPass {
using Element = std::variant<Entity, std::unique_ptr<EntityPass>>;
using BackdropFilterProc = std::function<std::shared_ptr<FilterContents>(
FilterInput::Ref,
const Matrix& effect_transform)>;
const Matrix& effect_transform,
bool is_subpass)>;

EntityPass();

Expand Down