On Tue, Feb 13, 2018 at 03:23:42PM -0800, Jason Ekstrand wrote:
> On Tue, Feb 13, 2018 at 3:18 PM, Nanley Chery <nanleych...@gmail.com> wrote:
>
> > 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,
> > > + };
> >
> > I kind of liked reusing the VkAttachmentReference struct instead of
> > having to roll our own.
>
>
> 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.
>
>
> > 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;
> > }
> >
>
> 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.
>
Ah, okay. Yeah, for depth we'd probably have to go so far as to inspect
the image view.
-Nanley
> --Jason
>
>
> > -Nanley
> >
> >
> > > 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].att
> > achment;
> > > @@ -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
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
