From f3d1bf552d23dd6a47e6a08655321677e7c53cf5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Dec 2023 13:21:28 -0800 Subject: [PATCH 01/15] [Impeller] made blur subpasses as big as the hint --- .../contents/filters/gaussian_blur_filter_contents.cc | 11 +++++------ .../contents/filters/gaussian_blur_filter_contents.h | 4 ++-- .../gaussian_blur_filter_contents_unittests.cc | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index f61439d7f047e..bc84c4c8a2e9f 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -309,8 +309,9 @@ std::optional GaussianBlurFilterContents::RenderFilter( ISize(round(downsampled_size.x), round(downsampled_size.y)); Vector2 effective_scalar = subpass_size / padded_size; + Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); Quad uvs = - CalculateUVs(inputs[0], entity, input_snapshot->texture->GetSize()); + CalculateUVs(inputs[0], entity, source_rect); std::shared_ptr pass1_out_texture = MakeDownsampleSubpass( renderer, input_snapshot->texture, input_snapshot->sampler_descriptor, @@ -360,14 +361,12 @@ Scalar GaussianBlurFilterContents::CalculateBlurRadius(Scalar sigma) { Quad GaussianBlurFilterContents::CalculateUVs( const std::shared_ptr& filter_input, const Entity& entity, - const ISize& texture_size) { + const Rect& source_rect) { 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}); + {1.0f / source_rect.size.width, 1.0f / source_rect.size.height, 1.0f}); return uv_transform.Transform(coverage_quad); } diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 7932291fd9ee8..477fd3ec870ac 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -41,10 +41,10 @@ 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 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); /// 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..98ec2e5a195f0 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -286,7 +286,7 @@ TEST_P(GaussianBlurFilterContentsTest, CalculateUVsSimple) { auto filter_input = FilterInput::Make(texture); Entity entity; Quad uvs = GaussianBlurFilterContents::CalculateUVs(filter_input, entity, - ISize(100, 100)); + Rect::MakeSize(ISize(100, 100))); std::optional uvs_bounds = Rect::MakePointBounds(uvs); EXPECT_TRUE(uvs_bounds.has_value()); if (uvs_bounds.has_value()) { From f201e5c1242032dede983c80a5676de87a6b2315 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Dec 2023 13:29:08 -0800 Subject: [PATCH 02/15] ++ --- .../contents/filters/gaussian_blur_filter_contents.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index bc84c4c8a2e9f..c6436b51a8c50 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -300,16 +300,15 @@ 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; + Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); + Rect source_rect_padded = source_rect.Expand(padding); + Vector2 downsampled_size = source_rect_padded.size * downsample_scalar; // TODO(gaaclarke): I don't think we are correctly handling this fractional // amount we are throwing away. 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; - Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); Quad uvs = CalculateUVs(inputs[0], entity, source_rect); From 7c6e658b37d1e8e6fa66de5de26e87590b9f45d8 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Dec 2023 13:31:45 -0800 Subject: [PATCH 03/15] ++ --- .../entity/contents/filters/gaussian_blur_filter_contents.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index c6436b51a8c50..9485f4fe79349 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -271,11 +271,12 @@ std::optional GaussianBlurFilterContents::RenderFilter( CalculateBlurRadius(scaled_sigma.y)}; Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); + Matrix source_to_local = entity.GetTransform() * effect_transform; // 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); + coverage_hint, source_to_local, 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 From 9429eb6b1a8a140c44c689bc7a521d3cc2c70429 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Dec 2023 13:49:32 -0800 Subject: [PATCH 04/15] ++ --- .../filters/gaussian_blur_filter_contents.cc | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 9485f4fe79349..087fcbdfd29e1 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -48,12 +48,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 +81,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,22 +97,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]}, + {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; @@ -302,20 +286,19 @@ std::optional GaussianBlurFilterContents::RenderFilter( // pass. Vector2 downsample_scalar(desired_scalar, desired_scalar); Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); - Rect source_rect_padded = source_rect.Expand(padding); - Vector2 downsampled_size = source_rect_padded.size * downsample_scalar; + Vector2 downsampled_size = source_rect.size * downsample_scalar; // TODO(gaaclarke): I don't think we are correctly handling this fractional // amount we are throwing away. ISize subpass_size = ISize(round(downsampled_size.x), round(downsampled_size.y)); - Vector2 effective_scalar = Vector2(subpass_size) / source_rect_padded.size; + Vector2 effective_scalar = Vector2(subpass_size) / source_rect.size; Quad uvs = CalculateUVs(inputs[0], entity, source_rect); 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()); @@ -347,7 +330,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( 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}, From 9acb2cdbd3e8a3c02be695b842555088848f873c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 18 Dec 2023 15:55:55 -0800 Subject: [PATCH 05/15] ++ --- .../contents/filters/filter_contents.cc | 2 + .../filters/gaussian_blur_filter_contents.cc | 54 +++++++++++-------- .../filters/gaussian_blur_filter_contents.h | 3 +- ...gaussian_blur_filter_contents_unittests.cc | 4 +- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index 5f2c310f78d9e..c4ffaff85726d 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -50,6 +50,8 @@ std::shared_ptr FilterContents::MakeDirectionalGaussianBlur( return blur; } +#define IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER 1 + std::shared_ptr FilterContents::MakeGaussianBlur( const FilterInput::Ref& input, Sigma sigma_x, diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 087fcbdfd29e1..0fe8bf368ce30 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -97,14 +97,13 @@ std::shared_ptr MakeDownsampleSubpass( frame_info.texture_sampler_y_coord_scale = 1.0; frame_info.alpha = 1.0; - BindVertices( - cmd, host_buffer, - { - {Point(0, 0), uvs[0]}, - {Point(1, 0), uvs[1]}, - {Point(0, 1), uvs[2]}, - {Point(1, 1), 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); @@ -255,12 +254,11 @@ std::optional GaussianBlurFilterContents::RenderFilter( CalculateBlurRadius(scaled_sigma.y)}; Vector2 padding(ceil(blur_radius.x), ceil(blur_radius.y)); - Matrix source_to_local = entity.GetTransform() * effect_transform; // 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, source_to_local, padding); + 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 @@ -281,20 +279,31 @@ std::optional GaussianBlurFilterContents::RenderFilter( Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); + desired_scalar = 1.0; // TODO(jonahwilliams): If desired_scalar is 1.0 and we fully acquired the // gutter from the expanded_coverage_hint, we can skip the downsample pass. // pass. Vector2 downsample_scalar(desired_scalar, desired_scalar); Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); - Vector2 downsampled_size = source_rect.size * downsample_scalar; + Rect source_rect_padded = source_rect; + Matrix padding_snapshot_adjustment; + if (coverage_hint.has_value() && input_snapshot->GetCoverage().has_value() && + coverage_hint->Contains(input_snapshot->GetCoverage().value())) { + // This means the expanded_coverage_hint was requested in order to get more + // content to sample from, but none was available. So, we'll add our own + // padding. + source_rect_padded = source_rect_padded.Expand(padding); + padding_snapshot_adjustment = Matrix::MakeTranslation(-padding); + } + Vector2 downsampled_size = source_rect_padded.size * downsample_scalar; // TODO(gaaclarke): I don't think we are correctly handling this fractional // amount we are throwing away. ISize subpass_size = ISize(round(downsampled_size.x), round(downsampled_size.y)); - Vector2 effective_scalar = Vector2(subpass_size) / source_rect.size; + Vector2 effective_scalar = Vector2(subpass_size) / source_rect_padded.size; - Quad uvs = - CalculateUVs(inputs[0], entity, source_rect); + 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, @@ -327,12 +336,12 @@ std::optional GaussianBlurFilterContents::RenderFilter( MinMagFilter::kLinear, SamplerAddressMode::kClampToEdge); return Entity::FromSnapshot( - Snapshot{ - .texture = pass3_out_texture, - .transform = input_snapshot->transform * - 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()); } @@ -343,12 +352,13 @@ Scalar GaussianBlurFilterContents::CalculateBlurRadius(Scalar sigma) { Quad GaussianBlurFilterContents::CalculateUVs( const std::shared_ptr& filter_input, const Entity& entity, - const Rect& source_rect) { + const Rect& source_rect, + const ISize& texture_size) { Matrix input_transform = filter_input->GetLocalTransform(entity); Quad coverage_quad = source_rect.GetTransformedPoints(input_transform); Matrix uv_transform = Matrix::MakeScale( - {1.0f / source_rect.size.width, 1.0f / source_rect.size.height, 1.0f}); + {1.0f / texture_size.width, 1.0f / texture_size.height, 1.0f}); return uv_transform.Transform(coverage_quad); } diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 477fd3ec870ac..772dcbdb81d98 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -44,7 +44,8 @@ class GaussianBlurFilterContents final : public FilterContents { /// @param texture_size The rect to convert in source coordinates. static Quad CalculateUVs(const std::shared_ptr& filter_input, const Entity& entity, - const Rect& source_rect); + 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 98ec2e5a195f0..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, - Rect::MakeSize(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()) { From 597879e1bc5e1f99880a302df16d873ed4aefcf1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 09:38:42 -0800 Subject: [PATCH 06/15] flipped the branch --- .../contents/filters/gaussian_blur_filter_contents.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 0fe8bf368ce30..99a6d681dcfe3 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -288,10 +288,9 @@ std::optional GaussianBlurFilterContents::RenderFilter( Rect source_rect_padded = source_rect; Matrix padding_snapshot_adjustment; if (coverage_hint.has_value() && input_snapshot->GetCoverage().has_value() && - coverage_hint->Contains(input_snapshot->GetCoverage().value())) { - // This means the expanded_coverage_hint was requested in order to get more - // content to sample from, but none was available. So, we'll add our own - // padding. + !input_snapshot->GetCoverage()->Contains(coverage_hint.value())) { + // This means that the snapshot does not contain all the data it needs to + // render, so we add extra padding for the blur halo to render. source_rect_padded = source_rect_padded.Expand(padding); padding_snapshot_adjustment = Matrix::MakeTranslation(-padding); } From af9c37c8bc8ba747a46e22a234824ece77926ebe Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 09:51:56 -0800 Subject: [PATCH 07/15] removed downsample size hack --- .../entity/contents/filters/gaussian_blur_filter_contents.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 99a6d681dcfe3..88713ef6d2fd9 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -279,7 +279,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); - desired_scalar = 1.0; // TODO(jonahwilliams): If desired_scalar is 1.0 and we fully acquired the // gutter from the expanded_coverage_hint, we can skip the downsample pass. // pass. From e7efc25d700ce0a6abe1b31a2309a6c49d74d971 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 10:10:50 -0800 Subject: [PATCH 08/15] updated docstring --- impeller/entity/contents/filters/gaussian_blur_filter_contents.h | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 772dcbdb81d98..a139771b3f8b1 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -41,6 +41,7 @@ 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 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, From 9177933ad77cb6b2a8bb89a0e11c9ed4c4ffa5f4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 13:24:17 -0800 Subject: [PATCH 09/15] fixed linter --- .../entity/contents/filters/gaussian_blur_filter_contents.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 88713ef6d2fd9..9af4071a6ef54 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -271,6 +271,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( if (!input_snapshot.has_value()) { return std::nullopt; } + std::optional input_snapshot_coverage = input_snapshot->GetCoverage(); if (scaled_sigma.x < kEhCloseEnough && scaled_sigma.y < kEhCloseEnough) { return Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode(), @@ -286,8 +287,8 @@ std::optional GaussianBlurFilterContents::RenderFilter( Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); Rect source_rect_padded = source_rect; Matrix padding_snapshot_adjustment; - if (coverage_hint.has_value() && input_snapshot->GetCoverage().has_value() && - !input_snapshot->GetCoverage()->Contains(coverage_hint.value())) { + if (coverage_hint.has_value() && input_snapshot_coverage.has_value() && + !input_snapshot_coverage->Contains(coverage_hint.value())) { // This means that the snapshot does not contain all the data it needs to // render, so we add extra padding for the blur halo to render. source_rect_padded = source_rect_padded.Expand(padding); From c855424f459765d11b484a3e26608ec478566e6f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 13:27:37 -0800 Subject: [PATCH 10/15] updated todo --- .../contents/filters/gaussian_blur_filter_contents.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 9af4071a6ef54..6dec2fa70d0c6 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -259,11 +259,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( // 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 input_snapshot = inputs[0]->GetSnapshot("GaussianBlur", renderer, entity, @@ -291,6 +286,9 @@ std::optional GaussianBlurFilterContents::RenderFilter( !input_snapshot_coverage->Contains(coverage_hint.value())) { // This means that the snapshot does not contain all the data it needs to // render, so we add extra padding for the blur halo to render. + // TODO(gaaclarke): This adds a gutter for the blur halo that is uniform, + // if we could only add padding where necessary that would + // be more efficient. source_rect_padded = source_rect_padded.Expand(padding); padding_snapshot_adjustment = Matrix::MakeTranslation(-padding); } From f17e00acae37529e44fbb240b36c2c68d57c244d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 13:53:47 -0800 Subject: [PATCH 11/15] fixed tests and turned off hack --- impeller/entity/contents/filters/filter_contents.cc | 2 -- .../entity/contents/filters/gaussian_blur_filter_contents.cc | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index c4ffaff85726d..5f2c310f78d9e 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -50,8 +50,6 @@ std::shared_ptr FilterContents::MakeDirectionalGaussianBlur( return blur; } -#define IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER 1 - std::shared_ptr FilterContents::MakeGaussianBlur( const FilterInput::Ref& input, Sigma sigma_x, diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 6dec2fa70d0c6..2f01528bfb6ed 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -282,8 +282,9 @@ std::optional GaussianBlurFilterContents::RenderFilter( Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); Rect source_rect_padded = source_rect; Matrix padding_snapshot_adjustment; - if (coverage_hint.has_value() && input_snapshot_coverage.has_value() && - !input_snapshot_coverage->Contains(coverage_hint.value())) { + if (!coverage_hint.has_value() || + (input_snapshot_coverage.has_value() && + !input_snapshot_coverage->Contains(coverage_hint.value()))) { // This means that the snapshot does not contain all the data it needs to // render, so we add extra padding for the blur halo to render. // TODO(gaaclarke): This adds a gutter for the blur halo that is uniform, From bb87029aef2a0873ab4248ed205b71c4b633d33d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 14:04:06 -0800 Subject: [PATCH 12/15] added test --- ...gaussian_blur_filter_contents_unittests.cc | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) 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 4350c9318b7c3..747020c0f933d 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -205,6 +205,40 @@ TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverage) { } } +TEST_P(GaussianBlurFilterContentsTest, RenderCoverageNoBlurHalo) { + TextureDescriptor desc = { + .storage_mode = StorageMode::kDevicePrivate, + .format = PixelFormat::kB8G8R8A8UNormInt, + .size = ISize(100, 100), + }; + std::shared_ptr texture = MakeTexture(desc); + Scalar sigma_radius_1 = CalculateSigmaForBlurRadius(1.0); + auto contents = std::make_unique( + sigma_radius_1, sigma_radius_1, Entity::TileMode::kDecal); + contents->SetInputs({FilterInput::Make(texture)}); + std::shared_ptr renderer = GetContentContext(); + + Entity entity; + std::optional result = contents->GetEntity( + *renderer, entity, /*coverage_hint=*/Rect::MakeLTRB(25, 25, 75, 75)); + EXPECT_TRUE(result.has_value()); + if (result.has_value()) { + EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); + std::optional result_coverage = result.value().GetCoverage(); + std::optional contents_coverage = contents->GetCoverage(entity); + EXPECT_TRUE(result_coverage.has_value()); + EXPECT_TRUE(contents_coverage.has_value()); + if (result_coverage.has_value() && contents_coverage.has_value()) { + EXPECT_TRUE(RectNear(contents_coverage.value(), + Rect::MakeLTRB(-1, -1, 101, 101))); + // The result coverage is inside the coverage as a result of an + // optimization that avoids adding a halo gutter. + EXPECT_TRUE( + RectNear(result_coverage.value(), Rect::MakeLTRB(0, 0, 100, 100))); + } + } +} + TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { TextureDescriptor desc = { From 8947a453f24b46cd8071f5ed15ab3bd61edb74b7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Dec 2023 09:49:35 -0800 Subject: [PATCH 13/15] Removed the optimization, fixed local padding calculation --- impeller/aiks/aiks_unittests.cc | 1 - .../filters/gaussian_blur_filter_contents.cc | 44 +++++++------------ ...gaussian_blur_filter_contents_unittests.cc | 34 -------------- impeller/geometry/rect.h | 3 +- 4 files changed, 18 insertions(+), 64 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 997d5ffe30f76..77137566d893a 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3930,6 +3930,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 2f01528bfb6ed..5a47d9583229b 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; @@ -253,12 +243,16 @@ 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); + 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, @@ -266,7 +260,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( if (!input_snapshot.has_value()) { return std::nullopt; } - std::optional input_snapshot_coverage = input_snapshot->GetCoverage(); if (scaled_sigma.x < kEhCloseEnough && scaled_sigma.y < kEhCloseEnough) { return Entity::FromSnapshot(input_snapshot.value(), entity.GetBlendMode(), @@ -280,22 +273,17 @@ std::optional GaussianBlurFilterContents::RenderFilter( // pass. Vector2 downsample_scalar(desired_scalar, desired_scalar); Rect source_rect = Rect::MakeSize(input_snapshot->texture->GetSize()); - Rect source_rect_padded = source_rect; - Matrix padding_snapshot_adjustment; - if (!coverage_hint.has_value() || - (input_snapshot_coverage.has_value() && - !input_snapshot_coverage->Contains(coverage_hint.value()))) { - // This means that the snapshot does not contain all the data it needs to - // render, so we add extra padding for the blur halo to render. - // TODO(gaaclarke): This adds a gutter for the blur halo that is uniform, - // if we could only add padding where necessary that would - // be more efficient. - source_rect_padded = source_rect_padded.Expand(padding); - padding_snapshot_adjustment = Matrix::MakeTranslation(-padding); - } + 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; - // TODO(gaaclarke): I don't think we are correctly handling this fractional - // amount we are throwing away. ISize subpass_size = ISize(round(downsampled_size.x), round(downsampled_size.y)); Vector2 effective_scalar = Vector2(subpass_size) / source_rect_padded.size; 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 747020c0f933d..4350c9318b7c3 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -205,40 +205,6 @@ TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverage) { } } -TEST_P(GaussianBlurFilterContentsTest, RenderCoverageNoBlurHalo) { - TextureDescriptor desc = { - .storage_mode = StorageMode::kDevicePrivate, - .format = PixelFormat::kB8G8R8A8UNormInt, - .size = ISize(100, 100), - }; - std::shared_ptr texture = MakeTexture(desc); - Scalar sigma_radius_1 = CalculateSigmaForBlurRadius(1.0); - auto contents = std::make_unique( - sigma_radius_1, sigma_radius_1, Entity::TileMode::kDecal); - contents->SetInputs({FilterInput::Make(texture)}); - std::shared_ptr renderer = GetContentContext(); - - Entity entity; - std::optional result = contents->GetEntity( - *renderer, entity, /*coverage_hint=*/Rect::MakeLTRB(25, 25, 75, 75)); - EXPECT_TRUE(result.has_value()); - if (result.has_value()) { - EXPECT_EQ(result.value().GetBlendMode(), BlendMode::kSourceOver); - std::optional result_coverage = result.value().GetCoverage(); - std::optional contents_coverage = contents->GetCoverage(entity); - EXPECT_TRUE(result_coverage.has_value()); - EXPECT_TRUE(contents_coverage.has_value()); - if (result_coverage.has_value() && contents_coverage.has_value()) { - EXPECT_TRUE(RectNear(contents_coverage.value(), - Rect::MakeLTRB(-1, -1, 101, 101))); - // The result coverage is inside the coverage as a result of an - // optimization that avoids adding a halo gutter. - EXPECT_TRUE( - RectNear(result_coverage.value(), Rect::MakeLTRB(0, 0, 100, 100))); - } - } -} - TEST_P(GaussianBlurFilterContentsTest, RenderCoverageMatchesGetCoverageTranslate) { TextureDescriptor desc = { diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h index c371512ccf9cb..05b162dfed2f9 100644 --- a/impeller/geometry/rect.h +++ b/impeller/geometry/rect.h @@ -127,7 +127,8 @@ struct TRect { } constexpr bool Contains(const TRect& o) const { - return Union(o).size == size; + return o.GetLeft() >= GetLeft() && o.GetTop() >= GetTop() && + o.GetRight() <= GetRight() && o.GetBottom() <= GetBottom(); } /// Returns true if either of the width or height are 0, negative, or NaN. From f265eb6a8892c801b483cd1ad0abc4f6736a39a2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Dec 2023 10:53:48 -0800 Subject: [PATCH 14/15] removed rect fix for now --- impeller/geometry/rect.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h index 05b162dfed2f9..c371512ccf9cb 100644 --- a/impeller/geometry/rect.h +++ b/impeller/geometry/rect.h @@ -127,8 +127,7 @@ struct TRect { } constexpr bool Contains(const TRect& o) const { - return o.GetLeft() >= GetLeft() && o.GetTop() >= GetTop() && - o.GetRight() <= GetRight() && o.GetBottom() <= GetBottom(); + return Union(o).size == size; } /// Returns true if either of the width or height are 0, negative, or NaN. From c799e298e7ca1d0affa7837ca42dc6f8e3405587 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 20 Dec 2023 11:00:30 -0800 Subject: [PATCH 15/15] format --- .../entity/contents/filters/gaussian_blur_filter_contents.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 5a47d9583229b..d44928fffc669 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -244,7 +244,8 @@ std::optional GaussianBlurFilterContents::RenderFilter( 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(); + (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