From dabd93170d607f666e4c5699860ef16ed7967258 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 08:46:17 -0700 Subject: [PATCH 01/12] [Impeller] moved blur to unrotated local space, started respecting `respect_ctm` flag --- .../filters/gaussian_blur_filter_contents.cc | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 918d145a75dfd..9018bbc9eb1ff 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -379,6 +379,14 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( return expanded_source_coverage.TransformBounds(entity.GetTransform()); } +namespace { +Vector2 ExtractScale(const Matrix& matrix) { + Vector2 entity_scale_x = matrix * Vector2(1.0, 0.0); + Vector2 entity_scale_y = matrix * Vector2(0.0, 1.0); + return Vector2(entity_scale_x.GetLength(), entity_scale_y.GetLength()); +} +} // namespace + std::optional GaussianBlurFilterContents::RenderFilter( const FilterInput::Vector& inputs, const ContentContext& renderer, @@ -390,7 +398,12 @@ std::optional GaussianBlurFilterContents::RenderFilter( return std::nullopt; } + const Vector2 source_space_scalar = + ExtractScale(entity.GetTransform().Basis()); + Vector2 scaled_sigma = (effect_transform.Basis() * + Matrix::MakeScale({source_space_scalar.x, + source_space_scalar.y, 1}) * // Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); @@ -417,11 +430,13 @@ std::optional GaussianBlurFilterContents::RenderFilter( } Entity snapshot_entity = entity.Clone(); - snapshot_entity.SetTransform(Matrix()); + snapshot_entity.SetTransform( + Matrix::MakeScale({source_space_scalar.x, source_space_scalar.y, 1})); std::optional source_expanded_coverage_hint; if (expanded_coverage_hint.has_value()) { - source_expanded_coverage_hint = - expanded_coverage_hint->TransformBounds(entity.GetTransform().Invert()); + source_expanded_coverage_hint = expanded_coverage_hint->TransformBounds( + Matrix::MakeScale({source_space_scalar.x, source_space_scalar.y, 1}) * + entity.GetTransform().Invert()); } std::optional input_snapshot = @@ -436,7 +451,10 @@ std::optional GaussianBlurFilterContents::RenderFilter( Entity result = Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode()); // No blur to render. - result.SetTransform(entity.GetTransform() * input_snapshot->transform); + result.SetTransform(entity.GetTransform() * + Matrix::MakeScale({1.f / source_space_scalar.x, + 1.f / source_space_scalar.y, 1}) * + input_snapshot->transform); return result; } @@ -562,12 +580,16 @@ std::optional GaussianBlurFilterContents::RenderFilter( MinMagFilter::kLinear, SamplerAddressMode::kClampToEdge); Entity blur_output_entity = Entity::FromSnapshot( - Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(), - .transform = entity.GetTransform() * input_snapshot->transform * - padding_snapshot_adjustment * - Matrix::MakeScale(1 / effective_scalar), - .sampler_descriptor = sampler_desc, - .opacity = input_snapshot->opacity}, + Snapshot{ + .texture = pass3_out.value().GetRenderTargetTexture(), + .transform = entity.GetTransform() * // + Matrix::MakeScale({1.f / source_space_scalar.x, + 1.f / source_space_scalar.y, 1}) * // + input_snapshot->transform * // + padding_snapshot_adjustment * // + Matrix::MakeScale(1 / effective_scalar), + .sampler_descriptor = sampler_desc, + .opacity = input_snapshot->opacity}, entity.GetBlendMode()); return ApplyBlurStyle(mask_blur_style_, entity, inputs[0], From 1550833a343713f97a3a7301cb4adcb19123d821 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 12 Jun 2024 20:29:45 -0700 Subject: [PATCH 02/12] [Impeller] Respect the respect_ctm flag for mask blurred text & absorb entity scale when rendering the 2-pass blur input. --- impeller/aiks/canvas.cc | 4 ++-- impeller/aiks/experimental_canvas.cc | 4 ++-- impeller/aiks/paint.cc | 18 +++++++++++++----- impeller/aiks/paint.h | 9 +++++++-- impeller/display_list/dl_dispatcher.cc | 1 + 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index f396c42e25ba4..9f7544a952826 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -909,8 +909,8 @@ void Canvas::DrawTextFrame(const std::shared_ptr& text_frame, // 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))); + entity.SetContents(paint.WithFilters(paint.WithMaskBlur( + std::move(text_contents), true, GetCurrentTransform()))); AddRenderEntityToCurrentPass(std::move(entity)); } diff --git a/impeller/aiks/experimental_canvas.cc b/impeller/aiks/experimental_canvas.cc index 2bbc8b186d8e6..6d6565e92da7d 100644 --- a/impeller/aiks/experimental_canvas.cc +++ b/impeller/aiks/experimental_canvas.cc @@ -409,8 +409,8 @@ void ExperimentalCanvas::DrawTextFrame( // 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))); + entity.SetContents(paint.WithFilters(paint.WithMaskBlur( + std::move(text_contents), true, GetCurrentTransform()))); AddRenderEntityToCurrentPass(std::move(entity), false); } diff --git a/impeller/aiks/paint.cc b/impeller/aiks/paint.cc index aa7350c031493..12a5a0adc045a 100644 --- a/impeller/aiks/paint.cc +++ b/impeller/aiks/paint.cc @@ -78,10 +78,11 @@ std::shared_ptr Paint::WithFiltersForSubpassTarget( } std::shared_ptr Paint::WithMaskBlur(std::shared_ptr input, - bool is_solid_color) const { + bool is_solid_color, + const Matrix& ctm) const { if (mask_blur_descriptor.has_value()) { input = mask_blur_descriptor->CreateMaskBlur(FilterInput::Make(input), - is_solid_color); + is_solid_color, ctm); } return input; } @@ -203,12 +204,19 @@ std::shared_ptr Paint::MaskBlurDescriptor::CreateMaskBlur( std::shared_ptr Paint::MaskBlurDescriptor::CreateMaskBlur( const FilterInput::Ref& input, - bool is_solid_color) const { + bool is_solid_color, + const Matrix& ctm) const { + Vector2 blur_sigma(sigma.sigma, sigma.sigma); + if (!respect_ctm) { + blur_sigma /= Vector2(ctm.GetBasisX().Length(), ctm.GetBasisY().Length()); + } if (is_solid_color) { - return FilterContents::MakeGaussianBlur(input, sigma, sigma, + return FilterContents::MakeGaussianBlur(input, Sigma(blur_sigma.x), + Sigma(blur_sigma.y), Entity::TileMode::kDecal, style); } - return FilterContents::MakeBorderMaskBlur(input, sigma, sigma, style); + return FilterContents::MakeBorderMaskBlur(input, Sigma(blur_sigma.x), + Sigma(blur_sigma.y), style); } std::shared_ptr Paint::GetColorFilter() const { diff --git a/impeller/aiks/paint.h b/impeller/aiks/paint.h index 7aa6da5be3200..660ca03d98533 100644 --- a/impeller/aiks/paint.h +++ b/impeller/aiks/paint.h @@ -48,6 +48,9 @@ struct Paint { struct MaskBlurDescriptor { FilterContents::BlurStyle style; Sigma sigma; + /// Text mask blurs need to not apply the CTM to the blur kernel. + /// See: https://github.com/flutter/flutter/issues/115112 + bool respect_ctm = true; std::shared_ptr CreateMaskBlur( std::shared_ptr color_source_contents, @@ -58,7 +61,8 @@ struct Paint { std::shared_ptr CreateMaskBlur( const FilterInput::Ref& input, - bool is_solid_color) const; + bool is_solid_color, + const Matrix& ctm) const; }; Color color = Color::Black(); @@ -104,7 +108,8 @@ struct Paint { bool HasColorFilter() const; std::shared_ptr WithMaskBlur(std::shared_ptr input, - bool is_solid_color) const; + bool is_solid_color, + const Matrix& ctm) const; std::shared_ptr WithImageFilter( const FilterInput::Variant& input, diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 5c5e30b53a383..6dbf67c41c669 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -511,6 +511,7 @@ void DlDispatcherBase::setMaskFilter(const flutter::DlMaskFilter* filter) { paint_.mask_blur_descriptor = { .style = ToBlurStyle(blur->style()), .sigma = Sigma(blur->sigma()), + .respect_ctm = blur->respectCTM(), }; break; } From 43498684a2fad28b81476dc362038d9242a57017 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 10:15:59 -0700 Subject: [PATCH 03/12] added golden tests --- impeller/display_list/BUILD.gn | 1 + .../display_list/dl_golden_blur_unittests.cc | 102 ++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 impeller/display_list/dl_golden_blur_unittests.cc diff --git a/impeller/display_list/BUILD.gn b/impeller/display_list/BUILD.gn index 449128bccfd48..de4abfebd6460 100644 --- a/impeller/display_list/BUILD.gn +++ b/impeller/display_list/BUILD.gn @@ -56,6 +56,7 @@ template("display_list_unittests_component") { "aiks_dl_gradient_unittests.cc", "aiks_dl_opacity_unittests.cc", "aiks_dl_path_unittests.cc", + "dl_golden_blur_unittests.cc", "dl_golden_unittests.cc", "dl_playground.cc", "dl_playground.h", diff --git a/impeller/display_list/dl_golden_blur_unittests.cc b/impeller/display_list/dl_golden_blur_unittests.cc new file mode 100644 index 0000000000000..422c121ca584a --- /dev/null +++ b/impeller/display_list/dl_golden_blur_unittests.cc @@ -0,0 +1,102 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/display_list/dl_golden_unittests.h" + +#include "flutter/display_list/dl_builder.h" +#include "flutter/display_list/effects/dl_mask_filter.h" +#include "flutter/testing/testing.h" +#include "gtest/gtest.h" +#include "impeller/typographer/backends/skia/text_frame_skia.h" +#include "txt/platform.h" + +namespace flutter { +namespace testing { + +using impeller::Font; + +namespace { +struct TextRenderOptions { + bool stroke = false; + SkScalar font_size = 50; + DlColor color = DlColor::kYellow(); + std::shared_ptr mask_filter; +}; + +bool RenderTextInCanvasSkia(DlCanvas* canvas, + const std::string& text, + const std::string_view& font_fixture, + SkPoint position, + TextRenderOptions options = {}) { + auto c_font_fixture = std::string(font_fixture); + auto mapping = flutter::testing::OpenFixtureAsSkData(c_font_fixture.c_str()); + if (!mapping) { + return false; + } + sk_sp font_mgr = txt::GetDefaultFontManager(); + SkFont sk_font(font_mgr->makeFromData(mapping), options.font_size); + auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font); + if (!blob) { + return false; + } + + auto frame = impeller::MakeTextFrameFromTextBlobSkia(blob); + + DlPaint text_paint; + text_paint.setColor(options.color); + text_paint.setMaskFilter(options.mask_filter); + // text_paint.mask_blur_descriptor = options.mask_blur_descriptor; + // text_paint.stroke_width = 1; + // text_paint.style = + // options.stroke ? Paint::Style::kStroke : Paint::Style::kFill; + canvas->DrawTextFrame(frame, position.x(), position.y(), text_paint); + return true; +} + +} // namespace + +TEST_P(DlGoldenTest, TextBlurMaskFilterRespectCTM) { + impeller::Point content_scale = GetContentScale(); + auto draw = [&](DlCanvas* canvas, + const std::vector>& images) { + canvas->Scale(content_scale.x, content_scale.y); + canvas->Scale(2, 2); + TextRenderOptions options; + options.mask_filter = + DlBlurMaskFilter::Make(DlBlurStyle::kNormal, /*sigma=*/10, + /*respect_ctm=*/true); + ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", + "Roboto-Regular.ttf", + SkPoint::Make(100, 100), options)); + }; + + DisplayListBuilder builder; + draw(&builder, /*images=*/{}); + + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + +TEST_P(DlGoldenTest, TextBlurMaskFilterDisrespectCTM) { + impeller::Point content_scale = GetContentScale(); + auto draw = [&](DlCanvas* canvas, + const std::vector>& images) { + canvas->Scale(content_scale.x, content_scale.y); + canvas->Scale(2, 2); + TextRenderOptions options; + options.mask_filter = + DlBlurMaskFilter::Make(DlBlurStyle::kNormal, /*sigma=*/10, + /*respect_ctm=*/false); + ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", + "Roboto-Regular.ttf", + SkPoint::Make(100, 100), options)); + }; + + DisplayListBuilder builder; + draw(&builder, /*images=*/{}); + + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + +} // namespace testing +} // namespace flutter From 298830faa8823a2a111362a6ff4a6b55f79e3138 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 10:34:31 -0700 Subject: [PATCH 04/12] updated docs --- .../filters/gaussian_blur_filter_contents.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 9018bbc9eb1ff..fd8eceb712e28 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -387,6 +387,14 @@ Vector2 ExtractScale(const Matrix& matrix) { } } // namespace +// A brief overview how this works: +// 1) Snapshot the filter input. +// 2) Perform downsample pass. This also inserts the gutter around the input +// snapshot since the blur can render outside the bounds of the snapshot. +// 3) Perform 1D horizontal blur pass. +// 4) Perform 1D vertical blur pass. +// 5) Apply the blur style to the blur result. This may just mask the output or +// draw the original snapshot over the result. std::optional GaussianBlurFilterContents::RenderFilter( const FilterInput::Vector& inputs, const ContentContext& renderer, @@ -398,6 +406,10 @@ std::optional GaussianBlurFilterContents::RenderFilter( return std::nullopt; } + // Source space here is scaled by the entity's transform. This is a + // requirement for text to be rendered correctly. You can think of this as + // "scaled source space" or "un-rotated local space". The entity's rotation is + // applied to the result of the blur as part of the result's transform. const Vector2 source_space_scalar = ExtractScale(entity.GetTransform().Basis()); From e7a6953998c64b9ae53da93904012b7958644e4c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 13:28:53 -0700 Subject: [PATCH 05/12] cleanup --- .../display_list/dl_golden_blur_unittests.cc | 10 ++++++ .../filters/gaussian_blur_filter_contents.cc | 35 ++++++++----------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/impeller/display_list/dl_golden_blur_unittests.cc b/impeller/display_list/dl_golden_blur_unittests.cc index 422c121ca584a..e6f22af120b2f 100644 --- a/impeller/display_list/dl_golden_blur_unittests.cc +++ b/impeller/display_list/dl_golden_blur_unittests.cc @@ -66,6 +66,11 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterRespectCTM) { options.mask_filter = DlBlurMaskFilter::Make(DlBlurStyle::kNormal, /*sigma=*/10, /*respect_ctm=*/true); + ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", + "Roboto-Regular.ttf", + SkPoint::Make(101, 101), options)); + options.mask_filter = nullptr; + options.color = DlColor::kRed(); ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", "Roboto-Regular.ttf", SkPoint::Make(100, 100), options)); @@ -87,6 +92,11 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterDisrespectCTM) { options.mask_filter = DlBlurMaskFilter::Make(DlBlurStyle::kNormal, /*sigma=*/10, /*respect_ctm=*/false); + ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", + "Roboto-Regular.ttf", + SkPoint::Make(101, 101), options)); + options.mask_filter = nullptr; + options.color = DlColor::kRed(); ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", "Roboto-Regular.ttf", SkPoint::Make(100, 100), options)); diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index fd8eceb712e28..b0c1b8d8fb13c 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -413,11 +413,10 @@ std::optional GaussianBlurFilterContents::RenderFilter( const Vector2 source_space_scalar = ExtractScale(entity.GetTransform().Basis()); - Vector2 scaled_sigma = (effect_transform.Basis() * - Matrix::MakeScale({source_space_scalar.x, - source_space_scalar.y, 1}) * // - Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) - .Abs(); + Vector2 scaled_sigma = + (effect_transform.Basis() * Matrix::MakeScale(source_space_scalar) * // + Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) + .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); scaled_sigma.y = std::min(scaled_sigma.y, kMaxSigma); Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), @@ -442,12 +441,11 @@ std::optional GaussianBlurFilterContents::RenderFilter( } Entity snapshot_entity = entity.Clone(); - snapshot_entity.SetTransform( - Matrix::MakeScale({source_space_scalar.x, source_space_scalar.y, 1})); + snapshot_entity.SetTransform(Matrix::MakeScale(source_space_scalar)); std::optional source_expanded_coverage_hint; if (expanded_coverage_hint.has_value()) { source_expanded_coverage_hint = expanded_coverage_hint->TransformBounds( - Matrix::MakeScale({source_space_scalar.x, source_space_scalar.y, 1}) * + Matrix::MakeScale(source_space_scalar) * entity.GetTransform().Invert()); } @@ -464,8 +462,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode()); // No blur to render. result.SetTransform(entity.GetTransform() * - Matrix::MakeScale({1.f / source_space_scalar.x, - 1.f / source_space_scalar.y, 1}) * + Matrix::MakeScale(1.f / source_space_scalar) * input_snapshot->transform); return result; } @@ -592,16 +589,14 @@ std::optional GaussianBlurFilterContents::RenderFilter( MinMagFilter::kLinear, SamplerAddressMode::kClampToEdge); Entity blur_output_entity = Entity::FromSnapshot( - Snapshot{ - .texture = pass3_out.value().GetRenderTargetTexture(), - .transform = entity.GetTransform() * // - Matrix::MakeScale({1.f / source_space_scalar.x, - 1.f / source_space_scalar.y, 1}) * // - input_snapshot->transform * // - padding_snapshot_adjustment * // - Matrix::MakeScale(1 / effective_scalar), - .sampler_descriptor = sampler_desc, - .opacity = input_snapshot->opacity}, + Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(), + .transform = entity.GetTransform() * // + Matrix::MakeScale(1.f / source_space_scalar) * // + input_snapshot->transform * // + padding_snapshot_adjustment * // + Matrix::MakeScale(1 / effective_scalar), + .sampler_descriptor = sampler_desc, + .opacity = input_snapshot->opacity}, entity.GetBlendMode()); return ApplyBlurStyle(mask_blur_style_, entity, inputs[0], From eab54b06543bccb18b463115287c4cc29173b27f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 15:27:59 -0700 Subject: [PATCH 06/12] fixed coverage test --- .../filters/gaussian_blur_filter_contents.cc | 35 ++++++++++--------- ...gaussian_blur_filter_contents_unittests.cc | 5 ++- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index b0c1b8d8fb13c..410774ad1c67b 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -352,6 +352,14 @@ std::optional GaussianBlurFilterContents::GetFilterSourceCoverage( return output_limit.Expand(Point(blur_radii.x, blur_radii.y)); } +namespace { +Vector2 ExtractScale(const Matrix& matrix) { + Vector2 entity_scale_x = matrix * Vector2(1.0, 0.0); + Vector2 entity_scale_y = matrix * Vector2(0.0, 1.0); + return Vector2(entity_scale_x.GetLength(), entity_scale_y.GetLength()); +} +} // namespace + std::optional GaussianBlurFilterContents::GetFilterCoverage( const FilterInput::Vector& inputs, const Entity& entity, @@ -359,15 +367,14 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( if (inputs.empty()) { return {}; } - - Entity snapshot_entity = entity.Clone(); - snapshot_entity.SetTransform(Matrix()); - std::optional source_coverage = inputs[0]->GetCoverage(snapshot_entity); - if (!source_coverage.has_value()) { + std::optional input_coverage = inputs[0]->GetCoverage(entity); + if (!input_coverage.has_value()) { return {}; } - Vector2 scaled_sigma = (effect_transform.Basis() * + const Vector2 source_space_scalar = + ExtractScale(entity.GetTransform().Basis()); + Vector2 scaled_sigma = (Matrix::MakeScale(source_space_scalar) * Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); @@ -375,17 +382,10 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)); Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Rect expanded_source_coverage = source_coverage->Expand(padding); - return expanded_source_coverage.TransformBounds(entity.GetTransform()); -} - -namespace { -Vector2 ExtractScale(const Matrix& matrix) { - Vector2 entity_scale_x = matrix * Vector2(1.0, 0.0); - Vector2 entity_scale_y = matrix * Vector2(0.0, 1.0); - return Vector2(entity_scale_x.GetLength(), entity_scale_y.GetLength()); + Vector2 local_padding = + (Matrix::MakeScale(source_space_scalar) * padding).Abs(); + return input_coverage.value().Expand(Point(local_padding.x, local_padding.y)); } -} // namespace // A brief overview how this works: // 1) Snapshot the filter input. @@ -422,7 +422,8 @@ std::optional GaussianBlurFilterContents::RenderFilter( Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)); Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Vector2 local_padding = (entity.GetTransform().Basis() * padding).Abs(); + Vector2 local_padding = + (Matrix::MakeScale(source_space_scalar) * padding).Abs(); // Apply as much of the desired padding as possible from the source. This may // be ignored so must be accounted for in the downsample pass by adding a diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc index cf4b955f1311a..b29dd6175eaad 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -400,8 +400,11 @@ TEST_P(GaussianBlurFilterContentsTest, EXPECT_TRUE(contents_coverage.has_value()); if (result_coverage.has_value() && contents_coverage.has_value()) { EXPECT_TRUE(RectNear(result_coverage.value(), contents_coverage.value())); + // Scaling a blurred entity doesn't seem to scale the blur radius linearly + // when comparing results with rrect_blur. That's why this is not + // Rect::MakeXYWH(98.f, 78.f, 204.0f, 204.f). EXPECT_TRUE(RectNear(contents_coverage.value(), - Rect::MakeXYWH(98.f, 78.f, 204.0f, 204.f))); + Rect::MakeXYWH(94.f, 74.f, 212.0f, 212.f))); } } } From b19a8ade41afff4a053a2cde669dd898bbcc9a46 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 15:29:39 -0700 Subject: [PATCH 07/12] updated golden list --- testing/impeller_golden_tests_output.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index 79412098cf562..1451a30eacc3b 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -819,3 +819,9 @@ impeller_Play_DlGoldenTest_GaussianVsRRectBlurScaled_Vulkan.png impeller_Play_DlGoldenTest_GaussianVsRRectBlur_Metal.png impeller_Play_DlGoldenTest_GaussianVsRRectBlur_OpenGLES.png impeller_Play_DlGoldenTest_GaussianVsRRectBlur_Vulkan.png +impeller_Play_DlGoldenTest_TextBlurMaskFilterDisrespectCTM_Metal.png +impeller_Play_DlGoldenTest_TextBlurMaskFilterDisrespectCTM_OpenGLES.png +impeller_Play_DlGoldenTest_TextBlurMaskFilterDisrespectCTM_Vulkan.png +impeller_Play_DlGoldenTest_TextBlurMaskFilterRespectCTM_Metal.png +impeller_Play_DlGoldenTest_TextBlurMaskFilterRespectCTM_OpenGLES.png +impeller_Play_DlGoldenTest_TextBlurMaskFilterRespectCTM_Vulkan.png From e6e87bcae04c8f1e6db158aca2ba0e3f096bc725 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 13 Jun 2024 15:31:44 -0700 Subject: [PATCH 08/12] tidy --- impeller/display_list/dl_golden_blur_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/display_list/dl_golden_blur_unittests.cc b/impeller/display_list/dl_golden_blur_unittests.cc index e6f22af120b2f..995397eaa6da0 100644 --- a/impeller/display_list/dl_golden_blur_unittests.cc +++ b/impeller/display_list/dl_golden_blur_unittests.cc @@ -28,7 +28,7 @@ bool RenderTextInCanvasSkia(DlCanvas* canvas, const std::string& text, const std::string_view& font_fixture, SkPoint position, - TextRenderOptions options = {}) { + const TextRenderOptions& options = {}) { auto c_font_fixture = std::string(font_fixture); auto mapping = flutter::testing::OpenFixtureAsSkData(c_font_fixture.c_str()); if (!mapping) { From 1099da192d080fd01cc59a16a54c152d6bbadece Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2024 09:06:26 -0700 Subject: [PATCH 09/12] license --- ci/licenses_golden/excluded_files | 1 + .../filters/gaussian_blur_filter_contents.cc | 66 +++++-------------- 2 files changed, 19 insertions(+), 48 deletions(-) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 97f87f2cf6ae3..c38006097e7d2 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -144,6 +144,7 @@ ../../../flutter/impeller/display_list/aiks_dl_gradient_unittests.cc ../../../flutter/impeller/display_list/aiks_dl_opacity_unittests.cc ../../../flutter/impeller/display_list/aiks_dl_path_unittests.cc +../../../flutter/impeller/display_list/dl_golden_blur_unittests.cc ../../../flutter/impeller/display_list/dl_golden_unittests.cc ../../../flutter/impeller/display_list/dl_golden_unittests.h ../../../flutter/impeller/display_list/dl_unittests.cc diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 410774ad1c67b..918d145a75dfd 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -352,14 +352,6 @@ std::optional GaussianBlurFilterContents::GetFilterSourceCoverage( return output_limit.Expand(Point(blur_radii.x, blur_radii.y)); } -namespace { -Vector2 ExtractScale(const Matrix& matrix) { - Vector2 entity_scale_x = matrix * Vector2(1.0, 0.0); - Vector2 entity_scale_y = matrix * Vector2(0.0, 1.0); - return Vector2(entity_scale_x.GetLength(), entity_scale_y.GetLength()); -} -} // namespace - std::optional GaussianBlurFilterContents::GetFilterCoverage( const FilterInput::Vector& inputs, const Entity& entity, @@ -367,14 +359,15 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( if (inputs.empty()) { return {}; } - std::optional input_coverage = inputs[0]->GetCoverage(entity); - if (!input_coverage.has_value()) { + + Entity snapshot_entity = entity.Clone(); + snapshot_entity.SetTransform(Matrix()); + std::optional source_coverage = inputs[0]->GetCoverage(snapshot_entity); + if (!source_coverage.has_value()) { return {}; } - const Vector2 source_space_scalar = - ExtractScale(entity.GetTransform().Basis()); - Vector2 scaled_sigma = (Matrix::MakeScale(source_space_scalar) * + Vector2 scaled_sigma = (effect_transform.Basis() * Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); @@ -382,19 +375,10 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)); Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Vector2 local_padding = - (Matrix::MakeScale(source_space_scalar) * padding).Abs(); - return input_coverage.value().Expand(Point(local_padding.x, local_padding.y)); + Rect expanded_source_coverage = source_coverage->Expand(padding); + return expanded_source_coverage.TransformBounds(entity.GetTransform()); } -// A brief overview how this works: -// 1) Snapshot the filter input. -// 2) Perform downsample pass. This also inserts the gutter around the input -// snapshot since the blur can render outside the bounds of the snapshot. -// 3) Perform 1D horizontal blur pass. -// 4) Perform 1D vertical blur pass. -// 5) Apply the blur style to the blur result. This may just mask the output or -// draw the original snapshot over the result. std::optional GaussianBlurFilterContents::RenderFilter( const FilterInput::Vector& inputs, const ContentContext& renderer, @@ -406,24 +390,15 @@ std::optional GaussianBlurFilterContents::RenderFilter( return std::nullopt; } - // Source space here is scaled by the entity's transform. This is a - // requirement for text to be rendered correctly. You can think of this as - // "scaled source space" or "un-rotated local space". The entity's rotation is - // applied to the result of the blur as part of the result's transform. - const Vector2 source_space_scalar = - ExtractScale(entity.GetTransform().Basis()); - - Vector2 scaled_sigma = - (effect_transform.Basis() * Matrix::MakeScale(source_space_scalar) * // - Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) - .Abs(); + Vector2 scaled_sigma = (effect_transform.Basis() * + Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) + .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); scaled_sigma.y = std::min(scaled_sigma.y, kMaxSigma); Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)); Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Vector2 local_padding = - (Matrix::MakeScale(source_space_scalar) * padding).Abs(); + Vector2 local_padding = (entity.GetTransform().Basis() * padding).Abs(); // Apply as much of the desired padding as possible from the source. This may // be ignored so must be accounted for in the downsample pass by adding a @@ -442,12 +417,11 @@ std::optional GaussianBlurFilterContents::RenderFilter( } Entity snapshot_entity = entity.Clone(); - snapshot_entity.SetTransform(Matrix::MakeScale(source_space_scalar)); + snapshot_entity.SetTransform(Matrix()); std::optional source_expanded_coverage_hint; if (expanded_coverage_hint.has_value()) { - source_expanded_coverage_hint = expanded_coverage_hint->TransformBounds( - Matrix::MakeScale(source_space_scalar) * - entity.GetTransform().Invert()); + source_expanded_coverage_hint = + expanded_coverage_hint->TransformBounds(entity.GetTransform().Invert()); } std::optional input_snapshot = @@ -462,9 +436,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( Entity result = Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode()); // No blur to render. - result.SetTransform(entity.GetTransform() * - Matrix::MakeScale(1.f / source_space_scalar) * - input_snapshot->transform); + result.SetTransform(entity.GetTransform() * input_snapshot->transform); return result; } @@ -591,10 +563,8 @@ std::optional GaussianBlurFilterContents::RenderFilter( Entity blur_output_entity = Entity::FromSnapshot( Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(), - .transform = entity.GetTransform() * // - Matrix::MakeScale(1.f / source_space_scalar) * // - input_snapshot->transform * // - padding_snapshot_adjustment * // + .transform = entity.GetTransform() * input_snapshot->transform * + padding_snapshot_adjustment * Matrix::MakeScale(1 / effective_scalar), .sampler_descriptor = sampler_desc, .opacity = input_snapshot->opacity}, From 1ead6dad0310c8acdf551bfc5528a8c6c9c59635 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2024 09:12:38 -0700 Subject: [PATCH 10/12] changed test background --- impeller/display_list/dl_golden_blur_unittests.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/display_list/dl_golden_blur_unittests.cc b/impeller/display_list/dl_golden_blur_unittests.cc index 995397eaa6da0..bd9e293080c8f 100644 --- a/impeller/display_list/dl_golden_blur_unittests.cc +++ b/impeller/display_list/dl_golden_blur_unittests.cc @@ -60,6 +60,7 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterRespectCTM) { impeller::Point content_scale = GetContentScale(); auto draw = [&](DlCanvas* canvas, const std::vector>& images) { + canvas->DrawColor(DlColor(0xff111111)); canvas->Scale(content_scale.x, content_scale.y); canvas->Scale(2, 2); TextRenderOptions options; @@ -86,6 +87,7 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterDisrespectCTM) { impeller::Point content_scale = GetContentScale(); auto draw = [&](DlCanvas* canvas, const std::vector>& images) { + canvas->DrawColor(DlColor(0xff111111)); canvas->Scale(content_scale.x, content_scale.y); canvas->Scale(2, 2); TextRenderOptions options; From 97871918015e131e3f885fad77b0b3991c07dd29 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2024 09:16:13 -0700 Subject: [PATCH 11/12] oops Revert "license" This reverts commit 1099da192d080fd01cc59a16a54c152d6bbadece. --- .../filters/gaussian_blur_filter_contents.cc | 66 ++++++++++++++----- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 918d145a75dfd..410774ad1c67b 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -352,6 +352,14 @@ std::optional GaussianBlurFilterContents::GetFilterSourceCoverage( return output_limit.Expand(Point(blur_radii.x, blur_radii.y)); } +namespace { +Vector2 ExtractScale(const Matrix& matrix) { + Vector2 entity_scale_x = matrix * Vector2(1.0, 0.0); + Vector2 entity_scale_y = matrix * Vector2(0.0, 1.0); + return Vector2(entity_scale_x.GetLength(), entity_scale_y.GetLength()); +} +} // namespace + std::optional GaussianBlurFilterContents::GetFilterCoverage( const FilterInput::Vector& inputs, const Entity& entity, @@ -359,15 +367,14 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( if (inputs.empty()) { return {}; } - - Entity snapshot_entity = entity.Clone(); - snapshot_entity.SetTransform(Matrix()); - std::optional source_coverage = inputs[0]->GetCoverage(snapshot_entity); - if (!source_coverage.has_value()) { + std::optional input_coverage = inputs[0]->GetCoverage(entity); + if (!input_coverage.has_value()) { return {}; } - Vector2 scaled_sigma = (effect_transform.Basis() * + const Vector2 source_space_scalar = + ExtractScale(entity.GetTransform().Basis()); + Vector2 scaled_sigma = (Matrix::MakeScale(source_space_scalar) * Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); @@ -375,10 +382,19 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)); Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Rect expanded_source_coverage = source_coverage->Expand(padding); - return expanded_source_coverage.TransformBounds(entity.GetTransform()); + Vector2 local_padding = + (Matrix::MakeScale(source_space_scalar) * padding).Abs(); + return input_coverage.value().Expand(Point(local_padding.x, local_padding.y)); } +// A brief overview how this works: +// 1) Snapshot the filter input. +// 2) Perform downsample pass. This also inserts the gutter around the input +// snapshot since the blur can render outside the bounds of the snapshot. +// 3) Perform 1D horizontal blur pass. +// 4) Perform 1D vertical blur pass. +// 5) Apply the blur style to the blur result. This may just mask the output or +// draw the original snapshot over the result. std::optional GaussianBlurFilterContents::RenderFilter( const FilterInput::Vector& inputs, const ContentContext& renderer, @@ -390,15 +406,24 @@ std::optional GaussianBlurFilterContents::RenderFilter( return std::nullopt; } - Vector2 scaled_sigma = (effect_transform.Basis() * - Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) - .Abs(); + // Source space here is scaled by the entity's transform. This is a + // requirement for text to be rendered correctly. You can think of this as + // "scaled source space" or "un-rotated local space". The entity's rotation is + // applied to the result of the blur as part of the result's transform. + const Vector2 source_space_scalar = + ExtractScale(entity.GetTransform().Basis()); + + Vector2 scaled_sigma = + (effect_transform.Basis() * Matrix::MakeScale(source_space_scalar) * // + Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_))) + .Abs(); scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma); scaled_sigma.y = std::min(scaled_sigma.y, kMaxSigma); Vector2 blur_radius = Vector2(CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)); Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Vector2 local_padding = (entity.GetTransform().Basis() * padding).Abs(); + Vector2 local_padding = + (Matrix::MakeScale(source_space_scalar) * padding).Abs(); // Apply as much of the desired padding as possible from the source. This may // be ignored so must be accounted for in the downsample pass by adding a @@ -417,11 +442,12 @@ std::optional GaussianBlurFilterContents::RenderFilter( } Entity snapshot_entity = entity.Clone(); - snapshot_entity.SetTransform(Matrix()); + snapshot_entity.SetTransform(Matrix::MakeScale(source_space_scalar)); std::optional source_expanded_coverage_hint; if (expanded_coverage_hint.has_value()) { - source_expanded_coverage_hint = - expanded_coverage_hint->TransformBounds(entity.GetTransform().Invert()); + source_expanded_coverage_hint = expanded_coverage_hint->TransformBounds( + Matrix::MakeScale(source_space_scalar) * + entity.GetTransform().Invert()); } std::optional input_snapshot = @@ -436,7 +462,9 @@ std::optional GaussianBlurFilterContents::RenderFilter( Entity result = Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode()); // No blur to render. - result.SetTransform(entity.GetTransform() * input_snapshot->transform); + result.SetTransform(entity.GetTransform() * + Matrix::MakeScale(1.f / source_space_scalar) * + input_snapshot->transform); return result; } @@ -563,8 +591,10 @@ std::optional GaussianBlurFilterContents::RenderFilter( Entity blur_output_entity = Entity::FromSnapshot( Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(), - .transform = entity.GetTransform() * input_snapshot->transform * - padding_snapshot_adjustment * + .transform = entity.GetTransform() * // + Matrix::MakeScale(1.f / source_space_scalar) * // + input_snapshot->transform * // + padding_snapshot_adjustment * // Matrix::MakeScale(1 / effective_scalar), .sampler_descriptor = sampler_desc, .opacity = input_snapshot->opacity}, From a3a03f2da03f0be62337cfd2f5a28d1a8aae896c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2024 09:45:31 -0700 Subject: [PATCH 12/12] fixed solid blur style --- .../filters/gaussian_blur_filter_contents.cc | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 410774ad1c67b..035109e51951f 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -245,7 +245,8 @@ Entity ApplyBlurStyle(FilterContents::BlurStyle blur_style, const std::shared_ptr& input, const Snapshot& input_snapshot, Entity blur_entity, - const std::shared_ptr& geometry) { + const std::shared_ptr& geometry, + Vector2 source_space_scalar) { switch (blur_style) { case FilterContents::BlurStyle::kNormal: return blur_entity; @@ -266,22 +267,25 @@ Entity ApplyBlurStyle(FilterContents::BlurStyle blur_style, Matrix snapshot_transform = entity.GetTransform() * snapshot_entity.GetTransform(); result.SetContents(Contents::MakeAnonymous( - fml::MakeCopyable([blur_entity = blur_entity.Clone(), - blurred_transform, snapshot_transform, - snapshot_entity = std::move(snapshot_entity)]( - const ContentContext& renderer, - const Entity& entity, - RenderPass& pass) mutable { - bool result = true; - blur_entity.SetClipDepth(entity.GetClipDepth()); - blur_entity.SetTransform(entity.GetTransform() * blurred_transform); - result = result && blur_entity.Render(renderer, pass); - snapshot_entity.SetTransform(entity.GetTransform() * - snapshot_transform); - snapshot_entity.SetClipDepth(entity.GetClipDepth()); - result = result && snapshot_entity.Render(renderer, pass); - return result; - }), + fml::MakeCopyable( + [blur_entity = blur_entity.Clone(), blurred_transform, + source_space_scalar, snapshot_transform, + snapshot_entity = std::move(snapshot_entity)]( + const ContentContext& renderer, const Entity& entity, + RenderPass& pass) mutable { + bool result = true; + blur_entity.SetClipDepth(entity.GetClipDepth()); + blur_entity.SetTransform(entity.GetTransform() * + blurred_transform); + result = result && blur_entity.Render(renderer, pass); + snapshot_entity.SetTransform( + entity.GetTransform() * + Matrix::MakeScale(1.f / source_space_scalar) * + snapshot_transform); + snapshot_entity.SetClipDepth(entity.GetClipDepth()); + result = result && snapshot_entity.Render(renderer, pass); + return result; + }), fml::MakeCopyable([blur_entity = blur_entity.Clone(), blurred_transform](const Entity& entity) mutable { blur_entity.SetTransform(entity.GetTransform() * blurred_transform); @@ -602,7 +606,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( return ApplyBlurStyle(mask_blur_style_, entity, inputs[0], input_snapshot.value(), std::move(blur_output_entity), - mask_geometry_); + mask_geometry_, source_space_scalar); } Scalar GaussianBlurFilterContents::CalculateBlurRadius(Scalar sigma) {