Skip to content

Commit

Permalink
Keep a reference to ApplicationDelegate in NSWindowDelegate and NSView
Browse files Browse the repository at this point in the history
The delegate is only weakly referenced by NSApplication, so getting it
from there may fail if the event loop has been dropped.
  • Loading branch information
madsmtm committed May 5, 2024
1 parent 7f8771a commit d1a593c
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 20 deletions.
3 changes: 3 additions & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@ The migration guide could reference other migration examples in the current
changelog entry.

## Unreleased

### Fixed
- On macOS, fix panic on exit when dropping windows outside the event loop.
2 changes: 2 additions & 0 deletions src/platform_impl/macos/app_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
6 changes: 4 additions & 2 deletions src/platform_impl/macos/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}
Expand Down Expand Up @@ -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)
}
Expand Down
28 changes: 20 additions & 8 deletions src/platform_impl/macos/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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] };

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion src/platform_impl/macos/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
28 changes: 19 additions & 9 deletions src/platform_impl/macos/window_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit d1a593c

Please sign in to comment.