<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 13, 2018 at 3:18 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_8274797247009232337HOEnZb"><div class="m_8274797247009232337h5">On Mon, Feb 05, 2018 at 02:34:59PM -0800, Jason Ekstrand wrote: > This requires us to ditch the VkAttachmentReference struct in favor of > an anv-specific struct. However, we can now easily identify from just > the subpass attachment what kind of an attachment it is. This will make > iteration over anv_subpass::attachments a little easier in some case. > --- > src/intel/vulkan/anv_pass.c | 35 +++++++++++++++++++++++++++-------- > src/intel/vulkan/anv_private.h | 16 +++++++++++----- > src/intel/vulkan/genX_cmd_buffer.c | 2 +- > 3 files changed, 39 insertions(+), 14 deletions(-) > > diff --git a/src/intel/vulkan/anv_pass.c b/src/intel/vulkan/anv_pass.c > index a77e52b..5b8b138 100644 > --- a/src/intel/vulkan/anv_pass.c > +++ b/src/intel/vulkan/anv_pass.c > @@ -65,7 +65,7 @@ VkResult anv_CreateRenderPass( > anv_multialloc_add(&ma, &attachments, pCreateInfo->attachmentCount); > anv_multialloc_add(&ma, &subpass_flushes, pCreateInfo->subpassCount + 1); > > - VkAttachmentReference *subpass_attachments; > + struct anv_subpass_attachment *subpass_attachments; > uint32_t subpass_attachment_count = 0; > for (uint32_t i = 0; i < pCreateInfo->subpassCount; i++) { > subpass_attachment_count += > @@ -117,7 +117,11 @@ VkResult anv_CreateRenderPass( > > for (uint32_t j = 0; j < desc->inputAttachmentCount; j++) { > uint32_t a = desc->pInputAttachments[j].attachment; > - subpass->input_attachments[j] = desc->pInputAttachments[j]; > + subpass->input_attachments[j] = (struct anv_subpass_attachment) { > + .usage = VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT, > + .attachment = desc->pInputAttachments[j].attachment, > + .layout = desc->pInputAttachments[j].layout, > + }; > if (a != VK_ATTACHMENT_UNUSED) { > has_input = true; > pass->attachments[a].usage |= VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT; > @@ -138,7 +142,11 @@ VkResult anv_CreateRenderPass( > > for (uint32_t j = 0; j < desc->colorAttachmentCount; j++) { > uint32_t a = desc->pColorAttachments[j].attachment; > - subpass->color_attachments[j] = desc->pColorAttachments[j]; > + subpass->color_attachments[j] = (struct anv_subpass_attachment) { > + .usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT, > + .attachment = desc->pColorAttachments[j].attachment, > + .layout = desc->pColorAttachments[j].layout, > + }; </div></div>I kind of liked reusing the VkAttachmentReference struct instead of having to roll our own.</blockquote><div> </div><div>I used to think that way. However, as we extend stuff, it tends to stop being practical. There are a number of cases where we started off with Vulkan structures and then ended up no longer using them. This is especially prone to happen with extensions. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">What do you think about creating helper functions, like the following instead: static bool is_color_attachment(struct anv_subpass* subpass, VkAttachmentReference* att_ref) { return att_ref >= subpass->color_attachments && att_ref < subpass->color_attachments + subpass->color_count; } </blockquote><div> </div><div>I did exactly that in earlier versions of this series. However, when I found myself trying to write an is_depth_attachment function it got messy and I decided it was easier to just stash the usage. It's actually rather convenient. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -Nanley <div><div class="m_8274797247009232337h5"> > if (a != VK_ATTACHMENT_UNUSED) { > has_color = true; > pass->attachments[a].usage |= VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT; > @@ -157,7 +165,11 @@ VkResult anv_CreateRenderPass( > > for (uint32_t j = 0; j < desc->colorAttachmentCount; j++) { > uint32_t a = desc->pResolveAttachments[j].attachment; > - subpass->resolve_attachments[j] = desc->pResolveAttachments[j]; > + subpass->resolve_attachments[j] = (struct anv_subpass_attachment) { > + .usage = VK_IMAGE_USAGE_TRANSFER_DST_BIT, > + .attachment = desc->pResolveAttachments[j].attachment, > + .layout = desc->pResolveAttachments[j].layout, > + }; > if (a != VK_ATTACHMENT_UNUSED) { > subpass->has_resolve = true; > uint32_t color_att = desc->pColorAttachments[j].attachment; > @@ -174,8 +186,12 @@ VkResult anv_CreateRenderPass( > > if (desc->pDepthStencilAttachment) { > uint32_t a = desc->pDepthStencilAttachment->attachment; > - *subpass_attachments++ = subpass->depth_stencil_attachment = > - *desc->pDepthStencilAttachment; > + subpass->depth_stencil_attachment = (struct anv_subpass_attachment) { > + .usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT, > + .attachment = desc->pDepthStencilAttachment->attachment, > + .layout = desc->pDepthStencilAttachment->layout, > + }; > + *subpass_attachments++ = subpass->depth_stencil_attachment; > if (a != VK_ATTACHMENT_UNUSED) { > has_depth = true; > pass->attachments[a].usage |= > @@ -186,8 +202,11 @@ VkResult anv_CreateRenderPass( > *desc->pDepthStencilAttachment); > } > } else { > - subpass->depth_stencil_attachment.attachment = VK_ATTACHMENT_UNUSED; > - subpass->depth_stencil_attachment.layout = VK_IMAGE_LAYOUT_UNDEFINED; > + subpass->depth_stencil_attachment = (struct anv_subpass_attachment) { > + .usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT, > + .attachment = VK_ATTACHMENT_UNUSED, > + .layout = VK_IMAGE_LAYOUT_UNDEFINED, > + }; > } > } > > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h > index d424498..9a8da2b 100644 > --- a/src/intel/vulkan/anv_private.h > +++ b/src/intel/vulkan/anv_private.h > @@ -2865,6 +2865,12 @@ struct anv_framebuffer { > struct anv_image_view * attachments[0]; > }; > > +struct anv_subpass_attachment { > + VkImageUsageFlagBits usage; > + uint32_t attachment; > + VkImageLayout layout; > +}; > + > struct anv_subpass { > uint32_t attachment_count; > > @@ -2872,14 +2878,14 @@ struct anv_subpass { > * A pointer to all attachment references used in this subpass. > * Only valid if ::attachment_count > 0. > */ > - VkAttachmentReference * attachments; > + struct anv_subpass_attachment * attachments; > uint32_t input_count; > - VkAttachmentReference * input_attachments; > + struct anv_subpass_attachment * input_attachments; > uint32_t color_count; > - VkAttachmentReference * color_attachments; > - VkAttachmentReference * resolve_attachments; > + struct anv_subpass_attachment * color_attachments; > + struct anv_subpass_attachment * resolve_attachments; > > - VkAttachmentReference depth_stencil_attachment; > + struct anv_subpass_attachment depth_stencil_attachment; > > uint32_t view_mask; > > diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c > index 2590ea3..f92e86f 100644 > --- a/src/intel/vulkan/genX_cmd_buffer.c > +++ b/src/intel/vulkan/genX_cmd_buffer.c > @@ -3310,7 +3310,7 @@ cmd_buffer_subpass_transition_layouts(struct anv_cmd_buffer * const cmd_buffer, > assert(subpass->attachments); > > /* Iterate over the array of attachment references. */ > - for (const VkAttachmentReference *att_ref = subpass->attachments; > + for (const struct anv_subpass_attachment *att_ref = subpass->attachments; > att_ref < subpass->attachments + subpass->attachment_count; att_ref++) { > > /* If the attachment is unused, we can't perform a layout transition. */ > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>