[Impeller] added a fallback that will make sure the blur fragment shader doesn't overflow (flutter/engine#53466)

issue: https://github.com/flutter/flutter/issues/150462

This makes that usage never crash.  I would like to reevaluate for a better fix later though.  Something like:
1) Rework the downsample logic to make sure this gap doesn't exist.
1) Simply increase the kernel size.  We already did that before and technically we should be clamped to the 500 sigma so we won't have to do it forever.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
gaaclarke 2024-06-24 11:01:00 -07:00 committed by GitHub
parent c2f5afd59a
commit b6e6849f95
3 changed files with 50 additions and 15 deletions

View File

@ -23,8 +23,6 @@ const int32_t GaussianBlurFilterContents::kBlurFilterRequiredMipCount = 4;
namespace { namespace {
// 48 comes from gaussian.frag.
const int32_t kMaxKernelSize = 50;
constexpr Scalar kMaxSigma = 500.0f; constexpr Scalar kMaxSigma = 500.0f;
SamplerDescriptor MakeSamplerDescriptor(MinMagFilter filter, SamplerDescriptor MakeSamplerDescriptor(MinMagFilter filter,
@ -174,7 +172,7 @@ fml::StatusOr<RenderTarget> MakeBlurSubpass(
pass, host_buffer.EmplaceUniform(frame_info)); pass, host_buffer.EmplaceUniform(frame_info));
GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples = GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples =
LerpHackKernelSamples(GenerateBlurInfo(blur_info)); LerpHackKernelSamples(GenerateBlurInfo(blur_info));
FML_CHECK(kernel_samples.sample_count < kMaxKernelSize); FML_CHECK(kernel_samples.sample_count <= kGaussianBlurMaxKernelSize);
GaussianBlurFragmentShader::BindKernelSamples( GaussianBlurFragmentShader::BindKernelSamples(
pass, host_buffer.EmplaceUniform(kernel_samples)); pass, host_buffer.EmplaceUniform(kernel_samples));
return pass.Draw().ok(); return pass.Draw().ok();
@ -639,9 +637,8 @@ Scalar GaussianBlurFilterContents::ScaleSigma(Scalar sigma) {
return clamped * scalar; return clamped * scalar;
} }
GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo( KernelSamples GenerateBlurInfo(BlurParameters parameters) {
BlurParameters parameters) { KernelSamples result;
GaussianBlurPipeline::FragmentShader::KernelSamples result;
result.sample_count = result.sample_count =
((2 * parameters.blur_radius) / parameters.step_size) + 1; ((2 * parameters.blur_radius) / parameters.step_size) + 1;
@ -653,6 +650,18 @@ GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo(
x_offset = 1; 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; Scalar tally = 0.0f;
for (int i = 0; i < result.sample_count; ++i) { for (int i = 0; i < result.sample_count; ++i) {
int x = x_offset + (i * parameters.step_size) - parameters.blur_radius; 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 // This works by shrinking the kernel size by 2 and relying on lerp to read
// between the samples. // between the samples.
GaussianBlurPipeline::FragmentShader::KernelSamples LerpHackKernelSamples( GaussianBlurPipeline::FragmentShader::KernelSamples LerpHackKernelSamples(
GaussianBlurPipeline::FragmentShader::KernelSamples parameters) { KernelSamples parameters) {
GaussianBlurPipeline::FragmentShader::KernelSamples result; GaussianBlurPipeline::FragmentShader::KernelSamples result;
result.sample_count = ((parameters.sample_count - 1) / 2) + 1; result.sample_count = ((parameters.sample_count - 1) / 2) + 1;
int32_t middle = result.sample_count / 2; int32_t middle = result.sample_count / 2;
int32_t j = 0; int32_t j = 0;
FML_DCHECK(result.sample_count <= kGaussianBlurMaxKernelSize);
for (int i = 0; i < result.sample_count; i++) { for (int i = 0; i < result.sample_count; i++) {
if (i == middle) { if (i == middle) {
result.samples[i] = parameters.samples[j++]; result.samples[i] = parameters.samples[j++];

View File

@ -12,6 +12,9 @@
namespace impeller { namespace impeller {
// Comes from gaussian.frag.
static constexpr int32_t kGaussianBlurMaxKernelSize = 50;
struct BlurParameters { struct BlurParameters {
Point blur_uv_offset; Point blur_uv_offset;
Scalar blur_sigma; Scalar blur_sigma;
@ -19,13 +22,23 @@ struct BlurParameters {
int step_size; int step_size;
}; };
GaussianBlurPipeline::FragmentShader::KernelSamples GenerateBlurInfo( /// A larger mirror of GaussianBlurPipeline::FragmentShader::KernelSamples.
BlurParameters parameters); ///
/// 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 /// This will shrink the size of a kernel by roughly half by sampling between
/// samples and relying on linear interpolation between the samples. /// samples and relying on linear interpolation between the samples.
GaussianBlurPipeline::FragmentShader::KernelSamples LerpHackKernelSamples( GaussianBlurPipeline::FragmentShader::KernelSamples LerpHackKernelSamples(
GaussianBlurPipeline::FragmentShader::KernelSamples samples); KernelSamples samples);
/// Performs a bidirectional Gaussian blur. /// Performs a bidirectional Gaussian blur.
/// ///

View File

@ -461,8 +461,7 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) {
.blur_sigma = 1, .blur_sigma = 1,
.blur_radius = 5, .blur_radius = 5,
.step_size = 1}; .step_size = 1};
GaussianBlurPipeline::FragmentShader::KernelSamples samples = KernelSamples samples = GenerateBlurInfo(parameters);
GenerateBlurInfo(parameters);
EXPECT_EQ(samples.sample_count, 9); EXPECT_EQ(samples.sample_count, 9);
// Coefficients should add up to 1. // Coefficients should add up to 1.
@ -482,7 +481,7 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) {
} }
TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesSimple) { TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesSimple) {
GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples = { KernelSamples kernel_samples = {
.sample_count = 5, .sample_count = 5,
.samples = .samples =
{ {
@ -567,8 +566,7 @@ TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesComplex) {
.blur_sigma = sigma, .blur_sigma = sigma,
.blur_radius = blur_radius, .blur_radius = blur_radius,
.step_size = 1}; .step_size = 1};
GaussianBlurPipeline::FragmentShader::KernelSamples kernel_samples = KernelSamples kernel_samples = GenerateBlurInfo(parameters);
GenerateBlurInfo(parameters);
EXPECT_EQ(kernel_samples.sample_count, 33); EXPECT_EQ(kernel_samples.sample_count, 33);
GaussianBlurPipeline::FragmentShader::KernelSamples fast_kernel_samples = GaussianBlurPipeline::FragmentShader::KernelSamples fast_kernel_samples =
LerpHackKernelSamples(kernel_samples); LerpHackKernelSamples(kernel_samples);
@ -614,5 +612,19 @@ TEST(GaussianBlurFilterContentsTest, LerpHackKernelSamplesComplex) {
EXPECT_NEAR(output, fast_output, 0.1); EXPECT_NEAR(output, fast_output, 0.1);
} }
TEST(GaussianBlurFilterContentsTest, ChopHugeBlurs) {
Scalar sigma = 30.5f;
int32_t blur_radius = static_cast<int32_t>(
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 testing
} // namespace impeller } // namespace impeller