Skip to content

fix(converter): use vkGetDescriptorEXT for descriptor buffer population#13

Merged
hgaiser merged 1 commit into
hgaiser:mainfrom
porkloin:descriptor-buffer-fix
May 10, 2026
Merged

fix(converter): use vkGetDescriptorEXT for descriptor buffer population#13
hgaiser merged 1 commit into
hgaiser:mainfrom
porkloin:descriptor-buffer-fix

Conversation

@porkloin
Copy link
Copy Markdown
Contributor

@porkloin porkloin commented May 8, 2026

#11 seems to have introduced a regression that causes me to get a completely green video channel when streaming via moonshine HEAD. It looks like it was caused by ColorConverter shader was reading uninitialized descriptors. As far as I can tell, I think we need to use vkGetDescriptorEXT instead.

Tested on 9070XT with HEAD/main from moonshine, with git bisect on pixelforge and it looks like #11 definitely introduced it and this PR fixes it, but an extra set of eyes and test on non-AMD hardware would be appreciated.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a descriptor-buffer regression introduced after updating ash (#11) where the converter’s compute shader could read invalid/uninitialized descriptor data (observed as a constant-green output channel when streaming), by switching descriptor population to Vulkan’s runtime descriptor writer API.

Changes:

  • Populate descriptor-buffer contents via vkGetDescriptorEXT (ash::ext::descriptor_buffer::Device::get_descriptor) instead of vkGet*OpaqueCaptureDescriptorDataEXT.
  • Use the correct runtime descriptor sizes (*_descriptor_size) from VkPhysicalDeviceDescriptorBufferPropertiesEXT.
  • Track the output buffer’s size and device address to build a VkDescriptorAddressInfoEXT for the STORAGE_BUFFER binding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/converter/pipeline.rs Adds output buffer device-address usage/address tracking and switches to runtime descriptor-size queries.
src/converter/mod.rs Replaces opaque capture descriptor flow with per-frame vkGetDescriptorEXT writes into the mapped descriptor buffer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/converter/pipeline.rs
Comment thread src/converter/pipeline.rs
Comment thread src/converter/mod.rs
@hgaiser
Copy link
Copy Markdown
Owner

hgaiser commented May 10, 2026

Thanks! I didn't test this yet in Moonshine. I used the verify_all example, which interestingly did work fine.

@hgaiser hgaiser merged commit 7fa5904 into hgaiser:main May 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants