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

Panic in wasm builds #11

Open
rparrett opened this issue Mar 27, 2024 · 4 comments
Open

Panic in wasm builds #11

rparrett opened this issue Mar 27, 2024 · 4 comments

Comments

@rparrett
Copy link
Contributor

rparrett commented Mar 27, 2024

Version tested

Tested v0.3.0.

Bisected to aa0b278

Info

AdapterInfo { name: "ANGLE (Apple, ANGLE Metal Renderer: Apple M1 Max, Unspecified Version)", vendor: 4203, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Gl }

Reproduction

With wasm-server-runner:
cargo run --example sparks --target=wasm32-unknown-unknown

Panic

(index):159 panicked at /Users/me/.cargo/registry/src/index.crates.io-6f17d22bba15001f/glow-0.13.1/src/web_sys.rs:3184:9:
Tex storage 2D multisample is not supported

Stack:

Error
    at imports.wbg.__wbg_new_abda76e883ba8a5f (http://127.0.0.1:1334/api/wasm.js:482:13)
    at __wbg_new_abda76e883ba8a5f externref shim (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[331084]:0x5b9d84a)
    at console_error_panic_hook::Error::new::h8a92a4862c5a1462 (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[237327]:0x56ded84)
    at console_error_panic_hook::hook_impl::hbd888f2d619deb19 (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[45590]:0x37de1ad)
    at console_error_panic_hook::hook::h7220c272d9834ace (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[289425]:0x59fb7f3)
    at core::ops::function::Fn::call::h9b1dea7da821a03f (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[256753]:0x58310b1)
    at std::panicking::rust_panic_with_hook::h6b00a1542b7e827c (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[94864]:0x459e600)
    at std::panicking::begin_panic_handler::{{closure}}::hcb858dc6b4beb4ae (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[123059]:0x4ac35eb)
    at std::sys_common::backtrace::__rust_end_short_backtrace::hbb248f505066e51b (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[333575]:0x5ba5b70)
    at rust_begin_unwind (http://127.0.0.1:1334/api/wasm.wasm:wasm-function[264653]:0x58aca92)

Notes

With Msaa disabled with

    app.add_plugins(DefaultPlugins)
        .insert_resource(Msaa::Off)
        .add_plugins(ParticleSystemPlugin)

there's a different panic:

wgpu error: Validation Error

Caused by:
    In Device::create_bind_group
      note: label = `Firework Uniform Bindgroup`
    Texture binding 1 expects multisampled = true, but given a view with samples = 1

And just a note: For the pbr example, Msaa must be disabled before the ParticleSystemPlugin is added, otherwise, there's a different panic:

wgpu error: Validation Error

Caused by:
    In Device::create_render_pipeline
      note: label = `Firework Pipeline`
    Error matching ShaderStages(FRAGMENT) shader requirements against the pipeline
    Shader global ResourceBinding { group: 1, binding: 1 } is not available in the pipeline layout
    Texture class Depth { multi: true } doesn't match the shader Depth { multi: false }
@mbrea-c
Copy link
Owner

mbrea-c commented Mar 27, 2024

The first panic seems to be a wgpu bug: gfx-rs/wgpu#5263

The second one is my bad, I forgot to add code to determine when to enable multisampling for texture bindings: https://github.com/mbrea-c/bevy_firework/blob/master/src/render.rs#L297. I'm surprised this didn't come up before tbh, I'll try to get it fixed this evening.

Thanks for reporting!

@mbrea-c
Copy link
Owner

mbrea-c commented Mar 27, 2024

PR #13 should fix the second crash, but the third will need slightly more work. I think it's happening because the Msaa setting is being changed after the uniform bindgroup layout is generated. One solution could be to add a system to re-create the uniform bindgroup layout and the dummy textureview whenever the Msaa resource changes.

@rparrett
Copy link
Contributor Author

Great! I'll check that out soon.

I'd hope that some sort of workaround could be found to allow msaa to be used eventually, but that fix will be helpful.

For the third, I'd personally be content with a "documentation fix," just making sure the examples work with wasm and showing users how to do that.

@mbrea-c
Copy link
Owner

mbrea-c commented Apr 1, 2024

I'd hope that some sort of workaround could be found to allow msaa to be used eventually, but that fix will be helpful.

Agreed

For the third, I'd personally be content with a "documentation fix," just making sure the examples work with wasm and showing users how to do that.

The last few days have been busy, but I can get back to this now.
I can start by merging the PR without the fix for the third crash (just so the plugin can be used at all without msaa), and open a follow-up for a proper fix. It shouldn't be that bad, I just need to look at some upstream Bevy code to figure out the best way of solving it.

mbrea-c added a commit that referenced this issue Apr 1, 2024
The current rendering code was assuming that Msaa was always on due to
an oversight. This PR will determines whether to bind a texture with
multisampling enabled based on the actual msaa configuration.

Fixes the second crash mentioned in #11, and works around the first in
the example code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants