From b161b24ce4a818433e88a2e44ad14a22c1e751fa Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Sun, 14 Aug 2022 06:05:57 +0100 Subject: [PATCH] Make AndroidApp Send + Sync In Winit then we ideally want to be able to associate a cloned reference to the AndroidApp with the `Window` and `MonitorHandle` types (since that's the most practical way to access the native_window and config state for the application) but for that `AndroidApp` needs to be Send + Sync --- android-activity/CHANGELOG.md | 1 + android-activity/src/game_activity/mod.rs | 59 +++++++++++++-------- android-activity/src/lib.rs | 47 ++++++++-------- android-activity/src/native_activity/mod.rs | 51 +++++++++++------- examples/agdk-winit-wgpu/.idea/gradle.xml | 1 - 5 files changed, 92 insertions(+), 67 deletions(-) diff --git a/android-activity/CHANGELOG.md b/android-activity/CHANGELOG.md index 6218955..9a69611 100644 --- a/android-activity/CHANGELOG.md +++ b/android-activity/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Emit an `InputAvailable` event for new input with `NativeActivity` and `GameActivity` enabling gui apps that don't render continuously - Oboe and Cpal audio examples added +- `AndroidApp` is now `Send` + `Sync` ### Changed - *Breaking*: updates to `ndk 0.7` and `ndk-sys 0.4` diff --git a/android-activity/src/game_activity/mod.rs b/android-activity/src/game_activity/mod.rs index 2b66977..470d166 100644 --- a/android-activity/src/game_activity/mod.rs +++ b/android-activity/src/game_activity/mod.rs @@ -4,6 +4,7 @@ use std::ffi::{CStr, CString}; use std::fs::File; use std::io::{BufRead, BufReader}; use std::marker::PhantomData; +use std::ops::Deref; use std::os::raw; use std::os::unix::prelude::*; use std::ptr::NonNull; @@ -45,7 +46,7 @@ impl<'a> StateSaver<'a> { // via libc::malloc since it will automatically handle freeing the data once it // has been handed over to the Java Activity / main thread. unsafe { - let app_ptr = self.app.ptr.as_ptr(); + let app_ptr = self.app.native_app.as_ptr(); // In case the application calls store() multiple times for some reason we // make sure to free any pre-existing state... @@ -82,7 +83,7 @@ pub struct StateLoader<'a> { impl<'a> StateLoader<'a> { pub fn load(&self) -> Option> { unsafe { - let app_ptr = self.app.ptr.as_ptr(); + let app_ptr = self.app.native_app.as_ptr(); if (*app_ptr).savedState != ptr::null_mut() && (*app_ptr).savedStateSize > 0 { let buf: &mut [u8] = std::slice::from_raw_parts_mut( (*app_ptr).savedState.cast(), @@ -126,18 +127,32 @@ impl AndroidApp { let config = Configuration::clone_from_ptr(NonNull::new_unchecked((*ptr.as_ptr()).config)); Self { - inner: Arc::new(AndroidAppInner { - ptr, + inner: Arc::new(RwLock::new(AndroidAppInner { + native_app: NativeAppGlue { ptr }, config: RwLock::new(config), native_window: Default::default(), - }), + })), } } } #[derive(Debug)] -pub struct AndroidAppInner { +struct NativeAppGlue { ptr: NonNull, +} +impl Deref for NativeAppGlue { + type Target = NonNull; + + fn deref(&self) -> &Self::Target { + &self.ptr + } +} +unsafe impl Send for NativeAppGlue {} +unsafe impl Sync for NativeAppGlue {} + +#[derive(Debug)] +pub struct AndroidAppInner { + native_app: NativeAppGlue, config: RwLock, native_window: RwLock>, } @@ -159,7 +174,7 @@ impl AndroidAppInner { trace!("poll_events"); unsafe { - let app_ptr = self.ptr; + let native_app = &self.native_app; let mut fd: i32 = 0; let mut events: i32 = 0; @@ -181,7 +196,7 @@ impl AndroidAppInner { ffi::ALOOPER_POLL_WAKE => { trace!("ALooper_pollAll returned POLL_WAKE"); - if ffi::android_app_input_available_wake_up(app_ptr.as_ptr()) { + if ffi::android_app_input_available_wake_up(native_app.as_ptr()) { log::debug!("Notifying Input Available"); callback(PollEvent::Main(MainEvent::InputAvailable)); } @@ -213,7 +228,7 @@ impl AndroidAppInner { trace!("ALooper_pollAll returned ID_MAIN"); let source: *mut ffi::android_poll_source = source.cast(); if source != ptr::null_mut() { - let cmd_i = ffi::android_app_read_cmd(app_ptr.as_ptr()); + let cmd_i = ffi::android_app_read_cmd(native_app.as_ptr()); let cmd = match cmd_i as u32 { //NativeAppGlueAppCmd_UNUSED_APP_CMD_INPUT_CHANGED => AndroidAppMainEvent::InputChanged, @@ -265,16 +280,16 @@ impl AndroidAppInner { trace!("Read ID_MAIN command {cmd_i} = {cmd:?}"); trace!("Calling android_app_pre_exec_cmd({cmd_i})"); - ffi::android_app_pre_exec_cmd(app_ptr.as_ptr(), cmd_i); + ffi::android_app_pre_exec_cmd(native_app.as_ptr(), cmd_i); match cmd { MainEvent::ConfigChanged => { *self.config.write().unwrap() = Configuration::clone_from_ptr(NonNull::new_unchecked( - (*app_ptr.as_ptr()).config, + (*native_app.as_ptr()).config, )); } MainEvent::InitWindow { .. } => { - let win_ptr = (*app_ptr.as_ptr()).window; + let win_ptr = (*native_app.as_ptr()).window; // It's important that we use ::clone_from_ptr() here // because NativeWindow has a Drop implementation that // will unconditionally _release() the native window @@ -293,7 +308,7 @@ impl AndroidAppInner { callback(PollEvent::Main(cmd)); trace!("Calling android_app_post_exec_cmd({cmd_i})"); - ffi::android_app_post_exec_cmd(app_ptr.as_ptr(), cmd_i); + ffi::android_app_post_exec_cmd(native_app.as_ptr(), cmd_i); } else { panic!("ALooper_pollAll returned ID_MAIN event with NULL android_poll_source!"); } @@ -320,11 +335,11 @@ impl AndroidAppInner { } } - pub fn enable_motion_axis(&self, axis: Axis) { + pub fn enable_motion_axis(&mut self, axis: Axis) { unsafe { ffi::GameActivityPointerAxes_enableAxis(axis as i32) } } - pub fn disable_motion_axis(&self, axis: Axis) { + pub fn disable_motion_axis(&mut self, axis: Axis) { unsafe { ffi::GameActivityPointerAxes_disableAxis(axis as i32) } } @@ -332,7 +347,7 @@ impl AndroidAppInner { unsafe { // From the application's pov we assume the app_ptr and looper pointer // have static lifetimes and we can safely assume they are never NULL. - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); AndroidAppWaker { looper: NonNull::new_unchecked((*app_ptr).looper), } @@ -345,7 +360,7 @@ impl AndroidAppInner { pub fn content_rect(&self) -> Rect { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); Rect { left: (*app_ptr).contentRect.left, right: (*app_ptr).contentRect.right, @@ -357,7 +372,7 @@ impl AndroidAppInner { pub fn asset_manager(&self) -> AssetManager { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); let am_ptr = NonNull::new_unchecked((*(*app_ptr).activity).assetManager); AssetManager::from_ptr(am_ptr) } @@ -368,7 +383,7 @@ impl AndroidAppInner { F: FnMut(&InputEvent), { let buf = unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); let input_buffer = ffi::android_app_swap_input_buffers(app_ptr); if input_buffer == ptr::null_mut() { return; @@ -386,21 +401,21 @@ impl AndroidAppInner { pub fn internal_data_path(&self) -> Option { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); util::try_get_path_from_ptr((*(*app_ptr).activity).internalDataPath) } } pub fn external_data_path(&self) -> Option { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); util::try_get_path_from_ptr((*(*app_ptr).activity).externalDataPath) } } pub fn obb_path(&self) -> Option { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); util::try_get_path_from_ptr((*(*app_ptr).activity).obbPath) } } diff --git a/android-activity/src/lib.rs b/android-activity/src/lib.rs index 048fad4..11ed79f 100644 --- a/android-activity/src/lib.rs +++ b/android-activity/src/lib.rs @@ -1,5 +1,6 @@ use std::hash::Hash; use std::ops::Deref; +use std::sync::RwLock; use std::time::Duration; use std::{os::unix::prelude::RawFd, sync::Arc}; @@ -207,7 +208,7 @@ pub use activity_impl::AndroidAppWaker; #[derive(Debug, Clone)] pub struct AndroidApp { - pub(crate) inner: Arc, + pub(crate) inner: Arc>, } impl PartialEq for AndroidApp { @@ -227,7 +228,7 @@ impl AndroidApp { #[cfg_attr(docsrs, doc(cfg(feature = "native-activity")))] #[cfg(feature = "native-activity")] pub(crate) fn native_activity(&self) -> *const ndk_sys::ANativeActivity { - self.inner.native_activity() + self.inner.read().unwrap().native_activity() } /// Queries the current [`NativeWindow`] for the application. @@ -236,7 +237,7 @@ impl AndroidApp { /// [`AndroidAppMainEvent::InitWindow`] and [`AndroidAppMainEvent::TerminateWindow`] /// events. pub fn native_window<'a>(&self) -> Option { - self.inner.native_window() + self.inner.read().unwrap().native_window() } /// Calls [`ALooper_pollAll`] on the looper associated with this AndroidApp as well @@ -250,14 +251,11 @@ impl AndroidApp { /// set to `None` once the callback returns, and this is also synchronized with the Java /// main thread. The [`MainEvent::SaveState`] event is also synchronized with the /// Java main thread. - /// - /// # Safety - /// This API must only be called from the application's main thread pub fn poll_events(&self, timeout: Option, callback: F) where F: FnMut(PollEvent), { - self.inner.poll_events(timeout, callback); + self.inner.read().unwrap().poll_events(timeout, callback); } /// Creates a means to wake up the main loop while it is blocked waiting for @@ -265,50 +263,41 @@ impl AndroidApp { /// /// Internally this uses [`ALooper_wake`] on the looper associated with this /// [AndroidApp]. - /// - /// # Safety - /// This API can be used from any thread pub fn create_waker(&self) -> activity_impl::AndroidAppWaker { - self.inner.create_waker() + self.inner.read().unwrap().create_waker() } /// Returns a deep copy of this application's [`Configuration`] pub fn config(&self) -> Configuration { - self.inner.config() + self.inner.read().unwrap().config() } /// Queries the current content rectangle of the window; this is the area where the /// window's content should be placed to be seen by the user. - /// - /// # Safety - /// This API must only be called from the applications main thread pub fn content_rect(&self) -> Rect { - self.inner.content_rect() + self.inner.read().unwrap().content_rect() } /// Queries the Asset Manager instance for the application. /// /// Use this to access binary assets bundled inside your application's .apk file. - /// - /// # Safety - /// This API must only be called from the applications main thread pub fn asset_manager(&self) -> AssetManager { - self.inner.asset_manager() + self.inner.read().unwrap().asset_manager() } pub fn enable_motion_axis(&self, axis: input::Axis) { - self.inner.enable_motion_axis(axis); + self.inner.write().unwrap().enable_motion_axis(axis); } pub fn disable_motion_axis(&self, axis: input::Axis) { - self.inner.disable_motion_axis(axis); + self.inner.write().unwrap().disable_motion_axis(axis); } pub fn input_events<'b, F>(&self, callback: F) where F: FnMut(&input::InputEvent), { - self.inner.input_events(callback); + self.inner.read().unwrap().input_events(callback); } /// The user-visible SDK version of the framework @@ -325,16 +314,22 @@ impl AndroidApp { /// Path to this application's internal data directory pub fn internal_data_path(&self) -> Option { - self.inner.internal_data_path() + self.inner.read().unwrap().internal_data_path() } /// Path to this application's external data directory pub fn external_data_path(&self) -> Option { - self.inner.external_data_path() + self.inner.read().unwrap().external_data_path() } /// Path to the directory containing the application's OBB files (if any). pub fn obb_path(&self) -> Option { - self.inner.obb_path() + self.inner.read().unwrap().obb_path() } } + +#[test] +fn test_app_is_send_sync() { + fn needs_send_sync() {} + needs_send_sync::(); +} diff --git a/android-activity/src/native_activity/mod.rs b/android-activity/src/native_activity/mod.rs index bf9aa8a..c315ac3 100644 --- a/android-activity/src/native_activity/mod.rs +++ b/android-activity/src/native_activity/mod.rs @@ -3,6 +3,7 @@ use std::ffi::{CStr, CString}; 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; @@ -47,7 +48,7 @@ impl<'a> StateSaver<'a> { // via libc::malloc since it will automatically handle freeing the data once it // has been handed over to the Java Activity / main thread. unsafe { - let app_ptr = self.app.ptr.as_ptr(); + let app_ptr = self.app.native_app.as_ptr(); // In case the application calls store() multiple times for some reason we // make sure to free any pre-existing state... @@ -84,7 +85,7 @@ pub struct StateLoader<'a> { impl<'a> StateLoader<'a> { pub fn load(&self) -> Option> { unsafe { - let app_ptr = self.app.ptr.as_ptr(); + let app_ptr = self.app.native_app.as_ptr(); if (*app_ptr).savedState != ptr::null_mut() && (*app_ptr).savedStateSize > 0 { let buf: &mut [u8] = std::slice::from_raw_parts_mut( (*app_ptr).savedState.cast(), @@ -128,18 +129,32 @@ impl AndroidApp { let config = Configuration::clone_from_ptr(NonNull::new_unchecked((*ptr.as_ptr()).config)); AndroidApp { - inner: Arc::new(AndroidAppInner { - ptr, + inner: Arc::new(RwLock::new(AndroidAppInner { + native_app: NativeAppGlue { ptr }, config: RwLock::new(config), native_window: Default::default(), - }), + })), } } } #[derive(Debug)] -pub(crate) struct AndroidAppInner { +struct NativeAppGlue { ptr: NonNull, +} +impl Deref for NativeAppGlue { + type Target = NonNull; + + fn deref(&self) -> &Self::Target { + &self.ptr + } +} +unsafe impl Send for NativeAppGlue {} +unsafe impl Sync for NativeAppGlue {} + +#[derive(Debug)] +pub(crate) struct AndroidAppInner { + native_app: NativeAppGlue, config: RwLock, native_window: RwLock>, } @@ -147,7 +162,7 @@ pub(crate) struct AndroidAppInner { impl AndroidAppInner { pub(crate) fn native_activity(&self) -> *const ndk_sys::ANativeActivity { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); (*app_ptr).activity.cast() } } @@ -168,7 +183,7 @@ impl AndroidAppInner { trace!("poll_events"); unsafe { - let app_ptr = self.ptr; + let native_app = &self.native_app; let mut fd: i32 = 0; let mut events: i32 = 0; @@ -217,7 +232,7 @@ impl AndroidAppInner { trace!("ALooper_pollAll returned ID_MAIN"); let source: *mut ffi::android_poll_source = source.cast(); if source != ptr::null_mut() { - let cmd_i = ffi::android_app_read_cmd(app_ptr.as_ptr()); + let cmd_i = ffi::android_app_read_cmd(native_app.as_ptr()); let cmd = match cmd_i as u32 { // We don't forward info about the AInputQueue to apps since it's @@ -256,7 +271,7 @@ impl AndroidAppInner { }; trace!("Calling android_app_pre_exec_cmd({cmd_i})"); - ffi::android_app_pre_exec_cmd(app_ptr.as_ptr(), cmd_i); + ffi::android_app_pre_exec_cmd(native_app.as_ptr(), cmd_i); if let Some(cmd) = cmd { trace!("Read ID_MAIN command {cmd_i} = {cmd:?}"); @@ -265,12 +280,12 @@ impl AndroidAppInner { *self.config.write().unwrap() = Configuration::clone_from_ptr( NonNull::new_unchecked( - (*app_ptr.as_ptr()).config, + (*native_app.as_ptr()).config, ), ); } MainEvent::InitWindow { .. } => { - let win_ptr = (*app_ptr.as_ptr()).window; + let win_ptr = (*native_app.as_ptr()).window; // It's important that we use ::clone_from_ptr() here // because NativeWindow has a Drop implementation that // will unconditionally _release() the native window @@ -290,7 +305,7 @@ impl AndroidAppInner { } trace!("Calling android_app_post_exec_cmd({cmd_i})"); - ffi::android_app_post_exec_cmd(app_ptr.as_ptr(), cmd_i); + ffi::android_app_post_exec_cmd(native_app.as_ptr(), cmd_i); } else { panic!("ALooper_pollAll returned ID_MAIN event with NULL android_poll_source!"); } @@ -302,7 +317,7 @@ impl AndroidAppInner { // input events then we only send one `InputAvailable` per iteration of input // handling. We re-attach the looper when the application calls // `AndroidApp::input_events()` - ffi::android_app_detach_input_queue_looper(app_ptr.as_ptr()); + ffi::android_app_detach_input_queue_looper(native_app.as_ptr()); callback(PollEvent::Main(MainEvent::InputAvailable)) } _ => { @@ -331,7 +346,7 @@ impl AndroidAppInner { unsafe { // From the application's pov we assume the app_ptr and looper pointer // have static lifetimes and we can safely assume they are never NULL. - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); AndroidAppWaker { looper: NonNull::new_unchecked((*app_ptr).looper), } @@ -344,7 +359,7 @@ impl AndroidAppInner { pub fn content_rect(&self) -> Rect { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); Rect { left: (*app_ptr).contentRect.left, right: (*app_ptr).contentRect.right, @@ -356,7 +371,7 @@ impl AndroidAppInner { pub fn asset_manager(&self) -> AssetManager { unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); let am_ptr = NonNull::new_unchecked((*(*app_ptr).activity).assetManager); AssetManager::from_ptr(am_ptr) } @@ -375,7 +390,7 @@ impl AndroidAppInner { F: FnMut(&input::InputEvent), { let queue = unsafe { - let app_ptr = self.ptr.as_ptr(); + let app_ptr = self.native_app.as_ptr(); if (*app_ptr).inputQueue == ptr::null_mut() { return; } diff --git a/examples/agdk-winit-wgpu/.idea/gradle.xml b/examples/agdk-winit-wgpu/.idea/gradle.xml index 526b4c2..a2d7c21 100644 --- a/examples/agdk-winit-wgpu/.idea/gradle.xml +++ b/examples/agdk-winit-wgpu/.idea/gradle.xml @@ -13,7 +13,6 @@ -