From bf73a978df8affb78709b61fdd1eadda5e86bcd2 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 27 May 2024 11:41:07 +0200 Subject: [PATCH] Only run observers in the default run loop mode --- clippy.toml | 2 ++ src/platform_impl/apple/appkit/app_state.rs | 8 ++------ src/platform_impl/apple/appkit/event_handler.rs | 4 ---- src/platform_impl/apple/appkit/event_loop.rs | 4 ++-- src/platform_impl/apple/appkit/observer.rs | 8 ++++---- src/platform_impl/apple/uikit/app_state.rs | 4 ++-- src/platform_impl/apple/uikit/event_loop.rs | 6 +++--- 7 files changed, 15 insertions(+), 21 deletions(-) diff --git a/clippy.toml b/clippy.toml index 84bf0d5e01..fea37fde93 100644 --- a/clippy.toml +++ b/clippy.toml @@ -12,4 +12,6 @@ disallowed-methods = [ { path = "web_sys::Document::fullscreen_element", reason = "Doesn't account for compatibility with Safari" }, { path = "objc2_app_kit::NSView::visibleRect", reason = "We expose a render target to the user, and visibility is not really relevant to that (and can break if you don't use the rectangle position as well). Use `frame` instead." }, { path = "objc2_app_kit::NSWindow::setFrameTopLeftPoint", reason = "Not sufficient when working with Winit's coordinate system, use `flip_window_screen_coordinates` instead" }, + { path = "core_foundation::runloop::kCFRunLoopCommonModes", reason = "Using the common modes includes the modal panel mode, which we usually want to avoid" }, + { path = "objc2_foundation::NSRunLoopCommonModes", reason = "Using the common modes includes the modal panel mode, which we usually want to avoid" } ] diff --git a/src/platform_impl/apple/appkit/app_state.rs b/src/platform_impl/apple/appkit/app_state.rs index 84d9a4fa53..790e51c1de 100644 --- a/src/platform_impl/apple/appkit/app_state.rs +++ b/src/platform_impl/apple/appkit/app_state.rs @@ -323,8 +323,7 @@ impl ApplicationDelegate { .upgrade() .expect("The panic info must exist here. This failure indicates a developer error."); - // Return when in event handler due to https://github.com/rust-windowing/winit/issues/1779 - if panic_info.is_panicking() || !self.ivars().event_handler.ready() || !self.is_running() { + if panic_info.is_panicking() || !self.is_running() { return; } @@ -356,10 +355,7 @@ impl ApplicationDelegate { .upgrade() .expect("The panic info must exist here. This failure indicates a developer error."); - // 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 panic_info.is_panicking() || !self.ivars().event_handler.ready() || !self.is_running() { + if panic_info.is_panicking() || !self.is_running() { return; } diff --git a/src/platform_impl/apple/appkit/event_handler.rs b/src/platform_impl/apple/appkit/event_handler.rs index c8a33ad915..a0b6a31c8b 100644 --- a/src/platform_impl/apple/appkit/event_handler.rs +++ b/src/platform_impl/apple/appkit/event_handler.rs @@ -104,10 +104,6 @@ impl EventHandler { self.inner.try_borrow().is_err() } - pub(crate) fn ready(&self) -> bool { - matches!(self.inner.try_borrow().as_deref(), Ok(Some(_))) - } - pub(crate) fn handle( &self, callback: impl FnOnce(&mut dyn ApplicationHandler, &RootActiveEventLoop), diff --git a/src/platform_impl/apple/appkit/event_loop.rs b/src/platform_impl/apple/appkit/event_loop.rs index 2d35bb4fb4..edb5b5294e 100644 --- a/src/platform_impl/apple/appkit/event_loop.rs +++ b/src/platform_impl/apple/appkit/event_loop.rs @@ -12,7 +12,7 @@ use std::time::{Duration, Instant}; use core_foundation::base::{CFIndex, CFRelease}; use core_foundation::runloop::{ - kCFRunLoopCommonModes, CFRunLoopAddSource, CFRunLoopGetMain, CFRunLoopSourceContext, + kCFRunLoopDefaultMode, CFRunLoopAddSource, CFRunLoopGetMain, CFRunLoopSourceContext, CFRunLoopSourceCreate, CFRunLoopSourceRef, CFRunLoopSourceSignal, CFRunLoopWakeUp, }; use objc2::rc::{autoreleasepool, Retained}; @@ -462,7 +462,7 @@ impl EventLoopProxy { perform: event_loop_proxy_handler, }; let source = CFRunLoopSourceCreate(ptr::null_mut(), CFIndex::MAX - 1, &mut context); - CFRunLoopAddSource(rl, source, kCFRunLoopCommonModes); + CFRunLoopAddSource(rl, source, kCFRunLoopDefaultMode); CFRunLoopWakeUp(rl); EventLoopProxy { proxy_wake_up, source } diff --git a/src/platform_impl/apple/appkit/observer.rs b/src/platform_impl/apple/appkit/observer.rs index 8339803086..0537ffedeb 100644 --- a/src/platform_impl/apple/appkit/observer.rs +++ b/src/platform_impl/apple/appkit/observer.rs @@ -13,8 +13,8 @@ use block2::Block; use core_foundation::base::{CFIndex, CFOptionFlags, CFRelease, CFTypeRef}; use core_foundation::date::CFAbsoluteTimeGetCurrent; use core_foundation::runloop::{ - kCFRunLoopAfterWaiting, kCFRunLoopBeforeWaiting, kCFRunLoopCommonModes, kCFRunLoopDefaultMode, - kCFRunLoopExit, CFRunLoopActivity, CFRunLoopAddObserver, CFRunLoopAddTimer, CFRunLoopGetMain, + kCFRunLoopAfterWaiting, kCFRunLoopBeforeWaiting, kCFRunLoopDefaultMode, kCFRunLoopExit, + CFRunLoopActivity, CFRunLoopAddObserver, CFRunLoopAddTimer, CFRunLoopGetMain, CFRunLoopObserverCallBack, CFRunLoopObserverContext, CFRunLoopObserverCreate, CFRunLoopObserverRef, CFRunLoopRef, CFRunLoopTimerCreate, CFRunLoopTimerInvalidate, CFRunLoopTimerRef, CFRunLoopTimerSetNextFireDate, CFRunLoopWakeUp, @@ -129,7 +129,7 @@ impl RunLoop { context, ) }; - unsafe { CFRunLoopAddObserver(self.0, observer, kCFRunLoopCommonModes) }; + unsafe { CFRunLoopAddObserver(self.0, observer, kCFRunLoopDefaultMode) }; } /// Submit a closure to run on the main thread as the next step in the run loop, before other @@ -267,7 +267,7 @@ impl EventLoopWaker { wakeup_main_loop, ptr::null_mut(), ); - CFRunLoopAddTimer(CFRunLoopGetMain(), timer, kCFRunLoopCommonModes); + CFRunLoopAddTimer(CFRunLoopGetMain(), timer, kCFRunLoopDefaultMode); Self { timer, start_instant: Instant::now(), next_fire_date: None } } } diff --git a/src/platform_impl/apple/uikit/app_state.rs b/src/platform_impl/apple/uikit/app_state.rs index a7cd751edb..53258e92f4 100644 --- a/src/platform_impl/apple/uikit/app_state.rs +++ b/src/platform_impl/apple/uikit/app_state.rs @@ -10,7 +10,7 @@ use std::{fmt, mem, ptr}; use core_foundation::base::CFRelease; use core_foundation::date::CFAbsoluteTimeGetCurrent; use core_foundation::runloop::{ - kCFRunLoopCommonModes, CFRunLoopAddTimer, CFRunLoopGetMain, CFRunLoopRef, CFRunLoopTimerCreate, + kCFRunLoopDefaultMode, CFRunLoopAddTimer, CFRunLoopGetMain, CFRunLoopRef, CFRunLoopTimerCreate, CFRunLoopTimerInvalidate, CFRunLoopTimerRef, CFRunLoopTimerSetNextFireDate, }; use objc2::rc::Retained; @@ -802,7 +802,7 @@ impl EventLoopWaker { wakeup_main_loop, ptr::null_mut(), ); - CFRunLoopAddTimer(rl, timer, kCFRunLoopCommonModes); + CFRunLoopAddTimer(rl, timer, kCFRunLoopDefaultMode); EventLoopWaker { timer } } diff --git a/src/platform_impl/apple/uikit/event_loop.rs b/src/platform_impl/apple/uikit/event_loop.rs index f30bbbb823..7e44ff6729 100644 --- a/src/platform_impl/apple/uikit/event_loop.rs +++ b/src/platform_impl/apple/uikit/event_loop.rs @@ -7,8 +7,8 @@ use std::sync::Arc; use core_foundation::base::{CFIndex, CFRelease}; use core_foundation::runloop::{ - kCFRunLoopAfterWaiting, kCFRunLoopBeforeWaiting, kCFRunLoopCommonModes, kCFRunLoopDefaultMode, - kCFRunLoopExit, CFRunLoopActivity, CFRunLoopAddObserver, CFRunLoopAddSource, CFRunLoopGetMain, + kCFRunLoopAfterWaiting, kCFRunLoopBeforeWaiting, kCFRunLoopDefaultMode, kCFRunLoopExit, + CFRunLoopActivity, CFRunLoopAddObserver, CFRunLoopAddSource, CFRunLoopGetMain, CFRunLoopObserverCreate, CFRunLoopObserverRef, CFRunLoopSourceContext, CFRunLoopSourceCreate, CFRunLoopSourceInvalidate, CFRunLoopSourceRef, CFRunLoopSourceSignal, CFRunLoopWakeUp, }; @@ -266,7 +266,7 @@ impl EventLoopProxy { perform: event_loop_proxy_handler, }; let source = CFRunLoopSourceCreate(ptr::null_mut(), CFIndex::MAX - 1, &mut context); - CFRunLoopAddSource(rl, source, kCFRunLoopCommonModes); + CFRunLoopAddSource(rl, source, kCFRunLoopDefaultMode); CFRunLoopWakeUp(rl); EventLoopProxy { proxy_wake_up, source }