From 15cdf6f53c85bdb44f5047c416442be4447afb36 Mon Sep 17 00:00:00 2001 From: Wu Wayne Date: Wed, 2 Nov 2022 12:38:02 +0800 Subject: [PATCH 1/3] On macOS, fix `WindowEvent::Destroyed` isn't emitted when the window is dropped --- CHANGELOG.md | 1 + src/platform_impl/macos/mod.rs | 2 +- src/platform_impl/macos/util/async.rs | 14 ++++++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b745bc148..af961b0697 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ And please only add new entries to the top of this list, right below the `# Unre # Unreleased +- On macOS, fix `WindowEvent::Destroyed` isn't emitted when the window is dropped. - On Windows, fix focusing menubar when pressing `Alt`. - On MacOS, made `accepts_first_mouse` configurable. - Migrated `WindowBuilderExtUnix::with_resize_increments` to `WindowBuilder`. diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index f4b585db9b..fa60c2041a 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -57,7 +57,7 @@ pub(crate) struct Window { impl Drop for Window { fn drop(&mut self) { // Ensure the window is closed - util::close_async(Id::into_super(self.window.clone())); + util::close_async(self.window.clone()); } } diff --git a/src/platform_impl/macos/util/async.rs b/src/platform_impl/macos/util/async.rs index 59f8e2ed54..f9888c3ef7 100644 --- a/src/platform_impl/macos/util/async.rs +++ b/src/platform_impl/macos/util/async.rs @@ -8,11 +8,15 @@ use objc2::rc::{autoreleasepool, Id, Shared}; use crate::{ dpi::LogicalSize, + event::{Event, WindowEvent}, platform_impl::platform::{ + app_state::AppState, appkit::{NSScreen, NSWindow, NSWindowLevel, NSWindowStyleMask}, + event::EventWrapper, ffi, - window::{SharedState, SharedStateMutexGuard}, + window::{SharedState, SharedStateMutexGuard, WinitWindow}, }, + window::WindowId, }; // Unsafe wrapper type that allows us to dispatch things that aren't Send. @@ -218,11 +222,17 @@ pub(crate) fn set_title_async(window: &NSWindow, title: String) { // // ArturKovacs: It's important that this operation keeps the underlying window alive // through the `Id` because otherwise it would dereference free'd memory -pub(crate) fn close_async(window: Id) { +pub(crate) fn close_async(window: Id) { let window = MainThreadSafe(window); Queue::main().exec_async(move || { autoreleasepool(move |_| { window.close(); + + let event = Event::WindowEvent { + window_id: WindowId(window.id()), + event: WindowEvent::Destroyed, + }; + AppState::queue_event(EventWrapper::StaticEvent(event)); }); }); } From eb534b583a421876ff0440c2c50e0a2dba5c5f66 Mon Sep 17 00:00:00 2001 From: Wu Wayne Date: Fri, 4 Nov 2022 13:42:39 +0800 Subject: [PATCH 2/3] Extend documentation of `WindowEvent::Destroyed` --- src/event.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/event.rs b/src/event.rs index da237672c7..326c097a99 100644 --- a/src/event.rs +++ b/src/event.rs @@ -331,6 +331,9 @@ pub enum WindowEvent<'a> { CloseRequested, /// The window has been destroyed. + /// + /// However, it won't get emitted if the window is closed by the event loop set to exit. + /// Use [`Event::LoopDestroyed`] to check such behaviour instead. Destroyed, /// A file has been dropped into the window. From 200e76f8731027d68846f549da3e8ec9307b9de4 Mon Sep 17 00:00:00 2001 From: Wu Wayne Date: Fri, 4 Nov 2022 14:36:06 +0800 Subject: [PATCH 3/3] Use delegate's window to close instead --- src/platform_impl/macos/mod.rs | 8 ++++---- src/platform_impl/macos/util/async.rs | 19 +++++-------------- src/platform_impl/macos/window_delegate.rs | 2 +- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index fa60c2041a..404aba2f4c 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -51,13 +51,13 @@ pub(crate) const DEVICE_ID: RootDeviceId = RootDeviceId(DeviceId); pub(crate) struct Window { pub(crate) window: Id, // We keep this around so that it doesn't get dropped until the window does. - _delegate: Id, + pub(crate) delegate: Id, } impl Drop for Window { fn drop(&mut self) { // Ensure the window is closed - util::close_async(self.window.clone()); + util::close_async(self.delegate.clone()); } } @@ -84,8 +84,8 @@ impl Window { attributes: WindowAttributes, pl_attribs: PlatformSpecificWindowBuilderAttributes, ) -> Result { - let (window, _delegate) = autoreleasepool(|_| WinitWindow::new(attributes, pl_attribs))?; - Ok(Window { window, _delegate }) + let (window, delegate) = autoreleasepool(|_| WinitWindow::new(attributes, pl_attribs))?; + Ok(Window { window, delegate }) } } diff --git a/src/platform_impl/macos/util/async.rs b/src/platform_impl/macos/util/async.rs index f9888c3ef7..50212720b4 100644 --- a/src/platform_impl/macos/util/async.rs +++ b/src/platform_impl/macos/util/async.rs @@ -6,17 +6,14 @@ use dispatch::Queue; use objc2::foundation::{is_main_thread, CGFloat, NSPoint, NSSize, NSString}; use objc2::rc::{autoreleasepool, Id, Shared}; +use crate::platform_impl::platform::window_delegate::WinitWindowDelegate; use crate::{ dpi::LogicalSize, - event::{Event, WindowEvent}, platform_impl::platform::{ - app_state::AppState, appkit::{NSScreen, NSWindow, NSWindowLevel, NSWindowStyleMask}, - event::EventWrapper, ffi, - window::{SharedState, SharedStateMutexGuard, WinitWindow}, + window::{SharedState, SharedStateMutexGuard}, }, - window::WindowId, }; // Unsafe wrapper type that allows us to dispatch things that aren't Send. @@ -222,17 +219,11 @@ pub(crate) fn set_title_async(window: &NSWindow, title: String) { // // ArturKovacs: It's important that this operation keeps the underlying window alive // through the `Id` because otherwise it would dereference free'd memory -pub(crate) fn close_async(window: Id) { - let window = MainThreadSafe(window); +pub(crate) fn close_async(delegate: Id) { + let delegate = MainThreadSafe(delegate); Queue::main().exec_async(move || { autoreleasepool(move |_| { - window.close(); - - let event = Event::WindowEvent { - window_id: WindowId(window.id()), - event: WindowEvent::Destroyed, - }; - AppState::queue_event(EventWrapper::StaticEvent(event)); + delegate.window.close(); }); }); } diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs index 539c626424..56a7eb8446 100644 --- a/src/platform_impl/macos/window_delegate.rs +++ b/src/platform_impl/macos/window_delegate.rs @@ -25,7 +25,7 @@ use crate::{ declare_class!( #[derive(Debug)] pub(crate) struct WinitWindowDelegate { - window: IvarDrop>, + pub(crate) window: IvarDrop>, // TODO: It's possible for delegate methods to be called asynchronously, // causing data races / `RefCell` panics.