From 1a2dfa352241048aed32eb90b2204cd3e2adaa40 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 26 Apr 2023 10:51:12 -0700 Subject: [PATCH] [Impeller] Store the root stencil on iOS (flutter/engine#41509) Switch the root RenderPass stencil from transient to device private, add validation logs, and dry the offscreen stencil setup. Also make the multisample root stencil setup valid in Vulkan (the attachment mismatched the texture -- this didn't matter before since we were always replacing it). Fixes a bug where the stencil gets erased between root render passes -- this issue was introduced when we removed the root blit on iOS. --- .../flutter/impeller/entity/entity_pass.cc | 43 +++++++---- .../renderer/backend/metal/surface_mtl.mm | 2 +- .../renderer/backend/vulkan/surface_vk.cc | 4 +- .../impeller/renderer/render_target.cc | 75 +++++++++---------- .../flutter/impeller/renderer/render_target.h | 7 ++ 5 files changed, 72 insertions(+), 59 deletions(-) diff --git a/engine/src/flutter/impeller/entity/entity_pass.cc b/engine/src/flutter/impeller/entity/entity_pass.cc index c9a00df681b..0b67412fed0 100644 --- a/engine/src/flutter/impeller/entity/entity_pass.cc +++ b/engine/src/flutter/impeller/entity/entity_pass.cc @@ -148,6 +148,15 @@ EntityPass* EntityPass::AddSubpass(std::unique_ptr pass) { return subpass_pointer; } +static RenderTarget::AttachmentConfig GetDefaultStencilConfig(bool readable) { + return RenderTarget::AttachmentConfig{ + .storage_mode = readable ? StorageMode::kDevicePrivate + : StorageMode::kDeviceTransient, + .load_action = LoadAction::kDontCare, + .store_action = StoreAction::kDontCare, + }; +} + static EntityPassTarget CreateRenderTarget(ContentContext& renderer, ISize size, bool readable, @@ -170,13 +179,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, .resolve_storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kDontCare, .store_action = StoreAction::kMultisampleResolve, - .clear_color = clear_color}, // color_attachment_config - RenderTarget::AttachmentConfig{ - .storage_mode = readable ? StorageMode::kDevicePrivate - : StorageMode::kDeviceTransient, - .load_action = LoadAction::kDontCare, - .store_action = StoreAction::kDontCare, - } // stencil_attachment_config + .clear_color = clear_color}, // color_attachment_config + GetDefaultStencilConfig(readable) // stencil_attachment_config ); } else { target = RenderTarget::CreateOffscreen( @@ -187,13 +191,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, .storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kDontCare, .store_action = StoreAction::kDontCare, - }, // color_attachment_config - RenderTarget::AttachmentConfig{ - .storage_mode = readable ? StorageMode::kDevicePrivate - : StorageMode::kDeviceTransient, - .load_action = LoadAction::kDontCare, - .store_action = StoreAction::kDontCare, - } // stencil_attachment_config + }, // color_attachment_config + GetDefaultStencilConfig(readable) // stencil_attachment_config ); } @@ -215,16 +214,28 @@ bool EntityPass::Render(ContentContext& renderer, return false; } + if (!render_target.GetStencilAttachment().has_value()) { + VALIDATION_LOG << "The root RenderTarget must have a stencil attachment."; + return false; + } + + auto stencil_texture = render_target.GetStencilAttachment()->texture; + if (!stencil_texture) { + VALIDATION_LOG << "The root RenderTarget must have a stencil texture."; + return false; + } + StencilCoverageStack stencil_coverage_stack = {StencilCoverageLayer{ .coverage = Rect::MakeSize(render_target.GetRenderTargetSize()), .stencil_depth = 0}}; - // bool supports_root_pass_reads = renderer.GetDeviceCapabilities().SupportsReadFromOnscreenTexture() && // If the backend doesn't have `SupportsReadFromResolve`, we need to flip // between two textures when restoring a previous MSAA pass. - renderer.GetDeviceCapabilities().SupportsReadFromResolve(); + renderer.GetDeviceCapabilities().SupportsReadFromResolve() && + stencil_texture->GetTextureDescriptor().storage_mode != + StorageMode::kDeviceTransient; if (!supports_root_pass_reads && GetTotalPassReads(renderer) > 0) { auto offscreen_target = CreateRenderTarget(renderer, render_target.GetRenderTargetSize(), true, diff --git a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm index 672923ca7cf..59ca1c0009c 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/surface_mtl.mm @@ -84,7 +84,7 @@ std::unique_ptr SurfaceMTL::WrapCurrentMetalLayerDrawable( color0.resolve_texture = resolve_tex; TextureDescriptor stencil_tex_desc; - stencil_tex_desc.storage_mode = StorageMode::kDeviceTransient; + stencil_tex_desc.storage_mode = StorageMode::kDevicePrivate; stencil_tex_desc.type = TextureType::kTexture2DMultisample; stencil_tex_desc.sample_count = SampleCount::kCount4; stencil_tex_desc.format = diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/surface_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/surface_vk.cc index 02992a35ebc..721b2fe7579 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/surface_vk.cc @@ -61,8 +61,8 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( color0.resolve_texture = resolve_tex; TextureDescriptor stencil0_tex; - stencil0_tex.storage_mode = StorageMode::kDeviceTransient; - stencil0_tex.type = TextureType::kTexture2D; + stencil0_tex.storage_mode = StorageMode::kDevicePrivate; + stencil0_tex.type = TextureType::kTexture2DMultisample; stencil0_tex.sample_count = SampleCount::kCount4; stencil0_tex.format = context->GetCapabilities()->GetDefaultStencilFormat(); stencil0_tex.size = msaa_tex_desc.size; diff --git a/engine/src/flutter/impeller/renderer/render_target.cc b/engine/src/flutter/impeller/renderer/render_target.cc index fe29d31df6e..67c18d95386 100644 --- a/engine/src/flutter/impeller/renderer/render_target.cc +++ b/engine/src/flutter/impeller/renderer/render_target.cc @@ -239,25 +239,8 @@ RenderTarget RenderTarget::CreateOffscreen( target.SetColorAttachment(color0, 0u); if (stencil_attachment_config.has_value()) { - TextureDescriptor stencil_tex0; - stencil_tex0.storage_mode = stencil_attachment_config->storage_mode; - stencil_tex0.format = context.GetCapabilities()->GetDefaultStencilFormat(); - stencil_tex0.size = size; - stencil_tex0.usage = - static_cast(TextureUsage::kRenderTarget); - - StencilAttachment stencil0; - stencil0.load_action = stencil_attachment_config->load_action; - stencil0.store_action = stencil_attachment_config->store_action; - stencil0.clear_stencil = 0u; - stencil0.texture = - context.GetResourceAllocator()->CreateTexture(stencil_tex0); - - if (!stencil0.texture) { - return {}; - } - stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str())); - target.SetStencilAttachment(std::move(stencil0)); + target.SetupStencilAttachment(context, size, false, label, + stencil_attachment_config.value()); } else { target.SetStencilAttachment(std::nullopt); } @@ -331,27 +314,8 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( // Create MSAA stencil texture. if (stencil_attachment_config.has_value()) { - TextureDescriptor stencil_tex0; - stencil_tex0.storage_mode = stencil_attachment_config->storage_mode; - stencil_tex0.type = TextureType::kTexture2DMultisample; - stencil_tex0.sample_count = SampleCount::kCount4; - stencil_tex0.format = context.GetCapabilities()->GetDefaultStencilFormat(); - stencil_tex0.size = size; - stencil_tex0.usage = - static_cast(TextureUsage::kRenderTarget); - - StencilAttachment stencil0; - stencil0.load_action = stencil_attachment_config->load_action; - stencil0.store_action = stencil_attachment_config->store_action; - stencil0.clear_stencil = 0u; - stencil0.texture = - context.GetResourceAllocator()->CreateTexture(stencil_tex0); - - if (!stencil0.texture) { - return {}; - } - stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str())); - target.SetStencilAttachment(std::move(stencil0)); + target.SetupStencilAttachment(context, size, true, label, + stencil_attachment_config.value()); } else { target.SetStencilAttachment(std::nullopt); } @@ -359,6 +323,37 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( return target; } +void RenderTarget::SetupStencilAttachment( + const Context& context, + ISize size, + bool msaa, + const std::string& label, + AttachmentConfig stencil_attachment_config) { + TextureDescriptor stencil_tex0; + stencil_tex0.storage_mode = stencil_attachment_config.storage_mode; + if (msaa) { + stencil_tex0.type = TextureType::kTexture2DMultisample; + stencil_tex0.sample_count = SampleCount::kCount4; + } + stencil_tex0.format = context.GetCapabilities()->GetDefaultStencilFormat(); + stencil_tex0.size = size; + stencil_tex0.usage = + static_cast(TextureUsage::kRenderTarget); + + StencilAttachment stencil0; + stencil0.load_action = stencil_attachment_config.load_action; + stencil0.store_action = stencil_attachment_config.store_action; + stencil0.clear_stencil = 0u; + stencil0.texture = + context.GetResourceAllocator()->CreateTexture(stencil_tex0); + + if (!stencil0.texture) { + return; // Error messages are handled by `Allocator::CreateTexture`. + } + stencil0.texture->SetLabel(SPrintF("%s Stencil Texture", label.c_str())); + SetStencilAttachment(std::move(stencil0)); +} + size_t RenderTarget::GetTotalAttachmentCount() const { size_t count = 0u; for (const auto& [_, color] : colors_) { diff --git a/engine/src/flutter/impeller/renderer/render_target.h b/engine/src/flutter/impeller/renderer/render_target.h index 9e1da327210..bc54976a030 100644 --- a/engine/src/flutter/impeller/renderer/render_target.h +++ b/engine/src/flutter/impeller/renderer/render_target.h @@ -76,6 +76,13 @@ class RenderTarget final { bool IsValid() const; + void SetupStencilAttachment(const Context& context, + ISize size, + bool msaa, + const std::string& label = "Offscreen", + AttachmentConfig stencil_attachment_config = + kDefaultStencilAttachmentConfig); + SampleCount GetSampleCount() const; bool HasColorAttachment(size_t index) const;