Skip to content

Commit

Permalink
android: Hold NativeWindow lock until after notifying the user with…
Browse files Browse the repository at this point in the history
… `Event::Suspended`

This applies rust-mobile/ndk#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
  • Loading branch information
MarijnS95 committed Jun 10, 2022
1 parent 44288f6 commit 50cd798
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
6 changes: 4 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 20 additions & 2 deletions src/platform_impl/android/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -238,6 +239,7 @@ pub struct EventLoop<T: 'static> {
start_cause: event::StartCause,
looper: ThreadLooper,
running: bool,
window_lock: Option<LockReadGuard<NativeWindow>>,
}

#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -267,6 +269,7 @@ impl<T: 'static> EventLoop<T> {
start_cause: event::StartCause::Init,
looper: ThreadLooper::for_thread().unwrap(),
running: false,
window_lock: None,
}
}

Expand Down Expand Up @@ -299,6 +302,17 @@ impl<T: 'static> EventLoop<T> {
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(),
Expand All @@ -315,6 +329,10 @@ impl<T: 'static> EventLoop<T> {
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,
Expand Down Expand Up @@ -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.");
Expand Down
15 changes: 12 additions & 3 deletions src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down

0 comments on commit 50cd798

Please sign in to comment.