From 72302e9dd09ad2b97dea2463c3f400fe415182c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20Frosteg=C3=A5rd?= Date: Mon, 28 Dec 2020 03:44:23 +0100 Subject: [PATCH] macOS: use CFRunLoopTimer, improve view release logic (#84) * macOS: fix property_no fn * Use CFRunLoopTimer instead if NSTimer This means the timer doesn't keep a reference to the view, which should make it easer to check retain_count in release. * macOS: take pointer instead of Arc in WindowState::setup_timer * Save retain count increase from build fn, use in release fn * macOS: in window setup, run build fn before doing parenting * macOS: clean up parenting * macOS: wrap WindowState in Box instead of Arc to improve clarity * macOS: use better names for ivar consts, move them to view.rs * Remove no longer used crate static_assertions * macOS: in view release fn, delete class when retain_count == 1 * macOS: set window state ivar to null after dropping * macOS: store retain count after build in WindowState * macOS: rename BASEVIEW_WINDOW_STATE_IVAR to BASEVIEW_STATE_IVAR --- Cargo.toml | 2 +- src/macos/view.rs | 67 ++++++++---------- src/macos/window.rs | 167 ++++++++++++++++++++++++-------------------- 3 files changed, 120 insertions(+), 116 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index baea30f..e9b1aa0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,6 @@ license = "MIT OR Apache-2.0" [dependencies] keyboard-types = { version = "0.5.0", default-features = false } raw-window-handle = "0.3.3" -static_assertions = "1.1.0" [target.'cfg(target_os="linux")'.dependencies] xcb = { version = "0.9", features = ["thread", "xlib_xcb", "dri2"] } @@ -29,6 +28,7 @@ winapi = { version = "0.3.8", features = ["libloaderapi", "winuser", "windef", " [target.'cfg(target_os="macos")'.dependencies] cocoa = "0.24.0" +core-foundation = "0.9.1" objc = "0.2.7" uuid = { version = "0.8", features = ["v4"] } diff --git a/src/macos/view.rs b/src/macos/view.rs index 87c9d11..0161b26 100644 --- a/src/macos/view.rs +++ b/src/macos/view.rs @@ -1,5 +1,4 @@ use std::ffi::c_void; -use std::sync::Arc; use cocoa::appkit::{NSEvent, NSView}; use cocoa::base::{id, nil, BOOL, YES, NO}; @@ -17,9 +16,11 @@ use uuid::Uuid; use crate::{Event, MouseButton, MouseEvent, Point, WindowOpenOptions}; use crate::MouseEvent::{ButtonPressed, ButtonReleased}; -use super::window::{ - WindowState, WINDOW_STATE_IVAR_NAME, FRAME_TIMER_IVAR_NAME -}; +use super::window::WindowState; + + +/// Name of the field used to store the `WindowState` pointer. +pub(super) const BASEVIEW_STATE_IVAR: &str = "baseview_state"; pub(super) unsafe fn create_view( @@ -66,14 +67,9 @@ unsafe fn create_view_class() -> &'static Class { accepts_first_mouse as extern "C" fn(&Object, Sel, id) -> BOOL ); - class.add_method( - sel!(triggerOnFrame:), - trigger_on_frame as extern "C" fn(&Object, Sel, id) - ); - class.add_method( sel!(release), - release as extern "C" fn(&Object, Sel) + release as extern "C" fn(&mut Object, Sel) ); class.add_method( sel!(viewWillMoveToWindow:), @@ -151,8 +147,7 @@ unsafe fn create_view_class() -> &'static Class { flags_changed as extern "C" fn(&Object, Sel, id), ); - class.add_ivar::<*mut c_void>(WINDOW_STATE_IVAR_NAME); - class.add_ivar::<*mut c_void>(FRAME_TIMER_IVAR_NAME); + class.add_ivar::<*mut c_void>(BASEVIEW_STATE_IVAR); class.register() } @@ -170,7 +165,7 @@ extern "C" fn property_no( _this: &Object, _sel: Sel, ) -> BOOL { - YES + NO } @@ -183,20 +178,7 @@ extern "C" fn accepts_first_mouse( } -extern "C" fn trigger_on_frame( - this: &Object, - _sel: Sel, - _event: id -){ - let state: &mut WindowState = unsafe { - WindowState::from_field(this) - }; - - state.trigger_frame(); -} - - -extern "C" fn release(this: &Object, _sel: Sel) { +extern "C" fn release(this: &mut Object, _sel: Sel) { unsafe { let superclass = msg_send![this, superclass]; @@ -206,19 +188,26 @@ extern "C" fn release(this: &Object, _sel: Sel) { unsafe { let retain_count: usize = msg_send![this, retainCount]; + let state_ptr: *mut c_void = *this.get_ivar(BASEVIEW_STATE_IVAR); + + if !state_ptr.is_null(){ + let retain_count_after_build = WindowState::from_field(this) + .retain_count_after_build; + + if retain_count <= retain_count_after_build { + WindowState::from_field(this).remove_timer(); + + this.set_ivar( + BASEVIEW_STATE_IVAR, + ::std::ptr::null() as *const c_void + ); + + // Drop WindowState + Box::from_raw(state_ptr as *mut WindowState); + } + } + if retain_count == 1 { - // Invalidate frame timer - let frame_timer_ptr: *mut c_void = *this.get_ivar( - FRAME_TIMER_IVAR_NAME - ); - let _: () = msg_send![frame_timer_ptr as id, invalidate]; - - // Drop WindowState - let state_ptr: *mut c_void = *this.get_ivar( - WINDOW_STATE_IVAR_NAME - ); - Arc::from_raw(state_ptr as *mut WindowState); - // Delete class let class = msg_send![this, class]; ::objc::runtime::objc_disposeClassPair(class); diff --git a/src/macos/window.rs b/src/macos/window.rs index 3fe4c22..d978da9 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -1,12 +1,15 @@ use std::ffi::c_void; -use std::sync::Arc; use cocoa::appkit::{ NSApp, NSApplication, NSApplicationActivationPolicyRegular, NSBackingStoreBuffered, NSWindow, NSWindowStyleMask, }; -use cocoa::base::{id, nil, NO, YES}; +use cocoa::base::{id, nil, NO}; use cocoa::foundation::{NSAutoreleasePool, NSPoint, NSRect, NSSize, NSString}; +use core_foundation::runloop::{ + CFRunLoop, CFRunLoopTimer, CFRunLoopTimerContext, __CFRunLoopTimer, + kCFRunLoopDefaultMode, +}; use keyboard_types::KeyboardEvent; use objc::{msg_send, runtime::Object, sel, sel_impl}; @@ -18,17 +21,10 @@ use crate::{ WindowScalePolicy, WindowInfo }; -use super::view::create_view; +use super::view::{create_view, BASEVIEW_STATE_IVAR}; use super::keyboard::KeyboardState; -/// Name of the field used to store the `WindowState` pointer in the custom -/// view class. -pub(super) const WINDOW_STATE_IVAR_NAME: &str = "WINDOW_STATE_IVAR_NAME"; - -pub(super) const FRAME_TIMER_IVAR_NAME: &str = "FRAME_TIMER"; - - pub struct AppRunner; impl AppRunner { @@ -61,38 +57,35 @@ impl Window { { let _pool = unsafe { NSAutoreleasePool::new(nil) }; - let (mut window, opt_app_runner) = match options.parent { - Parent::WithParent(parent) => { - if let RawWindowHandle::MacOS(handle) = parent { - let ns_view = handle.ns_view as *mut objc::runtime::Object; + let mut window = unsafe { + Window { + ns_window: None, + ns_view: create_view(&options), + } + }; - unsafe { - let subview = create_view(&options); + let window_handler = Box::new(build(&mut crate::Window(&mut window))); - let _: id = msg_send![ns_view, addSubview: subview]; + let retain_count_after_build: usize = unsafe { + msg_send![window.ns_view, retainCount] + }; - let window = Window { - ns_window: None, - ns_view: subview, - }; - - (window, None) - } - } else { - panic!("Not a macOS window"); + let opt_app_runner = match options.parent { + Parent::WithParent(RawWindowHandle::MacOS(handle)) => { + unsafe { + let () = msg_send![ + handle.ns_view as *mut Object, + addSubview: window.ns_view + ]; } + + None + }, + Parent::WithParent(_) => { + panic!("Not a macOS window"); }, Parent::AsIfParented => { - let ns_view = unsafe { - create_view(&options) - }; - - let window = Window { - ns_window: None, - ns_view, - }; - - (window, None) + None }, Parent::None => { // It seems prudent to run NSApp() here before doing other @@ -143,58 +136,30 @@ impl Window { ns_window.makeKeyAndOrderFront_(nil); - let subview = create_view(&options); + ns_window.setContentView_(window.ns_view); - ns_window.setContentView_(subview); + window.ns_window = Some(ns_window); - let window = Window { - ns_window: Some(ns_window), - ns_view: subview, - }; - - (window, Some(crate::AppRunner(AppRunner))) + Some(crate::AppRunner(AppRunner)) } }, }; - let window_handler = Box::new(build(&mut crate::Window(&mut window))); - - let window_state_arc = Arc::new(WindowState { + let window_state_ptr = Box::into_raw(Box::new(WindowState { window, window_handler, keyboard_state: KeyboardState::new(), - }); - - let window_state_pointer = Arc::into_raw( - window_state_arc.clone() - ) as *mut c_void; + frame_timer: None, + retain_count_after_build, + })); unsafe { - (*window_state_arc.window.ns_view).set_ivar( - WINDOW_STATE_IVAR_NAME, - window_state_pointer + (*(*window_state_ptr).window.ns_view).set_ivar( + BASEVIEW_STATE_IVAR, + window_state_ptr as *mut c_void ); - } - // Setup frame timer once window state is stored - unsafe { - let timer_interval = 0.015; - let selector = sel!(triggerOnFrame:); - - let timer: id = msg_send![ - ::objc::class!(NSTimer), - scheduledTimerWithTimeInterval:timer_interval - target:window_state_arc.window.ns_view - selector:selector - userInfo:nil - repeats:YES - ]; - - // Store pointer to timer for invalidation when view is released - (*window_state_arc.window.ns_view).set_ivar( - FRAME_TIMER_IVAR_NAME, - timer as *mut c_void, - ) + WindowState::setup_timer(window_state_ptr); } opt_app_runner @@ -206,6 +171,8 @@ pub(super) struct WindowState { window: Window, window_handler: Box, keyboard_state: KeyboardState, + frame_timer: Option, + pub retain_count_after_build: usize, } @@ -216,7 +183,7 @@ impl WindowState { /// WindowState. Apparently, macOS blocks for the duration of an event, /// callback, meaning that this shouldn't be a problem in practice. pub(super) unsafe fn from_field(obj: &Object) -> &mut Self { - let state_ptr: *mut c_void = *obj.get_ivar(WINDOW_STATE_IVAR_NAME); + let state_ptr: *mut c_void = *obj.get_ivar(BASEVIEW_STATE_IVAR); &mut *(state_ptr as *mut Self) } @@ -238,6 +205,54 @@ impl WindowState { ) -> Option { self.keyboard_state.process_native_event(event) } + + /// Don't call until WindowState pointer is stored in view + unsafe fn setup_timer(window_state_ptr: *mut WindowState){ + extern "C" fn timer_callback( + _: *mut __CFRunLoopTimer, + window_state_ptr: *mut c_void, + ){ + unsafe { + let window_state = &mut *( + window_state_ptr as *mut WindowState + ); + + window_state.trigger_frame(); + } + } + + let mut timer_context = CFRunLoopTimerContext { + version: 0, + info: window_state_ptr as *mut c_void, + retain: None, + release: None, + copyDescription: None, + }; + + let timer = CFRunLoopTimer::new( + 0.0, + 0.015, + 0, + 0, + timer_callback, + &mut timer_context, + ); + + CFRunLoop::get_current() + .add_timer(&timer, kCFRunLoopDefaultMode); + + let window_state = &mut *(window_state_ptr); + + window_state.frame_timer = Some(timer); + } + + /// Call when freeing view + pub(super) unsafe fn remove_timer(&mut self){ + if let Some(frame_timer) = self.frame_timer.take(){ + CFRunLoop::get_current() + .remove_timer(&frame_timer, kCFRunLoopDefaultMode); + } + } }