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 035109e519..efc56bc471 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 @@ -23,8 +23,6 @@ const int32_t GaussianBlurFilterContents::kBlurFilterRequiredMipCount = 4; namespace { -// 48 comes from gaussian.frag. -const int32_t kMaxKernelSize = 50; constexpr Scalar kMaxSigma = 500.0f; SamplerDescriptor MakeSamplerDescriptor(MinMagFilter filter, @@ -174,7 +172,7 @@ fml::StatusOr MakeBlurSubpass( pass, host_buffer.EmplaceUniform(frame_info)); GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples = LerpHackKernelSamples(GenerateBlurInfo(blur_info)); - FML_CHECK(kernel_samples.sample_count < kMaxKernelSize); + FML_CHECK(kernel_samples.sample_count <= kGaussianBlurMaxKernelSize); GaussianBlurFragmentShader::BindKernelSamples( pass, host_buffer.EmplaceUniform(kernel_samples)); return pass.Draw().ok(); @@ -639,9 +637,8 @@ Scalar GaussianBlurFilterContents::ScaleSigma(Scalar sigma) { return clamped * scalar; } -GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo( - BlurParameters parameters) { - GaussianBlurPipeline::FragmentShader::KernelSamples result; +KernelSamples GenerateBlurInfo(BlurParameters parameters) { + KernelSamples result; result.sample_count = ((2 * parameters.blur_radius) / parameters.step_size) + 1; @@ -653,6 +650,18 @@ GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo( x_offset = 1; } + // This is a safe-guard to make sure we don't overflow the fragment shader. + // The kernel size is multiplied by 2 since we'll use the lerp hack on the + // result. In practice this isn't throwing away much data since the blur radii + // are around 53 before the down-sampling and max sigma of 500 kick in. + // + // TODO(https://github.com/flutter/flutter/issues/150462): Come up with a more + // wholistic remedy for this. A proper downsample size should not make this + // required. Or we can increase the kernel size. + if (result.sample_count > KernelSamples::kMaxKernelSize) { + result.sample_count = KernelSamples::kMaxKernelSize; + } + Scalar tally = 0.0f; for (int i = 0; i < result.sample_count; ++i) { int x = x_offset + (i * parameters.step_size) - parameters.blur_radius; @@ -676,11 +685,12 @@ GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo( // This works by shrinking the kernel size by 2 and relying on lerp to read // between the samples. GaussianBlurPipeline::FragmentShader::KernelSamples LerpHackKernelSamples( - GaussianBlurPipeline::FragmentShader::KernelSamples parameters) { + KernelSamples parameters) { GaussianBlurPipeline::FragmentShader::KernelSamples result; result.sample_count = ((parameters.sample_count - 1) / 2) + 1; int32_t middle = result.sample_count / 2; int32_t j = 0; + FML_DCHECK(result.sample_count <= kGaussianBlurMaxKernelSize); for (int i = 0; i < result.sample_count; i++) { if (i == middle) { result.samples[i] = parameters.samples[j++]; 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 2cee099c43..6c53cfae95 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 @@ -12,6 +12,9 @@ namespace impeller { +// Comes from gaussian.frag. +static constexpr int32_t kGaussianBlurMaxKernelSize = 50; + struct BlurParameters { Point blur_uv_offset; Scalar blur_sigma; @@ -19,13 +22,23 @@ struct BlurParameters { int step_size; }; -GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo( - BlurParameters parameters); +/// A larger mirror of GaussianBlurPipeline::FragmentShader::KernelSamples. +/// +/// This is a mirror of GaussianBlurPipeline::FragmentShader::KernelSamples that +/// can hold 2x the max kernel size since it will get reduced with the lerp +/// hack. +struct KernelSamples { + static constexpr int kMaxKernelSize = kGaussianBlurMaxKernelSize * 2; + int sample_count; + GaussianBlurPipeline::FragmentShader::KernelSample samples[kMaxKernelSize]; +}; + +KernelSamples GenerateBlurInfo(BlurParameters parameters); /// This will shrink the size of a kernel by roughly half by sampling between /// samples and relying on linear interpolation between the samples. GaussianBlurPipeline::FragmentShader::KernelSamples LerpHackKernelSamples( - GaussianBlurPipeline::FragmentShader::KernelSamples samples); + KernelSamples samples); /// Performs a bidirectional Gaussian blur. /// 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 b29dd6175e..59618db565 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 @@ -461,8 +461,7 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) { .blur_sigma = 1, .blur_radius = 5, .step_size = 1}; - GaussianBlurPipeline::FragmentShader::KernelSamples samples = - GenerateBlurInfo(parameters); + KernelSamples samples = GenerateBlurInfo(parameters); EXPECT_EQ(samples.sample_count, 9); // Coefficients should add up to 1. @@ -482,7 +481,7 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) { } TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesSimple) { - GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples = { + KernelSamples kernel_samples = { .sample_count = 5, .samples = { @@ -567,8 +566,7 @@ TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesComplex) { .blur_sigma = sigma, .blur_radius = blur_radius, .step_size = 1}; - GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples = - GenerateBlurInfo(parameters); + KernelSamples kernel_samples = GenerateBlurInfo(parameters); EXPECT_EQ(kernel_samples.sample_count, 33); GaussianBlurPipeline::FragmentShader::KernelSamples fast_kernel_samples = LerpHackKernelSamples(kernel_samples); @@ -614,5 +612,19 @@ TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesComplex) { EXPECT_NEAR(output, fast_output, 0.1); } +TEST(GaussianBlurFilterContentsTest, ChopHugeBlurs) { + Scalar sigma = 30.5f; + int32_t blur_radius = static_cast( + std::ceil(GaussianBlurFilterContents::CalculateBlurRadius(sigma))); + BlurParameters parameters = {.blur_uv_offset = Point(1, 0), + .blur_sigma = sigma, + .blur_radius = blur_radius, + .step_size = 1}; + KernelSamples kernel_samples = GenerateBlurInfo(parameters); + GaussianBlurPipeline::FragmentShader::KernelSamples frag_kernel_samples = + LerpHackKernelSamples(kernel_samples); + EXPECT_TRUE(frag_kernel_samples.sample_count <= kGaussianBlurMaxKernelSize); +} + } // namespace testing } // namespace impeller