Enable wasm-bindgen feature with target wasm32-unknown-emscripten - #1349
Enable wasm-bindgen feature with target wasm32-unknown-emscripten#1349DouglasDwyer wants to merge 1 commit into
Conversation
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.
0eda773 to
ccac245
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>
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
There was a problem hiding this comment.
Thanks for the quick turnaround. Here's a few points, hopefully the last.
| | WASAPI / ASIO | Windows | 1.85 | Windows 8 | | ||
| | WASM (`wasm32-unknown`) | WebAssembly | 1.85 | — | | ||
| | WASM (`wasm32-unknown-unknown`) | WebAssembly | 1.85 | — | | ||
| | WASM (`wasm32-unknown-emscripten`) | WebAssembly | 1.85 | Emscripten 6.0.3, wasm-bindgen 0.2.127 | |
There was a problem hiding this comment.
If it's wasm-bindgen 0.2.127 and up specifically, then should that version not also be pinned in Cargo.toml?
| - **ALSA**: Update `alsa` dependency to 0.12. | ||
| - **Linux**: `realtime` can now promote threads without requiring `realtime-dbus`. | ||
| - **PipeWire**: Set `node.rate` property so that `default.clock.allowed-rates` PipeWire config works. | ||
| - **WebAudio**: the WebAudio host now works on `wasm32-unknown-emscripten` via [wasm-bindgen/Emscripten integration](https://github.com/wasm-bindgen/wasm-bindgen/issues/5237). |
There was a problem hiding this comment.
I propose to move this to the "Added" section, rephrasing it to "WebAudio: added support for Emscripten targets via ..."
| assert!( | ||
| WebAudioHost::is_available(), | ||
| "WebAudio is not available in this context; \ | ||
| AudioContext requires a Window (not a Worker or Service Worker)" |
| let current_time_bits_cb = current_time_bits.clone(); | ||
|
|
||
| let on_audio_process = Closure::wrap(Box::new(move |event: AudioProcessingEvent| { | ||
| let on_audio_process_fn = Box::new(move |event: AudioProcessingEvent| { |
There was a problem hiding this comment.
Are these changes to make it a _fn and wrap it later still necessary? Seems they can be removed and reduce the diff size, unless you think it reads a lot better or I'm missing something else.
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