Skip to content

Commit 1c80f39

Browse files
committed
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 #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 e47081e commit 1c80f39

File tree

3 files changed

+161
-47
lines changed

3 files changed

+161
-47
lines changed

src/changelog/unreleased.md

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ changelog entry.
7676
- Added `Window::surface_position`, which is the position of the surface inside the window.
7777
- Added `Window::safe_area`, which describes the area of the surface that is unobstructed.
7878
- On X11, Wayland, Windows and macOS, improved scancode conversions for more obscure key codes.
79+
- On macOS, no longer need control of the main `NSApplication` class (which means you can now override it yourself).
7980

8081
### Changed
8182

src/platform_impl/apple/appkit/app.rs

+152-35
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,114 @@
11
#![allow(clippy::unnecessary_cast)]
22

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

5-
use objc2::{declare_class, msg_send, mutability, ClassType, DeclaredClass};
6-
use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType, NSResponder};
7-
use objc2_foundation::{MainThreadMarker, NSObject};
7+
use objc2::runtime::{Imp, Sel};
8+
use objc2::sel;
9+
use objc2_app_kit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType};
10+
use objc2_foundation::MainThreadMarker;
811

912
use super::app_state::AppState;
1013
use crate::event::{DeviceEvent, ElementState};
1114

12-
declare_class!(
13-
pub(super) struct WinitApplication;
15+
// TODO(madsmtm): Use `MainThreadBound` once that is possible in `static`s.
16+
struct StaticMainThreadBound<T>(T);
1417

15-
unsafe impl ClassType for WinitApplication {
16-
#[inherits(NSResponder, NSObject)]
17-
type Super = NSApplication;
18-
type Mutability = mutability::MainThreadOnly;
19-
const NAME: &'static str = "WinitApplication";
18+
impl<T> StaticMainThreadBound<T> {
19+
const fn get(&self, _mtm: MainThreadMarker) -> &T {
20+
&self.0
2021
}
22+
}
2123

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

50113
fn maybe_dispatch_device_event(app_state: &Rc<AppState>, event: &NSEvent) {
51114
let event_type = unsafe { event.r#type() };
@@ -87,3 +150,57 @@ fn maybe_dispatch_device_event(app_state: &Rc<AppState>, event: &NSEvent) {
87150
_ => (),
88151
}
89152
}
153+
154+
#[cfg(test)]
155+
mod tests {
156+
use objc2::rc::Retained;
157+
use objc2::{declare_class, msg_send_id, mutability, ClassType, DeclaredClass};
158+
use objc2_app_kit::NSResponder;
159+
use objc2_foundation::NSObject;
160+
161+
use super::*;
162+
163+
#[test]
164+
fn test_override() {
165+
// This is a test, so main thread safety doesn't _really_ matter.
166+
let mtm = unsafe { MainThreadMarker::new_unchecked() };
167+
// Create a new application, without making it the shared application.
168+
let app = unsafe { NSApplication::new(mtm) };
169+
override_send_event(&app);
170+
// Test calling twice works.
171+
override_send_event(&app);
172+
173+
// FIXME(madsmtm): Can't test this yet, need some way to mock AppState.
174+
// unsafe {
175+
// let event = super::super::event::dummy_event().unwrap();
176+
// app.sendEvent(&event)
177+
// }
178+
}
179+
180+
#[test]
181+
fn test_custom_class() {
182+
declare_class!(
183+
pub(super) struct TestApplication;
184+
185+
unsafe impl ClassType for TestApplication {
186+
#[inherits(NSResponder, NSObject)]
187+
type Super = NSApplication;
188+
type Mutability = mutability::MainThreadOnly;
189+
const NAME: &'static str = "TestApplication";
190+
}
191+
192+
impl DeclaredClass for TestApplication {}
193+
194+
unsafe impl TestApplication {
195+
#[method(sendEvent:)]
196+
fn send_event(&self, _event: &NSEvent) {
197+
todo!()
198+
}
199+
}
200+
);
201+
202+
// This is a test, so main thread safety doesn't _really_ matter.
203+
let app: Retained<TestApplication> = unsafe { msg_send_id![TestApplication::class(), new] };
204+
override_send_event(&app);
205+
}
206+
}

src/platform_impl/apple/appkit/event_loop.rs

+8-12
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use core_foundation::runloop::{
1414
CFRunLoopSourceCreate, CFRunLoopSourceRef, CFRunLoopSourceSignal, CFRunLoopWakeUp,
1515
};
1616
use objc2::rc::{autoreleasepool, Retained};
17-
use objc2::{msg_send_id, sel, ClassType};
17+
use objc2::sel;
1818
use objc2_app_kit::{
1919
NSApplication, NSApplicationActivationPolicy, NSApplicationDidFinishLaunchingNotification,
2020
NSApplicationWillTerminateNotification, NSWindow,
@@ -23,7 +23,7 @@ use objc2_foundation::{MainThreadMarker, NSNotificationCenter, NSObject, NSObjec
2323
use rwh_06::HasDisplayHandle;
2424

2525
use super::super::notification_center::create_observer;
26-
use super::app::WinitApplication;
26+
use super::app::override_send_event;
2727
use super::app_state::AppState;
2828
use super::cursor::CustomCursor;
2929
use super::event::dummy_event;
@@ -207,16 +207,6 @@ impl EventLoop {
207207
let mtm = MainThreadMarker::new()
208208
.expect("on macOS, `EventLoop` must be created on the main thread!");
209209

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

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

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

0 commit comments

Comments
 (0)