attackgoat / screen-13

Screen 13 is an easy-to-use Vulkan rendering engine in the spirit of QBasic.
Apache License 2.0
251 stars 13 forks source link

Fix: Attempted write update to an immutable sampler descriptor. #76

Closed DGriffin91 closed 2 months ago

DGriffin91 commented 2 months ago

The image_sampler example with --separate (with either hlsl or glsl) was resulting in this vulkan validation error:

VUID-VkWriteDescriptorSet-descriptorType-02752(ERROR / SPEC): msgNum: -1422077154 - Validation Error: [ VUID-VkWriteDescriptorSet-descriptorType-02752 ] Object 0: handle = 0x625f640000000058, type = VK_OBJECT_TYPE_DESCRIPTOR_SET; 
| MessageID = 0xab3cd31e | vkUpdateDescriptorSets(): pDescriptorWrites[1] Attempted write update to an immutable sampler descriptor. The Vulkan spec states: If descriptorType is VK_DESCRIPTOR_TYPE_SAMPLER, then dstSet must not have been allocated with a layout that included immutable samplers for dstBinding (https://vulkan.lunarg.com/doc/view/1.3.280.0/windows/1.3-extensions/vkspec.html#VUID-VkWriteDescriptorSet-descriptorType-02752)

It was also crashing if run in render doc.

I think the sampler does not need to be updated in the descriptor binding in this case since it is already included immutably.

Sorry it took me so long to get around to testing this. Thanks so much for implementing it! Besides this issue it seems to be working great!

attackgoat commented 2 months ago

Thanks for debugging and fixing this!

After re-reviewing #72 I'm wondering why I decided to merge without fixing this. I do recall hitting this exact problem while making the change. At one point I was going to allow submitting dynamic samplers using the render graph, but the code is left in a half-way state in between these ideas.

Making things worse, there are additional validation errors introduced with the latest Vulkan SDK which are fixed on an unpublished branch based on #75, so there will be errors related to swapchain and uint8 regardless.

One tiny edit I'll make is that the entire for loop may be removed