attackgoat / screen-13

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

Unable to use multiple descriptor sets in graphics pipeline #53

Closed trevex closed 1 year ago

trevex commented 1 year ago

I tried to extend the bindless-example and also bind storage buffers to be used by the pipeline. The source-code can be seen in this gist. I copied in the BufferUploader helper class that I started using to bulk upload buffers to device-local/unmappable memory. (If it seems interest I can create a PR.)

When I run the code, the application crashes with the following error:

# ... lots lines about creating the images and copying buffers
2023-01-13T13:43:16.016728Z TRACE screen_13::driver::device: waiting...
2023-01-13T13:43:16.017945Z TRACE screen_13::driver::graphic: create
2023-01-13T13:43:16.019378Z  INFO screen_13::driver::shader: vertex attribute 0.0: R32_UINT (offset=0)
2023-01-13T13:43:16.019454Z TRACE screen_13::driver::shader: binding chunk_buffers: 2.0 = UniformBuffer[0]
2023-01-13T13:43:16.019554Z TRACE screen_13::driver::shader: binding : 1.0 = StorageBuffer(ReadWrite)[1]
2023-01-13T13:43:16.019668Z TRACE screen_13::driver::shader: binding textures: 0.0 = CombinedImageSampler[0]
2023-01-13T13:43:16.019725Z DEBUG screen_13::driver::shader: image binding textures using default sampler
2023-01-13T13:43:16.020901Z TRACE screen_13::driver::graphic: detected write attachment 0
2023-01-13T13:43:16.022442Z DEBUG screen_13::event_loop: first frame dt: 0.016666668
2023-01-13T13:43:16.022508Z TRACE screen_13::event_loop: ðŸŸĨðŸŸĐðŸŸĶ Event::RedrawRequested
2023-01-13T13:43:16.028956Z TRACE screen_13::event_loop: received 8 events
2023-01-13T13:43:16.029998Z DEBUG screen_13::driver::swapchain: Swapchain image count: 3
2023-01-13T13:43:16.030097Z DEBUG screen_13::driver::swapchain: Presentation mode: FIFO
2023-01-13T13:43:16.033281Z  INFO screen_13::driver::swapchain: Swapchain dimensions: 512x512
2023-01-13T13:43:16.035246Z TRACE screen_13::event_loop: ✅✅✅ render graph construction: 6159 ξs (0% load)
2023-01-13T13:43:16.035303Z TRACE screen_13::display: present_image
2023-01-13T13:43:16.036115Z DEBUG screen_13::graph::resolver: schedule: [0: Test]
2023-01-13T13:43:16.036212Z TRACE screen_13::graph::resolver: leasing [0: Test]
2023-01-13T13:43:16.038265Z TRACE screen_13::driver::render_pass: create
2023-01-13T13:43:16.038444Z TRACE screen_13::graph::resolver: recording pass [0: Test]
2023-01-13T13:43:16.039113Z TRACE screen_13::graph::resolver:   writing 3 descriptors (65 buffers, 64 images)
2023-01-13T13:43:16.039433Z  INFO screen_13::driver::instance: VUID-VkWriteDescriptorSet-descriptorType-00330
2023-01-13T13:43:16.039456Z ERROR screen_13::driver::instance: 🆘 vkUpdateDescriptorSets() pDescriptorWrites[0] failed write update validation for VkDescriptorSet 0x2158a00000000fc[] with error: Write update to VkDescriptorSet 0x2158a00000000fc[] allocated with VkDescriptorSetLayout 0x7675b400000000eb[] binding #0 failed with error message: Attempted write update to buffer descriptor failed due to: Buffer (VkBuffer 0xc4f3070000000021[]) with usage mask 0x22 being used for a descriptor update of type VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER does not have VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT set.. The Vulkan spec states: If descriptorType is VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER or VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC, the buffer member of each element of pBufferInfo must have been created with VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT set (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkWriteDescriptorSet-descriptorType-00330)
2023-01-13T13:43:16.039491Z DEBUG screen_13::driver::instance: 🛑 PARKING THREAD `main` -> attach debugger to pid 1567066!

I tried to dig through Screen 13's source, but am unsure why the chunk_buffers is logged as UniformBuffer. I suspect this is reflected incorrectly, but am yet to fully understand src/driver/shader.rs.

Any help would be appreciated. I ported the code from a prior experiment with Vulkano, so I either used Screen 13 wrong or there is indeed a bug.

trevex commented 1 year ago

Unfortunately the reported error is not always the same, but can also be:

2023-01-13T18:52:08.665297Z ERROR screen_13::driver::instance: 🆘 vkUpdateDescriptorSets() pDescriptorWrites[0] failed write update validation for VkDescriptorSet 0x2158a00000000fc[] with error: Attempting write update to VkDescriptorSet 0x2158a00000000fc[] allocated with VkDescriptorSetLayout 0x71a35600000000e9[] binding #0 with type VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER but update type is VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER. The Vulkan spec states: descriptorType must match the type of dstBinding within dstSet (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkWriteDescriptorSet-descriptorType-00319)

I did some debugging and set a breakpoint here: https://github.com/attackgoat/screen-13/blob/master/src/graph/resolver.rs#L2641

I double-checked the sets and bindings and the ids are correct and the application goes through each one exactly once before crashing. I did not double-check the descriptor type.

In another debugging session I set a breakpoint here just before updating the descriptor sets: https://github.com/attackgoat/screen-13/blob/master/src/graph/resolver.rs#L2830 rust-gdb was hesitant to let me inspect descriptors properly, but what was a surprise to me with my limited knowledge of Screen 13 is, that L2830-breakpoint is hit twice before crashing. I was expecting a single descriptor update.

EDIT: not sure if hitting breakpoint twice is due to breaking on the line of a macro

trevex commented 1 year ago

I added a debug!-log to Screen 13 and it looks like the bindless storage buffer array is not properly reflected as STORAGE_BUFFER but UNIFORM_BUFFER, but even so the above errors indicate that the DescriptorSets are mismatched, For the time being I added UNIFORM_BUFFER usage to the relevant buffers. This now also adds a new error to the list (the above errors still happen, just random which one I get):

2023-01-13T19:45:19.507570Z ERROR screen_13::driver::instance: 🆘 vkCreateShaderModule(): The SPIR-V Capability (StorageBuffer16BitAccess) was declared, but none of the requirements were met to use it. The Vulkan spec states: If pCode declares any of the capabilities listed in the SPIR-V Environment appendix, one of the corresponding requirements must be satisfied (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkShaderModuleCreateInfo-pCode-01091)

While I do not fully understand the code in src/driver/mod.rs this feature seems to be not available in Screen 13, yet.

Unfortunately my experience with Vulkan and Rust is limited so at this point I will halt my investigation.

attackgoat commented 1 year ago

This weekend I should be able to learn much more about this and the other issues; for 16bit buffers there is a device feature that needs to be enabled; I ran into that on a private branch and have a fix that I am ready to push up but the fallout changes are pretty large on device creation (user standpoint it gets easier) - this will be v0.9.

trevex commented 1 year ago

Looking forward to v0.9. :+1:

There seems to be two issues I am experiencing right now (excluding 16bit support):

  1. When I use a single DescriptorSet rather than 3, I get rid of the vkUpdateDescriptorSets errors, so there seems to be an issue with using multiple DescriptorSets.
  2. When I then remove the use of 16bit, I get the following error:
    2023-01-14T06:00:47.429667Z ERROR screen_13::driver::instance: 🆘 Type mismatch on descriptor slot 0.2 (expected `VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC`) but descriptor of type VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER The Vulkan spec states: layout must be consistent with all shaders specified in pStages (https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkGraphicsPipelineCreateInfo-layout-00756)

    Which brings me back to the binding that is reflected as UNIFORM_BUFFER, but should be a STORAGE_BUFFER:

    2023-01-14T06:00:47.411752Z TRACE screen_13::driver::shader: binding chunk_buffers: 0.2 = UniformBuffer[0]
    2023-01-14T06:00:47.411770Z TRACE screen_13::driver::shader: binding : 0.1 = StorageBuffer(ReadWrite)[1]
    2023-01-14T06:00:47.411785Z TRACE screen_13::driver::shader: binding textures: 0.0 = CombinedImageSampler[0]

    When I temporarily patch Screen 13 to return DescriptorInfo::StorageBuffer(...) instead, the code runs without errors: https://github.com/attackgoat/screen-13/blob/master/src/driver/shader.rs#L528

Well, I still see only a single triangle instead of a quad, but no validation errors, so most likely something I did wrong.

trevex commented 1 year ago

I fixed issues in my code and with the hacky patch mentioned above, the following code creates the same output as the bindless example: https://gist.github.com/trevex/6dd09a9a97a6a3eb8a091ba44c424fad

Regarding the second issue I suspect this might be a spirq issue, so I will try to reproduce this

trevex commented 1 year ago

Well was not a spirq issue but my fault. I did not specify the Vulkan version using inline-spirv (which I did when I previously used shaderc directly).

Related issue: https://github.com/PENGUINLIONG/spirq-rs/issues/78

So apart from 16bit support, the only real bug seems to be using multiple descriptor sets with Screen 13.

attackgoat commented 1 year ago

So three issues - and I think we have solutions for all now:

Both gists run without errors for me, and I can use multiple descriptor sets with different combinations. Let me know if you still have issues or see anything else - these are fantastic reports you're making!

attackgoat commented 1 year ago

I copied in the BufferUploader helper class that I started using to bulk upload buffers to device-local/unmappable memory. (If it seems interest I can create a PR.)

Certainly! If we can keep the code as unopinionated as possible, to allow for all types of Vulkan/graphics usage, then it would fit into the main crate. If not or it seems like it might cause trade-offs when selecting this type for a project we could give it a home in the contrib/ directory.

I don't currently publish the contrib/ crates, but this summer I hope to get to a beta stage where I start to do that.

trevex commented 1 year ago

Thanks you so much!

I am currently traveling, but will have a look ASAP and close this issue afterwards. I really like my Screen 13 experience so far, I really like how intuitive and nimble (for a lack of a better word) the API is.

Also kudos for tracking down the DescriptorSet issue.

Next on my todo-list is mipmapping and once I have that implemented I will draft up a PR. As it will use a compute shader it might be a nice extension to screen-13-fx.

trevex commented 1 year ago

Thanks works as expected both 16bit support and the DescriptorSet fix (tested on laptop with integrated AMD GPU and desktop pc with NVIDIA)