From 1c80f39b3cf3e62c56906720305483e1b3a05bcb Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 8 Dec 2024 23:54:47 +0100 Subject: [PATCH] Swizzle sendEvent: instead of subclassing NSApplication This is done to avoid order-dependent behavior that you'd otherwise encounter where `EventLoop::new` had to be called at the beginning of `fn main` to ensure that Winit's application was the one being registered as the main application by calling `sharedApplication`. Fixes https://github.com/rust-windowing/winit/issues/3772. This should also make it (more) possible to use multiple versions of Winit in the same application (though that's still untested). Finally, it should allow the user to override `NSApplication` themselves if they need to do that for some reason. --- src/changelog/unreleased.md | 1 + src/platform_impl/apple/appkit/app.rs | 187 +++++++++++++++---- src/platform_impl/apple/appkit/event_loop.rs | 20 +- 3 files changed, 161 insertions(+), 47 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 81d485122c..b37dd406bb 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -76,6 +76,7 @@ changelog entry. - Added `Window::surface_position`, which is the position of the surface inside the window. - Added `Window::safe_area`, which describes the area of the surface that is unobstructed. - On X11, Wayland, Windows and macOS, improved scancode conversions for more obscure key codes. +- On macOS, no longer need control of the main `NSApplication` class (which means you can now override it yourself). ### Changed diff --git a/src/platform_impl/apple/appkit/app.rs b/src/platform_impl/apple/appkit/app.rs index 747a17b4c5..caa9bb42f4 100644 --- a/src/platform_impl/apple/appkit/app.rs +++ b/src/platform_impl/apple/appkit/app.rs @@ -1,51 +1,114 @@ #![allow(clippy::unnecessary_cast)] +use std::cell::Cell; +use std::mem; use std::rc::Rc; -use objc2::{declare_class, msg_send, mutability, ClassType, DeclaredClass}; -use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType, NSResponder}; -use objc2_foundation::{MainThreadMarker, NSObject}; +use objc2::runtime::{Imp, Sel}; +use objc2::sel; +use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType}; +use objc2_foundation::MainThreadMarker; use super::app_state::AppState; use crate::event::{DeviceEvent, ElementState}; -declare_class!( - pub(super) struct WinitApplication; +// TODO(madsmtm): Use `MainThreadBound` once that is possible in `static`s. +struct StaticMainThreadBound(T); - unsafe impl ClassType for WinitApplication { - #[inherits(NSResponder, NSObject)] - type Super = NSApplication; - type Mutability = mutability::MainThreadOnly; - const NAME: &'static str = "WinitApplication"; +impl StaticMainThreadBound { + const fn get(&self, _mtm: MainThreadMarker) -> &T { + &self.0 } +} - impl DeclaredClass for WinitApplication {} - - unsafe impl WinitApplication { - // Normally, holding Cmd + any key never sends us a `keyUp` event for that key. - // Overriding `sendEvent:` like this fixes that. (https://stackoverflow.com/a/15294196) - // Fun fact: Firefox still has this bug! (https://bugzilla.mozilla.org/show_bug.cgi?id=1299553) - #[method(sendEvent:)] - fn send_event(&self, event: &NSEvent) { - // For posterity, there are some undocumented event types - // (https://github.com/servo/cocoa-rs/issues/155) - // but that doesn't really matter here. - let event_type = unsafe { event.r#type() }; - let modifier_flags = unsafe { event.modifierFlags() }; - if event_type == NSEventType::KeyUp - && modifier_flags.contains(NSEventModifierFlags::NSEventModifierFlagCommand) - { - if let Some(key_window) = self.keyWindow() { - key_window.sendEvent(event); - } - } else { - let app_state = AppState::get(MainThreadMarker::from(self)); - maybe_dispatch_device_event(&app_state, event); - unsafe { msg_send![super(self), sendEvent: event] } - } +unsafe impl Send for StaticMainThreadBound {} +unsafe impl Sync for StaticMainThreadBound {} + +// SAFETY: Creating `StaticMainThreadBound` in a `const` context, +// where there is no concept of the main thread. +static ORIGINAL: StaticMainThreadBound>> = + StaticMainThreadBound(Cell::new(None)); + +// FIXME(madsmtm): Use `extern "C-unwind"` once `objc2` supports that. +extern "C" fn send_event(app: &NSApplication, sel: Sel, event: &NSEvent) { + let mtm = MainThreadMarker::from(app); + + // Normally, holding Cmd + any key never sends us a `keyUp` event for that key. + // Overriding `sendEvent:` fixes that. (https://stackoverflow.com/a/15294196) + // Fun fact: Firefox still has this bug! (https://bugzilla.mozilla.org/show_bug.cgi?id=1299553) + // + // For posterity, there are some undocumented event types + // (https://github.com/servo/cocoa-rs/issues/155) + // but that doesn't really matter here. + let event_type = unsafe { event.r#type() }; + let modifier_flags = unsafe { event.modifierFlags() }; + if event_type == NSEventType::KeyUp + && modifier_flags.contains(NSEventModifierFlags::NSEventModifierFlagCommand) + { + if let Some(key_window) = app.keyWindow() { + key_window.sendEvent(event); } + return; } -); + + // Events are generally scoped to the window level, so the best way + // to get device events is to listen for them on NSApplication. + let app_state = AppState::get(mtm); + maybe_dispatch_device_event(&app_state, event); + + let original = ORIGINAL.get(mtm).get().expect("no existing sendEvent: handler set"); + original(app, sel, event) +} + +/// Override the [`sendEvent:`][NSApplication::sendEvent] method on the given application class. +/// +/// The previous implementation created a subclass of [`NSApplication`], however we would like to +/// give the user full control over their `NSApplication`, so we override the method here using +/// method swizzling instead. +/// +/// This _should_ also allow e.g. two versions of Winit to exist in the same application. +/// +/// See the following links for more info on method swizzling: +/// - +/// - +/// - +/// +/// NOTE: This function assumes that the passed in application object is the one returned from +/// [`NSApplication::sharedApplication`], i.e. the one and only global shared application object. +/// For testing though, we allow it to be a different object. +pub(crate) fn override_send_event(global_app: &NSApplication) { + let mtm = MainThreadMarker::from(global_app); + let class = global_app.class(); + + let method = + class.instance_method(sel!(sendEvent:)).expect("NSApplication must have sendEvent: method"); + + // SAFETY: Converting our `sendEvent:` implementation to an IMP. + let overridden = + unsafe { mem::transmute::(send_event) }; + + // If we've already overridden the method, don't do anything. + // FIXME(madsmtm): Use `std::ptr::fn_addr_eq` (Rust 1.85) once available in MSRV. + if overridden == method.implementation() { + return; + } + + // SAFETY: Our implementation has: + // 1. The same signature as `sendEvent:`. + // 2. Does not impose extra safety requirements on callers. + let original = unsafe { method.set_implementation(overridden) }; + + // SAFETY: This is the actual signature of `sendEvent:`. + let original = + unsafe { mem::transmute::(original) }; + + // NOTE: If NSApplication was safe to use from multiple threads, then this would potentially be + // a (checked) race-condition, since one could call `sendEvent:` before the original had been + // stored here. + // + // It is only usable from the main thread, however, so we're good! + ORIGINAL.get(mtm).set(Some(original)); +} fn maybe_dispatch_device_event(app_state: &Rc, event: &NSEvent) { let event_type = unsafe { event.r#type() }; @@ -87,3 +150,57 @@ fn maybe_dispatch_device_event(app_state: &Rc, event: &NSEvent) { _ => (), } } + +#[cfg(test)] +mod tests { + use objc2::rc::Retained; + use objc2::{declare_class, msg_send_id, mutability, ClassType, DeclaredClass}; + use objc2_app_kit::NSResponder; + use objc2_foundation::NSObject; + + use super::*; + + #[test] + fn test_override() { + // This is a test, so main thread safety doesn't _really_ matter. + let mtm = unsafe { MainThreadMarker::new_unchecked() }; + // Create a new application, without making it the shared application. + let app = unsafe { NSApplication::new(mtm) }; + override_send_event(&app); + // Test calling twice works. + override_send_event(&app); + + // FIXME(madsmtm): Can't test this yet, need some way to mock AppState. + // unsafe { + // let event = super::super::event::dummy_event().unwrap(); + // app.sendEvent(&event) + // } + } + + #[test] + fn test_custom_class() { + declare_class!( + pub(super) struct TestApplication; + + unsafe impl ClassType for TestApplication { + #[inherits(NSResponder, NSObject)] + type Super = NSApplication; + type Mutability = mutability::MainThreadOnly; + const NAME: &'static str = "TestApplication"; + } + + impl DeclaredClass for TestApplication {} + + unsafe impl TestApplication { + #[method(sendEvent:)] + fn send_event(&self, _event: &NSEvent) { + todo!() + } + } + ); + + // This is a test, so main thread safety doesn't _really_ matter. + let app: Retained = unsafe { msg_send_id![TestApplication::class(), new] }; + override_send_event(&app); + } +} diff --git a/src/platform_impl/apple/appkit/event_loop.rs b/src/platform_impl/apple/appkit/event_loop.rs index 4e8648d0f8..373e220c24 100644 --- a/src/platform_impl/apple/appkit/event_loop.rs +++ b/src/platform_impl/apple/appkit/event_loop.rs @@ -14,7 +14,7 @@ use core_foundation::runloop::{ CFRunLoopSourceCreate, CFRunLoopSourceRef, CFRunLoopSourceSignal, CFRunLoopWakeUp, }; use objc2::rc::{autoreleasepool, Retained}; -use objc2::{msg_send_id, sel, ClassType}; +use objc2::sel; use objc2_app_kit::{ NSApplication, NSApplicationActivationPolicy, NSApplicationDidFinishLaunchingNotification, NSApplicationWillTerminateNotification, NSWindow, @@ -23,7 +23,7 @@ use objc2_foundation::{MainThreadMarker, NSNotificationCenter, NSObject, NSObjec use rwh_06::HasDisplayHandle; use super::super::notification_center::create_observer; -use super::app::WinitApplication; +use super::app::override_send_event; use super::app_state::AppState; use super::cursor::CustomCursor; use super::event::dummy_event; @@ -207,16 +207,6 @@ impl EventLoop { let mtm = MainThreadMarker::new() .expect("on macOS, `EventLoop` must be created on the main thread!"); - let app: Retained = - unsafe { msg_send_id![WinitApplication::class(), sharedApplication] }; - - if !app.is_kind_of::() { - panic!( - "`winit` requires control over the principal class. You must create the event \ - loop before other parts of your application initialize NSApplication" - ); - } - let activation_policy = match attributes.activation_policy { None => None, Some(ActivationPolicy::Regular) => Some(NSApplicationActivationPolicy::Regular), @@ -231,6 +221,12 @@ impl EventLoop { attributes.activate_ignoring_other_apps, ); + // Initialize the application (if it has not already been). + let app = NSApplication::sharedApplication(mtm); + + // Override `sendEvent:` on the application to forward to our application state. + override_send_event(&app); + let center = unsafe { NSNotificationCenter::defaultCenter() }; let weak_app_state = Rc::downgrade(&app_state);