Add a new sample VK_KHR_device_address_commands. #1547
Conversation
|
This seems to run just fine for me, but I do get one validation error: |
There was a problem hiding this comment.
Pull request overview
This PR adds a new Vulkan-Samples extension sample showcasing VK_KHR_device_address_commands, demonstrating a GPU-driven rendering path that records bind/fill/update/draw operations using device addresses (instead of VkBuffer handles), along with accompanying documentation and build/docs integration.
Changes:
- Added a new
device_address_commandsextension sample implementation (C++), including runtime loading ofVK_KHR_device_address_commandsentry points and address-range barriers. - Added GLSL + Slang shader implementations for compute-driven animation/culling and indirect drawing.
- Integrated the sample into the extensions README, Antora navigation, and the sample build order list.
Reviewed changes
Copilot reviewed 16 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
samples/extensions/device_address_commands/device_address_commands.cpp |
New sample implementation using address-based fill/update/bind/draw + memory-range barriers |
samples/extensions/device_address_commands/device_address_commands.h |
Sample interface + shared constants and push-constant struct definitions |
samples/extensions/device_address_commands/CMakeLists.txt |
Build integration for the new sample and shader compilation inputs |
shaders/device_address_commands/glsl/update_objects.comp |
Compute shader (GLSL) that animates objects, culls, and emits indirect commands |
shaders/device_address_commands/glsl/render.vert |
Vertex shader (GLSL) reading camera/transforms via buffer references in push constants |
shaders/device_address_commands/glsl/render.frag |
Fragment shader (GLSL) outputting per-instance color |
shaders/device_address_commands/slang/update_objects.comp.slang |
Compute shader (Slang) equivalent using pointer-based access |
shaders/device_address_commands/slang/render.vert.slang |
Vertex shader (Slang) equivalent using pointer-based access |
shaders/device_address_commands/slang/render.frag.slang |
Fragment shader (Slang) equivalent |
samples/extensions/device_address_commands/README.adoc |
New tutorial-style documentation for the sample and extension concepts |
samples/extensions/README.adoc |
Adds the new sample to the Extensions samples README list |
samples/CMakeLists.txt |
Adds the sample to the global sample ordering list |
antora/modules/ROOT/nav.adoc |
Adds the new sample docs page to the Antora navigation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SaschaWillems
left a comment
There was a problem hiding this comment.
Sample is working for me on WIn11 and an nv RTX 4070.
I noticed a few naming/styling related thing in code that should be improved. But nothing functional.
| return false; | ||
| } | ||
|
|
||
| volkLoadDevice(get_device().get_handle()); |
There was a problem hiding this comment.
Do you need the device specific functions?
volkLoadInstance is already called by the framework. And I think, volkLoadDevice should be called there as well.
| // The compute shader sets firstInstance to the object index so the vertex | ||
| // shader can index the transform array. Non-zero firstInstance requires | ||
| // this core feature (Vulkan 1.0, universally available on desktop). | ||
| gpu.get_mutable_requested_features().drawIndirectFirstInstance = VK_TRUE; |
There was a problem hiding this comment.
Maybe check for gpu.get_features().drawIndirectFirstInstance support first?
|
|
||
| // vkCmdDrawIndexedIndirectCount2KHR is a multi-draw indirect variant; | ||
| // multiDrawIndirect is required when maxDrawCount > 1. | ||
| gpu.get_mutable_requested_features().multiDrawIndirect = VK_TRUE; |
There was a problem hiding this comment.
Maybe check for gpu.get_features().multiDrawIndirect support first?
| // Buffer device address is core in Vulkan 1.2 and required by | ||
| // VK_KHR_device_address_commands. We also request the KHR extension | ||
| // name so older loaders find it correctly. | ||
| add_device_extension(VK_KHR_BUFFER_DEVICE_ADDRESS_EXTENSION_NAME, /*optional=*/true); |
There was a problem hiding this comment.
As you already state in the comment, VK_KHR_device_address_commands is promoted to Vulkan 1.2. And as you already require Vulkan 1.3 (-> get_api_version), you don't need to add the VK_KHR_BUFFER_DEVICE_ADDRESS_EXTENSION_NAME extension.
Moreover, I think you're always using the non-KHR names, which makes adding this extension even more superfluous.
| // VK_KHR_device_address_commands. We also request the KHR extension | ||
| // name so older loaders find it correctly. | ||
| add_device_extension(VK_KHR_BUFFER_DEVICE_ADDRESS_EXTENSION_NAME, /*optional=*/true); | ||
| add_device_extension(VK_KHR_DRAW_INDIRECT_COUNT_EXTENSION_NAME); |
There was a problem hiding this comment.
VK_KHR_DRAW_INDIRECT_COUNT_EXTENSION_NAME is also promoted to Vulkan 1.2, so no need to add it here.
| add_device_extension(VK_KHR_BUFFER_DEVICE_ADDRESS_EXTENSION_NAME, /*optional=*/true); | ||
| ---- | ||
|
|
||
| === Loading function pointers |
| 0, sizeof(pc), &pc); | ||
|
|
||
| // 4 groups × 64 threads = 256 invocations, one per object. | ||
| vkCmdDispatch(cmd, kObjectCount / 64, 1, 1); |
There was a problem hiding this comment.
This only works with kObjectCount being an integer multiple of 64.
And where does that magic 64 come from?
| == Further reading | ||
|
|
||
| * Vulkan specification: https://registry.khronos.org/vulkan/specs/latest/man/html/VK_KHR_device_address_commands.html[VK_KHR_device_address_commands] | ||
| * Khronos proposal: https://docs.vulkan.org/features/latest/features/proposals/VK_KHR_device_address_commands.html[Feature proposal] |
There was a problem hiding this comment.
Maybe link to https://registry.khronos.org/vulkan/specs/latest/man/html/VK_KHR_device_address_commands.html, instead?
This requires updating Vulkan.
Tested in Ubuntu Linux on NVIDIA.
Description
Please include a summary of the change, new sample or fixed issue. Please also include relevant motivation and context.
Please read the contribution guidelines
Fixes #
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: