From 3657506f6edfbcbc82f349e2f29bbd57b521939f Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 3 Dec 2024 18:35:04 +0100 Subject: [PATCH] macOS: Avoid redundant initial resize event (#3913) The `NSViewFrameDidChangeNotification` that we listen to is emitted when `-[NSWindow setContentView]` is called, since that sets the frame of the view as well. So now we register the notification later, so that it's not triggered at window creation. This behaviour is well described in the documentation: https://developer.apple.com/documentation/appkit/nsview/postsframechangednotifications?language=objc --- src/changelog/unreleased.md | 1 + src/platform_impl/apple/appkit/view.rs | 54 +++++++------------ .../apple/appkit/window_delegate.rs | 31 ++++++++--- 3 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 47651d4242..776912b13d 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -211,3 +211,4 @@ changelog entry. - On iOS, fixed `SurfaceResized` and `Window::surface_size` not reporting the size of the actual surface. - On macOS, fixed the scancode conversion for audio volume keys. - On macOS, fixed the scancode conversion for `IntlBackslash`. +- On macOS, fixed redundant `SurfaceResized` event at window creation. diff --git a/src/platform_impl/apple/appkit/view.rs b/src/platform_impl/apple/appkit/view.rs index e8bdffa809..94994b025f 100644 --- a/src/platform_impl/apple/appkit/view.rs +++ b/src/platform_impl/apple/appkit/view.rs @@ -4,17 +4,17 @@ use std::collections::{HashMap, VecDeque}; use std::ptr; use std::rc::Rc; -use objc2::rc::{Retained, WeakId}; +use objc2::rc::Retained; use objc2::runtime::{AnyObject, Sel}; -use objc2::{declare_class, msg_send_id, mutability, sel, ClassType, DeclaredClass}; +use objc2::{declare_class, msg_send_id, mutability, ClassType, DeclaredClass}; use objc2_app_kit::{ NSApplication, NSCursor, NSEvent, NSEventPhase, NSResponder, NSTextInputClient, - NSTrackingRectTag, NSView, NSViewFrameDidChangeNotification, + NSTrackingRectTag, NSView, }; use objc2_foundation::{ MainThreadMarker, NSArray, NSAttributedString, NSAttributedStringKey, NSCopying, - NSMutableAttributedString, NSNotFound, NSNotificationCenter, NSObject, NSObjectProtocol, - NSPoint, NSRange, NSRect, NSSize, NSString, NSUInteger, + NSMutableAttributedString, NSNotFound, NSObject, NSObjectProtocol, NSPoint, NSRange, NSRect, + NSSize, NSString, NSUInteger, }; use super::app_state::AppState; @@ -134,9 +134,6 @@ pub struct ViewState { marked_text: RefCell>, accepts_first_mouse: bool, - // Weak reference because the window keeps a strong reference to the view - _ns_window: WeakId, - /// The state of the `Option` as `Alt`. option_as_alt: Cell, } @@ -177,9 +174,10 @@ declare_class!( self.ivars().tracking_rect.set(Some(tracking_rect)); } - #[method(frameDidChange:)] - fn frame_did_change(&self, _event: &NSEvent) { - trace_scope!("frameDidChange:"); + // Not a normal method on `NSView`, it's triggered by `NSViewFrameDidChangeNotification`. + #[method(viewFrameDidChangeNotification:)] + fn frame_did_change(&self, _notification: Option<&AnyObject>) { + trace_scope!("NSViewFrameDidChangeNotification"); if let Some(tracking_rect) = self.ivars().tracking_rect.take() { self.removeTrackingRect(tracking_rect); } @@ -203,10 +201,7 @@ declare_class!( fn draw_rect(&self, _rect: NSRect) { trace_scope!("drawRect:"); - // 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() { - self.ivars().app_state.handle_redraw(window.id()); - } + self.ivars().app_state.handle_redraw(self.window().id()); // This is a direct subclass of NSView, no need to call superclass' drawRect: } @@ -806,11 +801,10 @@ declare_class!( impl WinitView { pub(super) fn new( app_state: &Rc, - window: &WinitWindow, accepts_first_mouse: bool, option_as_alt: OptionAsAlt, + mtm: MainThreadMarker, ) -> Retained { - let mtm = MainThreadMarker::from(window); let this = mtm.alloc().set_ivars(ViewState { app_state: Rc::clone(app_state), cursor_state: Default::default(), @@ -825,34 +819,24 @@ impl WinitView { 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), }); let this: Retained = unsafe { msg_send_id![super(this), init] }; - this.setPostsFrameChangedNotifications(true); - let notification_center = unsafe { NSNotificationCenter::defaultCenter() }; - unsafe { - notification_center.addObserver_selector_name_object( - &this, - sel!(frameDidChange:), - Some(NSViewFrameDidChangeNotification), - Some(&this), - ) - } - *this.ivars().input_source.borrow_mut() = this.current_input_source(); this } fn window(&self) -> Retained { - // TODO: Simply use `window` property on `NSView`. - // That only returns a window _after_ the view has been attached though! - // (which is incompatible with `frameDidChange:`) - // - // unsafe { msg_send_id![self, window] } - self.ivars()._ns_window.load().expect("view to have a window") + let window = (**self).window().expect("view must be installed in a window"); + + if !window.isKindOfClass(WinitWindow::class()) { + unreachable!("view installed in non-WinitWindow"); + } + + // SAFETY: Just checked that the window is `WinitWindow` + unsafe { Retained::cast(window) } } fn queue_event(&self, event: WindowEvent) { diff --git a/src/platform_impl/apple/appkit/window_delegate.rs b/src/platform_impl/apple/appkit/window_delegate.rs index 92878ce980..6c2f434180 100644 --- a/src/platform_impl/apple/appkit/window_delegate.rs +++ b/src/platform_impl/apple/appkit/window_delegate.rs @@ -15,15 +15,15 @@ use objc2_app_kit::{ NSAppKitVersionNumber, NSAppKitVersionNumber10_12, NSAppearance, NSAppearanceCustomization, NSAppearanceNameAqua, NSApplication, NSApplicationPresentationOptions, NSBackingStoreType, NSColor, NSDraggingDestination, NSFilenamesPboardType, NSPasteboard, - NSRequestUserAttentionType, NSScreen, NSToolbar, NSView, NSWindowButton, NSWindowDelegate, - NSWindowFullScreenButton, NSWindowLevel, NSWindowOcclusionState, NSWindowOrderingMode, - NSWindowSharingType, NSWindowStyleMask, NSWindowTabbingMode, NSWindowTitleVisibility, - NSWindowToolbarStyle, + NSRequestUserAttentionType, NSScreen, NSToolbar, NSView, NSViewFrameDidChangeNotification, + NSWindowButton, NSWindowDelegate, NSWindowFullScreenButton, NSWindowLevel, + NSWindowOcclusionState, NSWindowOrderingMode, NSWindowSharingType, NSWindowStyleMask, + NSWindowTabbingMode, NSWindowTitleVisibility, NSWindowToolbarStyle, }; use objc2_foundation::{ ns_string, CGFloat, MainThreadMarker, NSArray, NSCopying, NSDictionary, NSEdgeInsets, NSKeyValueChangeKey, NSKeyValueChangeNewKey, NSKeyValueChangeOldKey, - NSKeyValueObservingOptions, NSObject, NSObjectNSDelayedPerforming, + NSKeyValueObservingOptions, NSNotificationCenter, NSObject, NSObjectNSDelayedPerforming, NSObjectNSKeyValueObserverRegistration, NSObjectProtocol, NSPoint, NSRect, NSSize, NSString, }; use tracing::{trace, warn}; @@ -170,7 +170,7 @@ declare_class!( #[method(windowDidResize:)] fn window_did_resize(&self, _: Option<&AnyObject>) { trace_scope!("windowDidResize:"); - // NOTE: WindowEvent::SurfaceResized is reported in frameDidChange. + // NOTE: WindowEvent::SurfaceResized is reported using NSViewFrameDidChangeNotification. self.emit_move_event(); } @@ -658,9 +658,9 @@ fn new_window( let view = WinitView::new( app_state, - &window, attrs.platform_specific.accepts_first_mouse, attrs.platform_specific.option_as_alt, + mtm, ); // The default value of `setWantsBestResolutionOpenGLSurface:` was `false` until @@ -682,6 +682,23 @@ fn new_window( window.setContentView(Some(&view)); window.setInitialFirstResponder(Some(&view)); + // Configure the view to send notifications whenever its frame rectangle changes. + // + // We explicitly do this _after_ setting the view as the content view of the window, to + // avoid a resize event when creating the window. + view.setPostsFrameChangedNotifications(true); + // `setPostsFrameChangedNotifications` posts the notification immediately, so register the + // observer _after_, again so that the event isn't triggered initially. + let notification_center = unsafe { NSNotificationCenter::defaultCenter() }; + unsafe { + notification_center.addObserver_selector_name_object( + &view, + sel!(viewFrameDidChangeNotification:), + Some(NSViewFrameDidChangeNotification), + Some(&view), + ) + } + if attrs.transparent { window.setOpaque(false); // See `set_transparent` for details on why we do this.