From 9de782c6ea8009a73dffe3f64f70a3c5135c45de Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 24 Aug 2023 15:30:09 -0700 Subject: [PATCH 1/3] [Impeller] split out creation of renderpass with setting the layout barrier --- .../renderer/backend/vulkan/render_pass_vk.cc | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 0a9f020ef6e59..eba8bdc5424cf 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -29,10 +29,8 @@ namespace impeller { -static vk::AttachmentDescription CreateAttachmentDescription( - const Attachment& attachment, - const std::shared_ptr& command_buffer, - bool resolve_texture = false) { +static vk::AttachmentDescription foo(const Attachment& attachment, + bool resolve_texture = false) { const auto& texture = resolve_texture ? attachment.resolve_texture : attachment.texture; if (!texture) { @@ -57,6 +55,31 @@ static vk::AttachmentDescription CreateAttachmentDescription( if (current_layout != vk::ImageLayout::ePresentSrcKHR && current_layout != vk::ImageLayout::eUndefined) { + // Note: This should incur a barrier. + current_layout = vk::ImageLayout::eGeneral; + } + + return CreateAttachmentDescription(desc.format, // + desc.sample_count, // + load_action, // + store_action, // + current_layout // + ); +} + +static vk::AttachmentDescription CreateAttachmentDescription( + const Attachment& attachment, + const std::shared_ptr& command_buffer, + bool resolve_texture = false) { + const auto& texture = + resolve_texture ? attachment.resolve_texture : attachment.texture; + if (!texture) { + return {}; + } + const auto& texture_vk = TextureVK::Cast(*texture); + vk::AttachmentDescription attachment_desc = foo(attachment, resolve_texture); + + if (attachment_desc.initialLayout == vk::ImageLayout::eGeneral) { BarrierVK barrier; barrier.new_layout = vk::ImageLayout::eGeneral; barrier.cmd_buffer = command_buffer->GetEncoder()->GetCommandBuffer(); @@ -68,17 +91,8 @@ static vk::AttachmentDescription CreateAttachmentDescription( vk::PipelineStageFlagBits::eTransfer; texture_vk.SetLayout(barrier); - current_layout = vk::ImageLayout::eGeneral; } - const auto attachment_desc = - CreateAttachmentDescription(desc.format, // - desc.sample_count, // - load_action, // - store_action, // - current_layout // - ); - // Instead of transitioning layouts manually using barriers, we are going to // make the subpass perform our transitions. texture_vk.SetLayoutWithoutEncoding(attachment_desc.finalLayout); From de988529f95b1679112598a16ab70bd804bc019b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 24 Aug 2023 15:56:04 -0700 Subject: [PATCH 2/3] split up creating the desc and setting the layout --- .../renderer/backend/vulkan/render_pass_vk.cc | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index eba8bdc5424cf..70420bba3c5fd 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -29,8 +29,9 @@ namespace impeller { -static vk::AttachmentDescription foo(const Attachment& attachment, - bool resolve_texture = false) { +static vk::AttachmentDescription CreateAttachmentDescription( + const Attachment& attachment, + bool resolve_texture = false) { const auto& texture = resolve_texture ? attachment.resolve_texture : attachment.texture; if (!texture) { @@ -67,17 +68,17 @@ static vk::AttachmentDescription foo(const Attachment& attachment, ); } -static vk::AttachmentDescription CreateAttachmentDescription( +static void SetTextureLayout( const Attachment& attachment, + const vk::AttachmentDescription& attachment_desc, const std::shared_ptr& command_buffer, bool resolve_texture = false) { const auto& texture = resolve_texture ? attachment.resolve_texture : attachment.texture; if (!texture) { - return {}; + return; } const auto& texture_vk = TextureVK::Cast(*texture); - vk::AttachmentDescription attachment_desc = foo(attachment, resolve_texture); if (attachment_desc.initialLayout == vk::ImageLayout::eGeneral) { BarrierVK barrier; @@ -96,8 +97,6 @@ static vk::AttachmentDescription CreateAttachmentDescription( // Instead of transitioning layouts manually using barriers, we are going to // make the subpass perform our transitions. texture_vk.SetLayoutWithoutEncoding(attachment_desc.finalLayout); - - return attachment_desc; } SharedHandleVK RenderPassVK::CreateVKRenderPass( @@ -127,13 +126,14 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( color_refs[bind_point] = vk::AttachmentReference{static_cast(attachments.size()), vk::ImageLayout::eColorAttachmentOptimal}; - attachments.emplace_back( - CreateAttachmentDescription(color, command_buffer)); + attachments.emplace_back(CreateAttachmentDescription(color)); + SetTextureLayout(color, attachments.back(), command_buffer); if (color.resolve_texture) { resolve_refs[bind_point] = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eGeneral}; attachments.emplace_back( - CreateAttachmentDescription(color, command_buffer, true)); + CreateAttachmentDescription(color, true)); + SetTextureLayout(color, attachments.back(), command_buffer, true); } } @@ -142,7 +142,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; attachments.emplace_back( - CreateAttachmentDescription(depth.value(), command_buffer)); + CreateAttachmentDescription(depth.value())); + SetTextureLayout(depth.value(), attachments.back(), command_buffer); } if (auto stencil = render_target_.GetStencilAttachment(); @@ -151,7 +152,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; attachments.emplace_back( - CreateAttachmentDescription(stencil.value(), command_buffer)); + CreateAttachmentDescription(stencil.value())); + SetTextureLayout(stencil.value(), attachments.back(), command_buffer); } vk::SubpassDescription subpass_desc; From 51c5c2da28f491dee1edaa83539f33ff9146ee2e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 24 Aug 2023 16:07:36 -0700 Subject: [PATCH 3/3] switched to ivar pointers --- .../renderer/backend/vulkan/render_pass_vk.cc | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 70420bba3c5fd..5d221971d531d 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -31,9 +31,8 @@ namespace impeller { static vk::AttachmentDescription CreateAttachmentDescription( const Attachment& attachment, - bool resolve_texture = false) { - const auto& texture = - resolve_texture ? attachment.resolve_texture : attachment.texture; + const std::shared_ptr Attachment::*texture_ptr) { + const auto& texture = attachment.*texture_ptr; if (!texture) { return {}; } @@ -50,7 +49,7 @@ static vk::AttachmentDescription CreateAttachmentDescription( if (desc.storage_mode == StorageMode::kDeviceTransient) { store_action = StoreAction::kDontCare; - } else if (resolve_texture) { + } else if (texture_ptr == &Attachment::resolve_texture) { store_action = StoreAction::kStore; } @@ -72,9 +71,8 @@ static void SetTextureLayout( const Attachment& attachment, const vk::AttachmentDescription& attachment_desc, const std::shared_ptr& command_buffer, - bool resolve_texture = false) { - const auto& texture = - resolve_texture ? attachment.resolve_texture : attachment.texture; + const std::shared_ptr Attachment::*texture_ptr) { + const auto& texture = attachment.*texture_ptr; if (!texture) { return; } @@ -126,14 +124,17 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( color_refs[bind_point] = vk::AttachmentReference{static_cast(attachments.size()), vk::ImageLayout::eColorAttachmentOptimal}; - attachments.emplace_back(CreateAttachmentDescription(color)); - SetTextureLayout(color, attachments.back(), command_buffer); + attachments.emplace_back( + CreateAttachmentDescription(color, &Attachment::texture)); + SetTextureLayout(color, attachments.back(), command_buffer, + &Attachment::texture); if (color.resolve_texture) { resolve_refs[bind_point] = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eGeneral}; attachments.emplace_back( - CreateAttachmentDescription(color, true)); - SetTextureLayout(color, attachments.back(), command_buffer, true); + CreateAttachmentDescription(color, &Attachment::resolve_texture)); + SetTextureLayout(color, attachments.back(), command_buffer, + &Attachment::resolve_texture); } } @@ -142,8 +143,9 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; attachments.emplace_back( - CreateAttachmentDescription(depth.value())); - SetTextureLayout(depth.value(), attachments.back(), command_buffer); + CreateAttachmentDescription(depth.value(), &Attachment::texture)); + SetTextureLayout(depth.value(), attachments.back(), command_buffer, + &Attachment::texture); } if (auto stencil = render_target_.GetStencilAttachment(); @@ -152,8 +154,9 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; attachments.emplace_back( - CreateAttachmentDescription(stencil.value())); - SetTextureLayout(stencil.value(), attachments.back(), command_buffer); + CreateAttachmentDescription(stencil.value(), &Attachment::texture)); + SetTextureLayout(stencil.value(), attachments.back(), command_buffer, + &Attachment::texture); } vk::SubpassDescription subpass_desc;