diff --git a/Cargo.toml b/Cargo.toml index ac824f2d..fd608013 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,10 +112,12 @@ objc2 = "0.5.2" [target.'cfg(target_os = "macos")'.dependencies] core-graphics = "0.23.1" +block2 = "0.5.1" [target.'cfg(target_os = "macos")'.dependencies.objc2-foundation] version = "0.2.2" features = [ + "block2", "dispatch", "NSArray", "NSAttributedString", diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 9fec6b62..e689a07f 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -57,3 +57,5 @@ changelog entry. - On macOS, fix panic on exit when dropping windows outside the event loop. - On macOS, fix window dragging glitches when dragging across a monitor boundary with different scale factor. - On macOS, fix the range in `Ime::Preedit`. +- On macOS, use the system's internal mechanisms for queuing events. +- On macOS, handle events directly instead of queuing when possible. diff --git a/src/platform_impl/macos/app.rs b/src/platform_impl/macos/app.rs index 23a1b071..1d1b21d7 100644 --- a/src/platform_impl/macos/app.rs +++ b/src/platform_impl/macos/app.rs @@ -57,25 +57,27 @@ fn maybe_dispatch_device_event(delegate: &ApplicationDelegate, event: &NSEvent) let delta_y = unsafe { event.deltaY() } as f64; if delta_x != 0.0 { - delegate.queue_device_event(DeviceEvent::Motion { axis: 0, value: delta_x }); + delegate.maybe_queue_device_event(DeviceEvent::Motion { axis: 0, value: delta_x }); } if delta_y != 0.0 { - delegate.queue_device_event(DeviceEvent::Motion { axis: 1, value: delta_y }) + delegate.maybe_queue_device_event(DeviceEvent::Motion { axis: 1, value: delta_y }) } if delta_x != 0.0 || delta_y != 0.0 { - delegate.queue_device_event(DeviceEvent::MouseMotion { delta: (delta_x, delta_y) }); + delegate.maybe_queue_device_event(DeviceEvent::MouseMotion { + delta: (delta_x, delta_y), + }); } }, NSEventType::LeftMouseDown | NSEventType::RightMouseDown | NSEventType::OtherMouseDown => { - delegate.queue_device_event(DeviceEvent::Button { + delegate.maybe_queue_device_event(DeviceEvent::Button { button: unsafe { event.buttonNumber() } as u32, state: ElementState::Pressed, }); }, NSEventType::LeftMouseUp | NSEventType::RightMouseUp | NSEventType::OtherMouseUp => { - delegate.queue_device_event(DeviceEvent::Button { + delegate.maybe_queue_device_event(DeviceEvent::Button { button: unsafe { event.buttonNumber() } as u32, state: ElementState::Released, }); diff --git a/src/platform_impl/macos/app_delegate.rs b/src/platform_impl/macos/app_delegate.rs index 9de572aa..08f1037a 100644 --- a/src/platform_impl/macos/app_delegate.rs +++ b/src/platform_impl/macos/app_delegate.rs @@ -1,23 +1,19 @@ use std::cell::{Cell, RefCell}; -use std::collections::VecDeque; use std::mem; use std::rc::Weak; -use std::sync::{Arc, Mutex}; use std::time::Instant; use objc2::rc::Retained; use objc2::runtime::AnyObject; use objc2::{declare_class, msg_send_id, mutability, ClassType, DeclaredClass}; use objc2_app_kit::{NSApplication, NSApplicationActivationPolicy, NSApplicationDelegate}; -use objc2_foundation::{MainThreadMarker, NSObject, NSObjectProtocol, NSSize}; +use objc2_foundation::{MainThreadMarker, NSObject, NSObjectProtocol}; use super::event_handler::EventHandler; use super::event_loop::{stop_app_immediately, ActiveEventLoop, PanicInfo}; use super::observer::{EventLoopWaker, RunLoop}; -use super::window::WinitWindow; use super::{menu, WindowId, DEVICE_ID}; -use crate::dpi::PhysicalSize; -use crate::event::{DeviceEvent, Event, InnerSizeWriter, StartCause, WindowEvent}; +use crate::event::{DeviceEvent, Event, StartCause, WindowEvent}; use crate::event_loop::{ActiveEventLoop as RootActiveEventLoop, ControlFlow}; use crate::window::WindowId as RootWindowId; @@ -35,6 +31,7 @@ pub(super) struct State { activation_policy: Policy, default_menu: bool, activate_ignoring_other_apps: bool, + run_loop: RunLoop, event_handler: EventHandler, stop_on_launch: Cell, stop_before_wait: Cell, @@ -50,7 +47,6 @@ pub(super) struct State { waker: RefCell, start_time: Cell>, wait_timeout: Cell>, - pending_events: RefCell>, pending_redraw: RefCell>, // NOTE: This is strongly referenced by our `NSWindowDelegate` and our `NSView` subclass, and // as such should be careful to not add fields that, in turn, strongly reference those. @@ -138,6 +134,7 @@ impl ApplicationDelegate { activation_policy: Policy(activation_policy), default_menu, activate_ignoring_other_apps, + run_loop: RunLoop::main(mtm), ..Default::default() }); unsafe { msg_send_id![super(this), init] } @@ -234,28 +231,16 @@ impl ApplicationDelegate { self.ivars().control_flow.get() } - pub fn queue_window_event(&self, window_id: WindowId, event: WindowEvent) { - self.ivars() - .pending_events - .borrow_mut() - .push_back(QueuedEvent::WindowEvent(window_id, event)); + pub fn maybe_queue_window_event(&self, window_id: WindowId, event: WindowEvent) { + self.maybe_queue_event(Event::WindowEvent { window_id: RootWindowId(window_id), event }); } - pub fn queue_device_event(&self, event: DeviceEvent) { - self.ivars().pending_events.borrow_mut().push_back(QueuedEvent::DeviceEvent(event)); + pub fn handle_window_event(&self, window_id: WindowId, event: WindowEvent) { + self.handle_event(Event::WindowEvent { window_id: RootWindowId(window_id), event }); } - pub fn queue_static_scale_factor_changed_event( - &self, - window: Retained, - suggested_size: PhysicalSize, - scale_factor: f64, - ) { - self.ivars().pending_events.borrow_mut().push_back(QueuedEvent::ScaleFactorChanged { - window, - suggested_size, - scale_factor, - }); + pub fn maybe_queue_device_event(&self, event: DeviceEvent) { + self.maybe_queue_event(Event::DeviceEvent { device_id: DEVICE_ID, event }); } pub fn handle_redraw(&self, window_id: WindowId) { @@ -283,9 +268,27 @@ impl ApplicationDelegate { if !pending_redraw.contains(&window_id) { pending_redraw.push(window_id); } - unsafe { RunLoop::get() }.wakeup(); + self.ivars().run_loop.wakeup(); } + #[track_caller] + fn maybe_queue_event(&self, event: Event) { + // Most programmer actions in AppKit (e.g. change window fullscreen, set focused, etc.) + // result in an event being queued, and applied at a later point. + // + // However, it is not documented which actions do this, and which ones are done immediately, + // so to make sure that we don't encounter re-entrancy issues, we first check if we're + // currently handling another event, and if we are, we queue the event instead. + if !self.ivars().event_handler.in_use() { + self.handle_event(event); + } else { + tracing::debug!(?event, "had to queue event since another is currently being handled"); + let this = self.retain(); + self.ivars().run_loop.queue_closure(move || this.handle_event(event)); + } + } + + #[track_caller] fn handle_event(&self, event: Event) { self.ivars().event_handler.handle_event(event, &ActiveEventLoop::new_root(self.retain())) } @@ -347,49 +350,6 @@ impl ApplicationDelegate { self.handle_event(Event::UserEvent(HandlePendingUserEvents)); - let events = mem::take(&mut *self.ivars().pending_events.borrow_mut()); - for event in events { - match event { - QueuedEvent::WindowEvent(window_id, event) => { - self.handle_event(Event::WindowEvent { - window_id: RootWindowId(window_id), - event, - }); - }, - QueuedEvent::DeviceEvent(event) => { - self.handle_event(Event::DeviceEvent { device_id: DEVICE_ID, event }); - }, - QueuedEvent::ScaleFactorChanged { window, suggested_size, scale_factor } => { - let new_inner_size = Arc::new(Mutex::new(suggested_size)); - let scale_factor_changed_event = Event::WindowEvent { - window_id: RootWindowId(window.id()), - event: WindowEvent::ScaleFactorChanged { - scale_factor, - inner_size_writer: InnerSizeWriter::new(Arc::downgrade( - &new_inner_size, - )), - }, - }; - - self.handle_event(scale_factor_changed_event); - - let physical_size = *new_inner_size.lock().unwrap(); - drop(new_inner_size); - if physical_size != suggested_size { - let logical_size = physical_size.to_logical(scale_factor); - let size = NSSize::new(logical_size.width, logical_size.height); - window.setContentSize(size); - } - - let resized_event = Event::WindowEvent { - window_id: RootWindowId(window.id()), - event: WindowEvent::Resized(physical_size), - }; - self.handle_event(resized_event); - }, - } - } - let redraw = mem::take(&mut *self.ivars().pending_redraw.borrow_mut()); for window_id in redraw { self.handle_event(Event::WindowEvent { @@ -420,17 +380,6 @@ impl ApplicationDelegate { } } -#[derive(Debug)] -pub(crate) enum QueuedEvent { - WindowEvent(WindowId, WindowEvent), - DeviceEvent(DeviceEvent), - ScaleFactorChanged { - window: Retained, - suggested_size: PhysicalSize, - scale_factor: f64, - }, -} - #[derive(Debug)] pub(crate) struct HandlePendingUserEvents; diff --git a/src/platform_impl/macos/event_loop.rs b/src/platform_impl/macos/event_loop.rs index 68f411ce..75333b60 100644 --- a/src/platform_impl/macos/event_loop.rs +++ b/src/platform_impl/macos/event_loop.rs @@ -252,7 +252,7 @@ impl EventLoop { }); let panic_info: Rc = Default::default(); - setup_control_flow_observers(Rc::downgrade(&panic_info)); + setup_control_flow_observers(mtm, Rc::downgrade(&panic_info)); let (sender, receiver) = mpsc::channel(); Ok(EventLoop { diff --git a/src/platform_impl/macos/observer.rs b/src/platform_impl/macos/observer.rs index 25571f09..732febc1 100644 --- a/src/platform_impl/macos/observer.rs +++ b/src/platform_impl/macos/observer.rs @@ -1,19 +1,26 @@ +//! Utilities for working with `CFRunLoop`. +//! +//! See Apple's documentation on Run Loops for details: +//! +use std::cell::Cell; use std::ffi::c_void; use std::panic::{AssertUnwindSafe, UnwindSafe}; use std::ptr; use std::rc::Weak; use std::time::Instant; -use core_foundation::base::{CFIndex, CFOptionFlags, CFRelease}; +use block2::Block; +use core_foundation::base::{CFIndex, CFOptionFlags, CFRelease, CFTypeRef}; use core_foundation::date::CFAbsoluteTimeGetCurrent; use core_foundation::runloop::{ - kCFRunLoopAfterWaiting, kCFRunLoopBeforeWaiting, kCFRunLoopCommonModes, kCFRunLoopExit, - CFRunLoopActivity, CFRunLoopAddObserver, CFRunLoopAddTimer, CFRunLoopGetMain, + kCFRunLoopAfterWaiting, kCFRunLoopBeforeWaiting, kCFRunLoopCommonModes, kCFRunLoopDefaultMode, + kCFRunLoopExit, CFRunLoopActivity, CFRunLoopAddObserver, CFRunLoopAddTimer, CFRunLoopGetMain, CFRunLoopObserverCallBack, CFRunLoopObserverContext, CFRunLoopObserverCreate, CFRunLoopObserverRef, CFRunLoopRef, CFRunLoopTimerCreate, CFRunLoopTimerInvalidate, CFRunLoopTimerRef, CFRunLoopTimerSetNextFireDate, CFRunLoopWakeUp, }; use objc2_foundation::MainThreadMarker; +use tracing::error; use super::app_delegate::ApplicationDelegate; use super::event_loop::{stop_app_on_panic, PanicInfo}; @@ -84,10 +91,20 @@ extern "C" fn control_flow_end_handler( } } +#[derive(Debug)] pub struct RunLoop(CFRunLoopRef); +impl Default for RunLoop { + fn default() -> Self { + Self(ptr::null_mut()) + } +} + impl RunLoop { - pub unsafe fn get() -> Self { + pub fn main(mtm: MainThreadMarker) -> Self { + // SAFETY: We have a MainThreadMarker here, which means we know we're on the main thread, so + // scheduling (and scheduling a non-`Send` block) to that thread is allowed. + let _ = mtm; RunLoop(unsafe { CFRunLoopGetMain() }) } @@ -114,9 +131,79 @@ impl RunLoop { }; unsafe { CFRunLoopAddObserver(self.0, observer, kCFRunLoopCommonModes) }; } + + /// Submit a closure to run on the main thread as the next step in the run loop, before other + /// event sources are processed. + /// + /// This is used for running event handlers, as those are not allowed to run re-entrantly. + /// + /// # Implementation + /// + /// This queuing could be implemented in the following several ways with subtle differences in + /// timing. This list is sorted in rough order in which they are run: + /// + /// 1. Using `CFRunLoopPerformBlock` or `-[NSRunLoop performBlock:]`. + /// + /// 2. Using `-[NSObject performSelectorOnMainThread:withObject:waitUntilDone:]` or wrapping the + /// event in `NSEvent` and posting that to `-[NSApplication postEvent:atStart:]` (both + /// creates a custom `CFRunLoopSource`, and signals that to wake up the main event loop). + /// + /// a. `atStart = true`. + /// + /// b. `atStart = false`. + /// + /// 3. `dispatch_async` or `dispatch_async_f`. Note that this may appear before 2b, it does not + /// respect the ordering that runloop events have. + /// + /// We choose the first one, both for ease-of-implementation, but mostly for consistency, as we + /// want the event to be queued in a way that preserves the order the events originally arrived + /// in. + /// + /// As an example, let's assume that we receive two events from the user, a mouse click which we + /// handled by queuing it, and a window resize which we handled immediately. If we allowed + /// AppKit to choose the ordering when queuing the mouse event, it might get put in the back of + /// the queue, and the events would appear out of order to the user of Winit. So we must instead + /// put the event at the very front of the queue, to be handled as soon as possible after + /// handling whatever event it's currently handling. + pub fn queue_closure(&self, closure: impl FnOnce() + 'static) { + extern "C" { + fn CFRunLoopPerformBlock(rl: CFRunLoopRef, mode: CFTypeRef, block: &Block); + } + + // Convert `FnOnce()` to `Block`. + let closure = Cell::new(Some(closure)); + let block = block2::RcBlock::new(move || { + if let Some(closure) = closure.take() { + closure() + } else { + error!("tried to execute queued closure on main thread twice"); + } + }); + + // There are a few common modes (`kCFRunLoopCommonModes`) defined by Cocoa: + // - `NSDefaultRunLoopMode`, alias of `kCFRunLoopDefaultMode`. + // - `NSEventTrackingRunLoopMode`, used when mouse-dragging and live-resizing a window. + // - `NSModalPanelRunLoopMode`, used when running a modal inside the Winit event loop. + // - `NSConnectionReplyMode`: TODO. + // + // We only want to run event handlers in the default mode, as we support running a blocking + // modal inside a Winit event handler (see [#1779]) which outrules the modal panel mode, and + // resizing such panel window enters the event tracking run loop mode, so we can't directly + // trigger events inside that mode either. + // + // Any events that are queued while running a modal or when live-resizing will instead wait, + // and be delivered to the application afterwards. + // + // [#1779]: https://github.com/rust-windowing/winit/issues/1779 + let mode = unsafe { kCFRunLoopDefaultMode as CFTypeRef }; + + // SAFETY: The runloop is valid, the mode is a `CFStringRef`, and the block is `'static`. + unsafe { CFRunLoopPerformBlock(self.0, mode, &block) } + } } -pub fn setup_control_flow_observers(panic_info: Weak) { +pub fn setup_control_flow_observers(mtm: MainThreadMarker, panic_info: Weak) { + let run_loop = RunLoop::main(mtm); unsafe { let mut context = CFRunLoopObserverContext { info: Weak::into_raw(panic_info) as *mut _, @@ -125,7 +212,6 @@ pub fn setup_control_flow_observers(panic_info: Weak) { release: None, copyDescription: None, }; - let run_loop = RunLoop::get(); run_loop.add_observer( kCFRunLoopAfterWaiting, CFIndex::min_value(), diff --git a/src/platform_impl/macos/view.rs b/src/platform_impl/macos/view.rs index f3f25597..8f9595b3 100644 --- a/src/platform_impl/macos/view.rs +++ b/src/platform_impl/macos/view.rs @@ -686,7 +686,7 @@ declare_class!( self.update_modifiers(event, false); - self.queue_device_event(DeviceEvent::MouseWheel { delta }); + self.ivars().app_delegate.maybe_queue_device_event(DeviceEvent::MouseWheel { delta }); self.queue_event(WindowEvent::MouseWheel { device_id: DEVICE_ID, delta, @@ -830,11 +830,7 @@ impl WinitView { } fn queue_event(&self, event: WindowEvent) { - self.ivars().app_delegate.queue_window_event(self.window().id(), event); - } - - fn queue_device_event(&self, event: DeviceEvent) { - self.ivars().app_delegate.queue_device_event(event); + self.ivars().app_delegate.maybe_queue_window_event(self.window().id(), event); } fn scale_factor(&self) -> f64 { diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs index 3909b98c..c2d35ae7 100644 --- a/src/platform_impl/macos/window_delegate.rs +++ b/src/platform_impl/macos/window_delegate.rs @@ -1,6 +1,7 @@ #![allow(clippy::unnecessary_cast)] use std::cell::{Cell, RefCell}; use std::collections::VecDeque; +use std::sync::{Arc, Mutex}; use core_graphics::display::{CGDisplay, CGPoint}; use monitor::VideoModeHandle; @@ -24,12 +25,13 @@ use objc2_foundation::{ use super::app_delegate::ApplicationDelegate; use super::cursor::cursor_from_icon; use super::monitor::{self, flip_window_screen_coordinates, get_display_id}; +use super::observer::RunLoop; use super::view::WinitView; use super::window::WinitWindow; use super::{ffi, Fullscreen, MonitorHandle, OsError, WindowId}; use crate::dpi::{LogicalPosition, LogicalSize, PhysicalPosition, PhysicalSize, Position, Size}; use crate::error::{ExternalError, NotSupportedError, OsError as RootOsError}; -use crate::event::WindowEvent; +use crate::event::{InnerSizeWriter, WindowEvent}; use crate::platform::macos::{OptionAsAlt, WindowExtMacOS}; use crate::window::{ Cursor, CursorGrabMode, Icon, ImePurpose, ResizeDirection, Theme, UserAttentionType, @@ -184,7 +186,17 @@ declare_class!( #[method(windowDidChangeBackingProperties:)] fn window_did_change_backing_properties(&self, _: Option<&AnyObject>) { trace_scope!("windowDidChangeBackingProperties:"); - self.queue_static_scale_factor_changed_event(); + let scale_factor = self.scale_factor(); + if scale_factor == self.ivars().previous_scale_factor.get() { + return; + }; + self.ivars().previous_scale_factor.set(scale_factor); + + let mtm = MainThreadMarker::from(self); + let this = self.retain(); + RunLoop::main(mtm).queue_closure(move || { + this.handle_scale_factor_changed(scale_factor); + }); } #[method(windowDidBecomeKey:)] @@ -681,7 +693,10 @@ impl WindowDelegate { let delegate: Retained = unsafe { msg_send_id![super(delegate), init] }; if scale_factor != 1.0 { - delegate.queue_static_scale_factor_changed_event(); + let delegate = delegate.clone(); + RunLoop::main(mtm).queue_closure(move || { + delegate.handle_scale_factor_changed(scale_factor); + }); } window.setDelegate(Some(ProtocolObject::from_ref(&*delegate))); @@ -754,24 +769,31 @@ impl WindowDelegate { } pub(crate) fn queue_event(&self, event: WindowEvent) { - self.ivars().app_delegate.queue_window_event(self.window().id(), event); + self.ivars().app_delegate.maybe_queue_window_event(self.window().id(), event); } - fn queue_static_scale_factor_changed_event(&self) { - let scale_factor = self.scale_factor(); - if scale_factor == self.ivars().previous_scale_factor.get() { - return; - }; + fn handle_scale_factor_changed(&self, scale_factor: CGFloat) { + let app_delegate = &self.ivars().app_delegate; + let window = self.window(); - self.ivars().previous_scale_factor.set(scale_factor); - let content_size = self.window().contentRectForFrameRect(self.window().frame()).size; + let content_size = window.contentRectForFrameRect(window.frame()).size; let content_size = LogicalSize::new(content_size.width, content_size.height); - self.ivars().app_delegate.queue_static_scale_factor_changed_event( - self.window().retain(), - content_size.to_physical(scale_factor), + let suggested_size = content_size.to_physical(scale_factor); + let new_inner_size = Arc::new(Mutex::new(suggested_size)); + app_delegate.handle_window_event(window.id(), WindowEvent::ScaleFactorChanged { scale_factor, - ); + inner_size_writer: InnerSizeWriter::new(Arc::downgrade(&new_inner_size)), + }); + let physical_size = *new_inner_size.lock().unwrap(); + drop(new_inner_size); + + if physical_size != suggested_size { + let logical_size = physical_size.to_logical(scale_factor); + let size = NSSize::new(logical_size.width, logical_size.height); + window.setContentSize(size); + } + app_delegate.handle_window_event(window.id(), WindowEvent::Resized(physical_size)); } fn emit_move_event(&self) {