Skip to content

Commit 675582b

Browse files
authored
Swizzle sendEvent: instead of subclassing NSApplication (#4036)
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.
1 parent 4d6fe7e commit 675582b

File tree

3 files changed

+146
-43
lines changed

3 files changed

+146
-43
lines changed

src/changelog/unreleased.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ changelog entry.
185185
- Updated `windows-sys` to `v0.59`.
186186
- To match the corresponding changes in `windows-sys`, the `HWND`, `HMONITOR`, and `HMENU` types
187187
now alias to `*mut c_void` instead of `isize`.
188+
- On macOS, no longer need control of the main `NSApplication` class (which means you can now override it yourself).
188189

189190
### Removed
190191

src/platform_impl/apple/appkit/app.rs

Lines changed: 137 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,101 @@
11
#![allow(clippy::unnecessary_cast)]
22

3+
use std::cell::Cell;
4+
use std::mem;
35
use std::rc::Rc;
46

5-
use objc2::{define_class, msg_send, MainThreadMarker};
6-
use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType, NSResponder};
7-
use objc2_foundation::NSObject;
7+
use dispatch2::MainThreadBound;
8+
use objc2::runtime::{Imp, Sel};
9+
use objc2::sel;
10+
use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType};
11+
use objc2_foundation::MainThreadMarker;
812

913
use super::app_state::AppState;
1014
use crate::event::{DeviceEvent, ElementState};
1115

12-
define_class!(
13-
#[unsafe(super(NSApplication, NSResponder, NSObject))]
14-
#[name = "WinitApplication"]
15-
pub(super) struct WinitApplication;
16-
17-
impl WinitApplication {
18-
// Normally, holding Cmd + any key never sends us a `keyUp` event for that key.
19-
// Overriding `sendEvent:` like this fixes that. (https://stackoverflow.com/a/15294196)
20-
// Fun fact: Firefox still has this bug! (https://bugzilla.mozilla.org/show_bug.cgi?id=1299553)
21-
#[unsafe(method(sendEvent:))]
22-
fn send_event(&self, event: &NSEvent) {
23-
// For posterity, there are some undocumented event types
24-
// (https://github.com/servo/cocoa-rs/issues/155)
25-
// but that doesn't really matter here.
26-
let event_type = unsafe { event.r#type() };
27-
let modifier_flags = unsafe { event.modifierFlags() };
28-
if event_type == NSEventType::KeyUp
29-
&& modifier_flags.contains(NSEventModifierFlags::Command)
30-
{
31-
if let Some(key_window) = self.keyWindow() {
32-
key_window.sendEvent(event);
33-
}
34-
} else {
35-
let app_state = AppState::get(MainThreadMarker::from(self));
36-
maybe_dispatch_device_event(&app_state, event);
37-
unsafe { msg_send![super(self), sendEvent: event] }
38-
}
16+
type SendEvent = extern "C-unwind" fn(&NSApplication, Sel, &NSEvent);
17+
18+
static ORIGINAL: MainThreadBound<Cell<Option<SendEvent>>> = {
19+
// SAFETY: Creating in a `const` context, where there is no concept of the main thread.
20+
MainThreadBound::new(Cell::new(None), unsafe { MainThreadMarker::new_unchecked() })
21+
};
22+
23+
extern "C-unwind" fn send_event(app: &NSApplication, sel: Sel, event: &NSEvent) {
24+
let mtm = MainThreadMarker::from(app);
25+
26+
// Normally, holding Cmd + any key never sends us a `keyUp` event for that key.
27+
// Overriding `sendEvent:` fixes that. (https://stackoverflow.com/a/15294196)
28+
// Fun fact: Firefox still has this bug! (https://bugzilla.mozilla.org/show_bug.cgi?id=1299553)
29+
//
30+
// For posterity, there are some undocumented event types
31+
// (https://github.com/servo/cocoa-rs/issues/155)
32+
// but that doesn't really matter here.
33+
let event_type = unsafe { event.r#type() };
34+
let modifier_flags = unsafe { event.modifierFlags() };
35+
if event_type == NSEventType::KeyUp && modifier_flags.contains(NSEventModifierFlags::Command) {
36+
if let Some(key_window) = app.keyWindow() {
37+
key_window.sendEvent(event);
3938
}
39+
return;
40+
}
41+
42+
// Events are generally scoped to the window level, so the best way
43+
// to get device events is to listen for them on NSApplication.
44+
let app_state = AppState::get(mtm);
45+
maybe_dispatch_device_event(&app_state, event);
46+
47+
let original = ORIGINAL.get(mtm).get().expect("no existing sendEvent: handler set");
48+
original(app, sel, event)
49+
}
50+
51+
/// Override the [`sendEvent:`][NSApplication::sendEvent] method on the given application class.
52+
///
53+
/// The previous implementation created a subclass of [`NSApplication`], however we would like to
54+
/// give the user full control over their `NSApplication`, so we override the method here using
55+
/// method swizzling instead.
56+
///
57+
/// This _should_ also allow two versions of Winit to exist in the same application.
58+
///
59+
/// See the following links for more info on method swizzling:
60+
/// - <https://nshipster.com/method-swizzling/>
61+
/// - <https://spin.atomicobject.com/method-swizzling-objective-c/>
62+
/// - <https://web.archive.org/web/20130308110627/http://cocoadev.com/wiki/MethodSwizzling>
63+
///
64+
/// NOTE: This function assumes that the passed in application object is the one returned from
65+
/// [`NSApplication::sharedApplication`], i.e. the one and only global shared application object.
66+
/// For testing though, we allow it to be a different object.
67+
pub(crate) fn override_send_event(global_app: &NSApplication) {
68+
let mtm = MainThreadMarker::from(global_app);
69+
let class = global_app.class();
70+
71+
let method =
72+
class.instance_method(sel!(sendEvent:)).expect("NSApplication must have sendEvent: method");
73+
74+
// SAFETY: Converting our `sendEvent:` implementation to an IMP.
75+
let overridden = unsafe { mem::transmute::<SendEvent, Imp>(send_event) };
76+
77+
// If we've already overridden the method, don't do anything.
78+
// FIXME(madsmtm): Use `std::ptr::fn_addr_eq` (Rust 1.85) once available in MSRV.
79+
#[allow(unknown_lints, unpredictable_function_pointer_comparisons)]
80+
if overridden == method.implementation() {
81+
return;
4082
}
41-
);
83+
84+
// SAFETY: Our implementation has:
85+
// 1. The same signature as `sendEvent:`.
86+
// 2. Does not impose extra safety requirements on callers.
87+
let original = unsafe { method.set_implementation(overridden) };
88+
89+
// SAFETY: This is the actual signature of `sendEvent:`.
90+
let original = unsafe { mem::transmute::<Imp, SendEvent>(original) };
91+
92+
// NOTE: If NSApplication was safe to use from multiple threads, then this would potentially be
93+
// a (checked) race-condition, since one could call `sendEvent:` before the original had been
94+
// stored here.
95+
//
96+
// It is only usable from the main thread, however, so we're good!
97+
ORIGINAL.get(mtm).set(Some(original));
98+
}
4299

43100
fn maybe_dispatch_device_event(app_state: &Rc<AppState>, event: &NSEvent) {
44101
let event_type = unsafe { event.r#type() };
@@ -80,3 +137,52 @@ fn maybe_dispatch_device_event(app_state: &Rc<AppState>, event: &NSEvent) {
80137
_ => (),
81138
}
82139
}
140+
141+
#[cfg(test)]
142+
mod tests {
143+
use objc2::rc::Retained;
144+
use objc2::{define_class, msg_send, ClassType};
145+
use objc2_app_kit::NSResponder;
146+
use objc2_foundation::NSObject;
147+
148+
use super::*;
149+
150+
#[test]
151+
fn test_override() {
152+
// FIXME(madsmtm): Ensure this always runs (maybe use cargo-nextest or `--test-threads=1`?)
153+
let Some(mtm) = MainThreadMarker::new() else { return };
154+
155+
// Create a new application, without making it the shared application.
156+
let app = unsafe { NSApplication::new(mtm) };
157+
override_send_event(&app);
158+
// Test calling twice works.
159+
override_send_event(&app);
160+
161+
// FIXME(madsmtm): Can't test this yet, need some way to mock AppState.
162+
// unsafe {
163+
// let event = super::super::event::dummy_event().unwrap();
164+
// app.sendEvent(&event)
165+
// }
166+
}
167+
168+
#[test]
169+
fn test_custom_class() {
170+
let Some(_mtm) = MainThreadMarker::new() else { return };
171+
172+
define_class!(
173+
#[unsafe(super(NSApplication, NSResponder, NSObject))]
174+
#[name = "TestApplication"]
175+
pub(super) struct TestApplication;
176+
177+
impl TestApplication {
178+
#[unsafe(method(sendEvent:))]
179+
fn send_event(&self, _event: &NSEvent) {
180+
todo!()
181+
}
182+
}
183+
);
184+
185+
let app: Retained<TestApplication> = unsafe { msg_send![TestApplication::class(), new] };
186+
override_send_event(&app);
187+
}
188+
}

src/platform_impl/apple/appkit/event_loop.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::time::{Duration, Instant};
1010

1111
use objc2::rc::{autoreleasepool, Retained};
1212
use objc2::runtime::ProtocolObject;
13-
use objc2::{available, msg_send, ClassType, MainThreadMarker};
13+
use objc2::{available, MainThreadMarker};
1414
use objc2_app_kit::{
1515
NSApplication, NSApplicationActivationPolicy, NSApplicationDidFinishLaunchingNotification,
1616
NSApplicationWillTerminateNotification, NSWindow,
@@ -24,7 +24,7 @@ use objc2_foundation::{NSNotificationCenter, NSObjectProtocol};
2424
use rwh_06::HasDisplayHandle;
2525

2626
use super::super::notification_center::create_observer;
27-
use super::app::WinitApplication;
27+
use super::app::override_send_event;
2828
use super::app_state::AppState;
2929
use super::cursor::CustomCursor;
3030
use super::event::dummy_event;
@@ -209,16 +209,6 @@ impl EventLoop {
209209
let mtm = MainThreadMarker::new()
210210
.expect("on macOS, `EventLoop` must be created on the main thread!");
211211

212-
let app: Retained<NSApplication> =
213-
unsafe { msg_send![WinitApplication::class(), sharedApplication] };
214-
215-
if !app.isKindOfClass(WinitApplication::class()) {
216-
panic!(
217-
"`winit` requires control over the principal class. You must create the event \
218-
loop before other parts of your application initialize NSApplication"
219-
);
220-
}
221-
222212
let activation_policy = match attributes.activation_policy {
223213
None => None,
224214
Some(ActivationPolicy::Regular) => Some(NSApplicationActivationPolicy::Regular),
@@ -233,6 +223,12 @@ impl EventLoop {
233223
attributes.activate_ignoring_other_apps,
234224
);
235225

226+
// Initialize the application (if it has not already been).
227+
let app = NSApplication::sharedApplication(mtm);
228+
229+
// Override `sendEvent:` on the application to forward to our application state.
230+
override_send_event(&app);
231+
236232
let center = unsafe { NSNotificationCenter::defaultCenter() };
237233

238234
let weak_app_state = Rc::downgrade(&app_state);

0 commit comments

Comments
 (0)