Skip to content

Commit 651c3f4

Browse files
committed
Refactor usage of CFRunLoopObserver
Added a common interface that: - Uses closures instead of static functions. This should allow easier refactoring in the future. - Returns a handle which is invalidated on `Drop`. This should avoid situations where the event loop has exited, but an observer is still called because the user spawned the application later on. - Is properly main-thread safe. This interface is placed in winit-common, to allow using it in both winit-appkit and winit-uikit.
1 parent 014fb68 commit 651c3f4

File tree

8 files changed

+285
-277
lines changed

8 files changed

+285
-277
lines changed

winit-appkit/src/app_state.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use dispatch2::MainThreadBound;
88
use objc2::MainThreadMarker;
99
use objc2_app_kit::{NSApplication, NSApplicationActivationPolicy, NSRunningApplication};
1010
use objc2_foundation::NSNotification;
11-
use winit_common::core_foundation::EventLoopProxy;
11+
use winit_common::core_foundation::{EventLoopProxy, MainRunLoop};
1212
use winit_common::event_handler::EventHandler;
1313
use winit_core::application::ApplicationHandler;
1414
use winit_core::event::{StartCause, WindowEvent};
@@ -17,15 +17,15 @@ use winit_core::window::WindowId;
1717

1818
use super::event_loop::{notify_windows_of_exit, stop_app_immediately, ActiveEventLoop};
1919
use super::menu;
20-
use super::observer::{EventLoopWaker, RunLoop};
20+
use super::observer::EventLoopWaker;
2121

2222
#[derive(Debug)]
2323
pub(super) struct AppState {
2424
mtm: MainThreadMarker,
2525
activation_policy: Option<NSApplicationActivationPolicy>,
2626
default_menu: bool,
2727
activate_ignoring_other_apps: bool,
28-
run_loop: RunLoop,
28+
run_loop: MainRunLoop,
2929
event_loop_proxy: Arc<EventLoopProxy>,
3030
event_handler: EventHandler,
3131
stop_on_launch: Cell<bool>,
@@ -68,7 +68,7 @@ impl AppState {
6868
activation_policy,
6969
default_menu,
7070
activate_ignoring_other_apps,
71-
run_loop: RunLoop::main(mtm),
71+
run_loop: MainRunLoop::get(mtm),
7272
event_loop_proxy,
7373
event_handler: EventHandler::new(),
7474
stop_on_launch: Cell::new(false),
@@ -265,7 +265,7 @@ impl AppState {
265265
if !pending_redraw.contains(&window_id) {
266266
pending_redraw.push(window_id);
267267
}
268-
self.run_loop.wakeup();
268+
self.run_loop.wake_up();
269269
}
270270

271271
#[track_caller]
@@ -310,6 +310,7 @@ impl AppState {
310310
// Called by RunLoopObserver after finishing waiting for new events
311311
pub fn wakeup(self: &Rc<Self>) {
312312
// Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779
313+
// (we have registered to observe all modes, including modal event loops).
313314
if !self.event_handler.ready() || !self.is_running() {
314315
return;
315316
}
@@ -338,8 +339,7 @@ impl AppState {
338339
// Called by RunLoopObserver before waiting for new events
339340
pub fn cleared(self: &Rc<Self>) {
340341
// Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779
341-
// XXX: how does it make sense that `event_handler.ready()` can ever return `false` here if
342-
// we're about to return to the `CFRunLoop` to poll for new events?
342+
// (we have registered to observe all modes, including modal event loops).
343343
if !self.event_handler.ready() || !self.is_running() {
344344
return;
345345
}

winit-appkit/src/event_loop.rs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ use objc2_app_kit::{
99
NSApplication, NSApplicationActivationPolicy, NSApplicationDidFinishLaunchingNotification,
1010
NSApplicationWillTerminateNotification, NSWindow,
1111
};
12+
use objc2_core_foundation::{kCFRunLoopCommonModes, CFIndex, CFRunLoopActivity};
1213
use objc2_foundation::{NSNotificationCenter, NSObjectProtocol};
1314
use rwh_06::HasDisplayHandle;
15+
use winit_common::core_foundation::{MainRunLoop, MainRunLoopObserver};
1416
use winit_core::application::ApplicationHandler;
1517
use winit_core::cursor::{CustomCursor as CoreCustomCursor, CustomCursorSource};
1618
use winit_core::error::{EventLoopError, RequestError};
@@ -28,7 +30,6 @@ use super::cursor::CustomCursor;
2830
use super::event::dummy_event;
2931
use super::monitor;
3032
use super::notification_center::create_observer;
31-
use super::observer::setup_control_flow_observers;
3233
use crate::window::Window;
3334
use crate::ActivationPolicy;
3435

@@ -150,6 +151,9 @@ pub struct EventLoop {
150151
// Though we do still need to keep the observers around to prevent them from being deallocated.
151152
_did_finish_launching_observer: Retained<ProtocolObject<dyn NSObjectProtocol>>,
152153
_will_terminate_observer: Retained<ProtocolObject<dyn NSObjectProtocol>>,
154+
155+
_before_waiting_observer: MainRunLoopObserver,
156+
_after_waiting_observer: MainRunLoopObserver,
153157
}
154158

155159
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@@ -217,14 +221,40 @@ impl EventLoop {
217221
},
218222
);
219223

220-
setup_control_flow_observers(mtm);
224+
let main_loop = MainRunLoop::get(mtm);
225+
let mode = unsafe { kCFRunLoopCommonModes }.unwrap();
226+
227+
let app_state_clone = Rc::clone(&app_state);
228+
let _before_waiting_observer = MainRunLoopObserver::new(
229+
mtm,
230+
CFRunLoopActivity::BeforeWaiting,
231+
true,
232+
// Queued with the lowest priority to ensure it is processed after other observers.
233+
// Without that, we'd get a `LoopExiting` after `AboutToWait`.
234+
CFIndex::MAX,
235+
move |_| app_state_clone.cleared(),
236+
);
237+
main_loop.add_observer(&_before_waiting_observer, mode);
238+
239+
let app_state_clone = Rc::clone(&app_state);
240+
let _after_waiting_observer = MainRunLoopObserver::new(
241+
mtm,
242+
CFRunLoopActivity::AfterWaiting,
243+
true,
244+
// Queued with the highest priority to ensure it is processed before other observers.
245+
CFIndex::MIN,
246+
move |_| app_state_clone.wakeup(),
247+
);
248+
main_loop.add_observer(&_after_waiting_observer, mode);
221249

222250
Ok(EventLoop {
223251
app,
224252
app_state: app_state.clone(),
225253
window_target: ActiveEventLoop { app_state, mtm },
226254
_did_finish_launching_observer,
227255
_will_terminate_observer,
256+
_before_waiting_observer,
257+
_after_waiting_observer,
228258
})
229259
}
230260

winit-appkit/src/observer.rs

Lines changed: 1 addition & 162 deletions
Original file line numberDiff line numberDiff line change
@@ -1,171 +1,10 @@
1-
//! Utilities for working with `CFRunLoop`.
2-
//!
3-
//! See Apple's documentation on Run Loops for details:
4-
//! <https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/RunLoopManagement.html>
5-
use std::cell::Cell;
61
use std::ffi::c_void;
72
use std::ptr;
83
use std::time::Instant;
94

10-
use objc2::MainThreadMarker;
115
use objc2_core_foundation::{
12-
kCFRunLoopCommonModes, kCFRunLoopDefaultMode, CFAbsoluteTimeGetCurrent, CFIndex, CFRetained,
13-
CFRunLoop, CFRunLoopActivity, CFRunLoopObserver, CFRunLoopObserverCallBack,
14-
CFRunLoopObserverContext, CFRunLoopTimer,
6+
kCFRunLoopCommonModes, CFAbsoluteTimeGetCurrent, CFRetained, CFRunLoop, CFRunLoopTimer,
157
};
16-
use tracing::error;
17-
18-
use super::app_state::AppState;
19-
20-
// begin is queued with the highest priority to ensure it is processed before other observers
21-
extern "C-unwind" fn control_flow_begin_handler(
22-
_: *mut CFRunLoopObserver,
23-
activity: CFRunLoopActivity,
24-
_info: *mut c_void,
25-
) {
26-
match activity {
27-
CFRunLoopActivity::AfterWaiting => {
28-
AppState::get(MainThreadMarker::new().unwrap()).wakeup();
29-
},
30-
_ => unreachable!(),
31-
}
32-
}
33-
34-
// end is queued with the lowest priority to ensure it is processed after other observers
35-
// without that, LoopExiting would get sent after AboutToWait
36-
extern "C-unwind" fn control_flow_end_handler(
37-
_: *mut CFRunLoopObserver,
38-
activity: CFRunLoopActivity,
39-
_info: *mut c_void,
40-
) {
41-
match activity {
42-
CFRunLoopActivity::BeforeWaiting => {
43-
AppState::get(MainThreadMarker::new().unwrap()).cleared();
44-
},
45-
CFRunLoopActivity::Exit => (), // unimplemented!(), // not expected to ever happen
46-
_ => unreachable!(),
47-
}
48-
}
49-
50-
#[derive(Debug)]
51-
pub struct RunLoop(CFRetained<CFRunLoop>);
52-
53-
impl RunLoop {
54-
pub fn main(mtm: MainThreadMarker) -> Self {
55-
// SAFETY: We have a MainThreadMarker here, which means we know we're on the main thread, so
56-
// scheduling (and scheduling a non-`Send` block) to that thread is allowed.
57-
let _ = mtm;
58-
RunLoop(CFRunLoop::main().unwrap())
59-
}
60-
61-
pub fn wakeup(&self) {
62-
self.0.wake_up();
63-
}
64-
65-
unsafe fn add_observer(
66-
&self,
67-
flags: CFRunLoopActivity,
68-
// The lower the value, the sooner this will run
69-
priority: CFIndex,
70-
handler: CFRunLoopObserverCallBack,
71-
context: *mut CFRunLoopObserverContext,
72-
) {
73-
let observer =
74-
unsafe { CFRunLoopObserver::new(None, flags.0, true, priority, handler, context) }
75-
.unwrap();
76-
self.0.add_observer(Some(&observer), unsafe { kCFRunLoopCommonModes });
77-
}
78-
79-
/// Submit a closure to run on the main thread as the next step in the run loop, before other
80-
/// event sources are processed.
81-
///
82-
/// This is used for running event handlers, as those are not allowed to run re-entrantly.
83-
///
84-
/// # Implementation
85-
///
86-
/// This queuing could be implemented in the following several ways with subtle differences in
87-
/// timing. This list is sorted in rough order in which they are run:
88-
///
89-
/// 1. Using `CFRunLoopPerformBlock` or `-[NSRunLoop performBlock:]`.
90-
///
91-
/// 2. Using `-[NSObject performSelectorOnMainThread:withObject:waitUntilDone:]` or wrapping the
92-
/// event in `NSEvent` and posting that to `-[NSApplication postEvent:atStart:]` (both
93-
/// creates a custom `CFRunLoopSource`, and signals that to wake up the main event loop).
94-
///
95-
/// a. `atStart = true`.
96-
///
97-
/// b. `atStart = false`.
98-
///
99-
/// 3. `dispatch_async` or `dispatch_async_f`. Note that this may appear before 2b, it does not
100-
/// respect the ordering that runloop events have.
101-
///
102-
/// We choose the first one, both for ease-of-implementation, but mostly for consistency, as we
103-
/// want the event to be queued in a way that preserves the order the events originally arrived
104-
/// in.
105-
///
106-
/// As an example, let's assume that we receive two events from the user, a mouse click which we
107-
/// handled by queuing it, and a window resize which we handled immediately. If we allowed
108-
/// AppKit to choose the ordering when queuing the mouse event, it might get put in the back of
109-
/// the queue, and the events would appear out of order to the user of Winit. So we must instead
110-
/// put the event at the very front of the queue, to be handled as soon as possible after
111-
/// handling whatever event it's currently handling.
112-
pub fn queue_closure(&self, closure: impl FnOnce() + 'static) {
113-
// Convert `FnOnce()` to `Block<dyn Fn()>`.
114-
let closure = Cell::new(Some(closure));
115-
let block = block2::RcBlock::new(move || {
116-
if let Some(closure) = closure.take() {
117-
closure()
118-
} else {
119-
error!("tried to execute queued closure on main thread twice");
120-
}
121-
});
122-
123-
// There are a few common modes (`kCFRunLoopCommonModes`) defined by Cocoa:
124-
// - `NSDefaultRunLoopMode`, alias of `kCFRunLoopDefaultMode`.
125-
// - `NSEventTrackingRunLoopMode`, used when mouse-dragging and live-resizing a window.
126-
// - `NSModalPanelRunLoopMode`, used when running a modal inside the Winit event loop.
127-
// - `NSConnectionReplyMode`: TODO.
128-
//
129-
// We only want to run event handlers in the default mode, as we support running a blocking
130-
// modal inside a Winit event handler (see [#1779]) which outrules the modal panel mode, and
131-
// resizing such panel window enters the event tracking run loop mode, so we can't directly
132-
// trigger events inside that mode either.
133-
//
134-
// Any events that are queued while running a modal or when live-resizing will instead wait,
135-
// and be delivered to the application afterwards.
136-
//
137-
// [#1779]: https://github.com/rust-windowing/winit/issues/1779
138-
let mode = unsafe { kCFRunLoopDefaultMode.unwrap() };
139-
140-
// SAFETY: The runloop is valid, the mode is a `CFStringRef`, and the block is `'static`.
141-
unsafe { self.0.perform_block(Some(mode), Some(&block)) }
142-
}
143-
}
144-
145-
pub fn setup_control_flow_observers(mtm: MainThreadMarker) {
146-
let run_loop = RunLoop::main(mtm);
147-
unsafe {
148-
let mut context = CFRunLoopObserverContext {
149-
info: ptr::null_mut(),
150-
version: 0,
151-
retain: None,
152-
release: None,
153-
copyDescription: None,
154-
};
155-
run_loop.add_observer(
156-
CFRunLoopActivity::AfterWaiting,
157-
CFIndex::MIN,
158-
Some(control_flow_begin_handler),
159-
&mut context as *mut _,
160-
);
161-
run_loop.add_observer(
162-
CFRunLoopActivity::Exit | CFRunLoopActivity::BeforeWaiting,
163-
CFIndex::MAX,
164-
Some(control_flow_end_handler),
165-
&mut context as *mut _,
166-
);
167-
}
168-
}
1698

1709
#[derive(Debug)]
17110
pub struct EventLoopWaker {

winit-appkit/src/window_delegate.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ use objc2_foundation::{
4040
NSRect, NSSize, NSString,
4141
};
4242
use tracing::{trace, warn};
43+
use winit_common::core_foundation::MainRunLoop;
4344
use winit_core::cursor::Cursor;
4445
use winit_core::error::{NotSupportedError, RequestError};
4546
use winit_core::event::{SurfaceSizeWriter, WindowEvent};
@@ -54,7 +55,6 @@ use super::app_state::AppState;
5455
use super::cursor::{cursor_from_icon, CustomCursor};
5556
use super::ffi;
5657
use super::monitor::{self, flip_window_screen_coordinates, get_display_id, MonitorHandle};
57-
use super::observer::RunLoop;
5858
use super::util::cgerr;
5959
use super::view::WinitView;
6060
use super::window::{window_id, WinitPanel, WinitWindow};
@@ -175,7 +175,7 @@ define_class!(
175175

176176
let mtm = MainThreadMarker::from(self);
177177
let this = self.retain();
178-
RunLoop::main(mtm).queue_closure(move || {
178+
MainRunLoop::get(mtm).queue_closure(move || {
179179
this.handle_scale_factor_changed(scale_factor);
180180
});
181181
}

winit-common/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ x11 = ["xkbcommon-dl?/x11", "dep:x11-dl"]
1818
xkb = ["dep:xkbcommon-dl", "dep:smol_str"]
1919

2020
# CoreFoundation
21-
core-foundation = ["dep:objc2", "dep:objc2-core-foundation"]
21+
core-foundation = ["dep:block2", "dep:objc2", "dep:objc2-core-foundation"]
2222

2323
[dependencies]
2424
smol_str = { workspace = true, optional = true }
@@ -31,9 +31,11 @@ x11-dl = { workspace = true, optional = true }
3131
xkbcommon-dl = { workspace = true, optional = true }
3232

3333
# CoreFoundation
34+
block2 = { workspace = true, optional = true }
3435
objc2 = { workspace = true, optional = true }
3536
objc2-core-foundation = { workspace = true, optional = true, features = [
3637
"std",
38+
"block2",
3739
"CFRunLoop",
3840
"CFString",
3941
] }

0 commit comments

Comments
 (0)