Missing image layout validation on draw

[ShabbyX]

I came across a bug recently where an image was not transitioned to the right layout, and the validation layers didn't catch this. In short, the application does the following:

1. Transition image 1 to COLOR_ATTACHMENT_OPTIMAL
2. Transition image 2 to SHADER_READ_ONLY_OPTIMAL
3. Draw call (that copies image 2 into image 1 with some transformations)
4. Transition image 3 to COLOR_ATTACHMENT_OPTIMAL
5. Transition image 1 to SHADER_READ_ONLY_OPTIMAL
6. Draw call (that copies image 1 into image 3 for CPU read back)
7. Transition image 3 to TRANSFER_SRC_OPTIMAL
8. Copy image 3 to buffer

The bug was that step 5 was missing, i.e. image 1 was in COLOR_ATTACHMENT_OPTIMAL but was being read by a fragment shader. In effect, the color cache was not flushed and the changes in step 3 were not visible to step 6.

It looks like the validation layers don't validate the image layouts on draw call. As a test, I change step 5 to transition to TRANSFER_SRC_OPTIMAL with no complaint from the layers either.

This page indicates the image layouts are validated only in the following functions (though the page is dated):

vkCreateRenderPass
vkMapMemory
vkQueuePresentKHR
vkQueueSubmit
vkCmdCopyImage
vkCmdCopyImageToBuffer
vkCmdWaitEvents
VkCmdPipelineBarrier

Is there a reason why they are not validated on draw/dispatch calls too?

[jzulauf-lunarg]

@ShabbyX -- is there a test case you can share? This case should be covered by the existing code, and it would be good to discover and resolve any latent bugs in the image layout transition and validation code.

Background

Image layout validation is done in multiple places throughout the core_validation layer. When it is possible to know the layout at command buffer record time (for example if an image layout transition was done earlier in that command buffer), the current image layout is compared against the binding/usage in copy, dispatch, and draw calls. Where the image layout state cannot be determined that check is deferred until queue submission of the command buffer, when the required state of the image layout from the point of view of the command buffer is compared to the current/global state of the image layout.

[ShabbyX]

@jzulauf-lunarg I don't have a simple test case, but this is on ANGLE, so you can give it a try. Give me a day or two to land the change that's affected by this, then I can provide you a patch that disables the layout transition I'm talking about + instructions on how to build and what to run (and therefore trace).

[ShabbyX]

Sorry for the late reply. Here's the patch to ANGLE that can repro the issue:

https://chromium-review.googlesource.com/c/angle/angle/+/1401020

To perform the test, follow these (untried) instructions:

# Set up Google stuff and get angle. See https://chromium.googlesource.com/angle/angle/+/master/doc/DevSetup.md

# depot_tools: https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html
$ git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git
$ export PATH=$PATH:$PWD/depot_tools

# angle:
$ git clone https://chromium.googlesource.com/angle/angle
$ cd angle
$ python scripts/bootstrap.py
$ gclient sync
$ git checkout -b validation-layer-bug-repro origin/master
$ ./build/install-build-deps.sh

# apply the patch:
$ git fetch https://chromium.googlesource.com/angle/angle refs/changes/20/1401020/3 && git cherry-pick FETCH_HEAD

# build:
$ gn gen out/dbg
$ gn args -C out/dbg
# an editor is opened here.  Put `is_debug=true` for debug build.  Save and exit.
$ cd out/dbg
$ ninja

# test:
$ ./angle_end2end_tests --gtest_filter=CopyTexImageTest.RGBAToRGB/ES2_VULKAN

Observe that the test output tells that the image is in the wrong layout. That error is generated (part of the repro patch) when using an image for shader read, but the layout is neither GENERAL nor SHADER_READ_ONLY_OPTIMAL.

This indeed caused failures on some tests on AMD where the image was written to as color attachment, then read from in a shader. The validation layers don't complain about this missing transition/barrier.

[jzulauf-lunarg]

triage notes: currently the descriptor set validation at draw doesn't have the information needed to do this check (it currently isn't extracted with the other spirv static analysis). Needs to be added to the shader_validation path and probably can be plumbed through with an additional descriptor_req bit.

[jzulauf-lunarg]

"Upon further review" -- it looks like the best way to catch this is by implementing VUID-VkWriteDescriptorSet-descriptorType-01403 and allow the existing checks for descriptor layout and image layout to catch any draw time discrepancy. However ...1403 looks to be out of date, which I'm addressing separately.