Retain ApplicationDelegate in NSWindowDelegate and NSView

The delegate is only weakly referenced by NSApplication, so getting it
from there may fail if the event loop has been dropped.

Fixes #3668.
This commit is contained in:
Mads Marquart 2024-05-06 16:29:07 +02:00 committed by GitHub
parent 16fd2baba0
commit 0a3cacd577
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 55 additions and 23 deletions

View file

@ -52,6 +52,8 @@ pub(super) struct State {
wait_timeout: Cell<Option<Instant>>,
pending_events: RefCell<VecDeque<QueuedEvent>>,
pending_redraw: RefCell<Vec<WindowId>>,
// 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.
}
declare_class!(
@ -71,7 +73,7 @@ declare_class!(
unsafe impl NSObjectProtocol for ApplicationDelegate {}
unsafe impl NSApplicationDelegate for ApplicationDelegate {
// Note: This will, globally, only be run once, no matter how many
// NOTE: This will, globally, only be run once, no matter how many
// `EventLoop`s the user creates.
#[method(applicationDidFinishLaunching:)]
fn did_finish_launching(&self, _sender: Option<&AnyObject>) {
@ -106,7 +108,7 @@ declare_class!(
// In this case we still want to consider Winit's `EventLoop` to be "running",
// so we call `start_running()` above.
if self.ivars().stop_on_launch.get() {
// Note: the original idea had been to only stop the underlying `RunLoop`
// 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).
//
@ -188,7 +190,7 @@ impl ApplicationDelegate {
/// 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,
/// 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) {
self.handle_event(Event::LoopExiting);

View file

@ -78,6 +78,10 @@ impl ActiveEventLoop {
RootWindowTarget { p, _marker: PhantomData }
}
pub(super) fn app_delegate(&self) -> &ApplicationDelegate {
&self.delegate
}
pub fn create_custom_cursor(&self, source: CustomCursorSource) -> RootCustomCursor {
RootCustomCursor { inner: CustomCursor::new(source.inner) }
}
@ -133,9 +137,7 @@ impl ActiveEventLoop {
pub(crate) fn owned_display_handle(&self) -> OwnedDisplayHandle {
OwnedDisplayHandle
}
}
impl ActiveEventLoop {
pub(crate) fn hide_application(&self) {
NSApplication::sharedApplication(self.mtm).hide(None)
}

View file

@ -110,8 +110,11 @@ fn get_left_modifier_code(key: &Key) -> KeyCode {
}
}
#[derive(Debug, Default)]
#[derive(Debug)]
pub struct ViewState {
/// Strong reference to the global application state.
app_delegate: Id<ApplicationDelegate>,
cursor_state: RefCell<CursorState>,
ime_position: Cell<NSPoint>,
ime_size: Cell<NSSize>,
@ -204,8 +207,7 @@ declare_class!(
// It's a workaround for https://github.com/rust-windowing/winit/issues/2640, don't replace with `self.window_id()`.
if let Some(window) = self.ivars()._ns_window.load() {
let app_delegate = ApplicationDelegate::get(MainThreadMarker::from(self));
app_delegate.handle_redraw(window.id());
self.ivars().app_delegate.handle_redraw(window.id());
}
#[allow(clippy::let_unit_value)]
@ -773,16 +775,28 @@ declare_class!(
impl WinitView {
pub(super) fn new(
app_delegate: &ApplicationDelegate,
window: &WinitWindow,
accepts_first_mouse: bool,
option_as_alt: OptionAsAlt,
) -> Id<Self> {
let mtm = MainThreadMarker::from(window);
let this = mtm.alloc().set_ivars(ViewState {
app_delegate: app_delegate.retain(),
cursor_state: Default::default(),
ime_position: Default::default(),
ime_size: Default::default(),
modifiers: Default::default(),
phys_modifiers: Default::default(),
tracking_rect: Default::default(),
ime_state: Default::default(),
input_source: Default::default(),
ime_allowed: Default::default(),
forward_key_to_app: Default::default(),
marked_text: Default::default(),
accepts_first_mouse,
_ns_window: WeakId::new(&window.retain()),
option_as_alt: Cell::new(option_as_alt),
..Default::default()
});
let this: Id<Self> = unsafe { msg_send_id![super(this), init] };
@ -818,13 +832,11 @@ impl WinitView {
}
fn queue_event(&self, event: WindowEvent) {
let app_delegate = ApplicationDelegate::get(MainThreadMarker::from(self));
app_delegate.queue_window_event(self.window().id(), event);
self.ivars().app_delegate.queue_window_event(self.window().id(), event);
}
fn queue_device_event(&self, event: DeviceEvent) {
let app_delegate = ApplicationDelegate::get(MainThreadMarker::from(self));
app_delegate.queue_device_event(event);
self.ivars().app_delegate.queue_device_event(event);
}
fn scale_factor(&self) -> f64 {

View file

@ -28,7 +28,9 @@ impl Window {
attributes: WindowAttributes,
) -> Result<Self, RootOsError> {
let mtm = window_target.mtm;
let delegate = autoreleasepool(|_| WindowDelegate::new(attributes, mtm))?;
let delegate = autoreleasepool(|_| {
WindowDelegate::new(window_target.app_delegate(), attributes, mtm)
})?;
Ok(Window {
window: MainThreadBound::new(delegate.window().retain(), mtm),
delegate: MainThreadBound::new(delegate, mtm),

View file

@ -74,6 +74,9 @@ impl Default for PlatformSpecificWindowAttributes {
#[derive(Debug)]
pub(crate) struct State {
/// Strong reference to the global application state.
app_delegate: Id<ApplicationDelegate>,
window: Id<WinitWindow>,
current_theme: Cell<Option<Theme>>,
@ -442,7 +445,11 @@ declare_class!(
}
);
fn new_window(attrs: &WindowAttributes, mtm: MainThreadMarker) -> Option<Id<WinitWindow>> {
fn new_window(
app_delegate: &ApplicationDelegate,
attrs: &WindowAttributes,
mtm: MainThreadMarker,
) -> Option<Id<WinitWindow>> {
autoreleasepool(|_| {
let screen = match attrs.fullscreen.clone().map(Into::into) {
Some(Fullscreen::Borderless(Some(monitor)))
@ -579,6 +586,7 @@ fn new_window(attrs: &WindowAttributes, mtm: MainThreadMarker) -> Option<Id<Wini
}
let view = WinitView::new(
app_delegate,
&window,
attrs.platform_specific.accepts_first_mouse,
attrs.platform_specific.option_as_alt,
@ -618,8 +626,12 @@ fn new_window(attrs: &WindowAttributes, mtm: MainThreadMarker) -> Option<Id<Wini
}
impl WindowDelegate {
pub fn new(attrs: WindowAttributes, mtm: MainThreadMarker) -> Result<Id<Self>, RootOsError> {
let window = new_window(&attrs, mtm)
pub(super) fn new(
app_delegate: &ApplicationDelegate,
attrs: WindowAttributes,
mtm: MainThreadMarker,
) -> Result<Id<Self>, RootOsError> {
let window = new_window(app_delegate, &attrs, mtm)
.ok_or_else(|| os_error!(OsError::CreationError("couldn't create `NSWindow`")))?;
#[cfg(feature = "rwh_06")]
@ -660,6 +672,7 @@ impl WindowDelegate {
};
let delegate = mtm.alloc().set_ivars(State {
app_delegate: app_delegate.retain(),
window: window.retain(),
current_theme: Cell::new(current_theme),
previous_position: Cell::new(None),
@ -756,8 +769,7 @@ impl WindowDelegate {
}
pub(crate) fn queue_event(&self, event: WindowEvent) {
let app_delegate = ApplicationDelegate::get(MainThreadMarker::from(self));
app_delegate.queue_window_event(self.window().id(), event);
self.ivars().app_delegate.queue_window_event(self.window().id(), event);
}
fn queue_static_scale_factor_changed_event(&self) {
@ -770,8 +782,7 @@ impl WindowDelegate {
let content_size = self.window().contentRectForFrameRect(self.window().frame()).size;
let content_size = LogicalSize::new(content_size.width, content_size.height);
let app_delegate = ApplicationDelegate::get(MainThreadMarker::from(self));
app_delegate.queue_static_scale_factor_changed_event(
self.ivars().app_delegate.queue_static_scale_factor_changed_event(
self.window().retain(),
content_size.to_physical(scale_factor),
scale_factor,
@ -833,8 +844,7 @@ impl WindowDelegate {
}
pub fn request_redraw(&self) {
let app_delegate = ApplicationDelegate::get(MainThreadMarker::from(self));
app_delegate.queue_redraw(self.window().id());
self.ivars().app_delegate.queue_redraw(self.window().id());
}
#[inline]