From 189d94fc77733297faf7fbbd6b72a3834c9d9f3d Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 25 Aug 2023 14:11:13 -0700 Subject: [PATCH] [Impeller] Fix mask blurs and the Gaussian blur coverage hint. (#45079) Resolves https://github.com/flutter/flutter/issues/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: https://github.com/flutter/engine/pull/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 53595c937df156b932850807869f539c15042919) --- impeller/aiks/aiks_unittests.cc | 35 +++++++++++++ impeller/aiks/canvas.cc | 23 +++++++-- impeller/aiks/paint.cc | 49 +++++++++++++++---- impeller/aiks/paint.h | 15 ++---- .../entity/contents/color_source_contents.cc | 4 ++ .../entity/contents/color_source_contents.h | 2 + .../filters/gaussian_blur_filter_contents.cc | 28 +++++------ .../entity/contents/solid_color_contents.cc | 4 ++ .../entity/contents/solid_color_contents.h | 3 ++ .../golden_playground_test_mac.cc | 3 ++ 10 files changed, 123 insertions(+), 43 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index d6ed92840a838..ccbb87d7507c0 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3124,5 +3124,40 @@ TEST_P(AiksTest, PipelineBlendSingleParameter) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, ClippedBlurFilterRendersCorrectlyInteractive) { + auto callback = [&](AiksContext& renderer, RenderTarget& render_target) { + auto point = IMPELLER_PLAYGROUND_POINT(Point(400, 400), 20, Color::Green()); + + Canvas canvas; + canvas.Translate(point - Point(400, 400)); + Paint paint; + paint.mask_blur_descriptor = Paint::MaskBlurDescriptor{ + .style = FilterContents::BlurStyle::kNormal, + .sigma = Radius{120 * 3}, + }; + paint.color = Color::Red(); + PathBuilder builder{}; + builder.AddRect(Rect::MakeLTRB(0, 0, 800, 800)); + canvas.DrawPath(builder.TakePath(), paint); + return renderer.Render(canvas.EndRecordingAsPicture(), render_target); + }; + ASSERT_TRUE(OpenPlaygroundHere(callback)); +} + +TEST_P(AiksTest, ClippedBlurFilterRendersCorrectly) { + Canvas canvas; + canvas.Translate(Point(0, -400)); + Paint paint; + paint.mask_blur_descriptor = Paint::MaskBlurDescriptor{ + .style = FilterContents::BlurStyle::kNormal, + .sigma = Radius{120 * 3}, + }; + paint.color = Color::Red(); + PathBuilder builder{}; + builder.AddRect(Rect::MakeLTRB(0, 0, 800, 800)); + canvas.DrawPath(builder.TakePath(), paint); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 84cb7544fdcd7..50a8f2d49f755 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -483,7 +483,7 @@ void Canvas::DrawImageRect(const std::shared_ptr& image, Entity entity; entity.SetBlendMode(paint.blend_mode); entity.SetStencilDepth(GetStencilDepth()); - entity.SetContents(paint.WithFilters(contents, false)); + entity.SetContents(paint.WithFilters(contents)); entity.SetTransformation(GetCurrentTransformation()); GetCurrentPass().AddEntity(entity); @@ -552,8 +552,14 @@ void Canvas::DrawTextFrame(const TextFrame& text_frame, color_text_contents->SetColorSourceContents( paint.color_source.GetContents(paint)); - entity.SetContents( - paint.WithFilters(std::move(color_text_contents), false)); + // TODO(bdero): This mask blur application is a hack. It will always wind up + // doing a gaussian blur that affects the color source itself + // instead of just the mask. The color filter text support + // needs to be reworked in order to interact correctly with + // mask filters. + // https://github.com/flutter/flutter/issues/133297 + entity.SetContents(paint.WithFilters( + paint.WithMaskBlur(std::move(color_text_contents), true))); GetCurrentPass().AddEntity(entity); return; @@ -564,7 +570,14 @@ void Canvas::DrawTextFrame(const TextFrame& text_frame, entity.SetTransformation(GetCurrentTransformation() * Matrix::MakeTranslation(position)); - entity.SetContents(paint.WithFilters(std::move(text_contents), true)); + // TODO(bdero): This mask blur application is a hack. It will always wind up + // doing a gaussian blur that affects the color source itself + // instead of just the mask. The color filter text support + // needs to be reworked in order to interact correctly with + // mask filters. + // https://github.com/flutter/flutter/issues/133297 + entity.SetContents( + paint.WithFilters(paint.WithMaskBlur(std::move(text_contents), true))); GetCurrentPass().AddEntity(entity); } @@ -673,7 +686,7 @@ void Canvas::DrawAtlas(const std::shared_ptr& atlas, entity.SetTransformation(GetCurrentTransformation()); entity.SetStencilDepth(GetStencilDepth()); entity.SetBlendMode(paint.blend_mode); - entity.SetContents(paint.WithFilters(contents, false)); + entity.SetContents(paint.WithFilters(contents)); GetCurrentPass().AddEntity(entity); } diff --git a/impeller/aiks/paint.cc b/impeller/aiks/paint.cc index 5cf342ee38cc4..07751623e26d1 100644 --- a/impeller/aiks/paint.cc +++ b/impeller/aiks/paint.cc @@ -34,21 +34,32 @@ std::shared_ptr Paint::CreateContentsForEntity(const Path& path, std::shared_ptr Paint::CreateContentsForGeometry( std::shared_ptr geometry) const { auto contents = color_source.GetContents(*this); + + // Attempt to apply the color filter on the CPU first. + // Note: This is not just an optimization; some color sources rely on + // CPU-applied color filters to behave properly. + bool needs_color_filter = !!color_filter; + if (color_filter && + contents->ApplyColorFilter(color_filter->GetCPUColorFilterProc())) { + needs_color_filter = false; + } + contents->SetGeometry(std::move(geometry)); if (mask_blur_descriptor.has_value()) { - return mask_blur_descriptor->CreateMaskBlur(contents); + // If there's a mask blur and we need to apply the color filter on the GPU, + // we need to be careful to only apply the color filter to the source + // colors. CreateMaskBlur is able to handle this case. + return mask_blur_descriptor->CreateMaskBlur( + contents, needs_color_filter ? color_filter : nullptr); } + return contents; } std::shared_ptr Paint::WithFilters( - std::shared_ptr input, - std::optional is_solid_color) const { - bool is_solid_color_val = is_solid_color.value_or(color_source.GetType() == - ColorSource::Type::kColor); + std::shared_ptr input) const { input = WithColorFilter(input, /*absorb_opacity=*/true); input = WithInvertFilter(input); - input = WithMaskBlur(input, is_solid_color_val); input = WithImageFilter(input, Matrix(), /*is_subpass=*/false); return input; } @@ -128,7 +139,16 @@ std::shared_ptr Paint::WithInvertFilter( } std::shared_ptr Paint::MaskBlurDescriptor::CreateMaskBlur( - std::shared_ptr color_source_contents) const { + std::shared_ptr color_source_contents, + const std::shared_ptr& color_filter) const { + // If it's a solid color and there is no color filter, then we can just get + // away with doing one Gaussian blur. + if (color_source_contents->IsSolidColor() && !color_filter) { + return FilterContents::MakeGaussianBlur( + FilterInput::Make(color_source_contents), sigma, sigma, style, + Entity::TileMode::kDecal, Matrix()); + } + /// 1. Create an opaque white mask of the original geometry. auto mask = std::make_shared(); @@ -152,11 +172,20 @@ std::shared_ptr Paint::MaskBlurDescriptor::CreateMaskBlur( color_source_contents->SetGeometry( Geometry::MakeRect(*expanded_local_bounds)); - /// 4. Composite the color source and mask together. + std::shared_ptr color_contents = color_source_contents; + + /// 4. Apply the user set color filter on the GPU, if applicable. + + if (color_filter) { + color_contents = color_filter->WrapWithGPUColorFilter( + FilterInput::Make(color_source_contents), true); + } + + /// 5. Composite the color source with the blurred mask. return ColorFilterContents::MakeBlend( - BlendMode::kSourceIn, {FilterInput::Make(blurred_mask), - FilterInput::Make(color_source_contents)}); + BlendMode::kSourceIn, + {FilterInput::Make(blurred_mask), FilterInput::Make(color_contents)}); } std::shared_ptr Paint::MaskBlurDescriptor::CreateMaskBlur( diff --git a/impeller/aiks/paint.h b/impeller/aiks/paint.h index 7f3839be18a17..c8a3434525507 100644 --- a/impeller/aiks/paint.h +++ b/impeller/aiks/paint.h @@ -42,7 +42,8 @@ struct Paint { Sigma sigma; std::shared_ptr CreateMaskBlur( - std::shared_ptr color_source_contents) const; + std::shared_ptr color_source_contents, + const std::shared_ptr& color_filter) const; std::shared_ptr CreateMaskBlur( const FilterInput::Ref& input, @@ -67,18 +68,10 @@ struct Paint { /// @brief Wrap this paint's configured filters to the given contents. /// @param[in] input The contents to wrap with paint's filters. - /// @param[in] is_solid_color Affects mask blurring behavior. If false, use - /// the image border for mask blurring. If true, - /// do a Gaussian blur to achieve the mask - /// blurring effect for arbitrary paths. If unset, - /// use the current paint configuration to infer - /// the result. /// @return The filter-wrapped contents. If there are no filters that need /// to be wrapped for the current paint configuration, the /// original contents is returned. - std::shared_ptr WithFilters( - std::shared_ptr input, - std::optional is_solid_color = std::nullopt) const; + std::shared_ptr WithFilters(std::shared_ptr input) const; /// @brief Wrap this paint's configured filters to the given contents of /// subpass target. @@ -101,10 +94,10 @@ struct Paint { /// @brief Whether this paint has a color filter that can apply opacity bool HasColorFilter() const; - private: std::shared_ptr WithMaskBlur(std::shared_ptr input, bool is_solid_color) const; + private: std::shared_ptr WithImageFilter(std::shared_ptr input, const Matrix& effect_transform, bool is_subpass) const; diff --git a/impeller/entity/contents/color_source_contents.cc b/impeller/entity/contents/color_source_contents.cc index 24b19566fccc0..8977b6d6026b1 100644 --- a/impeller/entity/contents/color_source_contents.cc +++ b/impeller/entity/contents/color_source_contents.cc @@ -38,6 +38,10 @@ const Matrix& ColorSourceContents::GetInverseEffectTransform() const { return inverse_matrix_; } +bool ColorSourceContents::IsSolidColor() const { + return false; +} + std::optional ColorSourceContents::GetCoverage( const Entity& entity) const { return geometry_->GetCoverage(entity.GetTransformation()); diff --git a/impeller/entity/contents/color_source_contents.h b/impeller/entity/contents/color_source_contents.h index 5ae6f15499bff..53b3faecd91c2 100644 --- a/impeller/entity/contents/color_source_contents.h +++ b/impeller/entity/contents/color_source_contents.h @@ -90,6 +90,8 @@ class ColorSourceContents : public Contents { /// Scalar GetOpacityFactor() const; + virtual bool IsSolidColor() const; + // |Contents| std::optional GetCoverage(const Entity& entity) const override; diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 0226507dbc8b6..b0b03975983d1 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -104,11 +104,19 @@ std::optional DirectionalGaussianBlurFilterContents::RenderFilter( // Limit the kernel size to 1000x1000 pixels, like Skia does. auto radius = std::min(Radius{blur_sigma_}.radius, 500.0f); + auto transform = entity.GetTransformation() * effect_transform.Basis(); + auto transformed_blur_radius = + transform.TransformDirection(blur_direction_ * radius); + + auto transformed_blur_radius_length = transformed_blur_radius.GetLength(); + // Input 0 snapshot. std::optional expanded_coverage_hint; if (coverage_hint.has_value()) { - auto r = Size(radius, radius).Abs(); + auto r = + Size(transformed_blur_radius_length, transformed_blur_radius_length) + .Abs(); expanded_coverage_hint = is_first_pass ? Rect(coverage_hint.value().origin - r, Size(coverage_hint.value().size + r * 2)) @@ -126,12 +134,6 @@ std::optional DirectionalGaussianBlurFilterContents::RenderFilter( entity.GetStencilDepth()); // No blur to render. } - auto transform = entity.GetTransformation() * effect_transform.Basis(); - auto transformed_blur_radius = - transform.TransformDirection(blur_direction_ * radius); - - auto transformed_blur_radius_length = transformed_blur_radius.GetLength(); - // If the radius length is < .5, the shader will take at most 1 sample, // resulting in no blur. if (transformed_blur_radius_length < .5) { @@ -158,16 +160,6 @@ std::optional DirectionalGaussianBlurFilterContents::RenderFilter( pass_texture_rect.origin.x -= transformed_blur_radius_length; pass_texture_rect.size.width += transformed_blur_radius_length * 2; - // Crop the pass texture with the rotated coverage hint if one was given. - if (expanded_coverage_hint.has_value()) { - auto maybe_pass_texture_rect = pass_texture_rect.Intersection( - expanded_coverage_hint->TransformBounds(texture_rotate)); - if (!maybe_pass_texture_rect.has_value()) { - return std::nullopt; - } - pass_texture_rect = *maybe_pass_texture_rect; - } - // Source override snapshot. auto source = source_override_ ? source_override_ : inputs[0]; @@ -335,6 +327,8 @@ std::optional DirectionalGaussianBlurFilterContents::RenderFilter( SamplerDescriptor sampler_desc; sampler_desc.min_filter = MinMagFilter::kLinear; sampler_desc.mag_filter = MinMagFilter::kLinear; + sampler_desc.width_address_mode = SamplerAddressMode::kClampToEdge; + sampler_desc.width_address_mode = SamplerAddressMode::kClampToEdge; return Entity::FromSnapshot( Snapshot{.texture = out_texture, diff --git a/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc index b90f3ffbe263f..d72b7e026ecff 100644 --- a/impeller/entity/contents/solid_color_contents.cc +++ b/impeller/entity/contents/solid_color_contents.cc @@ -24,6 +24,10 @@ Color SolidColorContents::GetColor() const { return color_.WithAlpha(color_.alpha * GetOpacityFactor()); } +bool SolidColorContents::IsSolidColor() const { + return true; +} + bool SolidColorContents::IsOpaque() const { return GetColor().IsOpaque(); } diff --git a/impeller/entity/contents/solid_color_contents.h b/impeller/entity/contents/solid_color_contents.h index 3d704b4ee7898..6b31e11761aff 100644 --- a/impeller/entity/contents/solid_color_contents.h +++ b/impeller/entity/contents/solid_color_contents.h @@ -34,6 +34,9 @@ class SolidColorContents final : public ColorSourceContents { Color GetColor() const; + // |ColorSourceContents| + bool IsSolidColor() const override; + // |Contents| bool IsOpaque() const override; diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 55209e7836746..86b0fff6d991a 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -27,6 +27,9 @@ static const std::vector kSkipTests = { "impeller_Play_AiksTest_CanRenderRadialGradientManyColors_Vulkan", "impeller_Play_AiksTest_CanRenderBackdropBlurInteractive_Metal", "impeller_Play_AiksTest_CanRenderBackdropBlurInteractive_Vulkan", + "impeller_Play_AiksTest_ClippedBlurFilterRendersCorrectlyInteractive_Metal", + "impeller_Play_AiksTest_ClippedBlurFilterRendersCorrectlyInteractive_" + "Vulkan", "impeller_Play_AiksTest_TextFrameSubpixelAlignment_Metal", "impeller_Play_AiksTest_TextFrameSubpixelAlignment_Vulkan", "impeller_Play_AiksTest_ColorWheel_Metal",