SPIR-V is rejected by ShaderProcessor if shader defs are present

[Shfty]

Bevy version

v0.9.1

What you did

Compiled a SPIR-V shader via rust-gpu which requires access to bevy_pbr's light and cluster data, but is prevented from binding read-only storage buffers due to upstream limitations.

I used WgpuSettings to set WgpuLimits::max_storage_buffers_per_shader_stage to 0, guaranteeing that the relevant data would be stored in uniform buffers, then tried to render a mesh with said shader.

What went wrong

The shader failed to load, with the following error originating from ShaderProcessor:
failed to process shader: This Shader's format does not support processing shader defs.

Additional information

This behaviour seems incorrect, given that SPIR-V is a precompiled format, and any conditional processing is thus implied to have taken place at compile time.

Most shader defs - such as VERTEX_POSITION or TONEMAP_IN_SHADER - are added to the RenderPipelineDescriptor before Material::specialize is called, and so can be cleared by downstream code as a workaround.

However, WGPU-predicated defs like NO_STORAGE_BUFFER_SUPPORT are injected after Material::specialize. This prevents user code from removing them before ShaderProcessor runs, thus effectively preventing SPIR-V from being used in any context where bevy_pbr's preferred GPU limitations are not met.

While there may be discussion to be had re. asserting the correctness of a given SPIR-V shader before allowing it to run, I think such functionality is currently outside the scope of ShaderProcessor. Removing the check would allow usage of correct SPIR-V shaders, and avoid the aforementioned clearing workaround.

I've put together an example repo with a working rust-gpu / bevy pipeline, which contains the shader crate in question (a Rust reimplementation of bevy_pbr), as well as the strata needed to compile it into SPIR-V, and a simple bevy app to render it alongside a StandardMaterial for comparison.

It depends on a patched fork of bevy 0.9.1 that removes the shader def error case, and the issue in question can be reproduced by pulling a local copy and modifying Cargo.toml to depend on the crates.io release.

I cherry-picked the change into main and created a separate branch, so have PRed that for the sake of retaining a 0.9.1 fork for my own use.

[brynnbrancamp]

This is super important! Needs to be fixed.