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 create_shader_module when naga parse Result is unwrapped #5019

Closed
waynr opened this issue Jan 9, 2024 · 3 comments
Closed

panic in create_shader_module when naga parse Result is unwrapped #5019

waynr opened this issue Jan 9, 2024 · 3 comments
Labels
help required We need community help to make this happen. type: bug Something isn't working

Comments

@waynr
Copy link
Contributor

waynr commented Jan 9, 2024

Description
I am building a shader livecoding tool using winit + wgpu where I have an off-main thread watching the shader file for changes as I edit it, recreating the render pipeline, then sending that via the winit EventLoopProxy as a user event to be handled by updating a struct holding the "active" render pipeline.

I have found that when I make shader coding mistake (ie no return value) a naga parse is attempted on the code and the resulting Err unwrapped: https://github.com/gfx-rs/wgpu/blob/trunk/wgpu/src/backend/direct.rs#L930

Repro steps
Run Device.create_shader_module on an invalid glsl shader.

Expected vs observed behavior
My filesystem watcher thread panics during the attempted shader recompile. I would expect that fallible operations on Device such as create_shader_module should return a Result so users like myself can decide how to handle them (in this case I would report the parse failure in my logs and continue).

Extra materials

thread 'notify-rs debouncer loop' panicked at /home/wayne/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.18.0/src/backend/direct.rs:924:61:
called `Result::unwrap()` on an `Err` value: [Error { kind: UnknownVariable("meow"), meta: Span { start: 2138, end: 2142 } }]
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/std/src/panicking.rs:597:5
   1: core::panicking::panic_fmt
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/panicking.rs:72:14
   2: core::result::unwrap_failed
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/result.rs:1652:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/a28077b28a02b92985b3a3faecf92813155f1ea1/library/core/src/result.rs:1077:23
   4: <wgpu::backend::direct::Context as wgpu::context::Context>::device_create_shader_module
             at /home/wayne/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.18.0/src/backend/direct.rs:924:30
   5: <T as wgpu::context::DynContext>::device_create_shader_module
             at /home/wayne/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.18.0/src/context.rs:2263:37
   6: wgpu::Device::create_shader_module
             at /home/wayne/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.18.0/src/lib.rs:2401:26
   7: shadrun::shader_manager::Shader::fragment_module
             at ./src/shader_manager.rs:475:9
   8: shadrun::shader_manager::Shader::render_pipeline
             at ./src/shader_manager.rs:501:31
   9: shadrun::shader_manager::Shader::recompile
             at ./src/shader_manager.rs:377:31
  10: shadrun::shader_manager::Shader::build
             at ./src/shader_manager.rs:354:9
  11: shadrun::shader_manager::ShaderEventHandler::rebuild_shader
             at ./src/shader_manager.rs:588:15
  12: shadrun::shader_manager::ShaderEventHandler::handle_single_event
             at ./src/shader_manager.rs:618:17
  13: shadrun::shader_manager::ShaderEventHandler::handle_events
             at ./src/shader_manager.rs:571:13
  14: <shadrun::shader_manager::ShaderEventHandler as notify_debouncer_full::DebounceEventHandler>::handle_event
             at ./src/shader_manager.rs:560:27
  15: notify_debouncer_full::new_debouncer_opt::{{closure}}
             at /home/wayne/.cargo/registry/src/index.crates.io-6f17d22bba15001f/notify-debouncer-full-0.3.1/src/lib.rs:607:17

Platform
Linux/X11
wgpu 0.18
winit 0.29.8
rust 1.74.1

@teoxoy teoxoy added type: bug Something isn't working help required We need community help to make this happen. labels Jan 9, 2024
@waynr
Copy link
Contributor Author

waynr commented Jan 10, 2024

@teoxoy hey I saw you added a "help wanted" label. Are you a maintainer? Can you tell me if this project would be open to making changes to the Device API to make this error addressable at runtime? ie, instead of panicking my preferenced would be that Device.create_shader_module returns some kind of Result reflecting the fact that shader compilation may fail. If y'all are open to this change (or alternatively adding a new fallible method eg Device.create_shader_module_fallible) let me know and I'll try taking a swing at it. Thanks!

@kpreid
Copy link
Contributor

kpreid commented Jan 11, 2024

This is a duplicate of #2545, I believe.

@cwfitzgerald
Copy link
Member

Closing in favor of that

@cwfitzgerald cwfitzgerald closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help required We need community help to make this happen. type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants