Incorrect image layer validation with secondary command buffers

[karl-lunarg]

Issue by fduranleau-gl (MIGRATED)
Tuesday May 08, 2018 at 13:32 GMT
Originally opened as KhronosGroup/Vulkan-LoaderAndValidationLayers#2653

---

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.

[karl-lunarg]

Comment by tobine (MIGRATED)
Tuesday May 08, 2018 at 13:40 GMT

---

@jzulauf-lunarg this replaces #2640

[karl-lunarg]

Comment by fduranleau-gl (MIGRATED)
Tuesday May 08, 2018 at 13:42 GMT

---

Indeed, I forgot to mention that.

[karl-lunarg]

Comment by chrisforbes (MIGRATED)
Tuesday May 08, 2018 at 16:05 GMT

---

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.

[karl-lunarg]

Comment by fduranleau-gl (MIGRATED)
Tuesday May 08, 2018 at 17:55 GMT

---

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.

[jzulauf-lunarg]

@chrisforbes -- it looks like the image layout errors from the secondary buffers can only be resolved at queue submit time, while the image layout issue within the primary command buffer could be (likely are) reported a command record time.

Does that match your understanding?

[chrisforbes]

@jzulauf-lunarg both primary and secondary cases should work pretty similarly for layout tracking -- either you have an existing requirement for the subresource layout and so validate against it, or you add a precondition to the CB.

CmdExecuteCommands is where it gets interesting -- the secondary preconditions and layout updates have to be merged into the primary at that point.

[jzulauf-lunarg]

@fduranleau-gl -- Should be fixed by PR 148

[jzulauf-lunarg]

PR merged in May. Cleaning up.