Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Refactor: Create attachment descriptions without setting layouts #45088

Merged
gaaclarke merged 3 commits into from
Aug 25, 2023
Merged

[Impeller] Refactor: Create attachment descriptions without setting layouts #45088

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9de782c
[Impeller] split out creation of renderpass with setting the layout b…
gaaclarke Aug 24, 2023
de98852
split up creating the desc and setting the layout
gaaclarke Aug 24, 2023
51c5c2d
switched to ivar pointers
gaaclarke Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 39 additions & 20 deletions impeller/renderer/backend/vulkan/render_pass_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ namespace impeller {

static vk::AttachmentDescription CreateAttachmentDescription(
const Attachment& attachment,
const std::shared_ptr<CommandBufferVK>& command_buffer,
bool resolve_texture = false) {
const auto& texture =
resolve_texture ? attachment.resolve_texture : attachment.texture;
const std::shared_ptr<Texture> Attachment::*texture_ptr) {
Copy link
Contributor

@jonahwilliams jonahwilliams Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this syntax?

Copy link
Member Author

@gaaclarke gaaclarke Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a pointer to a field. It allows us to specify if we want to grab Attachment::texture or Attachment::resolve_texture instead of using an optional bool argument. The pointers are const static. Its easier to check out the calling site.

const auto& texture = attachment.*texture_ptr;
if (!texture) {
return {};
}
Expand All @@ -51,12 +49,36 @@ 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;
}

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 void SetTextureLayout(
const Attachment& attachment,
const vk::AttachmentDescription& attachment_desc,
const std::shared_ptr<CommandBufferVK>& command_buffer,
const std::shared_ptr<Texture> Attachment::*texture_ptr) {
const auto& texture = attachment.*texture_ptr;
if (!texture) {
return;
}
const auto& texture_vk = TextureVK::Cast(*texture);

if (attachment_desc.initialLayout == vk::ImageLayout::eGeneral) {
BarrierVK barrier;
barrier.new_layout = vk::ImageLayout::eGeneral;
barrier.cmd_buffer = command_buffer->GetEncoder()->GetCommandBuffer();
Expand All @@ -68,22 +90,11 @@ 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);

return attachment_desc;
}

SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
Expand Down Expand Up @@ -114,12 +125,16 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
vk::AttachmentReference{static_cast<uint32_t>(attachments.size()),
vk::ImageLayout::eColorAttachmentOptimal};
attachments.emplace_back(
CreateAttachmentDescription(color, command_buffer));
CreateAttachmentDescription(color, &Attachment::texture));
SetTextureLayout(color, attachments.back(), command_buffer,
&Attachment::texture);
if (color.resolve_texture) {
resolve_refs[bind_point] = vk::AttachmentReference{
static_cast<uint32_t>(attachments.size()), vk::ImageLayout::eGeneral};
attachments.emplace_back(
CreateAttachmentDescription(color, command_buffer, true));
CreateAttachmentDescription(color, &Attachment::resolve_texture));
SetTextureLayout(color, attachments.back(), command_buffer,
&Attachment::resolve_texture);
}
}

Expand All @@ -128,7 +143,9 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
static_cast<uint32_t>(attachments.size()),
vk::ImageLayout::eDepthStencilAttachmentOptimal};
attachments.emplace_back(
CreateAttachmentDescription(depth.value(), command_buffer));
CreateAttachmentDescription(depth.value(), &Attachment::texture));
SetTextureLayout(depth.value(), attachments.back(), command_buffer,
&Attachment::texture);
}

if (auto stencil = render_target_.GetStencilAttachment();
Expand All @@ -137,7 +154,9 @@ SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass(
static_cast<uint32_t>(attachments.size()),
vk::ImageLayout::eDepthStencilAttachmentOptimal};
attachments.emplace_back(
CreateAttachmentDescription(stencil.value(), command_buffer));
CreateAttachmentDescription(stencil.value(), &Attachment::texture));
SetTextureLayout(stencil.value(), attachments.back(), command_buffer,
&Attachment::texture);
}

vk::SubpassDescription subpass_desc;
Expand Down