From b803359ce96ee4a705c04a56e0e90d4e946e1805 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 Jan 2024 11:19:08 -0800 Subject: [PATCH 01/28] [Impeller] new blur: added mipmap generation --- .../contents/filters/gaussian_blur_filter_contents.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index c83f0e0d79647..cc8aa2d529231 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -284,6 +284,17 @@ std::optional GaussianBlurFilterContents::RenderFilter( entity.GetClipDepth()); // No blur to render. } + if (!input_snapshot.value().texture->NeedsMipmapGeneration() && + input_snapshot.value().texture->GetMipCount() <= 1) { + std::shared_ptr mip_cmd_buffer = + renderer.GetContext()->CreateCommandBuffer(); + std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); + blit_pass->GenerateMipmap(input_snapshot.value().texture); + blit_pass->EncodeCommands(renderer.GetContext()->GetResourceAllocator()); + bool success = mip_cmd_buffer->SubmitCommands(/*callback=*/nullptr); + FML_CHECK(success); + } + Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); // TODO(jonahwilliams): If desired_scalar is 1.0 and we fully acquired the From 6f20569e8cd2afee701de6907bd55e72a7f401f7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 Jan 2024 13:31:16 -0800 Subject: [PATCH 02/28] added lod parameter --- .../contents/filters/gaussian_blur_filter_contents.cc | 2 ++ .../contents/filters/gaussian_blur_filter_contents.h | 1 + .../filters/gaussian_blur_filter_contents_unittests.cc | 5 ++++- impeller/entity/shaders/gaussian_blur/kernel.glsl | 8 +++++--- 4 files changed, 12 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 cc8aa2d529231..ba41c985ea0be 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -364,6 +364,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = static_cast(std::round(blur_radius.y * effective_scalar.y)), .step_size = 1, + .lod = -1, }, /*destination_target=*/std::nullopt, blur_uvs); @@ -446,6 +447,7 @@ Scalar GaussianBlurFilterContents::ScaleSigma(Scalar sigma) { KernelPipeline::FragmentShader::KernelSamples GenerateBlurInfo( BlurParameters parameters) { KernelPipeline::FragmentShader::KernelSamples result; + result.lod = parameters.lod; result.sample_count = ((2 * parameters.blur_radius) / parameters.step_size) + 1; // 32 comes from kernel.glsl. diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index 1bb594f3e4ae6..fdcd9ba3a6143 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -16,6 +16,7 @@ struct BlurParameters { Scalar blur_sigma; int blur_radius; int step_size; + Scalar lod; }; KernelPipeline::FragmentShader::KernelSamples GenerateBlurInfo( 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 ba3053483181f..c2dddf4052c07 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -462,7 +462,8 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) { BlurParameters parameters = {.blur_uv_offset = Point(1, 0), .blur_sigma = 1, .blur_radius = 5, - .step_size = 1}; + .step_size = 1, + .lod = -1.f}; KernelPipeline::FragmentShader::KernelSamples samples = GenerateBlurInfo(parameters); EXPECT_EQ(samples.sample_count, 9); @@ -481,6 +482,8 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) { EXPECT_TRUE(samples.samples[i + 1].coefficient > samples.samples[i].coefficient); } + + EXPECT_FLOAT_EQ(samples.lod, -1.f); } } // namespace testing diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index d8a544bf070f6..1b0ca6ae174a7 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -17,14 +17,15 @@ struct KernelSample { uniform KernelSamples { int sample_count; KernelSample samples[32]; + float lod; } blur_info; -f16vec4 Sample(f16sampler2D tex, vec2 coords) { +f16vec4 Sample(f16sampler2D tex, vec2 coords, float lod) { #if ENABLE_DECAL_SPECIALIZATION return IPHalfSampleDecal(tex, coords); #else - return texture(tex, coords); + return texture(tex, coords, lod); #endif } @@ -39,7 +40,8 @@ void main() { float16_t coefficient = float16_t(blur_info.samples[i].coefficient); total_color += coefficient * Sample(texture_sampler, - v_texture_coords + blur_info.samples[i].uv_offset); + v_texture_coords + blur_info.samples[i].uv_offset, + blur_info.lod); } frag_color = total_color; From 92859a26e4c0f8d8f27aae810e8b48c61ad92231 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 Jan 2024 13:45:44 -0800 Subject: [PATCH 03/28] started sending the correct lod --- .../contents/filters/gaussian_blur_filter_contents.cc | 7 ++++++- 1 file changed, 6 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 ba41c985ea0be..9575937997cff 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -193,6 +193,9 @@ Rect MakeReferenceUVs(const Rect& reference, const Rect& rect) { return result.Scale(1.0f / Vector2(reference.GetSize())); } +Scalar CalculateLOD(Scalar scalar) { + return fabsf(log2f(1.f / scalar)); +} } // namespace GaussianBlurFilterContents::GaussianBlurFilterContents( @@ -297,6 +300,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); + Scalar lod = CalculateLOD(desired_scalar); // 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. @@ -364,7 +368,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = static_cast(std::round(blur_radius.y * effective_scalar.y)), .step_size = 1, - .lod = -1, + .lod = lod, }, /*destination_target=*/std::nullopt, blur_uvs); @@ -387,6 +391,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = static_cast(std::round(blur_radius.x * effective_scalar.x)), .step_size = 1, + .lod = lod, }, pass3_destination, blur_uvs); From 48522bb7ea580138574cb3d64b9e269bfab0524b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 Jan 2024 13:51:30 -0800 Subject: [PATCH 04/28] textureLod --- impeller/entity/shaders/gaussian_blur/kernel.glsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index 1b0ca6ae174a7..7a6ff6f376d56 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -25,7 +25,7 @@ f16vec4 Sample(f16sampler2D tex, vec2 coords, float lod) { #if ENABLE_DECAL_SPECIALIZATION return IPHalfSampleDecal(tex, coords); #else - return texture(tex, coords, lod); + return textureLod(tex, coords, lod); #endif } From 7b248f79642dd1f29dd78e293f8e6154b0817b86 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 8 Jan 2024 16:14:25 -0800 Subject: [PATCH 05/28] hack to set mip level to 4 --- impeller/aiks/picture.cc | 1 + .../checkerboard_contents_unittests.cc | 3 ++- impeller/entity/contents/content_context.cc | 2 +- .../filters/gaussian_blur_filter_contents.cc | 3 +-- .../tiled_texture_contents_unittests.cc | 3 ++- .../contents/vertices_contents_unittests.cc | 3 ++- impeller/entity/entity_pass.cc | 24 +++++++++++-------- .../entity/entity_pass_target_unittests.cc | 3 ++- impeller/renderer/render_target.cc | 2 ++ impeller/renderer/render_target.h | 1 + 10 files changed, 28 insertions(+), 17 deletions(-) diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index ea07dfd27a58c..bd32f9068f895 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -64,6 +64,7 @@ std::shared_ptr Picture::RenderToTexture( *impeller_context, // context render_target_allocator, // allocator size, // size + /*mip_count=*/1, "Picture Snapshot MSAA", // label RenderTarget:: kDefaultColorAttachmentConfigMSAA, // color_attachment_config diff --git a/impeller/entity/contents/checkerboard_contents_unittests.cc b/impeller/entity/contents/checkerboard_contents_unittests.cc index 46328620026b6..63a2cbdd3aa07 100644 --- a/impeller/entity/contents/checkerboard_contents_unittests.cc +++ b/impeller/entity/contents/checkerboard_contents_unittests.cc @@ -35,7 +35,8 @@ TEST_P(EntityTest, RendersWithoutError) { auto buffer = content_context->GetContext()->CreateCommandBuffer(); auto render_target = RenderTarget::CreateOffscreenMSAA( *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}); + *GetContentContext()->GetRenderTargetCache(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); Entity entity; diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 634ae0b8e6fa7..6cd649eaa582d 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -418,7 +418,7 @@ fml::StatusOr ContentContext::MakeSubpass( RenderTarget subpass_target; if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) { subpass_target = RenderTarget::CreateOffscreenMSAA( - *context, *GetRenderTargetCache(), texture_size, + *context, *GetRenderTargetCache(), texture_size, /*mip_count=*/1, SPrintF("%s Offscreen", label.c_str()), RenderTarget::kDefaultColorAttachmentConfigMSAA, std::nullopt // stencil_attachment_config diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 9575937997cff..5ac0ec0035142 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -287,8 +287,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( entity.GetClipDepth()); // No blur to render. } - if (!input_snapshot.value().texture->NeedsMipmapGeneration() && - input_snapshot.value().texture->GetMipCount() <= 1) { + if (input_snapshot.value().texture->NeedsMipmapGeneration()) { std::shared_ptr mip_cmd_buffer = renderer.GetContext()->CreateCommandBuffer(); std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 2060f23437c2f..8fc5fecd99fb7 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -33,7 +33,8 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { auto buffer = content_context->GetContext()->CreateCommandBuffer(); auto render_target = RenderTarget::CreateOffscreenMSAA( *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}); + *GetContentContext()->GetRenderTargetCache(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *render_pass)); diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc index 173b13ea4bcfb..17fac72d722dc 100644 --- a/impeller/entity/contents/vertices_contents_unittests.cc +++ b/impeller/entity/contents/vertices_contents_unittests.cc @@ -61,7 +61,8 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) { auto buffer = content_context->GetContext()->CreateCommandBuffer(); auto render_target = RenderTarget::CreateOffscreenMSAA( *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}); + *GetContentContext()->GetRenderTargetCache(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); Entity entity; diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 67e343de8a6d5..aad8bd396a883 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -247,6 +247,7 @@ static const constexpr RenderTarget::AttachmentConfig kDefaultStencilConfig = static EntityPassTarget CreateRenderTarget(ContentContext& renderer, ISize size, + int mip_count, const Color& clear_color) { auto context = renderer.GetContext(); @@ -258,18 +259,20 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, RenderTarget target; if (context->GetCapabilities()->SupportsOffscreenMSAA()) { target = RenderTarget::CreateOffscreenMSAA( - *context, // context - *renderer.GetRenderTargetCache(), // allocator - size, // size - "EntityPass", // label + /*context=*/*context, + /*allocator=*/*renderer.GetRenderTargetCache(), + /*size=*/size, + /*mip_count=*/mip_count, + /*label=*/"EntityPass", + /*color_attachment_config=*/ RenderTarget::AttachmentConfigMSAA{ .storage_mode = StorageMode::kDeviceTransient, .resolve_storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kDontCare, .store_action = StoreAction::kMultisampleResolve, - .clear_color = clear_color}, // color_attachment_config - kDefaultStencilConfig // stencil_attachment_config - ); + .clear_color = clear_color}, + /*stencil_attachment_config=*/ + kDefaultStencilConfig); } else { target = RenderTarget::CreateOffscreen( *context, // context @@ -339,7 +342,7 @@ bool EntityPass::Render(ContentContext& renderer, // there's no need to set up a stencil attachment on the root render target. if (reads_from_onscreen_backdrop) { auto offscreen_target = CreateRenderTarget( - renderer, root_render_target.GetRenderTargetSize(), + renderer, root_render_target.GetRenderTargetSize(), /*mip_count=*/4, GetClearColorOrDefault(render_target.GetRenderTargetSize())); if (!OnRender(renderer, // renderer @@ -599,8 +602,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( } auto subpass_target = CreateRenderTarget( - renderer, // renderer - subpass_size, // size + renderer, // renderer + subpass_size, // size + /*mip_count=*/1, subpass->GetClearColorOrDefault(subpass_size)); // clear_color if (!subpass_target.IsValid()) { diff --git a/impeller/entity/entity_pass_target_unittests.cc b/impeller/entity/entity_pass_target_unittests.cc index 390f7929913f7..108da36230506 100644 --- a/impeller/entity/entity_pass_target_unittests.cc +++ b/impeller/entity/entity_pass_target_unittests.cc @@ -26,7 +26,8 @@ TEST_P(EntityPassTargetTest, SwapWithMSAATexture) { auto buffer = content_context->GetContext()->CreateCommandBuffer(); auto render_target = RenderTarget::CreateOffscreenMSAA( *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}); + *GetContentContext()->GetRenderTargetCache(), {100, 100}, + /*mip_count=*/1); auto entity_pass_target = EntityPassTarget(render_target, false, false); diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 3a9f1e0468e81..f2aca7f44d5d2 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -266,6 +266,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( const Context& context, RenderTargetAllocator& allocator, ISize size, + int mip_count, const std::string& label, AttachmentConfigMSAA color_attachment_config, std::optional stencil_attachment_config) { @@ -310,6 +311,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_resolve_tex_desc.usage = static_cast(TextureUsage::kRenderTarget) | static_cast(TextureUsage::kShaderRead); + color0_resolve_tex_desc.mip_count = mip_count; auto color0_resolve_tex = allocator.CreateTexture(color0_resolve_tex_desc); if (!color0_resolve_tex) { diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index dbd115a5c7612..388cfb7029fa8 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -95,6 +95,7 @@ class RenderTarget final { const Context& context, RenderTargetAllocator& allocator, ISize size, + int mip_count, const std::string& label = "Offscreen MSAA", AttachmentConfigMSAA color_attachment_config = kDefaultColorAttachmentConfigMSAA, From cd3136d5f6d0d97a857e3e631c13cfa6863182ba Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 9 Jan 2024 10:10:18 -0800 Subject: [PATCH 06/28] started invalidating mipmaps --- impeller/core/texture.cc | 4 ++++ impeller/core/texture.h | 2 ++ impeller/entity/contents/filters/filter_contents.cc | 2 ++ .../contents/filters/gaussian_blur_filter_contents.cc | 1 + impeller/entity/entity_pass.cc | 7 +++++++ impeller/renderer/backend/metal/texture_mtl.mm | 2 ++ impeller/renderer/render_target.cc | 4 ++++ 7 files changed, 22 insertions(+) diff --git a/impeller/core/texture.cc b/impeller/core/texture.cc index bb2f125aea503..f8ba083783700 100644 --- a/impeller/core/texture.cc +++ b/impeller/core/texture.cc @@ -86,4 +86,8 @@ bool Texture::NeedsMipmapGeneration() const { return !mipmap_generated_ && desc_.mip_count > 1; } +void Texture::InvalidateMipmap() { + mipmap_generated_ = false; +} + } // namespace impeller diff --git a/impeller/core/texture.h b/impeller/core/texture.h index b28dbb27ab5f3..d0bcc40432062 100644 --- a/impeller/core/texture.h +++ b/impeller/core/texture.h @@ -47,6 +47,8 @@ class Texture { bool NeedsMipmapGeneration() const; + void InvalidateMipmap(); + protected: explicit Texture(TextureDescriptor desc); 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 5ac0ec0035142..66721ac1ee594 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -288,6 +288,7 @@ std::optional GaussianBlurFilterContents::RenderFilter( } if (input_snapshot.value().texture->NeedsMipmapGeneration()) { + FML_LOG(ERROR) << "should generate mipmap"; std::shared_ptr mip_cmd_buffer = renderer.GetContext()->CreateCommandBuffer(); std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index aad8bd396a883..c74ee11a6ee0c 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -345,6 +345,13 @@ bool EntityPass::Render(ContentContext& renderer, renderer, root_render_target.GetRenderTargetSize(), /*mip_count=*/4, GetClearColorOrDefault(render_target.GetRenderTargetSize())); + offscreen_target.GetRenderTarget() + .GetRenderTargetTexture() + ->InvalidateMipmap(); + FML_LOG(ERROR) + << "invalidating mips: " + << offscreen_target.GetRenderTarget().GetRenderTargetTexture(); + if (!OnRender(renderer, // renderer capture, // capture offscreen_target.GetRenderTarget() diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index d65937e8b9f09..e37defc847fb9 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -139,6 +139,8 @@ new TextureMTL( return false; } + FML_LOG(ERROR) << "mipmap generated"; + [encoder generateMipmapsForTexture:texture]; mipmap_generated_ = true; diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index f2aca7f44d5d2..4d6500fcee036 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -314,6 +314,10 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_resolve_tex_desc.mip_count = mip_count; auto color0_resolve_tex = allocator.CreateTexture(color0_resolve_tex_desc); + if (mip_count > 1) { + FML_LOG(ERROR) << "created mip resolve texture: " << color0_resolve_tex + << " " << mip_count; + } if (!color0_resolve_tex) { VALIDATION_LOG << "Could not create color texture."; return {}; From 8bf8bb9df00f9a2757d4df1c7e86be3a39813727 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 10 Jan 2024 11:06:21 -0800 Subject: [PATCH 07/28] Added texture_mipmap --- .../filters/gaussian_blur_filter_contents.cc | 14 +++----- impeller/renderer/BUILD.gn | 2 ++ impeller/renderer/texture_mipmap.cc | 34 +++++++++++++++++++ impeller/renderer/texture_mipmap.h | 21 ++++++++++++ 4 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 impeller/renderer/texture_mipmap.cc create mode 100644 impeller/renderer/texture_mipmap.h diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 66721ac1ee594..674923520fd97 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -11,6 +11,7 @@ #include "impeller/entity/texture_fill.vert.h" #include "impeller/renderer/command.h" #include "impeller/renderer/render_pass.h" +#include "impeller/renderer/texture_mipmap.h" #include "impeller/renderer/vertex_buffer_builder.h" namespace impeller { @@ -287,16 +288,9 @@ std::optional GaussianBlurFilterContents::RenderFilter( entity.GetClipDepth()); // No blur to render. } - if (input_snapshot.value().texture->NeedsMipmapGeneration()) { - FML_LOG(ERROR) << "should generate mipmap"; - std::shared_ptr mip_cmd_buffer = - renderer.GetContext()->CreateCommandBuffer(); - std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); - blit_pass->GenerateMipmap(input_snapshot.value().texture); - blit_pass->EncodeCommands(renderer.GetContext()->GetResourceAllocator()); - bool success = mip_cmd_buffer->SubmitCommands(/*callback=*/nullptr); - FML_CHECK(success); - } + fml::Status mipmap_generation = + AddMipmapGeneration(renderer.GetContext(), input_snapshot->texture); + FML_CHECK(mipmap_generation.ok()); Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); diff --git a/impeller/renderer/BUILD.gn b/impeller/renderer/BUILD.gn index 6b17817b849f3..3e0c7b5770420 100644 --- a/impeller/renderer/BUILD.gn +++ b/impeller/renderer/BUILD.gn @@ -92,6 +92,8 @@ impeller_component("renderer") { "snapshot.h", "surface.cc", "surface.h", + "texture_mipmap.cc", + "texture_mipmap.h", "vertex_buffer_builder.cc", "vertex_buffer_builder.h", "vertex_descriptor.cc", diff --git a/impeller/renderer/texture_mipmap.cc b/impeller/renderer/texture_mipmap.cc new file mode 100644 index 0000000000000..ac020644c293a --- /dev/null +++ b/impeller/renderer/texture_mipmap.cc @@ -0,0 +1,34 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/renderer/texture_mipmap.h" +#include "impeller/renderer/blit_pass.h" +#include "impeller/renderer/command_buffer.h" + + +namespace impeller { + +fml::Status AddMipmapGeneration(const std::shared_ptr& context, + const std::shared_ptr& texture) { + if (texture->NeedsMipmapGeneration()) { + std::shared_ptr mip_cmd_buffer = + context->CreateCommandBuffer(); + std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); + bool success = blit_pass->GenerateMipmap(texture); + if (!success) { + return fml::Status(fml::StatusCode::kUnknown, ""); + } + success = blit_pass->EncodeCommands(context->GetResourceAllocator()); + if (!success) { + return fml::Status(fml::StatusCode::kUnknown, ""); + } + success = mip_cmd_buffer->SubmitCommands(/*callback=*/nullptr); + if (!success) { + return fml::Status(fml::StatusCode::kUnknown, ""); + } + } + return {}; +} + +} // namespace impeller diff --git a/impeller/renderer/texture_mipmap.h b/impeller/renderer/texture_mipmap.h new file mode 100644 index 0000000000000..f2f7d8cbb46d5 --- /dev/null +++ b/impeller/renderer/texture_mipmap.h @@ -0,0 +1,21 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_IMPELLER_TEXTURE_MIPMAP_H_ +#define FLUTTER_IMPELLER_TEXTURE_MIPMAP_H_ + +#include "flutter/fml/status.h" +#include "impeller/core/texture.h" +#include "impeller/renderer/context.h" + +namespace impeller { + +/// Adds a blit command to the render pass. +[[nodiscard]] fml::Status AddMipmapGeneration( + const std::shared_ptr& context, + const std::shared_ptr& texture); + +} // namespace impeller + +#endif // FLUTTER_IMPELLER_TEXTURE_MIPMAP_H_ From 97f5799425c6400dd35811e332a8cab0c916fe90 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 10 Jan 2024 12:55:22 -0800 Subject: [PATCH 08/28] moved mipmap generation to the spots identified by brandon --- .../filters/gaussian_blur_filter_contents.cc | 8 +++++--- impeller/entity/entity_pass.cc | 20 +++++++++++++++---- 2 files changed, 21 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 674923520fd97..2f48013dd82b2 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -288,9 +288,11 @@ std::optional GaussianBlurFilterContents::RenderFilter( entity.GetClipDepth()); // No blur to render. } - fml::Status mipmap_generation = - AddMipmapGeneration(renderer.GetContext(), input_snapshot->texture); - FML_CHECK(mipmap_generation.ok()); + // In order to avoid shimmering in downsampling step, we should have mips. + if (input_snapshot->texture->GetMipCount() <= 1) { + FML_DLOG(ERROR) << "Applying gaussian blur without mipmap."; + } + FML_DCHECK(!input_snapshot->texture->NeedsMipmapGeneration()); Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index c74ee11a6ee0c..e2db028a71d34 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -25,6 +25,7 @@ #include "impeller/geometry/color.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/command_buffer.h" +#include "impeller/renderer/texture_mipmap.h" #ifdef IMPELLER_DEBUG #include "impeller/entity/contents/checkerboard_contents.h" @@ -341,16 +342,15 @@ bool EntityPass::Render(ContentContext& renderer, // and then blit the results onto the onscreen texture. If using this branch, // there's no need to set up a stencil attachment on the root render target. if (reads_from_onscreen_backdrop) { - auto offscreen_target = CreateRenderTarget( + EntityPassTarget offscreen_target = CreateRenderTarget( renderer, root_render_target.GetRenderTargetSize(), /*mip_count=*/4, GetClearColorOrDefault(render_target.GetRenderTargetSize())); + // If the render target texture is recycled, make sure the mipmaps are + // regenerated. offscreen_target.GetRenderTarget() .GetRenderTargetTexture() ->InvalidateMipmap(); - FML_LOG(ERROR) - << "invalidating mips: " - << offscreen_target.GetRenderTarget().GetRenderTargetTexture(); if (!OnRender(renderer, // renderer capture, // capture @@ -558,6 +558,12 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( // rendering the backdrop, so if there's an active pass, end it prior to // rendering the subpass. pass_context.EndPass(); + + fml::Status mip_status = + AddMipmapGeneration(renderer.GetContext(), pass_context.GetTexture()); + if (!mip_status.ok()) { + return EntityPass::EntityResult::Failure(); + } } if (clip_coverage_stack.empty()) { @@ -654,6 +660,12 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( auto subpass_texture = subpass_target.GetRenderTarget().GetRenderTargetTexture(); + fml::Status mip_status = + AddMipmapGeneration(renderer.GetContext(), subpass_texture); + if (!mip_status.ok()) { + return EntityPass::EntityResult::Failure(); + } + auto offscreen_texture_contents = subpass->delegate_->CreateContentsForSubpassTarget( subpass_texture, From 80813fe21bbc7a63b3f3cae67422939968c24d1a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 10 Jan 2024 14:42:40 -0800 Subject: [PATCH 09/28] moved mipmap generation to the inline pass context --- impeller/entity/entity_pass.cc | 13 ------------- impeller/entity/inline_pass_context.cc | 11 +++++++++++ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index e2db028a71d34..4311566f027c1 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -25,7 +25,6 @@ #include "impeller/geometry/color.h" #include "impeller/geometry/rect.h" #include "impeller/renderer/command_buffer.h" -#include "impeller/renderer/texture_mipmap.h" #ifdef IMPELLER_DEBUG #include "impeller/entity/contents/checkerboard_contents.h" @@ -558,12 +557,6 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( // rendering the backdrop, so if there's an active pass, end it prior to // rendering the subpass. pass_context.EndPass(); - - fml::Status mip_status = - AddMipmapGeneration(renderer.GetContext(), pass_context.GetTexture()); - if (!mip_status.ok()) { - return EntityPass::EntityResult::Failure(); - } } if (clip_coverage_stack.empty()) { @@ -660,12 +653,6 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( auto subpass_texture = subpass_target.GetRenderTarget().GetRenderTargetTexture(); - fml::Status mip_status = - AddMipmapGeneration(renderer.GetContext(), subpass_texture); - if (!mip_status.ok()) { - return EntityPass::EntityResult::Failure(); - } - auto offscreen_texture_contents = subpass->delegate_->CreateContentsForSubpassTarget( subpass_texture, diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index b8295c2cb444a..20ffc7190cd86 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -7,10 +7,12 @@ #include #include "impeller/base/allocation.h" +#include "flutter/fml/status.h" #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/entity/entity_pass_target.h" #include "impeller/renderer/command_buffer.h" +#include "impeller/renderer/texture_mipmap.h" namespace impeller { @@ -64,6 +66,15 @@ bool InlinePassContext::EndPass() { } } + std::shared_ptr target_texture = + GetPassTarget().GetRenderTarget().GetRenderTargetTexture(); + if (target_texture->NeedsMipmapGeneration()) { + fml::Status mip_status = AddMipmapGeneration(context_, target_texture); + if (!mip_status.ok()) { + return false; + } + } + pass_ = nullptr; command_buffer_ = nullptr; From 2b5790e28047c20de6998aef478891420ea5fc01 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 10 Jan 2024 14:44:02 -0800 Subject: [PATCH 10/28] format --- impeller/renderer/texture_mipmap.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/impeller/renderer/texture_mipmap.cc b/impeller/renderer/texture_mipmap.cc index ac020644c293a..823b80d662b25 100644 --- a/impeller/renderer/texture_mipmap.cc +++ b/impeller/renderer/texture_mipmap.cc @@ -6,7 +6,6 @@ #include "impeller/renderer/blit_pass.h" #include "impeller/renderer/command_buffer.h" - namespace impeller { fml::Status AddMipmapGeneration(const std::shared_ptr& context, From 7204e7cebc78b51f67ee20762cbb296cca7b62c8 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 10 Jan 2024 14:51:42 -0800 Subject: [PATCH 11/28] removed logs --- impeller/renderer/backend/metal/texture_mtl.mm | 2 -- impeller/renderer/render_target.cc | 4 ---- 2 files changed, 6 deletions(-) diff --git a/impeller/renderer/backend/metal/texture_mtl.mm b/impeller/renderer/backend/metal/texture_mtl.mm index e37defc847fb9..d65937e8b9f09 100644 --- a/impeller/renderer/backend/metal/texture_mtl.mm +++ b/impeller/renderer/backend/metal/texture_mtl.mm @@ -139,8 +139,6 @@ new TextureMTL( return false; } - FML_LOG(ERROR) << "mipmap generated"; - [encoder generateMipmapsForTexture:texture]; mipmap_generated_ = true; diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 4d6500fcee036..f2aca7f44d5d2 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -314,10 +314,6 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_resolve_tex_desc.mip_count = mip_count; auto color0_resolve_tex = allocator.CreateTexture(color0_resolve_tex_desc); - if (mip_count > 1) { - FML_LOG(ERROR) << "created mip resolve texture: " << color0_resolve_tex - << " " << mip_count; - } if (!color0_resolve_tex) { VALIDATION_LOG << "Could not create color texture."; return {}; From afdc277229c337f700b594e8f0b79804416759c2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 09:21:45 -0800 Subject: [PATCH 12/28] Removed invalidation and updated the docstring on NeedsMipmapGeneration --- impeller/core/texture.cc | 4 ---- impeller/core/texture.h | 5 +++-- impeller/entity/entity_pass.cc | 6 ------ 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/impeller/core/texture.cc b/impeller/core/texture.cc index f8ba083783700..bb2f125aea503 100644 --- a/impeller/core/texture.cc +++ b/impeller/core/texture.cc @@ -86,8 +86,4 @@ bool Texture::NeedsMipmapGeneration() const { return !mipmap_generated_ && desc_.mip_count > 1; } -void Texture::InvalidateMipmap() { - mipmap_generated_ = false; -} - } // namespace impeller diff --git a/impeller/core/texture.h b/impeller/core/texture.h index d0bcc40432062..af14a9e9331c9 100644 --- a/impeller/core/texture.h +++ b/impeller/core/texture.h @@ -45,10 +45,11 @@ class Texture { virtual Scalar GetYCoordScale() const; + /// Returns true if mipmaps have never been generated. + /// The contents of the mipmap may be out of date if the root texture has been + /// modified and the mipmaps hasn't been regenerated. bool NeedsMipmapGeneration() const; - void InvalidateMipmap(); - protected: explicit Texture(TextureDescriptor desc); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 4311566f027c1..789c5615a5fab 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -345,12 +345,6 @@ bool EntityPass::Render(ContentContext& renderer, renderer, root_render_target.GetRenderTargetSize(), /*mip_count=*/4, GetClearColorOrDefault(render_target.GetRenderTargetSize())); - // If the render target texture is recycled, make sure the mipmaps are - // regenerated. - offscreen_target.GetRenderTarget() - .GetRenderTargetTexture() - ->InvalidateMipmap(); - if (!OnRender(renderer, // renderer capture, // capture offscreen_target.GetRenderTarget() From 6aa39c3b9a2a73d8861f866de208a2eaa126e7a4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 10:20:30 -0800 Subject: [PATCH 13/28] brought over the mip_count parameter to non msaa paths --- impeller/aiks/picture.cc | 7 ++++--- impeller/entity/contents/content_context.cc | 2 +- .../entity/contents/tiled_texture_contents_unittests.cc | 3 ++- impeller/entity/entity_pass.cc | 3 ++- impeller/entity/entity_unittests.cc | 5 +++-- impeller/entity/inline_pass_context.cc | 2 +- impeller/renderer/render_target.cc | 2 ++ impeller/renderer/render_target.h | 1 + impeller/renderer/renderer_unittests.cc | 8 ++++---- 9 files changed, 20 insertions(+), 13 deletions(-) diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index bd32f9068f895..8fc31dc31f45b 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -72,9 +72,10 @@ std::shared_ptr Picture::RenderToTexture( ); } else { target = RenderTarget::CreateOffscreen( - *impeller_context, // context - render_target_allocator, // allocator - size, // size + *impeller_context, // context + render_target_allocator, // allocator + size, // size + /*mip_count=*/1, "Picture Snapshot", // label RenderTarget::kDefaultColorAttachmentConfig, // color_attachment_config std::nullopt // stencil_attachment_config diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 6cd649eaa582d..7680481181b25 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -425,7 +425,7 @@ fml::StatusOr ContentContext::MakeSubpass( ); } else { subpass_target = RenderTarget::CreateOffscreen( - *context, *GetRenderTargetCache(), texture_size, + *context, *GetRenderTargetCache(), texture_size, /*mip_count=*/1, SPrintF("%s Offscreen", label.c_str()), RenderTarget::kDefaultColorAttachmentConfig, // std::nullopt // stencil_attachment_config diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 8fc5fecd99fb7..4715eaad79133 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -69,7 +69,8 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipelineExternalOES) { auto buffer = content_context->GetContext()->CreateCommandBuffer(); auto render_target = RenderTarget::CreateOffscreenMSAA( *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}); + *GetContentContext()->GetRenderTargetCache(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *render_pass)); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 789c5615a5fab..5d98e94cf7d63 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -278,7 +278,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, *context, // context *renderer.GetRenderTargetCache(), // allocator size, // size - "EntityPass", // label + /*mip_count=*/mip_count, + "EntityPass", // label RenderTarget::AttachmentConfig{ .storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kDontCare, diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 520d2ee9ee3db..a24e8e2b59931 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2511,8 +2511,9 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) { .store_action = StoreAction::kDontCare, .clear_color = Color::BlackTransparent()}; auto rt = RenderTarget::CreateOffscreen( - *GetContext(), *test_allocator, ISize::MakeWH(1000, 1000), "Offscreen", - RenderTarget::kDefaultColorAttachmentConfig, stencil_config); + *GetContext(), *test_allocator, ISize::MakeWH(1000, 1000), + /*mip_count=*/1, "Offscreen", RenderTarget::kDefaultColorAttachmentConfig, + stencil_config); auto content_context = ContentContext( GetContext(), TypographerContextSkia::Make(), test_allocator); pass->AddEntity(std::move(entity)); diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 20ffc7190cd86..73aeff1735c7a 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -6,8 +6,8 @@ #include -#include "impeller/base/allocation.h" #include "flutter/fml/status.h" +#include "impeller/base/allocation.h" #include "impeller/base/validation.h" #include "impeller/core/formats.h" #include "impeller/entity/entity_pass_target.h" diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index f2aca7f44d5d2..d4a1e3c6cf0bb 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -224,6 +224,7 @@ RenderTarget RenderTarget::CreateOffscreen( const Context& context, RenderTargetAllocator& allocator, ISize size, + int mip_count, const std::string& label, AttachmentConfig color_attachment_config, std::optional stencil_attachment_config) { @@ -237,6 +238,7 @@ RenderTarget RenderTarget::CreateOffscreen( color_tex0.storage_mode = color_attachment_config.storage_mode; color_tex0.format = pixel_format; color_tex0.size = size; + color_tex0.mip_count = mip_count; color_tex0.usage = static_cast(TextureUsage::kRenderTarget) | static_cast(TextureUsage::kShaderRead); diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 388cfb7029fa8..997ebac1b7eab 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -86,6 +86,7 @@ class RenderTarget final { const Context& context, RenderTargetAllocator& allocator, ISize size, + int mip_count, const std::string& label = "Offscreen", AttachmentConfig color_attachment_config = kDefaultColorAttachmentConfig, std::optional stencil_attachment_config = diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index dcc40359f9d27..f34b2440b6b6b 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -1286,8 +1286,8 @@ TEST_P(RendererTest, CanPreAllocateCommands) { auto render_target_cache = std::make_shared( GetContext()->GetResourceAllocator()); - auto render_target = - RenderTarget::CreateOffscreen(*context, *render_target_cache, {100, 100}); + auto render_target = RenderTarget::CreateOffscreen( + *context, *render_target_cache, {100, 100}, /*mip_count=*/1); auto render_pass = cmd_buffer->CreateRenderPass(render_target); render_pass->ReserveCommands(100u); @@ -1301,8 +1301,8 @@ TEST_P(RendererTest, CanLookupRenderTargetProperties) { auto render_target_cache = std::make_shared( GetContext()->GetResourceAllocator()); - auto render_target = - RenderTarget::CreateOffscreen(*context, *render_target_cache, {100, 100}); + auto render_target = RenderTarget::CreateOffscreen( + *context, *render_target_cache, {100, 100}, /*mip_count=*/1); auto render_pass = cmd_buffer->CreateRenderPass(render_target); EXPECT_EQ(render_pass->GetSampleCount(), render_target.GetSampleCount()); From 6896e32886c0cf466922832ff643321566a40d8d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 10:25:00 -0800 Subject: [PATCH 14/28] added todo --- impeller/entity/shaders/gaussian_blur/kernel.glsl | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index 7a6ff6f376d56..7667b6a34da56 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -23,6 +23,7 @@ blur_info; f16vec4 Sample(f16sampler2D tex, vec2 coords, float lod) { #if ENABLE_DECAL_SPECIALIZATION + // TODO(gaaclarke): Make a LOD variant here. return IPHalfSampleDecal(tex, coords); #else return textureLod(tex, coords, lod); From e59dc8c6e4ac480f889dcedc977f2b7b3eb4a247 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 11:48:46 -0800 Subject: [PATCH 15/28] license --- ci/licenses_golden/licenses_flutter | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 8b069e99fc686..8addf725c1d76 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -5553,6 +5553,8 @@ ORIGIN: ../../../flutter/impeller/renderer/snapshot.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/stroke.comp + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/surface.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/surface.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/texture_mipmap.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/texture_mipmap.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/threadgroup_sizing_test.comp + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/vertex_buffer_builder.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/vertex_buffer_builder.h + ../../../flutter/LICENSE @@ -8381,6 +8383,8 @@ FILE: ../../../flutter/impeller/renderer/snapshot.h FILE: ../../../flutter/impeller/renderer/stroke.comp FILE: ../../../flutter/impeller/renderer/surface.cc FILE: ../../../flutter/impeller/renderer/surface.h +FILE: ../../../flutter/impeller/renderer/texture_mipmap.cc +FILE: ../../../flutter/impeller/renderer/texture_mipmap.h FILE: ../../../flutter/impeller/renderer/threadgroup_sizing_test.comp FILE: ../../../flutter/impeller/renderer/vertex_buffer_builder.cc FILE: ../../../flutter/impeller/renderer/vertex_buffer_builder.h From 3f52295d3fee601227e16aedad137659b6f86a9c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 11:56:43 -0800 Subject: [PATCH 16/28] added decal lod support --- impeller/compiler/shader_lib/impeller/texture.glsl | 9 +++++++++ impeller/entity/shaders/gaussian_blur/kernel.glsl | 3 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/impeller/compiler/shader_lib/impeller/texture.glsl b/impeller/compiler/shader_lib/impeller/texture.glsl index 56be817085db3..d8673ab52e6b7 100644 --- a/impeller/compiler/shader_lib/impeller/texture.glsl +++ b/impeller/compiler/shader_lib/impeller/texture.glsl @@ -140,6 +140,15 @@ f16vec4 IPHalfSampleDecal(f16sampler2D texture_sampler, vec2 coords) { return texture(texture_sampler, coords, kDefaultMipBiasHalf); } +/// Sample a texture with decal tile mode at specified LOD. +f16vec4 IPHalfSampleDecalLod(f16sampler2D texture_sampler, vec2 coords, float lod) { + if (any(lessThan(coords, vec2(0))) || + any(greaterThanEqual(coords, vec2(1)))) { + return f16vec4(0.0); + } + return textureLod(texture_sampler, coords, lod); +} + /// Sample a texture, emulating a specific tile mode. /// /// This is useful for Impeller graphics backend that don't have native support diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index 7667b6a34da56..a45b874b581c1 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -23,8 +23,7 @@ blur_info; f16vec4 Sample(f16sampler2D tex, vec2 coords, float lod) { #if ENABLE_DECAL_SPECIALIZATION - // TODO(gaaclarke): Make a LOD variant here. - return IPHalfSampleDecal(tex, coords); + return IPHalfSampleDecalLod(tex, coords, lod); #else return textureLod(tex, coords, lod); #endif From a61094b5e0b9abf6d5f433a5064d38a632f8a680 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 16:08:15 -0800 Subject: [PATCH 17/28] removed check for needsmipmapgeneration --- impeller/entity/inline_pass_context.cc | 2 +- impeller/renderer/texture_mipmap.cc | 30 ++++++++++++-------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 73aeff1735c7a..1a4589c24d32f 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -68,7 +68,7 @@ bool InlinePassContext::EndPass() { std::shared_ptr target_texture = GetPassTarget().GetRenderTarget().GetRenderTargetTexture(); - if (target_texture->NeedsMipmapGeneration()) { + if (target_texture->GetMipCount() > 1) { fml::Status mip_status = AddMipmapGeneration(context_, target_texture); if (!mip_status.ok()) { return false; diff --git a/impeller/renderer/texture_mipmap.cc b/impeller/renderer/texture_mipmap.cc index 823b80d662b25..0a633c645958a 100644 --- a/impeller/renderer/texture_mipmap.cc +++ b/impeller/renderer/texture_mipmap.cc @@ -10,22 +10,20 @@ namespace impeller { fml::Status AddMipmapGeneration(const std::shared_ptr& context, const std::shared_ptr& texture) { - if (texture->NeedsMipmapGeneration()) { - std::shared_ptr mip_cmd_buffer = - context->CreateCommandBuffer(); - std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); - bool success = blit_pass->GenerateMipmap(texture); - if (!success) { - return fml::Status(fml::StatusCode::kUnknown, ""); - } - success = blit_pass->EncodeCommands(context->GetResourceAllocator()); - if (!success) { - return fml::Status(fml::StatusCode::kUnknown, ""); - } - success = mip_cmd_buffer->SubmitCommands(/*callback=*/nullptr); - if (!success) { - return fml::Status(fml::StatusCode::kUnknown, ""); - } + std::shared_ptr mip_cmd_buffer = + context->CreateCommandBuffer(); + std::shared_ptr blit_pass = mip_cmd_buffer->CreateBlitPass(); + bool success = blit_pass->GenerateMipmap(texture); + if (!success) { + return fml::Status(fml::StatusCode::kUnknown, ""); + } + success = blit_pass->EncodeCommands(context->GetResourceAllocator()); + if (!success) { + return fml::Status(fml::StatusCode::kUnknown, ""); + } + success = mip_cmd_buffer->SubmitCommands(/*callback=*/nullptr); + if (!success) { + return fml::Status(fml::StatusCode::kUnknown, ""); } return {}; } From 1930bfd62901938c59a929964b8936992e5f8851 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 16:30:05 -0800 Subject: [PATCH 18/28] Revert "added decal lod support" This reverts commit 423b717e0fde6e3dc3551eb8f8f17c8c1a60c222. --- impeller/compiler/shader_lib/impeller/texture.glsl | 9 --------- impeller/entity/shaders/gaussian_blur/kernel.glsl | 3 ++- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/impeller/compiler/shader_lib/impeller/texture.glsl b/impeller/compiler/shader_lib/impeller/texture.glsl index d8673ab52e6b7..56be817085db3 100644 --- a/impeller/compiler/shader_lib/impeller/texture.glsl +++ b/impeller/compiler/shader_lib/impeller/texture.glsl @@ -140,15 +140,6 @@ f16vec4 IPHalfSampleDecal(f16sampler2D texture_sampler, vec2 coords) { return texture(texture_sampler, coords, kDefaultMipBiasHalf); } -/// Sample a texture with decal tile mode at specified LOD. -f16vec4 IPHalfSampleDecalLod(f16sampler2D texture_sampler, vec2 coords, float lod) { - if (any(lessThan(coords, vec2(0))) || - any(greaterThanEqual(coords, vec2(1)))) { - return f16vec4(0.0); - } - return textureLod(texture_sampler, coords, lod); -} - /// Sample a texture, emulating a specific tile mode. /// /// This is useful for Impeller graphics backend that don't have native support diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index a45b874b581c1..7667b6a34da56 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -23,7 +23,8 @@ blur_info; f16vec4 Sample(f16sampler2D tex, vec2 coords, float lod) { #if ENABLE_DECAL_SPECIALIZATION - return IPHalfSampleDecalLod(tex, coords, lod); + // TODO(gaaclarke): Make a LOD variant here. + return IPHalfSampleDecal(tex, coords); #else return textureLod(tex, coords, lod); #endif From c42daef3476053dd3027a063882264d065368f01 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 16:33:56 -0800 Subject: [PATCH 19/28] removed explicit lod --- .../contents/filters/gaussian_blur_filter_contents.cc | 8 -------- .../contents/filters/gaussian_blur_filter_contents.h | 1 - .../filters/gaussian_blur_filter_contents_unittests.cc | 5 +---- impeller/entity/shaders/gaussian_blur/kernel.glsl | 8 +++----- 4 files changed, 4 insertions(+), 18 deletions(-) diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc index 2f48013dd82b2..11d192c0357e3 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc @@ -193,10 +193,6 @@ Rect MakeReferenceUVs(const Rect& reference, const Rect& rect) { rect.GetSize()); return result.Scale(1.0f / Vector2(reference.GetSize())); } - -Scalar CalculateLOD(Scalar scalar) { - return fabsf(log2f(1.f / scalar)); -} } // namespace GaussianBlurFilterContents::GaussianBlurFilterContents( @@ -296,7 +292,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( Scalar desired_scalar = std::min(CalculateScale(scaled_sigma.x), CalculateScale(scaled_sigma.y)); - Scalar lod = CalculateLOD(desired_scalar); // 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. @@ -364,7 +359,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = static_cast(std::round(blur_radius.y * effective_scalar.y)), .step_size = 1, - .lod = lod, }, /*destination_target=*/std::nullopt, blur_uvs); @@ -387,7 +381,6 @@ std::optional GaussianBlurFilterContents::RenderFilter( .blur_radius = static_cast(std::round(blur_radius.x * effective_scalar.x)), .step_size = 1, - .lod = lod, }, pass3_destination, blur_uvs); @@ -448,7 +441,6 @@ Scalar GaussianBlurFilterContents::ScaleSigma(Scalar sigma) { KernelPipeline::FragmentShader::KernelSamples GenerateBlurInfo( BlurParameters parameters) { KernelPipeline::FragmentShader::KernelSamples result; - result.lod = parameters.lod; result.sample_count = ((2 * parameters.blur_radius) / parameters.step_size) + 1; // 32 comes from kernel.glsl. diff --git a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h index fdcd9ba3a6143..1bb594f3e4ae6 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.h +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.h @@ -16,7 +16,6 @@ struct BlurParameters { Scalar blur_sigma; int blur_radius; int step_size; - Scalar lod; }; KernelPipeline::FragmentShader::KernelSamples GenerateBlurInfo( 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 c2dddf4052c07..ba3053483181f 100644 --- a/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc +++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents_unittests.cc @@ -462,8 +462,7 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) { BlurParameters parameters = {.blur_uv_offset = Point(1, 0), .blur_sigma = 1, .blur_radius = 5, - .step_size = 1, - .lod = -1.f}; + .step_size = 1}; KernelPipeline::FragmentShader::KernelSamples samples = GenerateBlurInfo(parameters); EXPECT_EQ(samples.sample_count, 9); @@ -482,8 +481,6 @@ TEST(GaussianBlurFilterContentsTest, Coefficients) { EXPECT_TRUE(samples.samples[i + 1].coefficient > samples.samples[i].coefficient); } - - EXPECT_FLOAT_EQ(samples.lod, -1.f); } } // namespace testing diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index 7667b6a34da56..c491acbbb0fc4 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -17,16 +17,15 @@ struct KernelSample { uniform KernelSamples { int sample_count; KernelSample samples[32]; - float lod; } blur_info; -f16vec4 Sample(f16sampler2D tex, vec2 coords, float lod) { +f16vec4 Sample(f16sampler2D tex, vec2 coords) { #if ENABLE_DECAL_SPECIALIZATION // TODO(gaaclarke): Make a LOD variant here. return IPHalfSampleDecal(tex, coords); #else - return textureLod(tex, coords, lod); + return texture(tex, coords); #endif } @@ -41,8 +40,7 @@ void main() { float16_t coefficient = float16_t(blur_info.samples[i].coefficient); total_color += coefficient * Sample(texture_sampler, - v_texture_coords + blur_info.samples[i].uv_offset, - blur_info.lod); + v_texture_coords + blur_info.samples[i].uv_offset); } frag_color = total_color; From eb8496463c201e8ffef6d6099cf7b6636b4c1129 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 17:32:11 -0800 Subject: [PATCH 20/28] only started incrementing the mip_count if we are using blur --- impeller/aiks/canvas.cc | 32 ++++++++++++++++++++ impeller/entity/entity_pass.cc | 45 ++++++++++++++++++++++++----- impeller/entity/entity_pass.h | 10 +++++++ impeller/entity/entity_unittests.cc | 1 + 4 files changed, 80 insertions(+), 8 deletions(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index c5b9ae9e598b7..49c6851bd3284 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -133,6 +133,35 @@ void Canvas::Save() { Save(false); } +namespace { +class MipCountVisitor : public ImageFilterVisitor { + public: + virtual void Visit(const BlurImageFilter& filter) { required_mip_count_ = 4; } + virtual void Visit(const LocalMatrixImageFilter& filter) { + required_mip_count_ = 1; + } + virtual void Visit(const DilateImageFilter& filter) { + required_mip_count_ = 1; + } + virtual void Visit(const ErodeImageFilter& filter) { + required_mip_count_ = 1; + } + virtual void Visit(const MatrixImageFilter& filter) { + required_mip_count_ = 1; + } + virtual void Visit(const ComposeImageFilter& filter) { + required_mip_count_ = 1; + } + virtual void Visit(const ColorImageFilter& filter) { + required_mip_count_ = 1; + } + int32_t GetRequiredMipCount() const { return required_mip_count_; } + + private: + int32_t required_mip_count_ = -1; +}; +} // namespace + void Canvas::Save(bool create_subpass, BlendMode blend_mode, const std::shared_ptr& backdrop_filter) { @@ -156,6 +185,9 @@ void Canvas::Save(bool create_subpass, return filter; }; subpass->SetBackdropFilter(backdrop_filter_proc); + MipCountVisitor mip_count_visitor; + backdrop_filter->Visit(mip_count_visitor); + subpass->SetRequiredMipCount(mip_count_visitor.GetRequiredMipCount()); } subpass->SetBlendMode(blend_mode); current_pass_ = GetCurrentPass().AddSubpass(std::move(subpass)); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 5d98e94cf7d63..576c3a97a6ff9 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -325,13 +325,23 @@ bool EntityPass::Render(ContentContext& renderer, Rect::MakeSize(root_render_target.GetRenderTargetSize()), {.readonly = true}); - IterateAllEntities([lazy_glyph_atlas = - renderer.GetLazyGlyphAtlas()](const Entity& entity) { - if (const auto& contents = entity.GetContents()) { - contents->PopulateGlyphAtlas(lazy_glyph_atlas, entity.DeriveTextScale()); - } - return true; - }); + int32_t required_mip_count = 1; + IterateAllElements( + [&required_mip_count, lazy_glyph_atlas = renderer.GetLazyGlyphAtlas()]( + const Element& element) { + if (auto entity = std::get_if(&element)) { + if (const auto& contents = entity->GetContents()) { + contents->PopulateGlyphAtlas(lazy_glyph_atlas, + entity->DeriveTextScale()); + } + } + if (auto subpass = std::get_if>(&element)) { + const EntityPass* entity_pass = subpass->get(); + required_mip_count = + std::max(required_mip_count, entity_pass->GetRequiredMipCount()); + } + return true; + }); ClipCoverageStack clip_coverage_stack = {ClipCoverageLayer{ .coverage = Rect::MakeSize(root_render_target.GetRenderTargetSize()), @@ -343,7 +353,7 @@ bool EntityPass::Render(ContentContext& renderer, // there's no need to set up a stencil attachment on the root render target. if (reads_from_onscreen_backdrop) { EntityPassTarget offscreen_target = CreateRenderTarget( - renderer, root_render_target.GetRenderTargetSize(), /*mip_count=*/4, + renderer, root_render_target.GetRenderTargetSize(), required_mip_count, GetClearColorOrDefault(render_target.GetRenderTargetSize())); if (!OnRender(renderer, // renderer @@ -1020,6 +1030,25 @@ void EntityPass::IterateAllElements( } } +void EntityPass::IterateAllElements( + const std::function& iterator) const { + /// TODO(gaaclarke): Remove duplication here between const and non-const + /// versions. + if (!iterator) { + return; + } + + for (auto& element : elements_) { + if (!iterator(element)) { + return; + } + if (auto subpass = std::get_if>(&element)) { + const EntityPass* entity_pass = subpass->get(); + entity_pass->IterateAllElements(iterator); + } + } +} + void EntityPass::IterateAllEntities( const std::function& iterator) { if (!iterator) { diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 39f5449ff323b..7de68d0d9ad11 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -101,6 +101,9 @@ class EntityPass { /// it's included in the stream before its children. void IterateAllElements(const std::function& iterator); + void IterateAllElements( + const std::function& iterator) const; + //---------------------------------------------------------------------------- /// @brief Iterate all entities in this pass, recursively including entities /// of child passes. The iteration order is depth-first. @@ -148,6 +151,12 @@ class EntityPass { void SetEnableOffscreenCheckerboard(bool enabled); + int32_t GetRequiredMipCount() const { return required_mip_count_; } + + void SetRequiredMipCount(int32_t mip_count) { + required_mip_count_ = mip_count; + } + //---------------------------------------------------------------------------- /// @brief Computes the coverage of a given subpass. This is used to /// determine the texture size of a given subpass before it's rendered @@ -297,6 +306,7 @@ class EntityPass { std::optional bounds_limit_; std::unique_ptr clip_replay_ = std::make_unique(); + int32_t required_mip_count_ = 1; /// These values are incremented whenever something is added to the pass that /// requires reading from the backdrop texture. Currently, this can happen in diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index a24e8e2b59931..4bde8ea6799f7 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -48,6 +48,7 @@ #include "impeller/renderer/command.h" #include "impeller/renderer/pipeline_descriptor.h" #include "impeller/renderer/render_pass.h" +#include "impeller/renderer/testing/mocks.h" #include "impeller/renderer/vertex_buffer_builder.h" #include "impeller/typographer/backends/skia/text_frame_skia.h" #include "impeller/typographer/backends/skia/typographer_context_skia.h" From 039b1b49027c4120cf907e56c56ad5ff72440793 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 17:39:59 -0800 Subject: [PATCH 21/28] put the mipcount behind the flag for the new blur --- impeller/aiks/canvas.cc | 4 +++- impeller/entity/contents/filters/filter_contents.cc | 6 ++++++ impeller/entity/contents/filters/filter_contents.h | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index 49c6851bd3284..f66da9a914cbf 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -136,7 +136,9 @@ void Canvas::Save() { namespace { class MipCountVisitor : public ImageFilterVisitor { public: - virtual void Visit(const BlurImageFilter& filter) { required_mip_count_ = 4; } + virtual void Visit(const BlurImageFilter& filter) { + required_mip_count_ = FilterContents::kBlurFilterRequiredMipCount; + } virtual void Visit(const LocalMatrixImageFilter& filter) { required_mip_count_ = 1; } diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index c4ffaff85726d..a9acd5ffd697d 100644 --- a/impeller/entity/contents/filters/filter_contents.cc +++ b/impeller/entity/contents/filters/filter_contents.cc @@ -52,6 +52,12 @@ std::shared_ptr FilterContents::MakeDirectionalGaussianBlur( #define IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER 1 +#ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER +const int32_t FilterContents::kBlurFilterRequiredMipCount = 4; +#else +const int32_t FilterContents::kBlurFilterRequiredMipCount = 1; +#endif + std::shared_ptr FilterContents::MakeGaussianBlur( const FilterInput::Ref& input, Sigma sigma_x, diff --git a/impeller/entity/contents/filters/filter_contents.h b/impeller/entity/contents/filters/filter_contents.h index 3e210d8f0fb70..076593f644dab 100644 --- a/impeller/entity/contents/filters/filter_contents.h +++ b/impeller/entity/contents/filters/filter_contents.h @@ -20,6 +20,8 @@ namespace impeller { class FilterContents : public Contents { public: + static const int32_t kBlurFilterRequiredMipCount; + enum class BlurStyle { /// Blurred inside and outside. kNormal, From f13d449ee5379a007ff6d35f3aecb97372117b4b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 18:57:48 -0800 Subject: [PATCH 22/28] added test --- impeller/aiks/aiks_unittests.cc | 27 +++++++++++++++++++ .../golden_playground_test_mac.cc | 17 +++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index e1f75aadc4099..620a59b19649f 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3716,5 +3716,32 @@ TEST_P(AiksTest, SubpassWithClearColorOptimization) { // will be filled with NaNs and may produce a magenta texture on macOS or iOS. ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } + +TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) { + // This makes sure if mip maps are recycled across invocations of blurs the + // contents get updated each frame correctly. If they aren't updated the color + // inside the blur and outside the blur will be different. + // + // If there is some change to render target caching this could display a false + // positive in the future. + int32_t count = 0; + auto callback = [&](AiksContext& renderer) -> std::optional { + Canvas canvas; + if (count++ == 0) { + canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); + } else { + canvas.DrawCircle({100, 100}, 50, {.color = Color::Chartreuse()}); + } + canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), {20, 20}); + canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt, + ImageFilter::MakeBlur(Sigma(30.0), Sigma(30.0), + FilterContents::BlurStyle::kNormal, + Entity::TileMode::kClamp)); + canvas.Restore(); + return canvas.EndRecordingAsPicture(); + }; + + ASSERT_TRUE(OpenPlaygroundHere(callback)); +} } // namespace testing } // namespace impeller diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index 81cec2c283a10..ef7bff92ac2c1 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -55,6 +55,8 @@ static const std::vector kSkipTests = { "impeller_Play_AiksTest_TextRotated_Vulkan", // Runtime stage based tests get confused with a Metal context. "impeller_Play_AiksTest_CanRenderClippedRuntimeEffects_Vulkan", + "impeller_Play_AiksTest_CaptureContext_Metal", + "impeller_Play_AiksTest_CaptureContext_Vulkan", }; namespace { @@ -153,7 +155,20 @@ bool GoldenPlaygroundTest::OpenPlaygroundHere(Picture picture) { bool GoldenPlaygroundTest::OpenPlaygroundHere( AiksPlaygroundCallback callback) { // NOLINT(performance-unnecessary-value-param) - return false; + AiksContext renderer(GetContext(), typographer_context_); + + std::optional picture; + std::unique_ptr screenshot; + for (int i = 0; i < 2; ++i) { + picture = callback(renderer); + if (!picture.has_value()) { + return false; + } + screenshot = pimpl_->screenshotter->MakeScreenshot( + renderer, picture.value(), pimpl_->window_size); + } + + return SaveScreenshot(std::move(screenshot)); } std::shared_ptr GoldenPlaygroundTest::CreateTextureForFixture( From 345cb30529e7fddbceb826243142b53a01e712fe Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 19:06:08 -0800 Subject: [PATCH 23/28] turned off hack --- impeller/entity/contents/filters/filter_contents.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc index a9acd5ffd697d..235a53134c5e3 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 - #ifdef IMPELLER_ENABLE_NEW_GAUSSIAN_FILTER const int32_t FilterContents::kBlurFilterRequiredMipCount = 4; #else From b04debfaecdf08e195d3e14230cd954ee80f26bd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 20:26:47 -0800 Subject: [PATCH 24/28] fixed linux usage of mip_count --- impeller/entity/contents/scene_contents.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/entity/contents/scene_contents.cc b/impeller/entity/contents/scene_contents.cc index 54eb9f5cd29a4..5a0b68cd427e8 100644 --- a/impeller/entity/contents/scene_contents.cc +++ b/impeller/entity/contents/scene_contents.cc @@ -50,6 +50,7 @@ bool SceneContents::Render(const ContentContext& renderer, *renderer.GetContext(), // context *renderer.GetRenderTargetCache(), // allocator ISize(coverage.value().GetSize()), // size + /*mip_count=*/1, "SceneContents", // label RenderTarget::AttachmentConfigMSAA{ .storage_mode = StorageMode::kDeviceTransient, @@ -68,6 +69,7 @@ bool SceneContents::Render(const ContentContext& renderer, *renderer.GetContext(), // context *renderer.GetRenderTargetCache(), // allocator ISize(coverage.value().GetSize()), // size + /*mip_count=*/1, "SceneContents", // label RenderTarget::AttachmentConfig{ .storage_mode = StorageMode::kDevicePrivate, From 12a1d30ac709a9aa868a132d44ccaf0dd8fc3d02 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 11 Jan 2024 20:35:24 -0800 Subject: [PATCH 25/28] added another test for setting mip count levels --- impeller/aiks/aiks_unittests.cc | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 620a59b19649f..91077989b7239 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3743,5 +3743,29 @@ TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) { ASSERT_TRUE(OpenPlaygroundHere(callback)); } + +TEST_P(AiksTest, GuassianBlurSetsMipCountOnPass) { + Canvas canvas; + canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); + canvas.SaveLayer({}, std::nullopt, + ImageFilter::MakeBlur(Sigma(3), Sigma(3), + FilterContents::BlurStyle::kNormal, + Entity::TileMode::kClamp)); + canvas.Restore(); + + Picture picture = canvas.EndRecordingAsPicture(); + + int32_t max_mip_count = 1; + picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool { + if (auto subpass = std::get_if>(&element)) { + max_mip_count = + std::max(max_mip_count, subpass->get()->GetRequiredMipCount()); + } + return true; + }); + + EXPECT_EQ(1, max_mip_count); +} + } // namespace testing } // namespace impeller From 4bea7dc29fc71fa6469cf645fcf381e4e69943e3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 12 Jan 2024 08:49:25 -0800 Subject: [PATCH 26/28] added another test --- impeller/aiks/aiks_context.cc | 8 ++++-- impeller/aiks/aiks_context.h | 6 ++++- impeller/aiks/aiks_unittests.cc | 30 ++++++++++++++++++++-- impeller/entity/contents/scene_contents.cc | 4 +-- impeller/entity/render_target_cache.h | 11 ++++++++ 5 files changed, 52 insertions(+), 7 deletions(-) diff --git a/impeller/aiks/aiks_context.cc b/impeller/aiks/aiks_context.cc index d001c189e3b0d..3be53f953713b 100644 --- a/impeller/aiks/aiks_context.cc +++ b/impeller/aiks/aiks_context.cc @@ -12,14 +12,18 @@ namespace impeller { AiksContext::AiksContext( std::shared_ptr context, - std::shared_ptr typographer_context) + std::shared_ptr typographer_context, + std::optional> + render_target_allocator) : context_(std::move(context)) { if (!context_ || !context_->IsValid()) { return; } content_context_ = std::make_unique( - context_, std::move(typographer_context)); + context_, std::move(typographer_context), + render_target_allocator.has_value() ? render_target_allocator.value() + : nullptr); if (!content_context_->IsValid()) { return; } diff --git a/impeller/aiks/aiks_context.h b/impeller/aiks/aiks_context.h index ad8d6429c4d01..5b35b63def700 100644 --- a/impeller/aiks/aiks_context.h +++ b/impeller/aiks/aiks_context.h @@ -28,8 +28,12 @@ class AiksContext { /// `nullptr` is supplied, then attempting to draw /// text with Aiks will result in validation /// errors. + /// @param render_target_allocator Injects a render target allocator or + /// allocates its own if none is supplied. AiksContext(std::shared_ptr context, - std::shared_ptr typographer_context); + std::shared_ptr typographer_context, + std::optional> + render_target_allocator = std::nullopt); ~AiksContext(); diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 91077989b7239..b0c5f057264bf 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/impeller/aiks/aiks_unittests.h" +#include #include #include #include @@ -26,6 +27,7 @@ #include "impeller/entity/contents/radial_gradient_contents.h" #include "impeller/entity/contents/solid_color_contents.h" #include "impeller/entity/contents/sweep_gradient_contents.h" +#include "impeller/entity/render_target_cache.h" #include "impeller/geometry/color.h" #include "impeller/geometry/constants.h" #include "impeller/geometry/geometry_asserts.h" @@ -3744,7 +3746,7 @@ TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) { ASSERT_TRUE(OpenPlaygroundHere(callback)); } -TEST_P(AiksTest, GuassianBlurSetsMipCountOnPass) { +TEST_P(AiksTest, GaussianBlurSetsMipCountOnPass) { Canvas canvas; canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); canvas.SaveLayer({}, std::nullopt, @@ -3755,7 +3757,7 @@ TEST_P(AiksTest, GuassianBlurSetsMipCountOnPass) { Picture picture = canvas.EndRecordingAsPicture(); - int32_t max_mip_count = 1; + int32_t max_mip_count = 0; picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool { if (auto subpass = std::get_if>(&element)) { max_mip_count = @@ -3767,5 +3769,29 @@ TEST_P(AiksTest, GuassianBlurSetsMipCountOnPass) { EXPECT_EQ(1, max_mip_count); } +TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { + Canvas canvas; + canvas.DrawCircle({100, 100}, 50, {.color = Color::CornflowerBlue()}); + canvas.SaveLayer({}, std::nullopt, + ImageFilter::MakeBlur(Sigma(3), Sigma(3), + FilterContents::BlurStyle::kNormal, + Entity::TileMode::kClamp)); + canvas.Restore(); + + Picture picture = canvas.EndRecordingAsPicture(); + std::shared_ptr cache = + std::make_shared(GetContext()->GetResourceAllocator()); + AiksContext aiks_context(GetContext(), nullptr, cache); + picture.ToImage(aiks_context, {100, 100}); + + size_t max_mip_count = 0; + for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); + ++it) { + max_mip_count = + std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); + } + EXPECT_EQ(max_mip_count, 1lu); +} + } // namespace testing } // namespace impeller diff --git a/impeller/entity/contents/scene_contents.cc b/impeller/entity/contents/scene_contents.cc index 5a0b68cd427e8..f5da62830ecbc 100644 --- a/impeller/entity/contents/scene_contents.cc +++ b/impeller/entity/contents/scene_contents.cc @@ -51,7 +51,7 @@ bool SceneContents::Render(const ContentContext& renderer, *renderer.GetRenderTargetCache(), // allocator ISize(coverage.value().GetSize()), // size /*mip_count=*/1, - "SceneContents", // label + "SceneContents", // label RenderTarget::AttachmentConfigMSAA{ .storage_mode = StorageMode::kDeviceTransient, .resolve_storage_mode = StorageMode::kDevicePrivate, @@ -70,7 +70,7 @@ bool SceneContents::Render(const ContentContext& renderer, *renderer.GetRenderTargetCache(), // allocator ISize(coverage.value().GetSize()), // size /*mip_count=*/1, - "SceneContents", // label + "SceneContents", // label RenderTarget::AttachmentConfig{ .storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kClear, diff --git a/impeller/entity/render_target_cache.h b/impeller/entity/render_target_cache.h index b59bd43983711..382781b793b7b 100644 --- a/impeller/entity/render_target_cache.h +++ b/impeller/entity/render_target_cache.h @@ -43,6 +43,17 @@ class RenderTargetCache : public RenderTargetAllocator { RenderTargetCache(const RenderTargetCache&) = delete; RenderTargetCache& operator=(const RenderTargetCache&) = delete; + + public: + /// Visible for testing. + std::vector::const_iterator GetTextureDataBegin() const { + return texture_data_.begin(); + } + + /// Visible for testing. + std::vector::const_iterator GetTextureDataEnd() const { + return texture_data_.end(); + } }; } // namespace impeller From 9bf4355ba78d0766106e9365bdb450bc3ee846e9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 12 Jan 2024 08:52:28 -0800 Subject: [PATCH 27/28] removed todo --- impeller/entity/shaders/gaussian_blur/kernel.glsl | 1 - 1 file changed, 1 deletion(-) diff --git a/impeller/entity/shaders/gaussian_blur/kernel.glsl b/impeller/entity/shaders/gaussian_blur/kernel.glsl index c491acbbb0fc4..d8a544bf070f6 100644 --- a/impeller/entity/shaders/gaussian_blur/kernel.glsl +++ b/impeller/entity/shaders/gaussian_blur/kernel.glsl @@ -22,7 +22,6 @@ blur_info; f16vec4 Sample(f16sampler2D tex, vec2 coords) { #if ENABLE_DECAL_SPECIALIZATION - // TODO(gaaclarke): Make a LOD variant here. return IPHalfSampleDecal(tex, coords); #else return texture(tex, coords); From 7cba038210052527b0f67b38abe7ac9073fda77f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 12 Jan 2024 08:58:52 -0800 Subject: [PATCH 28/28] updated comment --- impeller/aiks/aiks_unittests.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index b0c5f057264bf..c2b07397610a1 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3725,7 +3725,8 @@ TEST_P(AiksTest, GuassianBlurUpdatesMipmapContents) { // inside the blur and outside the blur will be different. // // If there is some change to render target caching this could display a false - // positive in the future. + // positive in the future. Also, if the LOD that is rendered is 1 it could + // present a false positive. int32_t count = 0; auto callback = [&](AiksContext& renderer) -> std::optional { Canvas canvas;