Skip to content

Commit 1ca01fc

Browse files
committed
[Impeller] Fix mask blurs and the Gaussian blur coverage hint. (flutter#45079)
Resolves flutter/flutter#132936. * In some cases, mask blurs were getting applied twice. * Coverage hint clipping was too aggressive on the second pass of the gaussian blur. * Rework the mask blur applicator to support gracefully falling back to GPU-based color filters when necessary. This fix requires us to cherry-pick: flutter#43519 Going a revert-based route to fix all of these problems would also be pretty complex. https://github.com/flutter/engine/assets/919017/9ee5e856-af7a-42b9-b135-ea268c2ba53f (cherry picked from commit 53595c9)
1 parent 65a2e86 commit 1ca01fc

File tree

10 files changed

+123
-43
lines changed

10 files changed

+123
-43
lines changed

impeller/aiks/aiks_unittests.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2970,5 +2970,40 @@ APPLY_COLOR_FILTER_GRADIENT_TEST(Radial);
29702970
APPLY_COLOR_FILTER_GRADIENT_TEST(Conical);
29712971
APPLY_COLOR_FILTER_GRADIENT_TEST(Sweep);
29722972

2973+
TEST_P(AiksTest, ClippedBlurFilterRendersCorrectlyInteractive) {
2974+
auto callback = [&](AiksContext& renderer, RenderTarget& render_target) {
2975+
auto point = IMPELLER_PLAYGROUND_POINT(Point(400, 400), 20, Color::Green());
2976+
2977+
Canvas canvas;
2978+
canvas.Translate(point - Point(400, 400));
2979+
Paint paint;
2980+
paint.mask_blur_descriptor = Paint::MaskBlurDescriptor{
2981+
.style = FilterContents::BlurStyle::kNormal,
2982+
.sigma = Radius{120 * 3},
2983+
};
2984+
paint.color = Color::Red();
2985+
PathBuilder builder{};
2986+
builder.AddRect(Rect::MakeLTRB(0, 0, 800, 800));
2987+
canvas.DrawPath(builder.TakePath(), paint);
2988+
return renderer.Render(canvas.EndRecordingAsPicture(), render_target);
2989+
};
2990+
ASSERT_TRUE(OpenPlaygroundHere(callback));
2991+
}
2992+
2993+
TEST_P(AiksTest, ClippedBlurFilterRendersCorrectly) {
2994+
Canvas canvas;
2995+
canvas.Translate(Point(0, -400));
2996+
Paint paint;
2997+
paint.mask_blur_descriptor = Paint::MaskBlurDescriptor{
2998+
.style = FilterContents::BlurStyle::kNormal,
2999+
.sigma = Radius{120 * 3},
3000+
};
3001+
paint.color = Color::Red();
3002+
PathBuilder builder{};
3003+
builder.AddRect(Rect::MakeLTRB(0, 0, 800, 800));
3004+
canvas.DrawPath(builder.TakePath(), paint);
3005+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
3006+
}
3007+
29733008
} // namespace testing
29743009
} // namespace impeller

impeller/aiks/canvas.cc

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ void Canvas::DrawImageRect(const std::shared_ptr<Image>& image,
456456
Entity entity;
457457
entity.SetBlendMode(paint.blend_mode);
458458
entity.SetStencilDepth(GetStencilDepth());
459-
entity.SetContents(paint.WithFilters(contents, false));
459+
entity.SetContents(paint.WithFilters(contents));
460460
entity.SetTransformation(GetCurrentTransformation());
461461

462462
GetCurrentPass().AddEntity(entity);
@@ -537,8 +537,14 @@ void Canvas::DrawTextFrame(const TextFrame& text_frame,
537537
color_text_contents->SetColorSourceContents(
538538
paint.color_source.GetContents(paint));
539539

540-
entity.SetContents(
541-
paint.WithFilters(std::move(color_text_contents), false));
540+
// TODO(bdero): This mask blur application is a hack. It will always wind up
541+
// doing a gaussian blur that affects the color source itself
542+
// instead of just the mask. The color filter text support
543+
// needs to be reworked in order to interact correctly with
544+
// mask filters.
545+
// https://github.com/flutter/flutter/issues/133297
546+
entity.SetContents(paint.WithFilters(
547+
paint.WithMaskBlur(std::move(color_text_contents), true)));
542548

543549
GetCurrentPass().AddEntity(entity);
544550
return;
@@ -549,7 +555,14 @@ void Canvas::DrawTextFrame(const TextFrame& text_frame,
549555
entity.SetTransformation(GetCurrentTransformation() *
550556
Matrix::MakeTranslation(position));
551557

552-
entity.SetContents(paint.WithFilters(std::move(text_contents), true));
558+
// TODO(bdero): This mask blur application is a hack. It will always wind up
559+
// doing a gaussian blur that affects the color source itself
560+
// instead of just the mask. The color filter text support
561+
// needs to be reworked in order to interact correctly with
562+
// mask filters.
563+
// https://github.com/flutter/flutter/issues/133297
564+
entity.SetContents(
565+
paint.WithFilters(paint.WithMaskBlur(std::move(text_contents), true)));
553566

554567
GetCurrentPass().AddEntity(entity);
555568
}
@@ -658,7 +671,7 @@ void Canvas::DrawAtlas(const std::shared_ptr<Image>& atlas,
658671
entity.SetTransformation(GetCurrentTransformation());
659672
entity.SetStencilDepth(GetStencilDepth());
660673
entity.SetBlendMode(paint.blend_mode);
661-
entity.SetContents(paint.WithFilters(contents, false));
674+
entity.SetContents(paint.WithFilters(contents));
662675

663676
GetCurrentPass().AddEntity(entity);
664677
}

impeller/aiks/paint.cc

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,21 +34,32 @@ std::shared_ptr<Contents> Paint::CreateContentsForEntity(const Path& path,
3434
std::shared_ptr<Contents> Paint::CreateContentsForGeometry(
3535
std::shared_ptr<Geometry> geometry) const {
3636
auto contents = color_source.GetContents(*this);
37+
38+
// Attempt to apply the color filter on the CPU first.
39+
// Note: This is not just an optimization; some color sources rely on
40+
// CPU-applied color filters to behave properly.
41+
bool needs_color_filter = !!color_filter;
42+
if (color_filter &&
43+
contents->ApplyColorFilter(color_filter->GetCPUColorFilterProc())) {
44+
needs_color_filter = false;
45+
}
46+
3747
contents->SetGeometry(std::move(geometry));
3848
if (mask_blur_descriptor.has_value()) {
39-
return mask_blur_descriptor->CreateMaskBlur(contents);
49+
// If there's a mask blur and we need to apply the color filter on the GPU,
50+
// we need to be careful to only apply the color filter to the source
51+
// colors. CreateMaskBlur is able to handle this case.
52+
return mask_blur_descriptor->CreateMaskBlur(
53+
contents, needs_color_filter ? color_filter : nullptr);
4054
}
55+
4156
return contents;
4257
}
4358

4459
std::shared_ptr<Contents> Paint::WithFilters(
45-
std::shared_ptr<Contents> input,
46-
std::optional<bool> is_solid_color) const {
47-
bool is_solid_color_val = is_solid_color.value_or(color_source.GetType() ==
48-
ColorSource::Type::kColor);
60+
std::shared_ptr<Contents> input) const {
4961
input = WithColorFilter(input, /*absorb_opacity=*/true);
5062
input = WithInvertFilter(input);
51-
input = WithMaskBlur(input, is_solid_color_val);
5263
input = WithImageFilter(input, Matrix(), /*is_subpass=*/false);
5364
return input;
5465
}
@@ -128,7 +139,16 @@ std::shared_ptr<Contents> Paint::WithInvertFilter(
128139
}
129140

130141
std::shared_ptr<FilterContents> Paint::MaskBlurDescriptor::CreateMaskBlur(
131-
std::shared_ptr<ColorSourceContents> color_source_contents) const {
142+
std::shared_ptr<ColorSourceContents> color_source_contents,
143+
const std::shared_ptr<ColorFilter>& color_filter) const {
144+
// If it's a solid color and there is no color filter, then we can just get
145+
// away with doing one Gaussian blur.
146+
if (color_source_contents->IsSolidColor() && !color_filter) {
147+
return FilterContents::MakeGaussianBlur(
148+
FilterInput::Make(color_source_contents), sigma, sigma, style,
149+
Entity::TileMode::kDecal, Matrix());
150+
}
151+
132152
/// 1. Create an opaque white mask of the original geometry.
133153

134154
auto mask = std::make_shared<SolidColorContents>();
@@ -152,11 +172,20 @@ std::shared_ptr<FilterContents> Paint::MaskBlurDescriptor::CreateMaskBlur(
152172
color_source_contents->SetGeometry(
153173
Geometry::MakeRect(*expanded_local_bounds));
154174

155-
/// 4. Composite the color source and mask together.
175+
std::shared_ptr<Contents> color_contents = color_source_contents;
176+
177+
/// 4. Apply the user set color filter on the GPU, if applicable.
178+
179+
if (color_filter) {
180+
color_contents = color_filter->WrapWithGPUColorFilter(
181+
FilterInput::Make(color_source_contents), true);
182+
}
183+
184+
/// 5. Composite the color source with the blurred mask.
156185

157186
return ColorFilterContents::MakeBlend(
158-
BlendMode::kSourceIn, {FilterInput::Make(blurred_mask),
159-
FilterInput::Make(color_source_contents)});
187+
BlendMode::kSourceIn,
188+
{FilterInput::Make(blurred_mask), FilterInput::Make(color_contents)});
160189
}
161190

162191
std::shared_ptr<FilterContents> Paint::MaskBlurDescriptor::CreateMaskBlur(

impeller/aiks/paint.h

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ struct Paint {
4242
Sigma sigma;
4343

4444
std::shared_ptr<FilterContents> CreateMaskBlur(
45-
std::shared_ptr<ColorSourceContents> color_source_contents) const;
45+
std::shared_ptr<ColorSourceContents> color_source_contents,
46+
const std::shared_ptr<ColorFilter>& color_filter) const;
4647

4748
std::shared_ptr<FilterContents> CreateMaskBlur(
4849
const FilterInput::Ref& input,
@@ -66,18 +67,10 @@ struct Paint {
6667

6768
/// @brief Wrap this paint's configured filters to the given contents.
6869
/// @param[in] input The contents to wrap with paint's filters.
69-
/// @param[in] is_solid_color Affects mask blurring behavior. If false, use
70-
/// the image border for mask blurring. If true,
71-
/// do a Gaussian blur to achieve the mask
72-
/// blurring effect for arbitrary paths. If unset,
73-
/// use the current paint configuration to infer
74-
/// the result.
7570
/// @return The filter-wrapped contents. If there are no filters that need
7671
/// to be wrapped for the current paint configuration, the
7772
/// original contents is returned.
78-
std::shared_ptr<Contents> WithFilters(
79-
std::shared_ptr<Contents> input,
80-
std::optional<bool> is_solid_color = std::nullopt) const;
73+
std::shared_ptr<Contents> WithFilters(std::shared_ptr<Contents> input) const;
8174

8275
/// @brief Wrap this paint's configured filters to the given contents of
8376
/// subpass target.
@@ -100,10 +93,10 @@ struct Paint {
10093
/// @brief Whether this paint has a color filter that can apply opacity
10194
bool HasColorFilter() const;
10295

103-
private:
10496
std::shared_ptr<Contents> WithMaskBlur(std::shared_ptr<Contents> input,
10597
bool is_solid_color) const;
10698

99+
private:
107100
std::shared_ptr<Contents> WithImageFilter(std::shared_ptr<Contents> input,
108101
const Matrix& effect_transform,
109102
bool is_subpass) const;

impeller/entity/contents/color_source_contents.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ const Matrix& ColorSourceContents::GetInverseMatrix() const {
3838
return inverse_matrix_;
3939
}
4040

41+
bool ColorSourceContents::IsSolidColor() const {
42+
return false;
43+
}
44+
4145
std::optional<Rect> ColorSourceContents::GetCoverage(
4246
const Entity& entity) const {
4347
return geometry_->GetCoverage(entity.GetTransformation());

impeller/entity/contents/color_source_contents.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ class ColorSourceContents : public Contents {
2626

2727
void SetOpacity(Scalar opacity);
2828

29+
virtual bool IsSolidColor() const;
30+
2931
// |Contents|
3032
std::optional<Rect> GetCoverage(const Entity& entity) const override;
3133

impeller/entity/contents/filters/gaussian_blur_filter_contents.cc

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,19 @@ std::optional<Entity> DirectionalGaussianBlurFilterContents::RenderFilter(
104104
// Limit the kernel size to 1000x1000 pixels, like Skia does.
105105
auto radius = std::min(Radius{blur_sigma_}.radius, 500.0f);
106106

107+
auto transform = entity.GetTransformation() * effect_transform.Basis();
108+
auto transformed_blur_radius =
109+
transform.TransformDirection(blur_direction_ * radius);
110+
111+
auto transformed_blur_radius_length = transformed_blur_radius.GetLength();
112+
107113
// Input 0 snapshot.
108114

109115
std::optional<Rect> expanded_coverage_hint;
110116
if (coverage_hint.has_value()) {
111-
auto r = Size(radius, radius).Abs();
117+
auto r =
118+
Size(transformed_blur_radius_length, transformed_blur_radius_length)
119+
.Abs();
112120
expanded_coverage_hint =
113121
is_first_pass ? Rect(coverage_hint.value().origin - r,
114122
Size(coverage_hint.value().size + r * 2))
@@ -126,12 +134,6 @@ std::optional<Entity> DirectionalGaussianBlurFilterContents::RenderFilter(
126134
entity.GetStencilDepth()); // No blur to render.
127135
}
128136

129-
auto transform = entity.GetTransformation() * effect_transform.Basis();
130-
auto transformed_blur_radius =
131-
transform.TransformDirection(blur_direction_ * radius);
132-
133-
auto transformed_blur_radius_length = transformed_blur_radius.GetLength();
134-
135137
// If the radius length is < .5, the shader will take at most 1 sample,
136138
// resulting in no blur.
137139
if (transformed_blur_radius_length < .5) {
@@ -158,16 +160,6 @@ std::optional<Entity> DirectionalGaussianBlurFilterContents::RenderFilter(
158160
pass_texture_rect.origin.x -= transformed_blur_radius_length;
159161
pass_texture_rect.size.width += transformed_blur_radius_length * 2;
160162

161-
// Crop the pass texture with the rotated coverage hint if one was given.
162-
if (expanded_coverage_hint.has_value()) {
163-
auto maybe_pass_texture_rect = pass_texture_rect.Intersection(
164-
expanded_coverage_hint->TransformBounds(texture_rotate));
165-
if (!maybe_pass_texture_rect.has_value()) {
166-
return std::nullopt;
167-
}
168-
pass_texture_rect = *maybe_pass_texture_rect;
169-
}
170-
171163
// Source override snapshot.
172164

173165
auto source = source_override_ ? source_override_ : inputs[0];
@@ -335,6 +327,8 @@ std::optional<Entity> DirectionalGaussianBlurFilterContents::RenderFilter(
335327
SamplerDescriptor sampler_desc;
336328
sampler_desc.min_filter = MinMagFilter::kLinear;
337329
sampler_desc.mag_filter = MinMagFilter::kLinear;
330+
sampler_desc.width_address_mode = SamplerAddressMode::kClampToEdge;
331+
sampler_desc.width_address_mode = SamplerAddressMode::kClampToEdge;
338332

339333
return Entity::FromSnapshot(
340334
Snapshot{.texture = out_texture,

impeller/entity/contents/solid_color_contents.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ Color SolidColorContents::GetColor() const {
2424
return color_.WithAlpha(color_.alpha * GetOpacity());
2525
}
2626

27+
bool SolidColorContents::IsSolidColor() const {
28+
return true;
29+
}
30+
2731
bool SolidColorContents::IsOpaque() const {
2832
return GetColor().IsOpaque();
2933
}

impeller/entity/contents/solid_color_contents.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ class SolidColorContents final : public ColorSourceContents {
3434

3535
Color GetColor() const;
3636

37+
// |ColorSourceContents|
38+
bool IsSolidColor() const override;
39+
3740
// |Contents|
3841
bool IsOpaque() const override;
3942

impeller/golden_tests/golden_playground_test_mac.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ static const std::vector<std::string> kSkipTests = {
2727
"impeller_Play_AiksTest_CanRenderRadialGradientManyColors_Vulkan",
2828
"impeller_Play_AiksTest_CanRenderBackdropBlurInteractive_Metal",
2929
"impeller_Play_AiksTest_CanRenderBackdropBlurInteractive_Vulkan",
30+
"impeller_Play_AiksTest_ClippedBlurFilterRendersCorrectlyInteractive_Metal",
31+
"impeller_Play_AiksTest_ClippedBlurFilterRendersCorrectlyInteractive_"
32+
"Vulkan",
3033
"impeller_Play_AiksTest_TextFrameSubpixelAlignment_Metal",
3134
"impeller_Play_AiksTest_TextFrameSubpixelAlignment_Vulkan",
3235
"impeller_Play_AiksTest_ColorWheel_Metal",

0 commit comments

Comments
 (0)