From b74cee8df1f62b7c5cf4e4cf075fe2f7474c5e32 Mon Sep 17 00:00:00 2001
From: Robert Bragg <robert@sixbynine.org>
Date: Sun, 23 Jul 2023 23:27:38 +0100
Subject: [PATCH] MacOS: implement pump_events_with_timeout internally

This layers pump_events on a pump_events_with_timeout API, like we have
for Linux and Android.

This is just an internal implementation detail for now but we could
consider making pump_events_with_timeout public, or just making it so
that pump_events() takes the timeout argument.
---
 src/platform_impl/macos/app_state.rs  | 84 ++++++++++++++++++++-------
 src/platform_impl/macos/event_loop.rs | 38 ++++++++++--
 src/platform_impl/macos/mod.rs        |  2 +-
 src/platform_impl/macos/observer.rs   | 55 +++++++++++++-----
 4 files changed, 138 insertions(+), 41 deletions(-)

diff --git a/src/platform_impl/macos/app_state.rs b/src/platform_impl/macos/app_state.rs
index 6569a7cf..c75bfc6a 100644
--- a/src/platform_impl/macos/app_state.rs
+++ b/src/platform_impl/macos/app_state.rs
@@ -121,16 +121,17 @@ impl<T> EventHandler for EventLoopHandler<T> {
 struct Handler {
     stop_app_on_launch: AtomicBool,
     stop_app_before_wait: AtomicBool,
+    stop_app_after_wait: AtomicBool,
     stop_app_on_redraw: AtomicBool,
     launched: AtomicBool,
     running: AtomicBool,
     in_callback: AtomicBool,
     control_flow: Mutex<ControlFlow>,
-    control_flow_prev: Mutex<ControlFlow>,
     start_time: Mutex<Option<Instant>>,
     callback: Mutex<Option<Box<dyn EventHandler>>>,
     pending_events: Mutex<VecDeque<EventWrapper>>,
     pending_redraw: Mutex<Vec<WindowId>>,
+    wait_timeout: Mutex<Option<Instant>>,
     waker: Mutex<EventLoopWaker>,
 }
 
@@ -214,10 +215,11 @@ impl Handler {
         // looks like there have been recuring re-entrancy issues with callback handling that might
         // make that awkward)
         self.running.store(false, Ordering::Relaxed);
-        *self.control_flow_prev.lock().unwrap() = ControlFlow::default();
         *self.control_flow.lock().unwrap() = ControlFlow::default();
         self.set_stop_app_on_redraw_requested(false);
         self.set_stop_app_before_wait(false);
+        self.set_stop_app_after_wait(false);
+        self.set_wait_timeout(None);
     }
 
     pub fn request_stop_app_on_launch(&self) {
@@ -245,6 +247,28 @@ impl Handler {
         self.stop_app_before_wait.load(Ordering::Relaxed)
     }
 
+    pub fn set_stop_app_after_wait(&self, stop_after_wait: bool) {
+        // Relaxed ordering because we don't actually have multiple threads involved, we just want
+        // interior mutability
+        self.stop_app_after_wait
+            .store(stop_after_wait, Ordering::Relaxed);
+    }
+
+    pub fn set_wait_timeout(&self, new_timeout: Option<Instant>) {
+        let mut timeout = self.wait_timeout.lock().unwrap();
+        *timeout = new_timeout;
+    }
+
+    pub fn wait_timeout(&self) -> Option<Instant> {
+        *self.wait_timeout.lock().unwrap()
+    }
+
+    pub fn should_stop_app_after_wait(&self) -> bool {
+        // Relaxed ordering because we don't actually have multiple threads involved, we just want
+        // interior mutability
+        self.stop_app_after_wait.load(Ordering::Relaxed)
+    }
+
     pub fn set_stop_app_on_redraw_requested(&self, stop_on_redraw: bool) {
         // Relaxed ordering because we don't actually have multiple threads involved, we just want
         // interior mutability
@@ -258,16 +282,8 @@ impl Handler {
         self.stop_app_on_redraw.load(Ordering::Relaxed)
     }
 
-    fn get_control_flow_and_update_prev(&self) -> ControlFlow {
-        let control_flow = self.control_flow.lock().unwrap();
-        *self.control_flow_prev.lock().unwrap() = *control_flow;
-        *control_flow
-    }
-
-    fn get_old_and_new_control_flow(&self) -> (ControlFlow, ControlFlow) {
-        let old = *self.control_flow_prev.lock().unwrap();
-        let new = *self.control_flow.lock().unwrap();
-        (old, new)
+    fn control_flow(&self) -> ControlFlow {
+        *self.control_flow.lock().unwrap()
     }
 
     fn get_start_time(&self) -> Option<Instant> {
@@ -402,12 +418,20 @@ impl AppState {
         HANDLER.set_stop_app_before_wait(stop_before_wait);
     }
 
+    pub fn set_stop_app_after_wait(stop_after_wait: bool) {
+        HANDLER.set_stop_app_after_wait(stop_after_wait);
+    }
+
+    pub fn set_wait_timeout(timeout: Option<Instant>) {
+        HANDLER.set_wait_timeout(timeout);
+    }
+
     pub fn set_stop_app_on_redraw_requested(stop_on_redraw: bool) {
         HANDLER.set_stop_app_on_redraw_requested(stop_on_redraw);
     }
 
     pub fn control_flow() -> ControlFlow {
-        HANDLER.get_old_and_new_control_flow().1
+        HANDLER.control_flow()
     }
 
     pub fn exit() -> i32 {
@@ -416,7 +440,7 @@ impl AppState {
         HANDLER.set_in_callback(false);
         HANDLER.exit();
         Self::clear_callback();
-        if let ControlFlow::ExitWithCode(code) = HANDLER.get_old_and_new_control_flow().1 {
+        if let ControlFlow::ExitWithCode(code) = HANDLER.control_flow() {
             code
         } else {
             0
@@ -493,8 +517,13 @@ impl AppState {
         {
             return;
         }
+
+        if HANDLER.should_stop_app_after_wait() {
+            Self::stop();
+        }
+
         let start = HANDLER.get_start_time().unwrap();
-        let cause = match HANDLER.get_control_flow_and_update_prev() {
+        let cause = match HANDLER.control_flow() {
             ControlFlow::Poll => StartCause::Poll,
             ControlFlow::Wait => StartCause::WaitCancelled {
                 start,
@@ -603,16 +632,27 @@ impl AppState {
             Self::stop();
         }
         HANDLER.update_start_time();
-        match HANDLER.get_old_and_new_control_flow() {
-            (ControlFlow::ExitWithCode(_), _) | (_, ControlFlow::ExitWithCode(_)) => (),
-            (old, new) if old == new => (),
-            (_, ControlFlow::Wait) => HANDLER.waker().stop(),
-            (_, ControlFlow::WaitUntil(instant)) => HANDLER.waker().start_at(instant),
-            (_, ControlFlow::Poll) => HANDLER.waker().start(),
-        }
+        let wait_timeout = HANDLER.wait_timeout(); // configured by pump_events_with_timeout
+        let app_timeout = match HANDLER.control_flow() {
+            ControlFlow::Wait => None,
+            ControlFlow::Poll | ControlFlow::ExitWithCode(_) => Some(Instant::now()),
+            ControlFlow::WaitUntil(instant) => Some(instant),
+        };
+        HANDLER
+            .waker()
+            .start_at(min_timeout(wait_timeout, app_timeout));
     }
 }
 
+/// Returns the minimum `Option<Instant>`, 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<Instant>, b: Option<Instant>) -> Option<Instant> {
+    a.map_or(b, |a_timeout| {
+        b.map_or(Some(a_timeout), |b_timeout| Some(a_timeout.min(b_timeout)))
+    })
+}
+
 /// A hack to make activation of multiple windows work when creating them before
 /// `applicationDidFinishLaunching:` / `Event::Event::NewEvents(StartCause::Init)`.
 ///
diff --git a/src/platform_impl/macos/event_loop.rs b/src/platform_impl/macos/event_loop.rs
index 23743859..2623b3e4 100644
--- a/src/platform_impl/macos/event_loop.rs
+++ b/src/platform_impl/macos/event_loop.rs
@@ -9,6 +9,7 @@ use std::{
     ptr,
     rc::{Rc, Weak},
     sync::mpsc,
+    time::{Duration, Instant},
 };
 
 use core_foundation::base::{CFIndex, CFRelease};
@@ -246,11 +247,16 @@ impl<T> EventLoop<T> {
             // catch panics to make sure we can't unwind without clearing the set callback
             // (which would leave the global `AppState` in an undefined, unsafe state)
             let catch_result = catch_unwind(AssertUnwindSafe(|| {
+                // clear / normalize pump_events state
+                AppState::set_wait_timeout(None);
+                AppState::set_stop_app_before_wait(false);
+                AppState::set_stop_app_after_wait(false);
+                AppState::set_stop_app_on_redraw_requested(false);
+
                 if AppState::is_launched() {
                     debug_assert!(!AppState::is_running());
                     AppState::start_running(); // Set is_running = true + dispatch `NewEvents(Init)` + `Resumed`
                 }
-                AppState::set_stop_app_before_wait(false);
                 unsafe { app.run() };
 
                 // While the app is running it's possible that we catch a panic
@@ -286,6 +292,13 @@ impl<T> EventLoop<T> {
     }
 
     pub fn pump_events<F>(&mut self, callback: F) -> PumpStatus
+    where
+        F: FnMut(Event<'_, T>, &RootWindowTarget<T>, &mut ControlFlow),
+    {
+        self.pump_events_with_timeout(Some(Duration::ZERO), callback)
+    }
+
+    fn pump_events_with_timeout<F>(&mut self, timeout: Option<Duration>, callback: F) -> PumpStatus
     where
         F: FnMut(Event<'_, T>, &RootWindowTarget<T>, &mut ControlFlow),
     {
@@ -343,11 +356,26 @@ impl<T> EventLoop<T> {
                     // we just starting to re-run the same `EventLoop` again.
                     AppState::start_running(); // Set is_running = true + dispatch `NewEvents(Init)` + `Resumed`
                 } else {
-                    // Make sure we can't block any external loop indefinitely by stopping the NSApp
-                    // and returning after dispatching any `RedrawRequested` event or whenever the
-                    // `RunLoop` needs to wait for new events from the OS
+                    // Only run the NSApp for as long as the given `Duration` allows so we
+                    // don't block the external loop.
+                    match timeout {
+                        Some(Duration::ZERO) => {
+                            AppState::set_wait_timeout(None);
+                            AppState::set_stop_app_before_wait(true);
+                        }
+                        Some(duration) => {
+                            AppState::set_stop_app_before_wait(false);
+                            let timeout = Instant::now() + duration;
+                            AppState::set_wait_timeout(Some(timeout));
+                            AppState::set_stop_app_after_wait(true);
+                        }
+                        None => {
+                            AppState::set_wait_timeout(None);
+                            AppState::set_stop_app_before_wait(false);
+                            AppState::set_stop_app_after_wait(true);
+                        }
+                    }
                     AppState::set_stop_app_on_redraw_requested(true);
-                    AppState::set_stop_app_before_wait(true);
                     unsafe {
                         app.run();
                     }
diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs
index 4559e0c9..141f03e6 100644
--- a/src/platform_impl/macos/mod.rs
+++ b/src/platform_impl/macos/mod.rs
@@ -35,7 +35,7 @@ use crate::{
 use objc2::rc::{autoreleasepool, Id, Shared};
 
 pub(crate) use crate::icon::NoIcon as PlatformIcon;
-pub(self) use crate::platform_impl::Fullscreen;
+pub(crate) use crate::platform_impl::Fullscreen;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct DeviceId;
diff --git a/src/platform_impl/macos/observer.rs b/src/platform_impl/macos/observer.rs
index bce63c87..1ff1f805 100644
--- a/src/platform_impl/macos/observer.rs
+++ b/src/platform_impl/macos/observer.rs
@@ -141,6 +141,16 @@ pub fn setup_control_flow_observers(panic_info: Weak<PanicInfo>) {
 
 pub struct EventLoopWaker {
     timer: CFRunLoopTimerRef,
+
+    /// An arbitrary instant in the past, that will trigger an immediate wake
+    /// We save this as the `next_fire_date` for consistency so we can
+    /// easily check if the next_fire_date needs updating.
+    start_instant: Instant,
+
+    /// This is what the `NextFireDate` has been set to.
+    /// `None` corresponds to `waker.stop()` and `start_instant` is used
+    /// for `waker.start()`
+    next_fire_date: Option<Instant>,
 }
 
 impl Drop for EventLoopWaker {
@@ -169,31 +179,50 @@ impl Default for EventLoopWaker {
                 ptr::null_mut(),
             );
             CFRunLoopAddTimer(CFRunLoopGetMain(), timer, kCFRunLoopCommonModes);
-            EventLoopWaker { timer }
+            EventLoopWaker {
+                timer,
+                start_instant: Instant::now(),
+                next_fire_date: None,
+            }
         }
     }
 }
 
 impl EventLoopWaker {
     pub fn stop(&mut self) {
-        unsafe { CFRunLoopTimerSetNextFireDate(self.timer, std::f64::MAX) }
+        if self.next_fire_date.is_some() {
+            self.next_fire_date = None;
+            unsafe { CFRunLoopTimerSetNextFireDate(self.timer, std::f64::MAX) }
+        }
     }
 
     pub fn start(&mut self) {
-        unsafe { CFRunLoopTimerSetNextFireDate(self.timer, std::f64::MIN) }
+        if self.next_fire_date != Some(self.start_instant) {
+            self.next_fire_date = Some(self.start_instant);
+            unsafe { CFRunLoopTimerSetNextFireDate(self.timer, std::f64::MIN) }
+        }
     }
 
-    pub fn start_at(&mut self, instant: Instant) {
+    pub fn start_at(&mut self, instant: Option<Instant>) {
         let now = Instant::now();
-        if now >= instant {
-            self.start();
-        } else {
-            unsafe {
-                let current = CFAbsoluteTimeGetCurrent();
-                let duration = instant - now;
-                let fsecs =
-                    duration.subsec_nanos() as f64 / 1_000_000_000.0 + duration.as_secs() as f64;
-                CFRunLoopTimerSetNextFireDate(self.timer, current + fsecs)
+        match instant {
+            Some(instant) if now >= instant => {
+                self.start();
+            }
+            Some(instant) => {
+                if self.next_fire_date != Some(instant) {
+                    self.next_fire_date = Some(instant);
+                    unsafe {
+                        let current = CFAbsoluteTimeGetCurrent();
+                        let duration = instant - now;
+                        let fsecs = duration.subsec_nanos() as f64 / 1_000_000_000.0
+                            + duration.as_secs() as f64;
+                        CFRunLoopTimerSetNextFireDate(self.timer, current + fsecs)
+                    }
+                }
+            }
+            None => {
+                self.stop();
             }
         }
     }