Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

Incorrect image layer validation with secondary command buffers #2653

Open
fduranleau-gl opened this issue May 8, 2018 · 4 comments
Open
Assignees
Labels
Milestone

Comments

@fduranleau-gl
Copy link
Contributor

We had this situation where we used an image in a render pass both as a depth attachment and as shader resource. The render pass set the image's layout to VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, but the same image was used in a descriptor with layout VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL. So obviously, we had image layout conflicts. However, commands for the render pass were recorded into secondary command buffers, and we had no errors being reported by core validation layer. As soon as we did not use secondary command buffers, we got the errors being reported.

There is no need record those secondary command buffes in parallel in other threads to get this error. Simply doing this sequence of commands will show the problem:

  1. begin the render pass, with flag to use secondary command buffers
  2. begin a secondary command buffer, with inheritance using render pass from 1.
  3. issue a draw command using one of the render pass' attachments in a descriptor, but with a different layout
  4. end command buffer
  5. execute command buffer
  6. end render pass

Doing the above, validation reports no errors. But doing this:

  1. begin the render pass
  2. issue a draw command using one of the render pass' attachments in a descriptor, but with a different layout
  3. end render pass

triggers the expected error.

For the record, if it changes something to analyse this problem, the whole render pass in our use case is in its own primary command buffer. The rest is in other primary command buffers.

@tobine tobine added the bug label May 8, 2018
@tobine tobine added this to the P1 milestone May 8, 2018
@tobine
Copy link
Contributor

tobine commented May 8, 2018

@jzulauf-lunarg this replaces #2640

@fduranleau-gl
Copy link
Contributor Author

Indeed, I forgot to mention that.

@chrisforbes
Copy link
Collaborator

Even without the layout conflict, this seems likely to not be valid; see this bit of 7.3:

Applications must ensure that all accesses to memory that backs image subresources used as
attachments in a given renderpass instance either happen-before the load operations for those
attachments, or happen-after the store operations for those attachments.

For depth/stencil attachments, each aspect can be used separately as attachments and nonattachments
as long as the non-attachment accesses are also via an image subresource in either the
VK_IMAGE_LAYOUT_DEPTH_READ_ONLY_STENCIL_ATTACHMENT_OPTIMAL layout or the
VK_IMAGE_LAYOUT_DEPTH_ATTACHMENT_STENCIL_READ_ONLY_OPTIMAL layout, and the attachment resource
uses whichever of those two layouts the image accesses do not. Use of non-attachment aspects in
this case is only well defined if the attachment is used in the subpass where the non-attachment
access is being made, or the layout of the image subresource is constant throughout the entire
render pass instance, including the initialLayout and finalLayout.

Note
These restrictions mean that the render pass has full knowledge of all uses of all of
the attachments, so that the implementation is able to make correct decisions
about when and how to perform layout transitions, when to overlap execution of
subpasses, etc.

@fduranleau-gl
Copy link
Contributor Author

Right, I did miss that part of the spec. The situation comes from a sample using our engine that worked fine with GL and Metal. We hit this while porting to Vulkan (we would need to use an input attachment and multiple subpasses). That being said, the problem remains. I still expect to get an error to be reported when I use a secondary command buffer, whether that error is a conflict of layout or a non-attachment access during a render pass.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants