From 1d15aa23e9aeefbfba4f770c2de036d1d2a5f573 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 27 Nov 2023 18:43:31 -0800 Subject: [PATCH 1/3] [Impeller] match sigma scaling to Skia scaling. --- .../filters/gaussian_blur_filter_contents.cc | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index af05805a602f1..ceedb8d180083 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -145,13 +145,15 @@ std::shared_ptr MakeBlurSubpass( } /// Calculate how much to scale down the texture depending on the blur radius. -/// This curve was taken from |DirectionalGaussianBlurFilterContents|. -Scalar CalculateScale(Scalar radius) { - constexpr Scalar decay = 4.0; // Larger is more gradual. - constexpr Scalar limit = 0.95; // The maximum percentage of the scaledown. - const Scalar curve = - std::min(1.0, decay / (std::max(1.0f, radius) + decay - 1.0)); - return (curve - 1) * limit + 1; +/// +/// This value was extracted from Skia, see: +/// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/ganesh/GrBlurUtils.cpp#L2561-L2576 +/// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/BlurUtils.h#L57 +Scalar CalculateScale(Scalar sigma) { + if (sigma <= 4) { + return 1.0; + } + return 4.0 / sigma; }; } // namespace @@ -211,7 +213,10 @@ std::optional GaussianBlurFilterContents::RenderFilter( } Scalar blur_radius = CalculateBlurRadius(sigma_); - Scalar desired_scalar = CalculateScale(blur_radius); + Scalar desired_scalar = CalculateScale(sigma_); + // TODO(jonahwilliams): if scaling value is 1.0, then skip the downsample + // pass. + Vector2 downsample_scalar(desired_scalar, desired_scalar); Vector2 padding(ceil(blur_radius), ceil(blur_radius)); From 505c2944bc9a8273d77a3f3b0db2370bc832ba3d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 28 Nov 2023 09:13:57 -0800 Subject: [PATCH 2/3] add unit test. --- .../filters/gaussian_blur_filter_contents.cc | 12 ++++++------ .../contents/filters/gaussian_blur_filter_contents.h | 5 +++++ .../gaussian_blur_filter_contents_unittests.cc | 9 +++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index ceedb8d180083..744f6691bfa8e 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -144,23 +144,23 @@ std::shared_ptr MakeBlurSubpass( return out_texture; } +} // namespace + +GaussianBlurFilterContents::GaussianBlurFilterContents(Scalar sigma) + : sigma_(sigma) {} + /// Calculate how much to scale down the texture depending on the blur radius. /// /// This value was extracted from Skia, see: /// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/ganesh/GrBlurUtils.cpp#L2561-L2576 /// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/BlurUtils.h#L57 -Scalar CalculateScale(Scalar sigma) { +Scalar GaussianBlurFilterContents::CalculateScale(Scalar sigma) { if (sigma <= 4) { return 1.0; } return 4.0 / sigma; }; -} // namespace - -GaussianBlurFilterContents::GaussianBlurFilterContents(Scalar sigma) - : sigma_(sigma) {} - std::optional GaussianBlurFilterContents::GetFilterSourceCoverage( const Matrix& effect_transform, const Rect& output_limit) const { diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 0b85fb17ca39f..3a68b8a67c6b9 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -42,6 +42,11 @@ class GaussianBlurFilterContents final : public FilterContents { const Entity& entity, const ISize& pass_size); + /// Calculate the scale factor for the given sigma value. + /// + /// Visible for testing. + static Scalar CalculateScale(Scalar sigma); + private: // |FilterContents| std::optional RenderFilter( 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 55ccd334c987b..02a7476858fbc 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -115,6 +115,15 @@ TEST(GaussianBlurFilterContentsTest, FilterSourceCoverage) { ASSERT_EQ(coverage, Rect::MakeLTRB(100 - 2, 100 - 2, 200 + 2, 200 + 2)); } +TEST(GaussianBlurFilterContentsTest, CalculateSigmaValues) { + EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(1.0f), 1); + EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(2.0f), 1); + EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(3.0f), 1); + EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(4.0f), 1); + EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(16.0f), 0.25); + EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(1024.0f), 4.f / 1024.f); +} + TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverage) { TextureDescriptor desc = { .storage_mode = StorageMode::kDevicePrivate, From 5a52513b55275163cfb6c58ece4eb22ed2b4b04b Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 28 Nov 2023 10:03:39 -0800 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: gaaclarke <30870216+gaaclarke@users.noreply.github.com> --- .../contents/filters/gaussian_blur_filter_contents.cc | 8 +++----- .../contents/filters/gaussian_blur_filter_contents.h | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 744f6691bfa8e..16f0c2dfa17a5 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -149,11 +149,9 @@ std::shared_ptr MakeBlurSubpass( GaussianBlurFilterContents::GaussianBlurFilterContents(Scalar sigma) : sigma_(sigma) {} -/// Calculate how much to scale down the texture depending on the blur radius. -/// -/// This value was extracted from Skia, see: -/// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/ganesh/GrBlurUtils.cpp#L2561-L2576 -/// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/BlurUtils.h#L57 +// This value was extracted from Skia, see: +// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/ganesh/GrBlurUtils.cpp#L2561-L2576 +// * https://github.com/google/skia/blob/d29cc3fe182f6e8a8539004a6a4ee8251677a6fd/src/gpu/BlurUtils.h#L57 Scalar GaussianBlurFilterContents::CalculateScale(Scalar sigma) { if (sigma <= 4) { return 1.0; diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 3a68b8a67c6b9..dfeaa8a6f473d 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -42,7 +42,7 @@ class GaussianBlurFilterContents final : public FilterContents { const Entity& entity, const ISize& pass_size); - /// Calculate the scale factor for the given sigma value. + /// Calculate the scale factor for the downsample pass given a sigma value. /// /// Visible for testing. static Scalar CalculateScale(Scalar sigma);