Skip to content

Commit

Permalink
ndk-glue: Wrap NativeWindow/InputQueue locks in a newtype (#288)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MarijnS95 authored Jul 8, 2022
1 parent c3ff9c9 commit 8526787
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 10 deletions.
2 changes: 2 additions & 0 deletions ndk-glue/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
79 changes: 69 additions & 10 deletions ndk-glue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,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;
Expand Down Expand Up @@ -65,16 +67,58 @@ pub fn native_activity() -> &'static NativeActivity {
unsafe { NATIVE_ACTIVITY.as_ref().unwrap() }
}

pub struct LockReadGuard<T: ?Sized + 'static>(RwLockReadGuard<'static, T>);

impl<T: ?Sized> Deref for LockReadGuard<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T: ?Sized + fmt::Debug> fmt::Debug for LockReadGuard<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.fmt(f)
}
}

impl<T: ?Sized + fmt::Display> fmt::Display for LockReadGuard<T> {
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
///
/// # Warning
/// This function accesses a `static` variable internally and must only be used if you are sure
/// there is exactly one version of `ndk_glue` in your dependency tree.
pub fn native_window() -> RwLockReadGuard<'static, Option<NativeWindow>> {
NATIVE_WINDOW.read().unwrap()
pub fn native_window() -> LockReadGuard<Option<NativeWindow>> {
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
///
/// # Warning
/// This function accesses a `static` variable internally and must only be used if you are sure
/// there is exactly one version of `ndk_glue` in your dependency tree.
pub fn input_queue() -> RwLockReadGuard<'static, Option<InputQueue>> {
INPUT_QUEUE.read().unwrap()
pub fn input_queue() -> LockReadGuard<Option<InputQueue>> {
LockReadGuard(INPUT_QUEUE.read().unwrap())
}

/// This function accesses a `static` variable internally and must only be used if you are sure
Expand Down Expand Up @@ -129,19 +173,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,
}
Expand Down

0 comments on commit 8526787

Please sign in to comment.