wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

Document web-sys version/API mismatch issues

Open KeKsBoTer opened this issue 1 year ago • 7 comments

Description Building a wgpu (version 0.19.1) project with wasm-bindgen results in an error:

 Compiling wgpu v0.19.1
error[E0061]: this function takes 1 argument but 3 arguments were supplied
   --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:375:22
    |
375 |     let mut mapped = web_sys::GpuDepthStencilState::new(
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
376 |         map_compare_function(desc.depth_compare),
    |         ---------------------------------------- unexpected argument of type `GpuCompareFunction`
377 |         desc.depth_write_enabled,
    |         ------------------------ unexpected argument of type `bool`
    |
note: associated function defined here
   --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuDepthStencilState.rs:27:12
    |
27  |     pub fn new(format: GpuTextureFormat) -> Self {
    |            ^^^
help: remove the extra arguments
    |
376 -         map_compare_function(desc.depth_compare),
376 +         map_texture_format(desc.format),
    |

error[E0061]: this function takes 1 argument but 2 arguments were supplied
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:1693:13
     |
1693 |             web_sys::GpuVertexState::new(desc.vertex.entry_point, &module.0);
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -------------------------
     |                                          |
     |                                          unexpected argument of type `&str`
     |                                          help: remove the extra argument
     |
note: associated function defined here
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuVertexState.rs:27:12
     |
27   |     pub fn new(module: &GpuShaderModule) -> Self {
     |            ^^^

error[E0061]: this function takes 2 arguments but 3 arguments were supplied
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:1768:17
     |
1768 |                 web_sys::GpuFragmentState::new(frag.entry_point, &module.0, &targets);
     |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ----------------             -------- unexpected argument of type `&Array`
     |                                                |
     |                                                expected `&GpuShaderModule`, found `&str`
     |
     = note: expected reference `&GpuShaderModule`
                found reference `&str`
note: associated function defined here
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuFragmentState.rs:27:12
     |
27   |     pub fn new(module: &GpuShaderModule, targets: &::wasm_bindgen::JsValue) -> Self {
     |            ^^^
help: remove the extra argument
     |
1768 -                 web_sys::GpuFragmentState::new(frag.entry_point, &module.0, &targets);
1768 +                 web_sys::GpuFragmentState::new(/* &GpuShaderModule */, &module.0);
     |

error[E0061]: this function takes 1 argument but 2 arguments were supplied
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:1793:13
     |
1793 |             web_sys::GpuProgrammableStage::new(desc.entry_point, &shader_module.0);
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ------------------
     |                                                |
     |                                                unexpected argument of type `&str`
     |                                                help: remove the extra argument
     |
note: associated function defined here
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/web-sys-0.3.68/src/features/gen_GpuProgrammableStage.rs:27:12
     |
27   |     pub fn new(module: &GpuShaderModule) -> Self {
     |            ^^^

error[E0599]: no method named `write_timestamp` found for struct `GpuCommandEncoder` in the current scope
    --> /home/simon/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.19.1/src/backend/webgpu.rs:2505:14
     |
2503 | /         encoder_data
2504 | |             .0
2505 | |             .write_timestamp(&query_set_data.0, query_index);
     | |             -^^^^^^^^^^^^^^^ method not found in `GpuCommandEncoder`
     | |_____________|
     | 

Some errors have detailed explanations: E0061, E0599.
For more information about an error, try `rustc --explain E0061`.
error: could not compile `wgpu` (lib) due to 5 previous errors

I think this is due to a change in the web-sys API. Version 0.3.67 has the expected API:

pub fn [new](https://docs.rs/web-sys/0.3.67/web_sys/struct.GpuDepthStencilState.html#method.new)(
    depth_compare: [GpuCompareFunction](https://docs.rs/web-sys/0.3.67/web_sys/enum.GpuCompareFunction.html),
    depth_write_enabled: [bool](https://doc.rust-lang.org/nightly/std/primitive.bool.html),
    format: [GpuTextureFormat](https://docs.rs/web-sys/0.3.67/web_sys/enum.GpuTextureFormat.html)
) -> Self

Construct a new GpuDepthStencilState.

This API requires the following crate features to be activated: GpuCompareFunction, GpuDepthStencilState, GpuTextureFormat

This API is unstable and requires --cfg=web_sys_unstable_apis to be activated, as [described in the wasm-bindgen guide](https://rustwasm.github.io/docs/wasm-bindgen/web-sys/unstable-apis.html)

Version 0.3.68 does not:

pub fn [new](https://docs.rs/web-sys/0.3.68/web_sys/struct.GpuDepthStencilState.html#method.new)(format: [GpuTextureFormat](https://docs.rs/web-sys/0.3.68/web_sys/enum.GpuTextureFormat.html)) -> Self

Construct a new GpuDepthStencilState.

This API requires the following crate features to be activated: GpuDepthStencilState, GpuTextureFormat

This API is unstable and requires --cfg=web_sys_unstable_apis to be activated, as [described in the wasm-bindgen guide](https://rustwasm.github.io/docs/wasm-bindgen/web-sys/unstable-apis.html)

This commit made the change.

Repro steps

Cargo.toml:

[package]
name = "wgpu_error"
version = "0.1.0"
edition = "2021"

[dependencies]
wgpu = { version = "0.19.1" }
wasm-bindgen = "0.2.84"

[lib]
path = "src/lib.rs"
crate-type = ["cdylib", "rlib"]

run

export RUSTFLAGS=--cfg=web_sys_unstable_apis 
cargo build \
    --no-default-features \
    --target wasm32-unknown-unknown \
    --lib

Expected vs observed behavior The build is successful

Platform OS: Ubuntu 22.04.2 LTS toolchain: stable-x86_64-unknown-linux-gnu (default) rustc 1.76.0 (07dca489a 2024-02-04)

KeKsBoTer avatar Feb 16 '24 09:02 KeKsBoTer

Forcing a downgrade web-sys = "=0.3.67" solves the issue. Why did wasm-bindgen introduce a API-breaking change in a minor version?

KeKsBoTer avatar Feb 16 '24 09:02 KeKsBoTer

Why did wasm-bindgen introduce a API-breaking change in a minor version?

Because as RUSTFLAGS=--cfg=web_sys_unstable_apis implies these are unstable apis. The semvar contract is not broken because the thing that changed were unstable APIs.

We recently discussed whether to pin the version on web-sys, but concluded we can't really on wgpu: if a user only is interested in using webgl, they may still want to use a newer web_sys version. If wgpu were to pin the minor version, you can't have another web-sys version live side by side since that's not allowed for crates that differ only on minor versions 😢

So.. known issue that we don't know how to solve.

I figure though we should do a better job at documenting this, which is why I keep this open. Something along the line "for webgpu users: you have to pin your web-sys version at XY and enable web_sys_unstable_apis"

Wumpf avatar Feb 16 '24 13:02 Wumpf

Seeing the Wgpu relies on an unstable API of web-sys, shouldn't the WebGPU backend be marked as unstable as well? Maybe it is and I didn't see it.

Alternatively Wgpu could stop relying on web-sys for these APIs and declare them in-crate. This could potentially be backported as well to solve current build failures in v0.19.

What would also be possible is to require at least web-sys v0.3.68 for Wgpu v0.20 and introduce custom bindings to Wgpu v0.19 only for the APIs that broke in a patch release.

daxpedda avatar Feb 17 '24 08:02 daxpedda

Seeing the Wgpu relies on an unstable API of web-sys, shouldn't the WebGPU backend be marked as unstable as well? Maybe it is and I didn't see it.

I'm not sure how we would do that. The WebGPU specification draft itself is arguably unstable.

Alternatively Wgpu could stop relying on web-sys for these APIs and declare them in-crate.

That would unnecessarily fork the ecosystem and cause a lot of extra effort on wgpu.

The strategy so far has been to make all "important" wgpu releases compatible with the latest web-sys, but this time around wgpu is a bit behind. Help welcome!

Wumpf avatar Feb 18 '24 09:02 Wumpf

That would unnecessarily fork the ecosystem and cause a lot of extra effort on wgpu.

I disagree on this one case - I think it is a decent idea to wholesale copy the web-sys WebGPU bindings inline. The important bit is that we never maintain them locally. We treat it as a black box. As long as all improvements are done against web-sys, then copied into wgpu, we get the best of both worlds.

cwfitzgerald avatar Feb 18 '24 23:02 cwfitzgerald

Would we get away though with just copying the webgpu parts? I figure we quickly run into issues where that doesn't interop with the stable web-sys that we'd still be depending on for all the rest. Might be worth a shot though

Wumpf avatar Feb 21 '24 08:02 Wumpf

Interop should not be a problem, types can be converted at zero-cost. If I don't get to it myself anytime soon, feel free to ping me for a review in a PR.

daxpedda avatar Feb 21 '24 14:02 daxpedda