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.

Sign up for GitHub

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

Fix Android crash on resume with wgpu #3847

Merged
merged 2 commits into from
Jan 22, 2024
Merged

Fix Android crash on resume with wgpu #3847

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b54e1c7
fixed android on resume crash
Garoven Jan 19, 2024
17aaf8b
Merge branch 'emilk:master' into android-fix
Garoven Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions crates/eframe/src/native/wgpu_integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,32 @@ impl WgpuWinitApp {
}
}

#[cfg(target_os = "android")]
fn recreate_window(
&self,
event_loop: &EventLoopWindowTarget<UserEvent>,
running: &WgpuWinitRunning,
) {
let SharedState {
egui_ctx,
viewports,
viewport_from_window,
painter,
..
} = &mut *running.shared.borrow_mut();

initialize_or_update_viewport(
egui_ctx,
viewports,
ViewportIdPair::ROOT,
ViewportClass::Root,
self.native_options.viewport.clone(),
None,
None,
)
.initialize_window(event_loop, egui_ctx, viewport_from_window, painter);
}

#[cfg(target_os = "android")]
fn drop_window(&mut self) -> Result<(), egui_wgpu::WgpuError> {
if let Some(running) = &mut self.running {
Expand Down Expand Up @@ -386,6 +412,8 @@ impl WinitApp for WgpuWinitApp {
log::debug!("Event::Resumed");

let running = if let Some(running) = &self.running {
#[cfg(target_os = "android")]
self.recreate_window(event_loop, running);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw winit is built to emit Resumed on all platforms. Can the initialization be done here, generically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the same way we're looking to consistently emit Suspended before the loop is destroyed, giving users a consistent place to clean up their swapchain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. On platforms other than Android, Ios and Web Resume event is only send once when loop starts running. So the recreation for other platforms won't happen and for Ios and Web the Suspend/Resume cycle is different.
  2. The same for Suspend event it only occurs for Android, Ios and Web.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On other platforms it is not about recreating, it is about initially creating this state in the same place instead of having unnecessary conditionals in eframe.

Destruction only happens in ::Suspended.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay this looks much harder to fix than thought. WgpuWinitRunning contains a lot of state that should easily live outside the Android surface lifecycle.

running
} else {
let storage = epi_integration::create_storage(
Expand Down
Loading