VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912 spec clarification

[juan-lunarg]

For context we received complaints about our validation of VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912

Here is the relevant documentation: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkCmdEndDebugUtilsLabelEXT.html

Here is the issue which started this: KhronosGroup/Vulkan-ValidationLayers#5671
Here is my PR: KhronosGroup/Vulkan-ValidationLayers#5941

Our validation of VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912 was invalid.

We were checking per command buffer instead of per queue.

VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912: There must be an outstanding vkCmdBeginDebugUtilsLabelEXT command prior to the vkCmdEndDebugUtilsLabelEXT on the queue that commandBuffer is submitted to

As a result I intended to move the validation to vkQueueSubmit/vkQueueSubmit2 (Which as @spencer-lunarg pointed out isn't enough. It also would need to happen at vkQueueBindSparse).

However, I ran into confusion when reading the spec

An application may open a debug label region in one command buffer and close it in another, or otherwise split debug label regions across multiple command buffers or multiple queue submissions. When viewed from the linear series of submissions to a single queue, the calls to vkCmdBeginDebugUtilsLabelEXT and vkCmdEndDebugUtilsLabelEXT must be matched and balanced.

The main trouble is the or multiple queue submissions part.

Which makes it sounds like you can open a debug label region on 1 queue. Submit. Then end the debug label, then submit. And it's still valid.

As a result I asked @MarkY-LunarG (The author of this extension) for clarification and they said

I was working with the AMD people who were using this, and we really can't validate it. I thought we added a note that we might not be able to validate it because of all this.

However, based on feedback from @pdaniell-nv I think we can ignore the GPU side and do the validation based only on the VkQueue object the command buffers are submitted on (Although @pdaniell-nv admitted they may be missing some needed context).

[akb825]

Speaking as a user of the extension, I rely on the "or multiple queue submissions" part. Certain operations require a queue submission that may not fit perfectly within logical debugging blocks. For example, when adding a fence or semaphore for synchronization you may want to submit it sooner to allow the GPU to process it before synchronizing on the CPU.

At least for my project, I have a higher level rendering library that has separate backend implementations for various graphics APIs, including Vulkan. In this case it would be especially annoying to be limited to matched push/pops on a single queue submission because the higher level code that sets up debug blocks may not be fully in control or aware of when queue submissions will occur.

[spencer-lunarg]

Discussed on the Working Group call this week

Agree this VU as-is, is not possible to validate. I assigned myself to look into possibly moving the VU or add the spec clarification

[juan-lunarg]

Speaking as a user of the extension, I rely on the "or multiple queue submissions" part. Certain operations require a queue submission that may not fit perfectly within logical debugging blocks. For example, when adding a fence or semaphore for synchronization you may want to submit it sooner to allow the GPU to process it before synchronizing on the CPU.

If this functionality is useful to developers it should be kept. It should be clarified better in the spec though to avoid confusion. This does ultimately mean the VU as-is is not possible to validate as @spencer-lunarg mentioned.

In particular the VUID currently says

VUID-vkCmdEndDebugUtilsLabelEXT-commandBuffer-01912 : There must be an outstanding vkCmdBeginDebugUtilsLabelEXT command prior to the vkCmdEndDebugUtilsLabelEXT on the queue that commandBuffer is submitted to

Which is confusing when you contrast it with "or multiple queue submissions" in the extension description.

IE: the VUID implies 1 queue. While the extension description implies 1 or more queues.

I'm unsure how to re-write the VUID properly to match the existing description.