Enable wasm-bindgen feature with target wasm32-unknown-emscripten - #1349
Enable wasm-bindgen feature with target wasm32-unknown-emscripten#1349DouglasDwyer wants to merge 5 commits into
Conversation
roderickvd
left a comment
roderickvd
left a comment
There was a problem hiding this comment.
That's great! Not so long ago we removed the old Emscripten host that had become defunct. This seems like a light-weight manner to get Emscripten support back.
Beyond the changes requested in the review points, please also consider updating README.md with Emscripten support.
ccac245 to
1620436
Compare
|
Thank you for your swift response! I have responded to the comments, and additionally updated the tables in the README to reflect support for the Emscripten target. |
roderickvd
left a comment
roderickvd
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround. Here's a few points, hopefully the last.
| assert!( | ||
| WebAudioHost::is_available(), | ||
| "WebAudio is not available in this context; \ | ||
| AudioContext requires a Window (not a Worker or Service Worker)" |
There was a problem hiding this comment.
Running on the browser's UI thread (and having access to the Window object) is still required, even with the Emscripten target. As such, I think this statement is clear and accurate - if you are running on one of Emscripten's worker threads, it won't work. We could change it to say "AudioContext requires the main UI thread" if that would be better?
There was a problem hiding this comment.
Actually, I would mention that we could call emscripten_proxy_sync when creating Streams, which would allow them to be created from any Emscripten thread. I didn't do that yet because I wanted to keep the scope of this PR very small. Would you be open to that addition as well? If so, I can add it as part of this PR or in a separate PR.
There was a problem hiding this comment.
Sure, maybe you can propose it as a separate commit, then we can judge to keep it in or do it separately.
There was a problem hiding this comment.
Following up - I have confirmed that the webaudio example in this repo (wasm32-unknown-unknown), as well as my own project (multithreaded wasm32-unknown-emscripten) work. I am able to get a beep going in Chrome.
1620436 to
40c1b2b
Compare
Enable the WebAudio host (and its wasm-bindgen/js-sys/web-sys deps) for `wasm32-unknown-emscripten`, gated on `any(target_os = "emscripten", target_os = "unknown")` so nothing wasm-bindgen-related is pulled in for `wasm32-wasip1`/`wasip2`. The AudioWorklet host stays `wasm32-unknown-unknown`-only. The three WebAudio JS callbacks keep using `Closure::wrap`. Dropping the `as Box<dyn FnMut(_)>` cast keeps the closures concrete, so their captures (all `UnwindSafe`) satisfy the `panic=unwind` bound on Emscripten without `wrap_aborting` -- a callback panic still surfaces as a JS exception rather than aborting the instance. Minimum `wasm-bindgen` stays at 0.2. README: document the `wasm32-unknown-emscripten` target (Emscripten 6.0.3, wasm-bindgen 0.2.127) and list it under the `wasm-bindgen` feature. Adds an Emscripten CI job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
40c1b2b to
3846df3
Compare
572e408 to
31691df
Compare
2 participants
Summary
The
wasm-bindgentool is finally getting support for integration with Emscripten. The feature is still quite new, but this means that most of Rust's web ecosystem can now work with the Emscripten target. I'd like to usecpalin a WASM/Emscripten project, but right now thewasm32-unknown-emscriptentarget is hard-coded to use the null backend. This PR eliminates the feature gate to make thewasm-bindgenfeature work withwasm32-unknown-emscriptentoo.This PR exposes the
webaudiobackend but not theaudioworkletbackend. That backend relies on re-instantiating the WASM module, but the way Emscripten modules get instantiated is different, so it wouldn't work without more changes.Changes
#[cfg(all(target_arch = "wasm32", target_os = "unknown", feature = "wasm-bindgen"))]with#[cfg(all(target_arch = "wasm32", feature = "wasm-bindgen"))]for thewebaudiobackendClosure::wrap_abortinginstead ofClosure::wrapso that the code properly compiles on WASM targets withpanic=unwindwasm-bindgendependency to0.2.110in order to useClosure::wrap_abortingTesting
In my own project, I have gotten
cpalaudio working with a Rust/Emscripten WASM module in Chrome. This PR also adds CI checks to ensure that compilation is successful.Related issues
#92 #413 #810