From 8a98a0be3386f6a757e23cf280249c7e0537da2d Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Fri, 2 Feb 2024 12:44:54 -0800 Subject: [PATCH] [Impeller] blur: removed ability to request out of bounds mip_counts (flutter/engine#50290) b/323402168 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../flutter/impeller/aiks/aiks_unittests.cc | 56 +++++++++++++++++++ engine/src/flutter/impeller/core/allocator.cc | 8 +++ .../impeller/entity/contents/contents.cc | 6 +- 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/impeller/aiks/aiks_unittests.cc b/engine/src/flutter/impeller/aiks/aiks_unittests.cc index ca5ba3cf37..c8c5a47bfa 100644 --- a/engine/src/flutter/impeller/aiks/aiks_unittests.cc +++ b/engine/src/flutter/impeller/aiks/aiks_unittests.cc @@ -3968,5 +3968,61 @@ TEST_P(AiksTest, CorrectClipDepthAssignedToEntities) { } } +// This addresses a bug where tiny blurs could result in mip maps that beyond +// the limits for the textures used for blurring. +// See also: b/323402168 +TEST_P(AiksTest, GaussianBlurSolidColorTinyMipMap) { + for (int32_t i = 1; i < 5; ++i) { + Canvas canvas; + Scalar fi = i; + canvas.DrawPath( + PathBuilder{} + .MoveTo({100, 100}) + .LineTo({100.f + fi, 100.f + fi}) + .TakePath(), + {.color = Color::Chartreuse(), + .image_filter = ImageFilter::MakeBlur( + Sigma(0.1), Sigma(0.1), FilterContents::BlurStyle::kNormal, + Entity::TileMode::kClamp)}); + + Picture picture = canvas.EndRecordingAsPicture(); + std::shared_ptr cache = + std::make_shared( + GetContext()->GetResourceAllocator()); + AiksContext aiks_context(GetContext(), nullptr, cache); + std::shared_ptr image = picture.ToImage(aiks_context, {1024, 768}); + EXPECT_TRUE(image) << " length " << i; + } +} + +// This addresses a bug where tiny blurs could result in mip maps that beyond +// the limits for the textures used for blurring. +// See also: b/323402168 +TEST_P(AiksTest, GaussianBlurBackdropTinyMipMap) { + for (int32_t i = 0; i < 5; ++i) { + Canvas canvas; + ISize clip_size = ISize(i, i); + canvas.ClipRect( + Rect::MakeXYWH(400, 400, clip_size.width, clip_size.height)); + canvas.DrawCircle( + {400, 400}, 200, + { + .color = Color::Green(), + .image_filter = ImageFilter::MakeBlur( + Sigma(0.1), Sigma(0.1), FilterContents::BlurStyle::kNormal, + Entity::TileMode::kDecal), + }); + canvas.Restore(); + + Picture picture = canvas.EndRecordingAsPicture(); + std::shared_ptr cache = + std::make_shared( + GetContext()->GetResourceAllocator()); + AiksContext aiks_context(GetContext(), nullptr, cache); + std::shared_ptr image = picture.ToImage(aiks_context, {1024, 768}); + EXPECT_TRUE(image) << " clip rect " << i; + } +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/core/allocator.cc b/engine/src/flutter/impeller/core/allocator.cc index d9cedbe9ee..19314c5bfc 100644 --- a/engine/src/flutter/impeller/core/allocator.cc +++ b/engine/src/flutter/impeller/core/allocator.cc @@ -55,6 +55,14 @@ std::shared_ptr Allocator::CreateTexture( return nullptr; } + if (desc.mip_count > desc.size.MipCount()) { + VALIDATION_LOG << "Requested mip_count " << desc.mip_count + << " exceeds maximum supported for size " << desc.size; + TextureDescriptor corrected_desc = desc; + corrected_desc.mip_count = desc.size.MipCount(); + return OnCreateTexture(corrected_desc); + } + return OnCreateTexture(desc); } diff --git a/engine/src/flutter/impeller/entity/contents/contents.cc b/engine/src/flutter/impeller/entity/contents/contents.cc index bbef8298ce..b8bfcc3e72 100644 --- a/engine/src/flutter/impeller/entity/contents/contents.cc +++ b/engine/src/flutter/impeller/entity/contents/contents.cc @@ -80,8 +80,9 @@ std::optional Contents::RenderToSnapshot( } } + ISize subpass_size = ISize::Ceil(coverage->GetSize()); fml::StatusOr render_target = renderer.MakeSubpass( - label, ISize::Ceil(coverage->GetSize()), + label, subpass_size, [&contents = *this, &entity, &coverage](const ContentContext& renderer, RenderPass& pass) -> bool { Entity sub_entity; @@ -91,7 +92,8 @@ std::optional Contents::RenderToSnapshot( entity.GetTransform()); return contents.Render(renderer, sub_entity, pass); }, - msaa_enabled, mip_count); + msaa_enabled, + std::min(mip_count, static_cast(subpass_size.MipCount()))); if (!render_target.ok()) { return std::nullopt;