gfx-rs / gfx

[maintenance mode] A low-overhead Vulkan-like GPU API for Rust.
http://gfx-rs.github.io/
Apache License 2.0
5.35k stars 551 forks source link

Read-only storage bindings #2933

Open kvark opened 4 years ago

kvark commented 4 years ago

Vulkan has this strange semantics: at the binding model level, there is no "read-only" option for storage resources. But at the shader level, one can specify a storage resource can be decorated with NonReadable or NonWritable. When such "NonWritable" is encountered by our DX12 backend (say, produced by "readonly" GLSL SSBO), the translated shader uses non-writable variant, e.g. ByteAddressBuffer as opposed to RWByteAddressBuffer. There is a crucial difference here because DX12 binding model considers the former to be SRV and the latter to be UAV... but we have no means to detect this at the pipeline creation time.

So there is an obvious conflict with DX12 binding model semantics. I could think of a number of ways to address this:

  1. Change our API to include this semantics in the descriptor set layout, potentially refactor the descriptor types completely. This is likely acceptable for all the clients except for Vulkan Portability, which we don't want to break.
  2. Convince the shader translator (be it SPIRV-Cross or the future Javelin) to declare these storage resources as RW all the time. This is probably the easiest path, but it may have unintended consequences, since DX12 driver would have to consider it being mutable now.
  3. In the Root Signature, for each storage type descriptor, generate both SRV and UAV entries. When the pipeline is created, we could extract this semantics from the shader and override the binding to either the SRV or UAV accordingly. This has a downside of wasting the descriptor space for storage types.

Also, a combination of these methods is possible: have the flag in the API that specifies read-only/write-only/both. Have the Root Signature creation code respect that setting, and for the "both" case still make the override based on the shader. This solution would allow both get the maximum of the API and keep the compatibility with Vulkan. We'd also consider upstreaming the API changes to VkPI in this case.

kvark commented 4 years ago

Good news is - we don't really need to introspect the SPIR-V and set up the overrides based on the "readonly" flag: Root Signature spaces for UAV and SRV are separate, so we can just bind both, and let it figure out which one is needed.