Skip to content

Commit

Permalink
[Impeller] blur: hold on to 1/8 downsample until the kernel overflows (
Browse files Browse the repository at this point in the history
…#50363)

issue: flutter/flutter#142753

After #50262 there were still some
sigmas that could show shimmering around the sigma [50, 100] range which
had a downsample amount of 1/16. This makes those ranges hang on to 1/8
for as long as possible. I'm unable to see any shimmering with
`AiksTest.GaussianBlurAnimatedBackdrop` after this PR. I've also
expanded that test so the clip region could be scaled to make sure that
there aren't sizes which cause it to reappear.

Testing: Expanded on manual testing. Since the error only manifests when
evaluating multiple frames of rendering we don't have infrastructure to
test that.

Here is the graph of `GaussianBlurFilterContents::CalculateScale` after
this change:

<img width="903" alt="Screenshot 2024-02-05 at 2 10 41 PM"
src="https://github.com/flutter/engine/assets/30870216/ac3b735a-95f0-4d7b-af12-58e1ae75278e">

## 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].

<!-- Links -->
[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
  • Loading branch information
gaaclarke authored Feb 6, 2024
1 parent 9c1b6c9 commit 8088863
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
4 changes: 4 additions & 0 deletions impeller/aiks/aiks_blur_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,10 @@ TEST_P(AiksTest, GaussianBlurAnimatedBackdrop) {
Point(1024 / 2 - boston->GetSize().width / 2,
(768 / 2 - boston->GetSize().height / 2) + y),
{});
auto [handle_a, handle_b] = IMPELLER_PLAYGROUND_LINE(
Point(100, 100), Point(900, 700), 20, Color::Red(), Color::Red());
canvas.ClipRect(
Rect::MakeLTRB(handle_a.x, handle_a.y, handle_b.x, handle_b.y));
canvas.ClipRect(Rect::MakeLTRB(100, 100, 900, 700));
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
ImageFilter::MakeBlur(Sigma(sigma), Sigma(sigma),
Expand Down
30 changes: 21 additions & 9 deletions impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ const int32_t GaussianBlurFilterContents::kBlurFilterRequiredMipCount = 4;

namespace {

// 48 comes from kernel.glsl.
const int32_t kMaxKernelSize = 48;

SamplerDescriptor MakeSamplerDescriptor(MinMagFilter filter,
SamplerAddressMode address_mode) {
SamplerDescriptor sampler_desc;
Expand Down Expand Up @@ -189,6 +192,10 @@ Rect MakeReferenceUVs(const Rect& reference, const Rect& rect) {
rect.GetSize());
return result.Scale(1.0f / Vector2(reference.GetSize()));
}

int ScaleBlurRadius(Scalar radius, Scalar scalar) {
return static_cast<int>(std::round(radius * scalar));
}
} // namespace

std::string_view GaussianBlurFilterContents::kNoMipsError =
Expand All @@ -207,13 +214,21 @@ Scalar GaussianBlurFilterContents::CalculateScale(Scalar sigma) {
if (sigma <= 4) {
return 1.0;
}
Scalar result = 4.0 / sigma;
Scalar raw_result = 4.0 / sigma;
// Round to the nearest 1/(2^n) to get the best quality down scaling.
Scalar exponent = round(log2f(result));
Scalar exponent = round(log2f(raw_result));
// Don't scale down below 1/16th to preserve signal.
exponent = std::max(-4.0f, exponent);
Scalar rounded = powf(2.0f, exponent);
return rounded;
Scalar result = rounded;
// Only drop below 1/8 if 1/8 would overflow our kernel.
if (rounded < 0.125f) {
Scalar rounded_plus = powf(2.0f, exponent + 1);
Scalar blur_radius = CalculateBlurRadius(sigma);
int kernel_size_plus = (ScaleBlurRadius(blur_radius, rounded_plus) * 2) + 1;
result = kernel_size_plus < kMaxKernelSize ? rounded_plus : rounded;
}
return result;
};

std::optional<Rect> GaussianBlurFilterContents::GetFilterSourceCoverage(
Expand Down Expand Up @@ -370,8 +385,7 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
BlurParameters{
.blur_uv_offset = Point(0.0, pass1_pixel_size.y),
.blur_sigma = scaled_sigma.y * effective_scalar.y,
.blur_radius =
static_cast<int>(std::round(blur_radius.y * effective_scalar.y)),
.blur_radius = ScaleBlurRadius(blur_radius.y, effective_scalar.y),
.step_size = 1,
},
/*destination_target=*/std::nullopt, blur_uvs);
Expand All @@ -392,8 +406,7 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
BlurParameters{
.blur_uv_offset = Point(pass1_pixel_size.x, 0.0),
.blur_sigma = scaled_sigma.x * effective_scalar.x,
.blur_radius =
static_cast<int>(std::round(blur_radius.x * effective_scalar.x)),
.blur_radius = ScaleBlurRadius(blur_radius.x, effective_scalar.x),
.step_size = 1,
},
pass3_destination, blur_uvs);
Expand Down Expand Up @@ -457,8 +470,7 @@ KernelPipeline::FragmentShader::KernelSamples GenerateBlurInfo(
KernelPipeline::FragmentShader::KernelSamples result;
result.sample_count =
((2 * parameters.blur_radius) / parameters.step_size) + 1;
// 48 comes from kernel.glsl.
FML_CHECK(result.sample_count < 48);
FML_CHECK(result.sample_count < kMaxKernelSize);

// Chop off the last samples if the radius >= 3 where they account for < 1.56%
// of the result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ TEST(GaussianBlurFilterContentsTest, CalculateSigmaValues) {
EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(3.0f), 1);
EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(4.0f), 1);
EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(16.0f), 0.25);
// Hang on to 1/8 as long as possible.
EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(100.0f), 0.125);
// Downsample clamped to 1/16th.
EXPECT_EQ(GaussianBlurFilterContents::CalculateScale(1024.0f), 0.0625);
}
Expand Down

0 comments on commit 8088863

Please sign in to comment.