From 348356802fc0c9cba6815a9960cb97db3775e2b9 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Fri, 10 Jun 2022 17:40:32 +0200 Subject: [PATCH] ndk-glue: Wrap `NativeWindow`/`InputQueue` locks in a newtype As recently recommended/requested, this prevents `ndk_glue` from leaking its underlying lock implementation, and relieves downstream crates from having to import and specify the appropriate lock type. Especially in light of a scheduled migration to `parking_lot`. Note that this won't prevent against API breakage introduced by the intended change of transposing the `Option` inside this `LockReadGuard` as that can't be done prior to switching to `parking_lot`, and it'll be its own free-standing API ergonomics improvement regardless (`Option` now only needs to be unwrapped/checked once). Finally add some more doc-comments to link to the new lock and explain how the locks should be used on the `*Created` events and getter functions too, instead of describing it only on the `*Destroyed` events. --- ndk-glue/CHANGELOG.md | 2 ++ ndk-glue/src/lib.rs | 75 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/ndk-glue/CHANGELOG.md b/ndk-glue/CHANGELOG.md index 8c53511d..eb95b776 100644 --- a/ndk-glue/CHANGELOG.md +++ b/ndk-glue/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- **Breaking:** Provide a `LockReadGuard` newtype around `NativeWindow`/`InputQueue` to hide the underlying lock implementation. (#288) + # 0.6.2 (2022-04-19) - Call `ndk_context::release_android_context()` function to remove `AndroidContext` when activity is destroyed. (#263) diff --git a/ndk-glue/src/lib.rs b/ndk-glue/src/lib.rs index 65678e9c..f94d5732 100644 --- a/ndk-glue/src/lib.rs +++ b/ndk-glue/src/lib.rs @@ -6,8 +6,10 @@ use ndk::native_window::NativeWindow; use ndk_sys::{AInputQueue, ANativeActivity, ANativeWindow, ARect}; use once_cell::sync::Lazy; use std::ffi::{CStr, CString}; +use std::fmt; use std::fs::File; use std::io::{BufRead, BufReader}; +use std::ops::Deref; use std::os::raw; use std::os::unix::prelude::*; use std::ptr::NonNull; @@ -58,12 +60,50 @@ pub fn native_activity() -> &'static NativeActivity { unsafe { NATIVE_ACTIVITY.as_ref().unwrap() } } -pub fn native_window() -> RwLockReadGuard<'static, Option> { - NATIVE_WINDOW.read().unwrap() +pub struct LockReadGuard(RwLockReadGuard<'static, T>); + +impl Deref for LockReadGuard { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl fmt::Debug for LockReadGuard { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } } -pub fn input_queue() -> RwLockReadGuard<'static, Option> { - INPUT_QUEUE.read().unwrap() +impl fmt::Display for LockReadGuard { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +/// Returns a [`NativeWindow`] held inside a lock, preventing Android from freeing it immediately +/// in [its `NativeWindow` destructor]. +/// +/// If the window is in use by e.g. a graphics API, make sure to hold on to this lock. +/// +/// After receiving [`Event::WindowDestroyed`] `ndk-glue` will block in Android's [`NativeWindow`] destructor +/// callback until the lock is released, returning to Android and allowing it to free the window. +/// +/// [its `NativeWindow` destructor]: https://developer.android.com/ndk/reference/struct/a-native-activity-callbacks#onnativewindowdestroyed +pub fn native_window() -> LockReadGuard> { + LockReadGuard(NATIVE_WINDOW.read().unwrap()) +} + +/// Returns an [`InputQueue`] held inside a lock, preventing Android from freeing it immediately +/// in [its `InputQueue` destructor]. +/// +/// After receiving [`Event::InputQueueDestroyed`] `ndk-glue` will block in Android's [`InputQueue`] destructor +/// callback until the lock is released, returning to Android and allowing it to free the window. +/// +/// [its `InputQueue` destructor]: https://developer.android.com/ndk/reference/struct/a-native-activity-callbacks#oninputqueuedestroyed +pub fn input_queue() -> LockReadGuard> { + LockReadGuard(INPUT_QUEUE.read().unwrap()) } pub fn content_rect() -> Rect { @@ -116,19 +156,34 @@ pub enum Event { LowMemory, WindowLostFocus, WindowHasFocus, + /// A [`NativeWindow`] is now available through [`native_window()`]. See that function for more + /// details about holding on to the returned [`LockReadGuard`]. + /// + /// Be sure to release any resources (e.g. Vulkan/OpenGL graphics surfaces) created from + /// it followed by releasing this lock upon receiving [`Event::WindowDestroyed`]. WindowCreated, WindowResized, WindowRedrawNeeded, - /// If the window is in use by ie. a graphics API, make sure the lock from + /// If the window is in use by e.g. a graphics API, make sure the [`LockReadGuard`] from /// [`native_window()`] is held on to until after freeing those resources. /// - /// After receiving this [`Event`] `ndk_glue` will block until that read-lock - /// is released before returning to Android and allowing it to free up the window. + /// After receiving this [`Event`] `ndk_glue` will block inside its [`NativeWindow`] destructor + /// until that read-lock is released before returning to Android and allowing it to free the + /// window. + /// + /// From this point [`native_window()`] will return [`None`] until receiving + /// [`Event::WindowCreated`] again. WindowDestroyed, + /// An [`InputQueue`] is now available through [`input_queue()`]. + /// + /// Be sure to release the returned lock upon receiving [`Event::InputQueueDestroyed`]. InputQueueCreated, - /// After receiving this [`Event`] `ndk_glue` will block until the read-lock from - /// [`input_queue()`] is released before returning to Android and allowing it to - /// free up the input queue. + /// After receiving this [`Event`] `ndk_glue` will block inside its [`InputQueue`] destructor + /// until the read-lock from [`input_queue()`] is released before returning to Android and + /// allowing it to free the input queue. + /// + /// From this point [`input_queue()`] will return [`None`] until receiving + /// [`Event::InputQueueCreated`] again. InputQueueDestroyed, ContentRectChanged, }