From 91d348e6e3d453e29b288d07007e8686fd42c4e0 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Mon, 1 Aug 2022 18:05:05 -0700 Subject: [PATCH] Move Sigma<->Radius conversion to geometry (flutter/engine#35069) --- .../ci/licenses_golden/licenses_flutter | 2 + .../display_list/display_list_dispatcher.cc | 7 ++- .../entity/contents/filters/filter_contents.h | 52 +---------------- .../impeller/entity/entity_unittests.cc | 10 ++-- engine/src/flutter/impeller/geometry/BUILD.gn | 2 + engine/src/flutter/impeller/geometry/sigma.cc | 19 +++++++ engine/src/flutter/impeller/geometry/sigma.h | 57 +++++++++++++++++++ 7 files changed, 89 insertions(+), 60 deletions(-) create mode 100644 engine/src/flutter/impeller/geometry/sigma.cc create mode 100644 engine/src/flutter/impeller/geometry/sigma.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index cf00c43c74d..e4550afff2c 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -654,6 +654,8 @@ FILE: ../../../flutter/impeller/geometry/rect.h FILE: ../../../flutter/impeller/geometry/scalar.h FILE: ../../../flutter/impeller/geometry/shear.cc FILE: ../../../flutter/impeller/geometry/shear.h +FILE: ../../../flutter/impeller/geometry/sigma.cc +FILE: ../../../flutter/impeller/geometry/sigma.h FILE: ../../../flutter/impeller/geometry/size.cc FILE: ../../../flutter/impeller/geometry/size.h FILE: ../../../flutter/impeller/geometry/type_traits.cc diff --git a/engine/src/flutter/impeller/display_list/display_list_dispatcher.cc b/engine/src/flutter/impeller/display_list/display_list_dispatcher.cc index b06e0ca57a7..dcc4daa9220 100644 --- a/engine/src/flutter/impeller/display_list/display_list_dispatcher.cc +++ b/engine/src/flutter/impeller/display_list/display_list_dispatcher.cc @@ -21,6 +21,7 @@ #include "impeller/geometry/path.h" #include "impeller/geometry/path_builder.h" #include "impeller/geometry/scalar.h" +#include "impeller/geometry/sigma.h" #include "impeller/geometry/vertices.h" #include "impeller/renderer/formats.h" #include "impeller/typographer/backends/skia/text_frame_skia.h" @@ -324,7 +325,7 @@ void DisplayListDispatcher::setMaskFilter(const flutter::DlMaskFilter* filter) { auto blur = filter->asBlur(); auto style = ToBlurStyle(blur->style()); - auto sigma = FilterContents::Sigma(blur->sigma()); + auto sigma = Sigma(blur->sigma()); paint_.mask_filter = [style, sigma](FilterInput::Ref input, bool is_solid_color) { @@ -350,8 +351,8 @@ static std::optional ToImageFilterProc( switch (filter->type()) { case flutter::DlImageFilterType::kBlur: { auto blur = filter->asBlur(); - auto sigma_x = FilterContents::Sigma(blur->sigma_x()); - auto sigma_y = FilterContents::Sigma(blur->sigma_y()); + auto sigma_x = Sigma(blur->sigma_x()); + auto sigma_y = Sigma(blur->sigma_y()); if (blur->tile_mode() != flutter::DlTileMode::kClamp) { // TODO(105072): Implement tile mode for blur filter. diff --git a/engine/src/flutter/impeller/entity/contents/filters/filter_contents.h b/engine/src/flutter/impeller/entity/contents/filters/filter_contents.h index 43d2af2512b..03231a3d78c 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/filter_contents.h +++ b/engine/src/flutter/impeller/entity/contents/filters/filter_contents.h @@ -11,6 +11,7 @@ #include "impeller/entity/contents/filters/inputs/filter_input.h" #include "impeller/entity/entity.h" +#include "impeller/geometry/sigma.h" #include "impeller/renderer/formats.h" namespace impeller { @@ -30,57 +31,6 @@ class FilterContents : public Contents { kInner, }; - /// For filters that use a Gaussian distribution, this is the `Radius` size to - /// use per `Sigma` (standard deviation). - /// - /// This cutoff (sqrt(3)) is taken from Flutter and Skia (where the - /// multiplicative inverse of this constant is used (1 / sqrt(3)): - /// https://api.flutter.dev/flutter/dart-ui/Shadow/convertRadiusToSigma.html - /// - /// In practice, this value is somewhat arbitrary, and can be changed to a - /// higher number to integrate more of the Gaussian function and render higher - /// quality blurs (with exponentially diminishing returns for the same sigma - /// input). Making this value any lower results in a noticable loss of - /// quality in the blur. - constexpr static float kKernelRadiusPerSigma = 1.73205080757; - - struct Radius; - - /// @brief In filters that use Gaussian distributions, "sigma" is a size of - /// one standard deviation in terms of the local space pixel grid of - /// the filter input. In other words, this determines how wide the - /// distribution stretches. - struct Sigma { - Scalar sigma = 0.0; - - constexpr Sigma() = default; - - explicit constexpr Sigma(Scalar p_sigma) : sigma(p_sigma) {} - - constexpr operator Radius() const { - return Radius{sigma > 0.5f ? (sigma - 0.5f) * kKernelRadiusPerSigma - : 0.0f}; - }; - }; - - /// @brief For convolution filters, the "radius" is the size of the - /// convolution kernel to use on the local space pixel grid of the - /// filter input. - /// For Gaussian blur kernels, this unit has a linear - /// relationship with `Sigma`. See `kKernelRadiusPerSigma` for - /// details on how this relationship works. - struct Radius { - Scalar radius = 0.0; - - constexpr Radius() = default; - - explicit constexpr Radius(Scalar p_radius) : radius(p_radius) {} - - constexpr operator Sigma() const { - return Sigma{radius > 0 ? radius / kKernelRadiusPerSigma + 0.5f : 0.0f}; - }; - }; - static std::shared_ptr MakeBlend( Entity::BlendMode blend_mode, FilterInput::Vector inputs, diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index dca12868062..b1f0e0b0c6a 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -20,6 +20,7 @@ #include "impeller/entity/entity_playground.h" #include "impeller/geometry/geometry_unittests.h" #include "impeller/geometry/path_builder.h" +#include "impeller/geometry/sigma.h" #include "impeller/playground/playground.h" #include "impeller/playground/widgets.h" #include "impeller/renderer/render_pass.h" @@ -913,13 +914,11 @@ TEST_P(EntityTest, GaussianBlurFilter) { } auto blur = FilterContents::MakeGaussianBlur( - FilterInput::Make(input), FilterContents::Sigma{blur_amount[0]}, - FilterContents::Sigma{blur_amount[1]}, + FilterInput::Make(input), Sigma{blur_amount[0]}, Sigma{blur_amount[1]}, blur_styles[selected_blur_style]); auto mask_blur = FilterContents::MakeBorderMaskBlur( - FilterInput::Make(input), FilterContents::Sigma{blur_amount[0]}, - FilterContents::Sigma{blur_amount[1]}, + FilterInput::Make(input), Sigma{blur_amount[0]}, Sigma{blur_amount[1]}, blur_styles[selected_blur_style]); auto ctm = Matrix::MakeScale(GetContentScale()) * @@ -1033,8 +1032,7 @@ TEST_P(EntityTest, BorderMaskBlurCoverageIsCorrect) { PathBuilder{}.AddRect(Rect::MakeXYWH(0, 0, 300, 400)).TakePath()); fill->SetColor(Color::CornflowerBlue()); auto border_mask_blur = FilterContents::MakeBorderMaskBlur( - FilterInput::Make(fill), FilterContents::Radius{3}, - FilterContents::Radius{4}); + FilterInput::Make(fill), Radius{3}, Radius{4}); { Entity e; diff --git a/engine/src/flutter/impeller/geometry/BUILD.gn b/engine/src/flutter/impeller/geometry/BUILD.gn index 65b99c1290a..0afe7a6dbd5 100644 --- a/engine/src/flutter/impeller/geometry/BUILD.gn +++ b/engine/src/flutter/impeller/geometry/BUILD.gn @@ -29,6 +29,8 @@ impeller_component("geometry") { "scalar.h", "shear.cc", "shear.h", + "sigma.cc", + "sigma.h", "size.cc", "size.h", "type_traits.cc", diff --git a/engine/src/flutter/impeller/geometry/sigma.cc b/engine/src/flutter/impeller/geometry/sigma.cc new file mode 100644 index 00000000000..a73bcd7cea8 --- /dev/null +++ b/engine/src/flutter/impeller/geometry/sigma.cc @@ -0,0 +1,19 @@ +// 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/geometry/sigma.h" + +#include + +namespace impeller { + +Sigma::operator Radius() const { + return Radius{sigma > 0.5f ? (sigma - 0.5f) * kKernelRadiusPerSigma : 0.0f}; +} + +Radius::operator Sigma() const { + return Sigma{radius > 0 ? radius / kKernelRadiusPerSigma + 0.5f : 0.0f}; +} + +} // namespace impeller diff --git a/engine/src/flutter/impeller/geometry/sigma.h b/engine/src/flutter/impeller/geometry/sigma.h new file mode 100644 index 00000000000..c909128be4b --- /dev/null +++ b/engine/src/flutter/impeller/geometry/sigma.h @@ -0,0 +1,57 @@ +// 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. + +#pragma once + +#include "impeller/geometry/scalar.h" + +namespace impeller { + +/// For filters that use a Gaussian distribution, this is the `Radius` size to +/// use per `Sigma` (standard deviation). +/// +/// This cutoff (sqrt(3)) is taken from Flutter and Skia (where the +/// multiplicative inverse of this constant is used (1 / sqrt(3)): +/// https://api.flutter.dev/flutter/dart-ui/Shadow/convertRadiusToSigma.html +/// +/// In practice, this value is somewhat arbitrary, and can be changed to a +/// higher number to integrate more of the Gaussian function and render higher +/// quality blurs (with exponentially diminishing returns for the same sigma +/// input). Making this value any lower results in a noticable loss of +/// quality in the blur. +constexpr static float kKernelRadiusPerSigma = 1.73205080757; + +struct Radius; + +/// @brief In filters that use Gaussian distributions, "sigma" is a size of +/// one standard deviation in terms of the local space pixel grid of +/// the filter input. In other words, this determines how wide the +/// distribution stretches. +struct Sigma { + Scalar sigma = 0.0; + + constexpr Sigma() = default; + + explicit constexpr Sigma(Scalar p_sigma) : sigma(p_sigma) {} + + operator Radius() const; // NOLINT(google-explicit-constructor) +}; + +/// @brief For convolution filters, the "radius" is the size of the +/// convolution kernel to use on the local space pixel grid of the +/// filter input. +/// For Gaussian blur kernels, this unit has a linear +/// relationship with `Sigma`. See `kKernelRadiusPerSigma` for +/// details on how this relationship works. +struct Radius { + Scalar radius = 0.0; + + constexpr Radius() = default; + + explicit constexpr Radius(Scalar p_radius) : radius(p_radius) {} + + operator Sigma() const; // NOLINT(google-explicit-constructor) +}; + +} // namespace impeller