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

[macOS/iOS] Crash when calling UI APIs from non-UI thread #63

Open
spencer741 opened this issue Mar 4, 2024 · 2 comments
Open

[macOS/iOS] Crash when calling UI APIs from non-UI thread #63

spencer741 opened this issue Mar 4, 2024 · 2 comments

Comments

@spencer741
Copy link

spencer741 commented Mar 4, 2024

Problem Description

When attempting to create a new surface on macOS/iOS platforms using which internally uses wgpu, the application crashes if the call is made from a non-UI thread. This issue specifically arises in the context of wgpu-hal when attempting to interact with Metal's get_metal_layer from a thread other than the main UI thread.

Running the many_layers demo on MacOS yields this crash.

[2024-03-04T20:28:25Z INFO  galileo::render::wgpu] Creating new surface
thread 'tokio-runtime-worker' panicked at ../.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-hal-0.19.3/src/metal/surface.rs:113:13:
get_metal_layer cannot be called in non-ui thread.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected Behavior

Don't crash on MacOS. Haven't tested on IOS, but assuming this is a problem?
Internally ensure that UI API calls are dispatched to the main thread to prevent such crashes.
This is where gfx-rs prevents create_surface calls from happening if not on the UI thread. gfx-rs/wgpu#2736

Possible Fixes

Looks like we spawn a new thread here which makes an otherwise synchronous event loop be asynchronous in nature, would there be a problem making this synchronous? Was looking through this commit:

@spencer741
Copy link
Author

spencer741 commented Mar 4, 2024

I am also excited to contribute, just wanted to have an architectural discussion on spawning threads in the event loop and get your thoughts.

Maybe not making the WGPU calls sync is a great (or even easily achievable concept)...

We might have to wait on something like this to flesh out.

rust-windowing/winit#3560

@Maximkaaa
Copy link
Owner

Thanks for reporting this. It makes sense that the crush happens since even wgpu docs state that is how it works on macOS/iOS. I remember reading it, but completely forgot about it when I was trying to make the Android example work.

Before one of the latest commits, the surface was created once before the event loop started, and that should work on macOS. But on Android this approach doesn't work since the window is not ready until the Resumed event is called. And this may happen many times during the lifetime of the application. So the renderer must be initialized inside the even loop.

From the architectural point of view, this is not very important. As this part of the code is only supposed to be used in sample apps and during development, not in production. In real applications the UI framework will take care of the even loop and the surface creation. So we could take a look at how the UI frameworks initialize wgpu and do the same here.

Another workable approach would be to block on the renderer initialization future instead of doing it in the background. This is fine, as this process doesn't take long, and only happens when an application starts or is reinitialized from background.

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