From 8acca62755ccf0bdfed7370031251b449ced4fbc Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 28 Feb 2024 12:33:51 -0800 Subject: [PATCH] [Impeller] fix render pass depth descriptor. (flutter/engine#51031) Depth+stencil should be treated as the same attachment. Adding more than one d/s attachment in the render pass descriptor seems to be confusing swiftshader. @bdero This fixes things for me locally, lets let CI take a spin --- .../src/flutter/impeller/entity/inline_pass_context.cc | 9 ++++++--- .../impeller/renderer/backend/vulkan/context_vk.cc | 6 ++---- .../impeller/renderer/backend/vulkan/pipeline_vk.cc | 4 +--- .../impeller/renderer/backend/vulkan/render_pass_vk.cc | 10 +++------- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/engine/src/flutter/impeller/entity/inline_pass_context.cc b/engine/src/flutter/impeller/entity/inline_pass_context.cc index 7fcaf515ad3..d39d41394fe 100644 --- a/engine/src/flutter/impeller/entity/inline_pass_context.cc +++ b/engine/src/flutter/impeller/entity/inline_pass_context.cc @@ -158,15 +158,18 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( pass_target_.target_.SetDepthAttachment(depth.value()); } + auto depth = pass_target_.GetRenderTarget().GetDepthAttachment(); auto stencil = pass_target_.GetRenderTarget().GetStencilAttachment(); - if (!stencil.has_value()) { - VALIDATION_LOG << "Stencil attachment unexpectedly missing from the " + if (!depth.has_value() || !stencil.has_value()) { + VALIDATION_LOG << "Stencil/Depth attachment unexpectedly missing from the " "EntityPass render target."; return {}; } - stencil->load_action = LoadAction::kClear; stencil->store_action = StoreAction::kDontCare; + depth->load_action = LoadAction::kClear; + depth->store_action = StoreAction::kDontCare; + pass_target_.target_.SetDepthAttachment(depth); pass_target_.target_.SetStencilAttachment(stencil.value()); pass_target_.target_.SetColorAttachment(color0, 0); diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc index d8fde32f375..4fdde323fb3 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc @@ -600,10 +600,8 @@ void ContextVK::InitializeCommonlyUsedShadersIfNeeded() const { depth->load_action, // depth->store_action // ); - } - - if (auto stencil = render_target.GetStencilAttachment(); - stencil.has_value()) { + } else if (auto stencil = render_target.GetStencilAttachment(); + stencil.has_value()) { builder.SetStencilAttachment( stencil->texture->GetTextureDescriptor().format, // stencil->texture->GetTextureDescriptor().sample_count, // diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc index 497fa2c3032..13c13ab337a 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/pipeline_vk.cc @@ -142,9 +142,7 @@ static vk::UniqueRenderPass CreateCompatRenderPassForPipeline( LoadAction::kDontCare, // StoreAction::kDontCare // ); - } - - if (desc.HasStencilAttachmentDescriptors()) { + } else if (desc.HasStencilAttachmentDescriptors()) { builder.SetStencilAttachment(desc.GetStencilPixelFormat(), // desc.GetSampleCount(), // LoadAction::kDontCare, // diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_vk.cc index b2f8bde4c11..7c049814b21 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -68,9 +68,7 @@ static std::vector GetVKClearValues( if (depth.has_value()) { clears.emplace_back(VKClearValueFromDepthStencil( stencil ? stencil->clear_stencil : 0u, depth->clear_depth)); - } - - if (stencil.has_value()) { + } else if (stencil.has_value()) { clears.emplace_back(VKClearValueFromDepthStencil( stencil->clear_stencil, depth ? depth->clear_depth : 0.0f)); } @@ -116,10 +114,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( depth->store_action // ); TextureVK::Cast(*depth->texture).SetLayout(barrier); - } - - if (auto stencil = render_target_.GetStencilAttachment(); - stencil.has_value()) { + } else if (auto stencil = render_target_.GetStencilAttachment(); + stencil.has_value()) { builder.SetStencilAttachment( stencil->texture->GetTextureDescriptor().format, // stencil->texture->GetTextureDescriptor().sample_count, //