From e315f81ba097ce616bbdfc6c8587fc061d791b9e Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Fri, 8 Dec 2023 16:57:40 -0800 Subject: [PATCH] [Impeller] Clamp new blur sigma (flutter/engine#48813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes `AiksTests.CanRenderBackdropBlurHugeSigma/Metal` I also tweaked the existing quadratic equation to make sure the minima is at 500 and `f(0) = 1`. ## before Screenshot 2023-12-07 at 4 36 40 PM ## after Screenshot 2023-12-07 at 4 32 11 PM ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --------- Co-authored-by: Jonah Williams --- ...rectional_gaussian_blur_filter_contents.cc | 15 ++--- .../filters/gaussian_blur_filter_contents.cc | 30 +++++++--- .../filters/gaussian_blur_filter_contents.h | 9 +++ ...gaussian_blur_filter_contents_unittests.cc | 58 +++++++++++++++++-- 4 files changed, 88 insertions(+), 24 deletions(-) diff --git a/engine/src/flutter/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc b/engine/src/flutter/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc index d9965ded5bd..98957e3582c 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/directional_gaussian_blur_filter_contents.cc @@ -24,20 +24,13 @@ namespace impeller { -// This function was derived with polynomial regression when comparing the -// results with Skia. Changing the curve below should invalidate this. -// -// The following data points were used: -// 0 | 1 -// 75 | 0.8 -// 150 | 0.5 -// 300 | 0.22 -// 400 | 0.2 -// 500 | 0.15 +// This function was calculated by observing Skia's behavior. Its blur at 500 +// seemed to be 0.15. Since we clamp at 500 I solved the quadratic equation +// that puts the minima there and a f(0)=1. Sigma ScaleSigma(Sigma sigma) { // Limit the kernel size to 1000x1000 pixels, like Skia does. Scalar clamped = std::min(sigma.sigma, 500.0f); - Scalar scalar = 1.02 - 3.89e-3 * clamped + 4.36e-06 * clamped * clamped; + Scalar scalar = 1.0 - 3.4e-3 * clamped + 3.4e-06 * clamped * clamped; return Sigma(clamped * scalar); } diff --git a/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 9824ef9b5a7..2df5f702497 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -210,7 +210,8 @@ Scalar GaussianBlurFilterContents::CalculateScale(Scalar sigma) { std::optional GaussianBlurFilterContents::GetFilterSourceCoverage( const Matrix& effect_transform, const Rect& output_limit) const { - Scalar blur_radius = CalculateBlurRadius(sigma_); + Scalar scaled_sigma = ScaleSigma(sigma_); + Scalar blur_radius = CalculateBlurRadius(scaled_sigma); Vector3 blur_radii = effect_transform.Basis() * Vector3{blur_radius, blur_radius, 0.0}; return output_limit.Expand(Point(blur_radii.x, blur_radii.y)); @@ -229,7 +230,8 @@ std::optional GaussianBlurFilterContents::GetFilterCoverage( return {}; } - Scalar blur_radius = CalculateBlurRadius(sigma_); + Scalar scaled_sigma = ScaleSigma(sigma_); + Scalar blur_radius = CalculateBlurRadius(scaled_sigma); Vector3 blur_radii = (inputs[0]->GetTransform(entity).Basis() * effect_transform.Basis() * Vector3{blur_radius, blur_radius, 0.0}) @@ -248,7 +250,8 @@ std::optional GaussianBlurFilterContents::RenderFilter( return std::nullopt; } - Scalar blur_radius = CalculateBlurRadius(sigma_); + Scalar scaled_sigma = ScaleSigma(sigma_); + Scalar blur_radius = CalculateBlurRadius(scaled_sigma); Vector2 padding(ceil(blur_radius), ceil(blur_radius)); // Apply as much of the desired padding as possible from the source. This may @@ -269,12 +272,12 @@ std::optional GaussianBlurFilterContents::RenderFilter( return std::nullopt; } - if (sigma_ < kEhCloseEnough) { + if (scaled_sigma < kEhCloseEnough) { return Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode(), entity.GetClipDepth()); // No blur to render. } - Scalar desired_scalar = CalculateScale(sigma_); + Scalar desired_scalar = CalculateScale(scaled_sigma); // TODO(jonahwilliams): If desired_scalar is 1.0 and we fully acquired the // gutter from the expanded_coverage_hint, we can skip the downsample pass. // pass. @@ -301,7 +304,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( renderer, pass1_out_texture, input_snapshot->sampler_descriptor, GaussianBlurFragmentShader::BlurInfo{ .blur_uv_offset = Point(0.0, pass1_pixel_size.y), - .blur_sigma = sigma_ * effective_scalar.y, + .blur_sigma = scaled_sigma * effective_scalar.y, .blur_radius = blur_radius * effective_scalar.y, .step_size = 1.0, }); @@ -311,7 +314,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( renderer, pass2_out_texture, input_snapshot->sampler_descriptor, GaussianBlurFragmentShader::BlurInfo{ .blur_uv_offset = Point(pass1_pixel_size.x, 0.0), - .blur_sigma = sigma_ * effective_scalar.x, + .blur_sigma = scaled_sigma * effective_scalar.x, .blur_radius = blur_radius * effective_scalar.x, .step_size = 1.0, }); @@ -348,4 +351,17 @@ Quad GaussianBlurFilterContents::CalculateUVs( return uv_transform.Transform(coverage_quad); } +// This function was calculated by observing Skia's behavior. Its blur at 500 +// seemed to be 0.15. Since we clamp at 500 I solved the quadratic equation +// that puts the minima there and a f(0)=1. +Scalar GaussianBlurFilterContents::ScaleSigma(Scalar sigma) { + // Limit the kernel size to 1000x1000 pixels, like Skia does. + Scalar clamped = std::min(sigma, 500.0f); + constexpr Scalar a = 3.4e-06; + constexpr Scalar b = -3.4e-3; + constexpr Scalar c = 1.f; + Scalar scalar = c + b * clamped + a * clamped * clamped; + return clamped * scalar; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index ee3e0636c38..7d390ddf87e 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -47,6 +47,15 @@ class GaussianBlurFilterContents final : public FilterContents { /// Visible for testing. static Scalar CalculateScale(Scalar sigma); + /// Scales down the sigma value to match Skia's behavior. + /// + /// effective_blur_radius = CalculateBlurRadius(ScaleSigma(sigma_)); + /// + /// This function was calculated by observing Skia's behavior. Its blur at + /// 500 seemed to be 0.15. Since we clamp at 500 I solved the quadratic + /// equation that puts the minima there and a f(0)=1. + static Scalar ScaleSigma(Scalar sigma); + private: // |FilterContents| std::optional RenderFilter( diff --git a/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc b/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc index efe135a5fc4..356870ccba9 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -16,9 +16,36 @@ namespace testing { namespace { -Scalar CalculateSigmaForBlurRadius(Scalar blur_radius) { - // See Sigma.h - return (blur_radius / kKernelRadiusPerSigma) + 0.5; +// Use newtonian method to give the closest answer to target where +// f(x) is less than the target. We do this because the value is `ceil`'d to +// grab fractional pixels. +float LowerBoundNewtonianMethod(const std::function& func, + float target, + float guess, + float tolerance) { + const float delta = 1e-6; + float x = guess; + float fx; + + do { + fx = func(x) - target; + float derivative = (func(x + delta) - func(x)) / delta; + x = x - fx / derivative; + + } while (std::abs(fx) > tolerance || + fx < 0.0); // fx < 0.0 makes this lower bound. + + return x; +} + +Scalar CalculateSigmaForBlurRadius(Scalar radius) { + auto f = [](Scalar x) -> Scalar { + return GaussianBlurFilterContents::CalculateBlurRadius( + GaussianBlurFilterContents::ScaleSigma(x)); + }; + // The newtonian method is used here since inverting the function is + // non-trivial because of conditional logic and would be fragile to changes. + return LowerBoundNewtonianMethod(f, radius, 2.f, 0.001f); } } // namespace @@ -67,7 +94,10 @@ TEST(GaussianBlurFilterContentsTest, CoverageWithSigma) { Entity entity; std::optional coverage = contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix()); - ASSERT_EQ(coverage, Rect::MakeLTRB(99, 99, 201, 201)); + EXPECT_TRUE(coverage.has_value()); + if (coverage.has_value()) { + EXPECT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(99, 99, 201, 201)); + } } TEST_P(GaussianBlurFilterContentsTest, CoverageWithTexture) { @@ -87,7 +117,10 @@ TEST_P(GaussianBlurFilterContentsTest, CoverageWithTexture) { entity.SetTransform(Matrix::MakeTranslation({100, 100, 0})); std::optional coverage = contents.GetFilterCoverage(inputs, entity, /*effect_transform=*/Matrix()); - ASSERT_EQ(coverage, Rect::MakeLTRB(99, 99, 201, 201)); + EXPECT_TRUE(coverage.has_value()); + if (coverage.has_value()) { + EXPECT_RECT_NEAR(coverage.value(), Rect::MakeLTRB(99, 99, 201, 201)); + } } TEST_P(GaussianBlurFilterContentsTest, CoverageWithEffectTransform) { @@ -107,7 +140,11 @@ TEST_P(GaussianBlurFilterContentsTest, CoverageWithEffectTransform) { entity.SetTransform(Matrix::MakeTranslation({100, 100, 0})); std::optional coverage = contents.GetFilterCoverage( inputs, entity, /*effect_transform=*/Matrix::MakeScale({2.0, 2.0, 1.0})); - ASSERT_EQ(coverage, Rect::MakeLTRB(100 - 2, 100 - 2, 200 + 2, 200 + 2)); + EXPECT_TRUE(coverage.has_value()); + if (coverage.has_value()) { + EXPECT_RECT_NEAR(coverage.value(), + Rect::MakeLTRB(100 - 2, 100 - 2, 200 + 2, 200 + 2)); + } } TEST(GaussianBlurFilterContentsTest, FilterSourceCoverage) { @@ -328,5 +365,14 @@ TEST_P(GaussianBlurFilterContentsTest, } } +TEST(GaussianBlurFilterContentsTest, CalculateSigmaForBlurRadius) { + Scalar sigma = 1.0; + Scalar radius = GaussianBlurFilterContents::CalculateBlurRadius( + GaussianBlurFilterContents::ScaleSigma(sigma)); + Scalar derived_sigma = CalculateSigmaForBlurRadius(radius); + + EXPECT_NEAR(sigma, derived_sigma, 0.01f); +} + } // namespace testing } // namespace impeller