From 43d6539c92f5dea50aa6cca5041d7f026de9a190 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 19 Dec 2023 16:18:26 -0800 Subject: [PATCH 1/4] [Impeller] limit blur uvs to the clip region --- .../filters/gaussian_blur_filter_contents.cc | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 453c37dd4b28b..3687197bd8b12 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -121,7 +121,8 @@ fml::StatusOr MakeBlurSubpass( const SamplerDescriptor& sampler_descriptor, Entity::TileMode tile_mode, const GaussianBlurFragmentShader::BlurInfo& blur_info, - std::optional destination_target) { + std::optional destination_target, + const Quad& blur_uvs) { if (blur_info.blur_sigma < kEhCloseEnough) { return input_pass; } @@ -153,10 +154,10 @@ fml::StatusOr MakeBlurSubpass( BindVertices(cmd, host_buffer, { - {Point(0, 0), Point(0, 0)}, - {Point(1, 0), Point(1, 0)}, - {Point(0, 1), Point(0, 1)}, - {Point(1, 1), Point(1, 1)}, + {blur_uvs[0], blur_uvs[0]}, + {blur_uvs[1], blur_uvs[1]}, + {blur_uvs[2], blur_uvs[2]}, + {blur_uvs[3], blur_uvs[3]}, }); SamplerDescriptor linear_sampler_descriptor = sampler_descriptor; @@ -183,6 +184,14 @@ fml::StatusOr MakeBlurSubpass( } } +/// Returns `rect` relative to `reference`, where Rect::MakeXYWH(0,0,1,1) will +/// be returned when `rect` == `reference`. +Rect MakeReferenceUVs(const Rect& reference, const Rect& rect) { + Rect result = Rect::MakeOriginSize(rect.GetOrigin() - reference.GetOrigin(), + rect.GetSize()); + return result.Scale(1.0f / Vector2(reference.GetSize())); +} + } // namespace GaussianBlurFilterContents::GaussianBlurFilterContents( @@ -311,6 +320,29 @@ std::optional GaussianBlurFilterContents::RenderFilter( Vector2 pass1_pixel_size = 1.0 / Vector2(pass1_out.value().GetRenderTargetTexture()->GetSize()); + std::optional input_snapshot_coverage = input_snapshot->GetCoverage(); + Quad blur_uvs = {Point(0, 0), Point(1, 0), Point(0, 1), Point(1, 1)}; + if (expanded_coverage_hint.has_value() && + input_snapshot_coverage.has_value() && + // TODO(tbd): Remove this condition. There is some flaw in coverage + // stopping us from using this today. I attempted to use source + // coordinates to calculate the uvs, but that didn't work + // either + // (https://github.com/flutter/engine/pull/49299#issuecomment-1865311438). + input_snapshot.has_value() && + input_snapshot.value().transform.IsTranslationScaleOnly()) { + // Only process the uvs where the blur is happening, not the whole texture. + std::optional uvs = MakeReferenceUVs(input_snapshot_coverage.value(), + expanded_coverage_hint.value()) + .Intersection(Rect::MakeSize(Size(1, 1))); + if (uvs.has_value()) { + blur_uvs[0] = uvs->GetLeftTop(); + blur_uvs[1] = uvs->GetRightTop(); + blur_uvs[2] = uvs->GetLeftBottom(); + blur_uvs[3] = uvs->GetRightBottom(); + } + } + fml::StatusOr pass2_out = MakeBlurSubpass(renderer, /*input_pass=*/pass1_out.value(), input_snapshot->sampler_descriptor, tile_mode_, @@ -320,7 +352,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = blur_radius.y * effective_scalar.y, .step_size = 1.0, }, - /*destination_target=*/std::nullopt); + /*destination_target=*/std::nullopt, blur_uvs); if (!pass2_out.ok()) { return std::nullopt; @@ -341,7 +373,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = blur_radius.x * effective_scalar.x, .step_size = 1.0, }, - pass3_destination); + pass3_destination, blur_uvs); if (!pass3_out.ok()) { return std::nullopt; From 8d92328395d4a95339560bb00924e56d8c685dae Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 2 Jan 2024 15:16:03 -0800 Subject: [PATCH 2/4] added assert --- .../entity/contents/filters/gaussian_blur_filter_contents.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 3687197bd8b12..9848790be5e19 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -335,6 +335,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( std::optional uvs = MakeReferenceUVs(input_snapshot_coverage.value(), expanded_coverage_hint.value()) .Intersection(Rect::MakeSize(Size(1, 1))); + FML_DCHECK(uvs.has_value()); if (uvs.has_value()) { blur_uvs[0] = uvs->GetLeftTop(); blur_uvs[1] = uvs->GetRightTop(); From 4a9b2afed01d6f70b24c60d71570ffffb6cc964f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 2 Jan 2024 15:45:33 -0800 Subject: [PATCH 3/4] brandon feedback --- impeller/aiks/aiks_unittests.cc | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index b7a7e0d8bc2f2..6448b1174f2e8 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -4003,6 +4003,30 @@ TEST_P(AiksTest, GaussianBlurRotatedAndClipped) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, GaussianBlurScaledAndClipped) { + Canvas canvas; + std::shared_ptr boston = CreateTextureForFixture("boston.jpg"); + Rect bounds = + Rect::MakeXYWH(0, 0, boston->GetSize().width, boston->GetSize().height); + Vector2 image_center = Vector2(bounds.GetSize() / 2); + Paint paint = {.image_filter = + ImageFilter::MakeBlur(Sigma(20.0), Sigma(20.0), + FilterContents::BlurStyle::kNormal, + Entity::TileMode::kDecal)}; + Vector2 clip_size = {150, 75}; + Vector2 center = Vector2(1024, 768) / 2; + canvas.Scale(GetContentScale()); + canvas.ClipRect( + Rect::MakeLTRB(center.x, center.y, center.x, center.y).Expand(clip_size)); + canvas.Translate({center.x, center.y, 0}); + canvas.Scale({0.6, 0.6, 1}); + + canvas.DrawImageRect(std::make_shared(boston), /*source=*/bounds, + /*dest=*/bounds.Shift(-image_center), paint); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, GaussianBlurRotatedAndClippedInteractive) { std::shared_ptr boston = CreateTextureForFixture("boston.jpg"); @@ -4013,11 +4037,13 @@ TEST_P(AiksTest, GaussianBlurRotatedAndClippedInteractive) { Entity::TileMode::kMirror, Entity::TileMode::kDecal}; static float rotation = 0; + static float scale = 0.6; static int selected_tile_mode = 3; ImGui::Begin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize); { ImGui::SliderFloat("Rotation (degrees)", &rotation, -180, 180); + ImGui::SliderFloat("Scale", &scale, 0, 2.0); ImGui::Combo("Tile mode", &selected_tile_mode, tile_mode_names, sizeof(tile_mode_names) / sizeof(char*)); } @@ -4038,7 +4064,7 @@ TEST_P(AiksTest, GaussianBlurRotatedAndClippedInteractive) { canvas.ClipRect( Rect::MakeLTRB(handle_a.x, handle_a.y, handle_b.x, handle_b.y)); canvas.Translate({center.x, center.y, 0}); - canvas.Scale({0.6, 0.6, 1}); + canvas.Scale({scale, scale, 1}); canvas.Rotate(Degrees(rotation)); canvas.DrawImageRect(std::make_shared(boston), /*source=*/bounds, From a809f2dcee1f600ad1bae98d1e2a0507175a3f2e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 3 Jan 2024 09:02:42 -0800 Subject: [PATCH 4/4] updated comment --- .../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 9848790be5e19..19aa2e46fa32b 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -324,11 +324,10 @@ std::optional GaussianBlurFilterContents::RenderFilter( Quad blur_uvs = {Point(0, 0), Point(1, 0), Point(0, 1), Point(1, 1)}; if (expanded_coverage_hint.has_value() && input_snapshot_coverage.has_value() && - // TODO(tbd): Remove this condition. There is some flaw in coverage - // stopping us from using this today. I attempted to use source - // coordinates to calculate the uvs, but that didn't work - // either - // (https://github.com/flutter/engine/pull/49299#issuecomment-1865311438). + // TODO(https://github.com/flutter/flutter/issues/140890): Remove this + // condition. There is some flaw in coverage stopping us from using this + // today. I attempted to use source coordinates to calculate the uvs, + // but that didn't work either. input_snapshot.has_value() && input_snapshot.value().transform.IsTranslationScaleOnly()) { // Only process the uvs where the blur is happening, not the whole texture.