Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Following upgrade to wasm-bindgen 0.2.95, Ruffle no longer works on Safari 15 and below #4227

Closed
danielhjacobs opened this issue Oct 25, 2024 · 9 comments
Labels

Comments

@danielhjacobs
Copy link

danielhjacobs commented Oct 25, 2024

Describe the Bug

We compile two wasm modules, one with target features for bulk-memory, simd128, nontrapping-fptoint, sign-ext, and reference-types and one without those target features. The one without those target features no longer works on Safari 15 and below. I suspect that may be caused by the combination of https://releases.rs/docs/1.82.0/#compatibility-notes (The WebAssembly target features multivalue and reference-types are now both enabled by default) and https://github.com/rustwasm/wasm-bindgen/blob/main/CHANGELOG.md#changed-2 ("Implicitly enable reference type and multivalue transformations if the module already makes use of the corresponding target features"), though it could be an unrelated issue. We'd like to try to generate a vanilla module like the one we created before, with those transformations disabled, but the Rust upgrade notes say "Generating a WebAssembly module that disables default features requires -Zbuild-std support from Cargo", and looking at https://doc.rust-lang.org/cargo/reference/unstable.html#build-std, that's still an unstable feature, so I looked to wasm-bindgen for a way to at least disable the transformation but I don't see one.

This is the script that builds the WASM modules: https://github.com/ruffle-rs/ruffle/blob/master/web/packages/core/tools/build_wasm.ts

This is the code that determines which module to load: https://github.com/ruffle-rs/ruffle/blob/master/web/packages/core/src/load-ruffle.ts#L38-L52

Steps to Reproduce

  1. Go to https://ruffle.rs/demo/ with Safari 15
  2. The error reported by someone using that browser says RuntimeError: Out of bounds table access (evaluating 'b._dyn_core__ops__function__FnMut__A____Output___R_as_wasm_bindgen__closure__WasmClosure___describe__invoke__h281f2f93d3d6cf28(n,e,t)')

Expected Behavior

The vanilla WASM module that loads should work on browsers without WebAssembly extensions.

Actual Behavior

The application panics.

@danielhjacobs
Copy link
Author

The code we use is as follows:

    if (extensions) {
        rustFlags.push(
            "-C",
            "target-feature=+bulk-memory,+simd128,+nontrapping-fptoint,+sign-ext,+reference-types",
        );
        wasmBindgenFlags.push("--reference-types");
        wasmOptFlags.push("--enable-reference-types");
    }

My initial thought was to change it to this:

    if (extensions) {
        rustFlags.push(
            "-C",
            "target-feature=+bulk-memory,+simd128,+nontrapping-fptoint,+sign-ext",
        );
        wasmBindgenFlags.push("--reference-types");
        wasmOptFlags.push("--enable-reference-types");
    } else {
        rustFlags.push(
            "-C",
            "target-feature=-reference-types,-multivalue",
        );
    }

But I'm not sure that will work based on the note that "Generating a WebAssembly module that disables default features requires -Zbuild-std support from Cargo"

@RunDevelopment
Copy link
Contributor

Not a maintainer, but maybe I can help.

As you already pointed out, disabling default features is not possible without nightly. So your only options in that regard are to either use nightly and rebuild std, or downgrade and pin Rust 1.81. The downgrade and pin path seems to be what other projects are doing btw.

However, while wasm bindgen can't disable those features AFAIK, it can generate JS glue code that doesn't use those features. Right now, WBG will automatically enable its code gen for multivalue and reference-types when those features are detected in the compiled binary. So there is currently #4213 to disable that, but the main maintainer of this repo is quite busy right now, so it might take a little until the fix is merged and released.

@danielhjacobs
Copy link
Author

Opened ruffle-rs/ruffle#18397

@daxpedda
Copy link
Collaborator

daxpedda commented Nov 7, 2024

As RunDevelopment already pointed out, this is more or less an issue with Rust and not with wasm-bindgen.

Rust v1.84 will add a new target wasm32v1-none which would fill that need.

@daxpedda daxpedda closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2024
@danielhjacobs
Copy link
Author

danielhjacobs commented Nov 7, 2024

Unfortunately that target won't really work for us since we need to build std, as discussed in ruffle-rs/ruffle#18412. Strangely, although Rust 1.82 is what introduced these as default target features, until we upgraded wasm-bindgen it wasn't causing any issues. I think the JS glue provided by wasm-bindgen is what broke Ruffle on those browsers, and the target features didn't actually cause any problems on their own.

@daxpedda
Copy link
Collaborator

daxpedda commented Nov 7, 2024

Unfortunately that target won't really work for us since we need to build std, as discussed in ruffle-rs/ruffle#18412.

Generally speaking it should definitely be possible. Arc and Mutex should be replaced with Rc and RefCell unless you are using multi-threading. HashMap can be replaced with hashbrown and so on.

If Ruffle intends to continue supporting older versions of Safari with newer versions of Rust and therefor LLVM, it should definitely consider supporting wasm32v1-none.

Strangely, although Rust 1.82 is what introduced these as default target features, until we upgraded wasm-bindgen it wasn't causing any issues. I think the JS glue provided by wasm-bindgen is what broke Ruffle on those browsers, and the target features didn't actually cause any problems on their own.

Indeed, the wasm-bindgen reference-type transformations are causing the problems. But disabling those would only be a temporary solution, e.g. if multi-value gets properly enabled by LLVM it would be emitted by Rust and not wasm-bindgen. This will continue as more and more proposals will be enabled by default which older versions of Safari don't support.

Again, I strongly recommend wasm32v1-none for that.

@danielhjacobs
Copy link
Author

According to moulins on our Discord:

removing our usage of std is certainly possible, if a little tedious, but can we reasonably get all of our dependencies to be no_std-compatible?

In particular, is https://github.com/gfx-rs/wgpu, which we rely on heavily to use WebGPU on web where supported or otherwise fall back to WebGL2 no_std-compatible?

@daxpedda
Copy link
Collaborator

daxpedda commented Nov 7, 2024

According to moulins on our Discord:

removing our usage of std is certainly possible, if a little tedious, but can we reasonably get all of our dependencies to be no_std-compatible?

In particular, is https://github.com/gfx-rs/wgpu, which we rely on heavily to use WebGPU on web where supported or otherwise fall back to WebGL2 no_std-compatible?

Yeah, getting Wgpu no_std compatible would require quite some work.

Unfortunately I don't have any more ideas apart from sticking to a compatible Rust version, ergo v1.81. Please let me know if there is something wasm-bindgen can do you can think of.

@danielhjacobs
Copy link
Author

Thanks. It's just unfortunate that with the latest stable Rust our minimum supported browsers can't be any older than LLVM's minimum supported browsers, and also it's unfortunate that even though Safari 15 is meant to support these proposals in reality they don't seem to work right.

We can change our enable_wasm.ts tool:

diff --git a/web/packages/core/tools/build_wasm.ts b/web/packages/core/tools/build_wasm.ts
index 61c93ea52..360f6d5c7 100644
--- a/web/packages/core/tools/build_wasm.ts
+++ b/web/packages/core/tools/build_wasm.ts
@@ -42,12 +42,14 @@ function cargoBuild({
     profile,
     features,
     rustFlags,
+    extensions
 }: {
     profile?: string;
     features?: string[];
     rustFlags?: string[];
+    extensions?: boolean;
 }) {
-    let args = ["build", "--locked", "--target", "wasm32-unknown-unknown"];
+    let args = !extensions && process.env["BUILD_WASM_MVP"] ? ["+nightly", "build", "--locked", "-Z", "build-std=std,panic_abort", "--target", "wasm32-unknown-unknown"] : ["build", "--locked", "--target", "wasm32-unknown-unknown"];
     if (profile) {
         args.push("--profile", profile);
     }
@@ -99,9 +101,16 @@ function buildWasm(
     let originalWasmPath;
     if (wasmSource === "cargo" || wasmSource === "cargo_and_store") {
         console.log(`Building ${flavor} with cargo...`);
+        if (process.env["BUILD_WASM_MVP"]) {
+            rustFlags.push(
+                "-C",
+                "target-feature=-reference-types,-multivalue",
+            );
+        }
         cargoBuild({
             profile,
             rustFlags,
+            extensions,
         });
         originalWasmPath = `../../../target/wasm32-unknown-unknown/${profile}/ruffle_web.wasm`;
         if (wasmSource === "cargo_and_store") {

I just don't like having to use nightly Rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants