Skip to content

Commit

Permalink
Swizzle sendEvent: instead of subclassing NSApplication (#4036)
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
madsmtm authored Feb 24, 2025
1 parent 4d6fe7e commit 675582b
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 43 deletions.
1 change: 1 addition & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ changelog entry.
- Updated `windows-sys` to `v0.59`.
- To match the corresponding changes in `windows-sys`, the `HWND`, `HMONITOR`, and `HMENU` types
now alias to `*mut c_void` instead of `isize`.
- On macOS, no longer need control of the main `NSApplication` class (which means you can now override it yourself).

### Removed

Expand Down
168 changes: 137 additions & 31 deletions src/platform_impl/apple/appkit/app.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,101 @@
#![allow(clippy::unnecessary_cast)]

use std::cell::Cell;
use std::mem;
use std::rc::Rc;

use objc2::{define_class, msg_send, MainThreadMarker};
use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType, NSResponder};
use objc2_foundation::NSObject;
use dispatch2::MainThreadBound;
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};

define_class!(
#[unsafe(super(NSApplication, NSResponder, NSObject))]
#[name = "WinitApplication"]
pub(super) struct WinitApplication;

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)
#[unsafe(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::Command)
{
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] }
}
type SendEvent = extern "C-unwind" fn(&NSApplication, Sel, &NSEvent);

static ORIGINAL: MainThreadBound<Cell<Option<SendEvent>>> = {
// SAFETY: Creating in a `const` context, where there is no concept of the main thread.
MainThreadBound::new(Cell::new(None), unsafe { MainThreadMarker::new_unchecked() })
};

extern "C-unwind" 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::Command) {
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 two versions of Winit to exist in the same application.
///
/// See the following links for more info on method swizzling:
/// - <https://nshipster.com/method-swizzling/>
/// - <https://spin.atomicobject.com/method-swizzling-objective-c/>
/// - <https://web.archive.org/web/20130308110627/http://cocoadev.com/wiki/MethodSwizzling>
///
/// 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::<SendEvent, Imp>(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.
#[allow(unknown_lints, unpredictable_function_pointer_comparisons)]
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::<Imp, SendEvent>(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<AppState>, event: &NSEvent) {
let event_type = unsafe { event.r#type() };
Expand Down Expand Up @@ -80,3 +137,52 @@ fn maybe_dispatch_device_event(app_state: &Rc<AppState>, event: &NSEvent) {
_ => (),
}
}

#[cfg(test)]
mod tests {
use objc2::rc::Retained;
use objc2::{define_class, msg_send, ClassType};
use objc2_app_kit::NSResponder;
use objc2_foundation::NSObject;

use super::*;

#[test]
fn test_override() {
// FIXME(madsmtm): Ensure this always runs (maybe use cargo-nextest or `--test-threads=1`?)
let Some(mtm) = MainThreadMarker::new() else { return };

// 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() {
let Some(_mtm) = MainThreadMarker::new() else { return };

define_class!(
#[unsafe(super(NSApplication, NSResponder, NSObject))]
#[name = "TestApplication"]
pub(super) struct TestApplication;

impl TestApplication {
#[unsafe(method(sendEvent:))]
fn send_event(&self, _event: &NSEvent) {
todo!()
}
}
);

let app: Retained<TestApplication> = unsafe { msg_send![TestApplication::class(), new] };
override_send_event(&app);
}
}
20 changes: 8 additions & 12 deletions src/platform_impl/apple/appkit/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::time::{Duration, Instant};

use objc2::rc::{autoreleasepool, Retained};
use objc2::runtime::ProtocolObject;
use objc2::{available, msg_send, ClassType, MainThreadMarker};
use objc2::{available, MainThreadMarker};
use objc2_app_kit::{
NSApplication, NSApplicationActivationPolicy, NSApplicationDidFinishLaunchingNotification,
NSApplicationWillTerminateNotification, NSWindow,
Expand All @@ -24,7 +24,7 @@ use objc2_foundation::{NSNotificationCenter, NSObjectProtocol};
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;
Expand Down Expand Up @@ -209,16 +209,6 @@ impl EventLoop {
let mtm = MainThreadMarker::new()
.expect("on macOS, `EventLoop` must be created on the main thread!");

let app: Retained<NSApplication> =
unsafe { msg_send![WinitApplication::class(), sharedApplication] };

if !app.isKindOfClass(WinitApplication::class()) {
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),
Expand All @@ -233,6 +223,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);
Expand Down

0 comments on commit 675582b

Please sign in to comment.