From 7b6227fde951be23343a94c88febfc9cf651958f Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Wed, 20 Dec 2023 11:45:23 -0800 Subject: [PATCH] [Impeller] new blur: refactored math and fixed expanded padding size (#49206) This refactors the math so that it makes it easier to conditionally add padding. That optimization was removed for now since it wasn't quite working satisfactorily yet. This also fixes the math used to expand the coverage hint. There is no visual test for it since it would only ever result in wasteful expansion that would show up in the benchmarks. ## 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]. [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 --- impeller/aiks/aiks_unittests.cc | 1 - .../filters/gaussian_blur_filter_contents.cc | 98 +++++++------------ .../filters/gaussian_blur_filter_contents.h | 6 +- ...gaussian_blur_filter_contents_unittests.cc | 4 +- 4 files changed, 44 insertions(+), 65 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 54d0b1de62401..805c91cdb4850 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3976,6 +3976,5 @@ TEST_P(AiksTest, SubpassWithClearColorOptimization) { // will be filled with NaNs and may produce a magenta texture on macOS or iOS. ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } - } // namespace testing } // namespace impeller diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index f61439d7f047e..d44928fffc669 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -19,16 +19,6 @@ using GaussianBlurFragmentShader = GaussianBlurPipeline::FragmentShader; namespace { -std::optional ExpandCoverageHint(const std::optional& coverage_hint, - const Matrix& source_to_local_transform, - const Vector2& padding) { - if (!coverage_hint.has_value()) { - return std::nullopt; - } - Vector2 transformed_padding = (source_to_local_transform * padding).Abs(); - return coverage_hint->Expand(transformed_padding); -} - SamplerDescriptor MakeSamplerDescriptor(MinMagFilter filter, SamplerAddressMode address_mode) { SamplerDescriptor sampler_desc; @@ -48,12 +38,6 @@ void BindVertices(Command& cmd, cmd.BindVertices(vtx_builder.CreateVertexBuffer(host_buffer)); } -Matrix MakeAnchorScale(const Point& anchor, Vector2 scale) { - return Matrix::MakeTranslation({anchor.x, anchor.y, 0}) * - Matrix::MakeScale(scale) * - Matrix::MakeTranslation({-anchor.x, -anchor.y, 0}); -} - void SetTileMode(SamplerDescriptor* descriptor, const ContentContext& renderer, Entity::TileMode tile_mode) { @@ -87,7 +71,6 @@ std::shared_ptr MakeDownsampleSubpass( const SamplerDescriptor& sampler_descriptor, const Quad& uvs, const ISize& subpass_size, - const Vector2 padding, Entity::TileMode tile_mode) { ContentContext::SubpassCallback subpass_callback = [&](const ContentContext& renderer, RenderPass& pass) { @@ -104,23 +87,13 @@ std::shared_ptr MakeDownsampleSubpass( frame_info.texture_sampler_y_coord_scale = 1.0; frame_info.alpha = 1.0; - // Insert transparent gutter around the downsampled image so the blur - // creates a halo effect. This compensates for when the expanded clip - // region can't give us the full gutter we want. - Vector2 texture_size = Vector2(input_texture->GetSize()); - Quad guttered_uvs = - MakeAnchorScale({0.5, 0.5}, - (texture_size + padding * 2) / texture_size) - .Transform(uvs); - - BindVertices( - cmd, host_buffer, - { - {Point(0, 0), guttered_uvs[0]}, - {Point(1, 0), guttered_uvs[1]}, - {Point(0, 1), guttered_uvs[2]}, - {Point(1, 1), guttered_uvs[3]}, - }); + BindVertices(cmd, host_buffer, + { + {Point(0, 0), uvs[0]}, + {Point(1, 0), uvs[1]}, + {Point(0, 1), uvs[2]}, + {Point(1, 1), uvs[3]}, + }); SamplerDescriptor linear_sampler_descriptor = sampler_descriptor; SetTileMode(&linear_sampler_descriptor, renderer, tile_mode); @@ -270,17 +243,17 @@ std::optional GaussianBlurFilterContents::RenderFilter( Vector2 blur_radius = {CalculateBlurRadius(scaled_sigma.x), CalculateBlurRadius(scaled_sigma.y)}; Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); + Vector2 local_padding = + (entity.GetTransform().Basis() * effect_transform.Basis() * padding) + .Abs(); // Apply as much of the desired padding as possible from the source. This may // be ignored so must be accounted for in the downsample pass by adding a // transparent gutter. - std::optional expanded_coverage_hint = ExpandCoverageHint( - coverage_hint, entity.GetTransform() * effect_transform, padding); - // TODO(gaaclarke): How much of the gutter is thrown away can be used to - // adjust the padding that is added in the downsample pass. - // For example, if we get all the padding we requested from - // the expanded_coverage_hint, there is no need to add a - // transparent gutter. + std::optional expanded_coverage_hint; + if (coverage_hint.has_value()) { + expanded_coverage_hint = coverage_hint->Expand(local_padding); + } std::optional input_snapshot = inputs[0]->GetSnapshot("GaussianBlur", renderer, entity, @@ -300,21 +273,28 @@ std::optional GaussianBlurFilterContents::RenderFilter( // gutter from the expanded_coverage_hint, we can skip the downsample pass. // pass. Vector2 downsample_scalar(desired_scalar, desired_scalar); - Vector2 padded_size = - Vector2(input_snapshot->texture->GetSize()) + 2.0 * padding; - Vector2 downsampled_size = padded_size * downsample_scalar; - // TODO(gaaclarke): I don't think we are correctly handling this fractional - // amount we are throwing away. + Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); + Rect source_rect_padded = source_rect.Expand(padding); + Matrix padding_snapshot_adjustment = Matrix::MakeTranslation(-padding); + // TODO(gaaclarke): The padding could be removed if we know it's not needed or + // resized to account for the expanded_clip_coverage. There doesn't appear + // to be the math to make those calculations though. The following + // optimization works, but causes a shimmer as a result of + // https://github.com/flutter/flutter/issues/140193 so it isn't applied. + // + // !input_snapshot->GetCoverage()->Expand(-local_padding) + // .Contains(coverage_hint.value())) + Vector2 downsampled_size = source_rect_padded.size * downsample_scalar; ISize subpass_size = ISize(round(downsampled_size.x), round(downsampled_size.y)); - Vector2 effective_scalar = subpass_size / padded_size; + Vector2 effective_scalar = Vector2(subpass_size) / source_rect_padded.size; - Quad uvs = - CalculateUVs(inputs[0], entity, input_snapshot->texture->GetSize()); + Quad uvs = CalculateUVs(inputs[0], entity, source_rect_padded, + input_snapshot->texture->GetSize()); std::shared_ptr pass1_out_texture = MakeDownsampleSubpass( renderer, input_snapshot->texture, input_snapshot->sampler_descriptor, - uvs, subpass_size, padding, tile_mode_); + uvs, subpass_size, tile_mode_); Vector2 pass1_pixel_size = 1.0 / Vector2(pass1_out_texture->GetSize()); @@ -343,13 +323,12 @@ std::optional GaussianBlurFilterContents::RenderFilter( MinMagFilter::kLinear, SamplerAddressMode::kClampToEdge); return Entity::FromSnapshot( - Snapshot{ - .texture = pass3_out_texture, - .transform = input_snapshot->transform * - Matrix::MakeTranslation({-padding.x, -padding.y, 0}) * - Matrix::MakeScale(1 / effective_scalar), - .sampler_descriptor = sampler_desc, - .opacity = input_snapshot->opacity}, + Snapshot{.texture = pass3_out_texture, + .transform = input_snapshot->transform * + padding_snapshot_adjustment * + Matrix::MakeScale(1 / effective_scalar), + .sampler_descriptor = sampler_desc, + .opacity = input_snapshot->opacity}, entity.GetBlendMode(), entity.GetClipDepth()); } @@ -360,11 +339,10 @@ Scalar GaussianBlurFilterContents::CalculateBlurRadius(Scalar sigma) { Quad GaussianBlurFilterContents::CalculateUVs( const std::shared_ptr& filter_input, const Entity& entity, + const Rect& source_rect, const ISize& texture_size) { Matrix input_transform = filter_input->GetLocalTransform(entity); - Rect snapshot_rect = - Rect::MakeXYWH(0, 0, texture_size.width, texture_size.height); - Quad coverage_quad = snapshot_rect.GetTransformedPoints(input_transform); + Quad coverage_quad = source_rect.GetTransformedPoints(input_transform); Matrix uv_transform = Matrix::MakeScale( {1.0f / texture_size.width, 1.0f / texture_size.height, 1.0f}); diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 7932291fd9ee8..a139771b3f8b1 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -41,10 +41,12 @@ class GaussianBlurFilterContents final : public FilterContents { /// Calculate the UV coordinates for rendering the filter_input. /// @param filter_input The FilterInput that should be rendered. /// @param entity The associated entity for the filter_input. - /// @param texture_size The size of the texture_size the uvs will be used for. + /// @param source_rect The rect in source coordinates to convert to uvs. + /// @param texture_size The rect to convert in source coordinates. static Quad CalculateUVs(const std::shared_ptr& filter_input, const Entity& entity, - const ISize& pass_size); + const Rect& source_rect, + const ISize& texture_size); /// Calculate the scale factor for the downsample pass given a sigma value. /// 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 40e36406cf065..4350c9318b7c3 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -285,8 +285,8 @@ TEST_P(GaussianBlurFilterContentsTest, CalculateUVsSimple) { std::shared_ptr texture = MakeTexture(desc); auto filter_input = FilterInput::Make(texture); Entity entity; - Quad uvs = GaussianBlurFilterContents::CalculateUVs(filter_input, entity, - ISize(100, 100)); + Quad uvs = GaussianBlurFilterContents::CalculateUVs( + filter_input, entity, Rect::MakeSize(ISize(100, 100)), ISize(100, 100)); std::optional uvs_bounds = Rect::MakePointBounds(uvs); EXPECT_TRUE(uvs_bounds.has_value()); if (uvs_bounds.has_value()) {