-
Notifications
You must be signed in to change notification settings - Fork 618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix fragment_shading_rate_dynamic sample #916
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, was a bit hasty with my approval. Even with this fix applied, the sample crashes for me with the following error:
error] [framework\core\instance.cpp:50] -1360208885 - VUID-vkQueueSubmit-pCommandBuffers-00070: Validation Error: [ VUID-vkQueueSubmit-pCommandBuffers-00070 ] Object 0: handle = 0x247718630e0, type = VK_OBJECT_TYPE_COMMAND_BUFFER; | MessageID = 0xaeecdc0b | vkQueueSubmit(): pSubmits[0].pCommandBuffers[0] VkCommandBuffer 0x247718630e0[] is unrecorded and contains no commands. The Vulkan spec states: Each element of the pCommandBuffers member of each element of pSubmits must be in the pending or executable state (https://vulkan.lunarg.com/doc/view/1.3.275.0/windows/1.3-extensions/vkspec.html#VUID-vkQueueSubmit-pCommandBuffers-00070)
I'm actually not sure why that sample works at all. It allocates the compute command buffers from the base class command pool, which may be limited to the graphics queue family index. Imo it should be adjusted to create it's own compute command pool, like we do in the other compute related samples. |
I'm actually a bit confused, because I recently did a batch test against my allocated-refactor branch which does NOT have this fix and it passed without crashing. And the error you're seeing is what I was getting before I made this change on the sample, so I think the sample is more subtle than I suspected at first...
I initially tried that approach because I noticed that I'm going to give that another shot and test it again checking both release and debug modes this time. |
OK, so using a dedicated compute queue for the compute operations would require significant refactoring of the example. While the acquire and release barriers in the compute command buffers are pretty straight-forward, it's not immediately clear to me where they'd go on the graphics side. It looks like the acquire would have to go in the |
Description
Fix the declaration of the compute_buffer in a loop as an r-value reference to a simple reference.
Fixes #913
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
Sample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: