Repository navigation
fix(converter): set DESCRIPTOR_BUFFER_EXT pipeline flag and use device-address-aware buffer allocation - #40
Merged
Conversation
…e-address-aware buffer allocation The color-space-conversion compute pipeline binds a descriptor buffer at dispatch time (the descriptor set layout is created with DESCRIPTOR_BUFFER_EXT), but the compute pipeline itself was created without that flag. Per the Vulkan spec this is undefined behaviour (VUID-vkCmdDispatch-None-08600) -- the shader effectively dispatches with unbound descriptor bindings. Also switches the output buffer allocation to the device-address-aware allocator, since get_buffer_device_address() on memory not allocated with VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT is likewise undefined behaviour.
Contributor
|
Code looks sane to me as well, though since I'd still merge this as a fix assuming it has no regressions with other GPU vendors, it's still good until the new extension can be made use of 👍 |
Owner
|
Apologies for the long wait, I will try to look into this soon! |
Owner
|
Looks good to me, good catch! |
hgaiser
pushed a commit
that referenced
this pull request
Oct 18, 2026
The converter binds two descriptors, a sampled source image and an output buffer, and rewrites both on every dispatch. It did that through VK_EXT_descriptor_buffer, which is now deprecated in favour of VK_EXT_descriptor_heap, and which was a heavy tool for two descriptors: it needed bufferDeviceAddress, descriptorBufferCaptureReplay, a mapped host buffer, per-binding offset queries, device-address memory, and a pipeline flag that was missing until #40 added it. Push descriptors write both straight into the command buffer. There is no descriptor memory, no device addresses and nothing to enable beyond VK_KHR_push_descriptor, which every desktop driver has had for years and which is core in Vulkan 1.4. Descriptor heaps would be the replacement for a large, long-lived descriptor set; this is not one. The shorter feature list matters most for a caller that hands pixelforge its own device, which would otherwise have to turn on descriptor buffers and capture-replay just so the converter can bind two descriptors. Removing the descriptor-buffer block also fixes two features that had ended up inside it by accident: samplerYcbcrConversion and the 2-plane 4:4:4 formats were only enabled on devices that also had descriptor buffers, even though the 4:4:4 extension itself was enabled regardless. BREAKING CHANGE: VideoContext::has_descriptor_buffer is replaced by VideoContext::has_push_descriptor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The color-space-conversion compute pipeline (
ColorConverter) binds adescriptor buffer at dispatch time -- its descriptor set layout is created
with
DESCRIPTOR_BUFFER_EXT-- but the compute pipeline itself was createdwithout that flag.
Per the Vulkan spec this is undefined behaviour
(
VUID-vkCmdDispatch-None-08600): the compute shader effectively dispatcheswith unbound descriptor bindings, which can manifest as GPU memory-safety
issues (arbitrary reads/writes) on every dispatch. On our hardware
(RDNA3, RX 7900 XTX) this reliably contributed to sporadic amdgpu/VCN
instability during long streaming sessions.
Changes
.flags(vk::PipelineCreateFlags::DESCRIPTOR_BUFFER_EXT)on theComputePipelineCreateInfoused to build the converter pipeline, matchingthe flag already used for its descriptor set layout.
create_bufferhelper to the crate's existingcreate_buffer_with_device_address, sinceget_buffer_device_address()on memory not allocated with
VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BITislikewise undefined behaviour. This also removes a duplicate, slightly
less robust local
create_buffer(it didn't clean up the buffer/memoryon allocation or bind failure, unlike the shared helper it now uses).
No public API changes; behaviour is otherwise unchanged.
Testing
cargo build --releasepasses.this fix) on RX 7900 XTX / Mesa RADV, where the previous undefined
behaviour was suspected of contributing to sporadic GPU crashes in
concurrently-running Vulkan/D3D12 applications sharing the same GPU.
🤖 AI-generated — posted by the
engineeragent via opencode, on behalf of @afriemann.personal note: frankly, I have no clue if this is right. It does seem to fix the issue but I might have just been lucky and haven't tested it long term. The code seems reasonable and I tried to verify as much as possible, but my rust is terrible and I don't know vulkan much. Feel free to just close if it's nonsense.