wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

[BREAKING] Make `Shader(Module)Source::Wgsl` optional

Open daxpedda opened this issue 3 years ago • 2 comments

Checklist

  • [x] Run cargo clippy.
  • [x] Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • [ ] Add change to CHANGELOG.md. See simple instructions inside file.

Connections Discussed in Matrix.

Description This is totally a breaking change! It also might turn out entirely unnecessary if the compile-time savings aren't worth the added complexity.

At the moment the naga wgsl-in crate feature is active even when completely unnecessary. This addresses it by gating Shader(Module)Source::Wgsl behind a new wgsl crate feature, like with ShaderSource::Glsl for example. The added wgsl crate feature is also active by default.

Code quality is definitely a bit worse with all of these #[cfg(...)] now. CI time is increased because --no-default-features has to be tested.

I also didn't know what to do about Shader(Module)Sources lifetime, at the moment the lifetime is not present when the only active variant is Naga, which I consider very user unfriendly and could only be fixed by either stop using Cow or adding a dummy lifetime to the Naga variant. ~~This could also be solved by #2903, so maybe we should merge that first.~~ (can't bind 'a to the 'static in Cow)

I also had to increase the MSRV to 1.60 to not unnecessarily pull in wgpu-core on WASM by using the weak dependency feature.

Testing Added additional runs with --no-default-features in the CI.

daxpedda avatar Jul 17 '22 17:07 daxpedda

Okay, CI just has issues installing Rust, I will try again later.

daxpedda avatar Jul 17 '22 17:07 daxpedda

Rebased after #2872.

daxpedda avatar Sep 03 '22 13:09 daxpedda

Rebased again. Too bad it didn't make the cut for 0.14. @cwfitzgerald can we get this in before it gets forgotten again?

daxpedda avatar Oct 06 '22 08:10 daxpedda

Done.

daxpedda avatar Oct 07 '22 17:10 daxpedda