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

Keep a reference to ApplicationDelegate in NSWindowDelegate and NSView #3684

Merged
merged 1 commit into from
May 6, 2024
Merged
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
4 changes: 4 additions & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,7 @@ 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.
8 changes: 5 additions & 3 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 All @@ -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>) {
Expand Down Expand Up @@ -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).
//
Expand Down Expand Up @@ -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);
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>,
madsmtm marked this conversation as resolved.
Show resolved Hide resolved

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
Loading