wgpu
wgpu copied to clipboard
Fix shaderf16 support on vulkan/nvidia
Connections N/A
Description On my Laptop 4090, vulkaninfo reports:
VkPhysicalDeviceVulkan11Features:
---------------------------------
storageBuffer16BitAccess = true
uniformAndStorageBuffer16BitAccess = true
storagePushConstant16 = true
storageInputOutput16 = false
However the wgpu vulkan backend requires all four to be true for the SHADER_F16 feature to be supported.
This doesn't seem necessary, and as far as I can tell, storage_input_output16 isn't even read anywhere in wgpu.
This is also how dawn handles f16 support checking here.
Testing My personal project using f16s (developed in chrome wasm which doesn't have this issue) was tested and works fine on Windows 11 native, Vulkan Backend.
Squash or Rebase? Squash
Checklist
- [x] Run
cargo fmt. - [ ] Run
taplo format. - [ ] Run
cargo clippy --tests. If applicable, add:- [ ]
--target wasm32-unknown-unknown
- [ ]
- [ ] Run
cargo xtask testto run tests. - [ ] If this contains user-facing changes, add a
CHANGELOG.mdentry.
storage_input_output16 is somehow implicitly used here:
https://github.com/gfx-rs/wgpu/blob/50eb207a77963a3818e8d05f65da34f1f6e6018c/naga/src/back/spv/writer.rs#L1207
As for Dawn https://issues.chromium.org/issues/42251212:
Tint can now polyfill f16 shader IO by using f32 types, so we can enable the F16 feature on Vulkan without this capability.
I think naga also does pollyfill somewhere.
Any idea where would be the best place to add the polyfill?
Related: https://github.com/gfx-rs/wgpu/issues/7468
This fixes an issue I had when trying to profile code using f16 in Nsight.
@cryvosh can we take this PR out of draft, and leave pollyfilling to a followup?
All of the Vulkan features are required for the f16 feature. Dawn gets away with not requiring storageInputOutput16 since they polyfill the functionality, naga doesn't currently have this polyfill.
Landing this PR as is would make the functionality of the SHADER_F16 feature non-portable. If we want to not require storageInputOutput16 we can do so under a new feature SHADER_F16_WO_IO until naga can do the polyfill.
CI seems to correctly catch this as a vulkan validation error.
As this isn't actually allowed, I'm going to close this PR - we would accept a followup which adds the polyfill using f32.
For future reference, what's not allowed?
Basically, if you use f16 as part of the input or output of a shader (like Vertex input or Fragment shader input) you need the input/output capability. However you can polyfill it by using f32, we just have not implemented this.
Oh gotcha, I thought you meant I broke some rule
I'll look into adding the polyfill and open new PR when I get the chance
(in meantime any suggestions on where / at what stage in the pipeline to add this would be appreciated)