diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index e8344b53c..68fa872dc 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -229,3 +229,4 @@ changelog entry. - On macOS, fixed the scancode conversion for audio volume keys. - On macOS, fixed the scancode conversion for `IntlBackslash`. - On macOS, fixed redundant `SurfaceResized` event at window creation. +- On macOS, fixed the behaviour of `pump_app_events` to match what the system expects. diff --git a/src/platform/pump_events.rs b/src/platform/pump_events.rs index 4d070feac..2b8f43b02 100644 --- a/src/platform/pump_events.rs +++ b/src/platform/pump_events.rs @@ -83,22 +83,18 @@ pub trait EventLoopExtPumpEvents { /// - **Windows**: The implementation will use `PeekMessage` when checking for window messages /// to avoid blocking your external event loop. /// - /// - **MacOS**: The implementation works in terms of stopping the global application whenever - /// the application `RunLoop` indicates that it is preparing to block and wait for new events. + /// - **MacOS**: Certain actions like resizing the window will enter a "modal" state, where + /// `pump_app_events` will process events internally, and block until the resize is over. /// - /// This is very different to the polling APIs that are available on other - /// platforms (the lower level polling primitives on MacOS are private - /// implementation details for `NSApplication` which aren't accessible to - /// application developers) + /// Thus, if you render or run your game code outside of `ApplicationHandler`, your + /// application will freeze while the window resizes. The recommended approach is to render + /// inside [`WindowEvent::RedrawRequested`] instead. /// - /// It's likely this will be less efficient than polling on other OSs and - /// it also means the `NSApplication` is stopped while outside of the Winit - /// event loop - and that's observable (for example to crates like `rfd`) - /// because the `NSApplication` is global state. + /// Furthermore, when pumping events the `NSApplication` is still considered stopped to + /// crates like `rfd` that inspect [`-[NSApplication isRunning]`][isrunning]. /// - /// If you render outside of Winit you are likely to see window resizing artifacts - /// since MacOS expects applications to render synchronously during any `drawRect` - /// callback. + /// [`WindowEvent::RedrawRequested`]: crate::event::WindowEvent::RedrawRequested + /// [isrunning]: https://developer.apple.com/documentation/appkit/nsapplication/isrunning?language=objc fn pump_app_events( &mut self, timeout: Option, diff --git a/src/platform_impl/apple/appkit/app_state.rs b/src/platform_impl/apple/appkit/app_state.rs index 767e8ecea..216d144dc 100644 --- a/src/platform_impl/apple/appkit/app_state.rs +++ b/src/platform_impl/apple/appkit/app_state.rs @@ -28,20 +28,13 @@ pub(super) struct AppState { run_loop: RunLoop, event_loop_proxy: Arc, event_handler: EventHandler, - stop_on_launch: Cell, - stop_before_wait: Cell, - stop_after_wait: Cell, - stop_on_redraw: Cell, - /// Whether `applicationDidFinishLaunching:` has been run or not. + /// Whether `NSApplicationDidFinishLaunchingNotification` has been sent. is_launched: Cell, - /// Whether an `EventLoop` is currently running. - is_running: Cell, /// Whether the user has requested the event loop to exit. exit: Cell, control_flow: Cell, waker: RefCell, start_time: Cell>, - wait_timeout: Cell>, pending_redraw: RefCell>, // NOTE: This is strongly referenced by our `NSWindowDelegate` and our `NSView` subclass, and // as such should be careful to not add fields that, in turn, strongly reference those. @@ -71,17 +64,11 @@ impl AppState { run_loop: RunLoop::main(mtm), event_loop_proxy, event_handler: EventHandler::new(), - stop_on_launch: Cell::new(false), - stop_before_wait: Cell::new(false), - stop_after_wait: Cell::new(false), - stop_on_redraw: Cell::new(false), is_launched: Cell::new(false), - is_running: Cell::new(false), exit: Cell::new(false), control_flow: Cell::new(ControlFlow::default()), waker: RefCell::new(EventLoopWaker::new()), start_time: Cell::new(None), - wait_timeout: Cell::new(None), pending_redraw: RefCell::new(vec![]), }); @@ -97,15 +84,17 @@ impl AppState { .clone() } - // NOTE: This notification will, globally, only be emitted once, - // no matter how many `EventLoop`s the user creates. pub fn did_finish_launching(self: &Rc, _notification: &NSNotification) { trace_scope!("NSApplicationDidFinishLaunchingNotification"); + // NOTE: This notification will, globally, only be emitted once, + // no matter how many `EventLoop`s the user creates. There is no other + // way to know this information, other than to keep track of it + // ourselves. self.is_launched.set(true); let app = NSApplication::sharedApplication(self.mtm); - // We need to delay setting the activation policy and activating the app - // until `applicationDidFinishLaunching` has been called. Otherwise the + // We need to delay setting the activation policy and activating the app until + // `NSApplicationDidFinishLaunchingNotification` has been sent. Otherwise the // menu bar is initially unresponsive on macOS 10.15. if let Some(activation_policy) = self.activation_policy { app.setActivationPolicy(activation_policy); @@ -135,23 +124,7 @@ impl AppState { self.waker.borrow_mut().start(); - self.set_is_running(true); self.dispatch_init_events(); - - // If the application is being launched via `EventLoop::pump_app_events()` then we'll - // want to stop the app once it is launched (and return to the external loop) - // - // In this case we still want to consider Winit's `EventLoop` to be "running", - // so we call `start_running()` above. - if self.stop_on_launch.get() { - // NOTE: the original idea had been to only stop the underlying `RunLoop` - // for the app but that didn't work as expected (`-[NSApplication run]` - // effectively ignored the attempt to stop the RunLoop and re-started it). - // - // So we return from `pump_events` by stopping the application. - let app = NSApplication::sharedApplication(self.mtm); - stop_app_immediately(&app); - } } pub fn will_terminate(self: &Rc, _notification: &NSNotification) { @@ -174,57 +147,16 @@ impl AppState { &self.event_loop_proxy } - /// If `pump_events` is called to progress the event loop then we - /// bootstrap the event loop via `-[NSApplication run]` but will use - /// `CFRunLoopRunInMode` for subsequent calls to `pump_events`. - pub fn set_stop_on_launch(&self) { - self.stop_on_launch.set(true); - } - - pub fn set_stop_before_wait(&self, value: bool) { - self.stop_before_wait.set(value) - } - - pub fn set_stop_after_wait(&self, value: bool) { - self.stop_after_wait.set(value) - } - - pub fn set_stop_on_redraw(&self, value: bool) { - self.stop_on_redraw.set(value) - } - - pub fn set_wait_timeout(&self, value: Option) { - self.wait_timeout.set(value) - } - - /// Clears the `running` state and resets the `control_flow` state when an `EventLoop` exits. - /// - /// NOTE: that if the `NSApplication` has been launched then that state is preserved, - /// and we won't need to re-launch the app if subsequent EventLoops are run. pub fn internal_exit(self: &Rc) { self.with_handler(|app, event_loop| { app.exiting(event_loop); }); - - self.set_is_running(false); - self.set_stop_on_redraw(false); - self.set_stop_before_wait(false); - self.set_stop_after_wait(false); - self.set_wait_timeout(None); } pub fn is_launched(&self) -> bool { self.is_launched.get() } - pub fn set_is_running(&self, value: bool) { - self.is_running.set(value) - } - - pub fn is_running(&self) -> bool { - self.is_running.get() - } - pub fn exit(&self) { self.exit.set(true) } @@ -252,14 +184,6 @@ impl AppState { self.with_handler(|app, event_loop| { app.window_event(event_loop, window_id, WindowEvent::RedrawRequested); }); - - // `pump_events` will request to stop immediately _after_ dispatching RedrawRequested - // events as a way to ensure that `pump_events` can't block an external loop - // indefinitely - if self.stop_on_redraw.get() { - let app = NSApplication::sharedApplication(self.mtm); - stop_app_immediately(&app); - } } } @@ -313,15 +237,10 @@ impl AppState { // Called by RunLoopObserver after finishing waiting for new events pub fn wakeup(self: &Rc) { // Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779 - if !self.event_handler.ready() || !self.is_running() { + if !self.event_handler.ready() { return; } - if self.stop_after_wait.get() { - let app = NSApplication::sharedApplication(self.mtm); - stop_app_immediately(&app); - } - let start = self.start_time.get().unwrap(); let cause = match self.control_flow() { ControlFlow::Poll => StartCause::Poll, @@ -343,7 +262,7 @@ impl AppState { // Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779 // XXX: how does it make sense that `event_handler.ready()` can ever return `false` here if // we're about to return to the `CFRunLoop` to poll for new events? - if !self.event_handler.ready() || !self.is_running() { + if !self.event_handler.ready() { return; } @@ -362,24 +281,12 @@ impl AppState { stop_app_immediately(&app); } - if self.stop_before_wait.get() { - let app = NSApplication::sharedApplication(self.mtm); - stop_app_immediately(&app); - } self.start_time.set(Some(Instant::now())); - let wait_timeout = self.wait_timeout.get(); // configured by pump_events let app_timeout = match self.control_flow() { ControlFlow::Wait => None, ControlFlow::Poll => Some(Instant::now()), ControlFlow::WaitUntil(instant) => Some(instant), }; - self.waker.borrow_mut().start_at(min_timeout(wait_timeout, app_timeout)); + self.waker.borrow_mut().start_at(app_timeout); } } - -/// Returns the minimum `Option`, taking into account that `None` -/// equates to an infinite timeout, not a zero timeout (so can't just use -/// `Option::min`) -fn min_timeout(a: Option, b: Option) -> Option { - a.map_or(b, |a_timeout| b.map_or(Some(a_timeout), |b_timeout| Some(a_timeout.min(b_timeout)))) -} diff --git a/src/platform_impl/apple/appkit/event_loop.rs b/src/platform_impl/apple/appkit/event_loop.rs index 8246efe86..b8053e35a 100644 --- a/src/platform_impl/apple/appkit/event_loop.rs +++ b/src/platform_impl/apple/appkit/event_loop.rs @@ -1,15 +1,17 @@ use std::rc::Rc; use std::sync::Arc; -use std::time::{Duration, Instant}; +use std::time::Duration; use objc2::rc::{autoreleasepool, Retained}; use objc2::runtime::ProtocolObject; use objc2::{available, MainThreadMarker}; use objc2_app_kit::{ NSApplication, NSApplicationActivationPolicy, NSApplicationDidFinishLaunchingNotification, - NSApplicationWillTerminateNotification, NSWindow, + NSApplicationWillTerminateNotification, NSEventMask, NSWindow, +}; +use objc2_foundation::{ + NSDate, NSDefaultRunLoopMode, NSNotificationCenter, NSObjectProtocol, NSTimeInterval, }; -use objc2_foundation::{NSNotificationCenter, NSObjectProtocol}; use rwh_06::HasDisplayHandle; use super::super::notification_center::create_observer; @@ -136,6 +138,9 @@ pub struct EventLoop { app: Retained, app_state: Rc, + /// Whether an outer event loop is running. + pump_has_sent_init: bool, + window_target: ActiveEventLoop, // Since macOS 10.11, we no longer need to remove the observers before they are deallocated; @@ -186,6 +191,15 @@ impl EventLoop { // Override `sendEvent:` on the application to forward to our application state. override_send_event(&app); + // Queue `NSApplicationDidFinishLaunchingNotification` and generally + // make sure the application is fully initialized (once the run loop + // starts). + // + // This is technically only necessary when using `pump_app_events` + // (`app.run()` will do it for us in `run_app_on_demand`), but we + // might as well do it everywhere. + unsafe { app.finishLaunching() }; + let center = unsafe { NSNotificationCenter::defaultCenter() }; let weak_app_state = Rc::downgrade(&app_state); @@ -217,6 +231,7 @@ impl EventLoop { Ok(EventLoop { app, app_state: app_state.clone(), + pump_has_sent_init: false, window_target: ActiveEventLoop { app_state, mtm }, _did_finish_launching_observer, _will_terminate_observer, @@ -231,10 +246,6 @@ impl EventLoop { self.run_app_on_demand(app) } - // NB: we don't base this on `pump_events` because for `MacOs` we can't support - // `pump_events` elegantly (we just ask to run the loop for a "short" amount of - // time and so a layered implementation would end up using a lot of CPU due to - // redundant wake ups. pub fn run_app_on_demand( &mut self, mut app: A, @@ -242,18 +253,19 @@ impl EventLoop { self.app_state.clear_exit(); self.app_state.set_event_handler(&mut app, || { autoreleasepool(|_| { - // clear / normalize pump_events state - self.app_state.set_wait_timeout(None); - self.app_state.set_stop_before_wait(false); - self.app_state.set_stop_after_wait(false); - self.app_state.set_stop_on_redraw(false); - if self.app_state.is_launched() { - debug_assert!(!self.app_state.is_running()); - self.app_state.set_is_running(true); + // The `NSApplicationDidFinishLaunchingNotification` notification is globally + // only delivered once, but for the purpose of our events, we want to act + // as-if an entirely new event loop has been started on each invocation of + // `run_app_on_demand`. self.app_state.dispatch_init_events(); } + // NOTE: We don't base this on `pump_events` because + // `nextEventMatchingMask:untilDate:inMode:dequeue:` is worse supported, + // especially as the top-level handler. In part because this sets the `isRunning` + // flag (which is used by crates like `rfd`), while `nextEventMatchingMask` won't. + // // NOTE: Make sure to not run the application re-entrantly, as that'd be confusing. self.app.run(); @@ -271,49 +283,43 @@ impl EventLoop { ) -> PumpStatus { self.app_state.set_event_handler(&mut app, || { autoreleasepool(|_| { - // As a special case, if the application hasn't been launched yet then we at least - // run the loop until it has fully launched. - if !self.app_state.is_launched() { - debug_assert!(!self.app_state.is_running()); - - self.app_state.set_stop_on_launch(); - self.app.run(); - - // Note: we dispatch `NewEvents(Init)` + `Resumed` events after the application - // has launched - } else if !self.app_state.is_running() { - // Even though the application may have been launched, it's possible we aren't - // running if the `EventLoop` was run before and has since - // exited. This indicates that we just starting to re-run - // the same `EventLoop` again. - self.app_state.set_is_running(true); + if self.app_state.is_launched() && !self.pump_has_sent_init { + // If the application is already launched, we won't get the re-initialization + // events. Dispatch them here instead. self.app_state.dispatch_init_events(); - } else { - // Only run for as long as the given `Duration` allows so we don't block the - // external loop. - match timeout { - Some(Duration::ZERO) => { - self.app_state.set_wait_timeout(None); - self.app_state.set_stop_before_wait(true); - }, - Some(duration) => { - self.app_state.set_stop_before_wait(false); - let timeout = Instant::now() + duration; - self.app_state.set_wait_timeout(Some(timeout)); - self.app_state.set_stop_after_wait(true); - }, - None => { - self.app_state.set_wait_timeout(None); - self.app_state.set_stop_before_wait(false); - self.app_state.set_stop_after_wait(true); - }, - } - self.app_state.set_stop_on_redraw(true); - self.app.run(); + } + self.pump_has_sent_init = true; + + // Only run for as long as the given `Duration` allows so we don't block the + // external loop. + let expiration_date = match timeout { + Some(Duration::ZERO) => unsafe { NSDate::distantPast() }, + Some(duration) => unsafe { + NSDate::dateWithTimeIntervalSinceNow( + duration.as_secs_f64() as NSTimeInterval + ) + }, + None => unsafe { NSDate::distantFuture() }, + }; + + // Wait for an event to arrive within the specified duration, + // and let the application handle it if one did. + let event = unsafe { + self.app.nextEventMatchingMask_untilDate_inMode_dequeue( + NSEventMask::Any, + Some(&expiration_date), + NSDefaultRunLoopMode, + true, + ) + }; + if let Some(event) = event { + unsafe { self.app.sendEvent(&event) }; } if self.app_state.exiting() { self.app_state.internal_exit(); + // If we start again, we'll emit a new set of initialization events. + self.pump_has_sent_init = false; PumpStatus::Exit(0) } else { PumpStatus::Continue