From 50cd79884e78d97ee904e576b4a43b60049cf7f6 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Sat, 28 May 2022 21:04:10 +0200 Subject: [PATCH] android: Hold `NativeWindow` lock until after notifying the user with `Event::Suspended` This applies https://github.com/rust-windowing/android-ndk-rs/issues/117 on the `winit` side: Android destroys its window/surface as soon as the user returns from [`onNativeWindowDestroyed`], and we "fixed" this on the `ndk-glue` side by sending the `WindowDestroyed` event before locking the window and removing it: this lock has to wait for any user of `ndk-glue` - ie. `winit` - to give up its readlock on the window, which is what we utilize here to give users of `winit` "time" to destroy any resource created on top of a `RawWindowHandle`. since we can't pass the user a `RawWindowHandle` through the `HasRawWindowHandle` trait we have to document this case explicitly and keep the lock alive on the `winit` side instead. [`onNativeWindowDestroyed`]: https://developer.android.com/ndk/reference/struct/a-native-activity-callbacks#onnativewindowdestroyed --- Cargo.toml | 6 ++++-- src/platform_impl/android/mod.rs | 22 ++++++++++++++++++++-- src/window.rs | 15 ++++++++++++--- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d98a6bc1c2d..6397f54abc2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,8 +39,10 @@ simple_logger = "2.1.0" [target.'cfg(target_os = "android")'.dependencies] # Coordinate the next winit release with android-ndk-rs: https://github.com/rust-windowing/winit/issues/1995 -ndk = { git = "https://github.com/rust-windowing/android-ndk-rs", rev = "7e33384" } -ndk-glue = { git = "https://github.com/rust-windowing/android-ndk-rs", rev = "7e33384" } +# https://github.com/rust-windowing/android-ndk-rs/pull/282 +# https://github.com/rust-windowing/android-ndk-rs/pull/288 +ndk = { git = "https://github.com/MarijnS95/android-ndk-rs", rev = "130d013" } +ndk-glue = { git = "https://github.com/MarijnS95/android-ndk-rs", rev = "130d013" } [target.'cfg(any(target_os = "ios", target_os = "macos"))'.dependencies] objc = "0.2.7" diff --git a/src/platform_impl/android/mod.rs b/src/platform_impl/android/mod.rs index c472e10723c..fa549bf2c18 100644 --- a/src/platform_impl/android/mod.rs +++ b/src/platform_impl/android/mod.rs @@ -10,8 +10,9 @@ use ndk::{ configuration::Configuration, event::{InputEvent, KeyAction, Keycode, MotionAction}, looper::{ForeignLooper, Poll, ThreadLooper}, + native_window::NativeWindow, }; -use ndk_glue::{Event, Rect}; +use ndk_glue::{Event, LockReadGuard, Rect}; use once_cell::sync::Lazy; use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; @@ -238,6 +239,7 @@ pub struct EventLoop { start_cause: event::StartCause, looper: ThreadLooper, running: bool, + window_lock: Option>, } #[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)] @@ -267,6 +269,7 @@ impl EventLoop { start_cause: event::StartCause::Init, looper: ThreadLooper::for_thread().unwrap(), running: false, + window_lock: None, } } @@ -299,6 +302,17 @@ impl EventLoop { match self.first_event.take() { Some(EventSource::Callback) => match ndk_glue::poll_events().unwrap() { Event::WindowCreated => { + // Acquire a lock on the window to prevent Android from destroying + // it until we've notified and waited for the user in Event::Suspended. + // XXX: Don't send this event if Android already removed the window + // before we managed to handle this event? + let next_window_lock = ndk_glue::native_window().expect( + "`WindowCreated` while ndk_glue::native_window() provides no window", + ); + assert!( + self.window_lock.replace(next_window_lock).is_none(), + "`WindowCreated` called while we were already holding a lock" + ); call_event_handler!( event_handler, self.window_target(), @@ -315,6 +329,10 @@ impl EventLoop { control_flow, event::Event::Suspended ); + // Release the lock, allowing Android to clean up this surface + self.window_lock.take().expect( + "`WindowDestroyed` called while we were not holding a window lock", + ); } Event::Pause => self.running = false, Event::Resume => self.running = true, @@ -786,7 +804,7 @@ impl Window { } pub fn raw_window_handle(&self) -> RawWindowHandle { - if let Some(native_window) = ndk_glue::native_window().as_ref() { + if let Some(native_window) = ndk_glue::native_window() { native_window.raw_window_handle() } else { panic!("Cannot get the native window, it's null and will always be null before Event::Resumed and after Event::Suspended. Make sure you only call this function between those events."); diff --git a/src/window.rs b/src/window.rs index 2f8cbbf50c1..ba0d037f485 100644 --- a/src/window.rs +++ b/src/window.rs @@ -1024,12 +1024,21 @@ impl Window { } unsafe impl raw_window_handle::HasRawWindowHandle for Window { - /// Returns a `raw_window_handle::RawWindowHandle` for the Window + /// Returns a [`raw_window_handle::RawWindowHandle`] for the Window /// /// ## Platform-specific /// - /// - **Android:** Only available after receiving the Resumed event and before Suspended. *If you* - /// *try to get the handle outside of that period, this function will panic*! + /// ### Android + /// + /// Only available after receiving [`Event::Resumed`] and before [`Event::Suspended`]. *If you + /// try to get the handle outside of that period, this function will panic*! + /// + /// Make sure to release or destroy any resources created from this `RawWindowHandle` (ie. Vulkan + /// or OpenGL surfaces) before returning from [`Event::Suspended`], at which point Android will + /// release the underlying window/surface: any subsequent interaction is undefined behavior. + /// + /// [`Event::Resumed`]: crate::event::Event::Resumed + /// [`Event::Suspended`]: crate::event::Event::Suspended fn raw_window_handle(&self) -> raw_window_handle::RawWindowHandle { self.window.raw_window_handle() }