Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only run observers in the default run loop mode #3767

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
]
8 changes: 2 additions & 6 deletions src/platform_impl/apple/appkit/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 0 additions & 4 deletions src/platform_impl/apple/appkit/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions src/platform_impl/apple/appkit/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 }
Expand Down
8 changes: 4 additions & 4 deletions src/platform_impl/apple/appkit/observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/platform_impl/apple/uikit/app_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -802,7 +802,7 @@ impl EventLoopWaker {
wakeup_main_loop,
ptr::null_mut(),
);
CFRunLoopAddTimer(rl, timer, kCFRunLoopCommonModes);
CFRunLoopAddTimer(rl, timer, kCFRunLoopDefaultMode);

EventLoopWaker { timer }
}
Expand Down
6 changes: 3 additions & 3 deletions src/platform_impl/apple/uikit/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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 }
Expand Down
Loading