Skip to content

Commit

Permalink
macOS: Avoid redundant initial resize event (#3913)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
madsmtm authored Dec 3, 2024
1 parent edca3eb commit 3657506
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 42 deletions.
1 change: 1 addition & 0 deletions src/changelog/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
54 changes: 19 additions & 35 deletions src/platform_impl/apple/appkit/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -134,9 +134,6 @@ pub struct ViewState {
marked_text: RefCell<Retained<NSMutableAttributedString>>,
accepts_first_mouse: bool,

// Weak reference because the window keeps a strong reference to the view
_ns_window: WeakId<WinitWindow>,

/// The state of the `Option` as `Alt`.
option_as_alt: Cell<OptionAsAlt>,
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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:
}
Expand Down Expand Up @@ -806,11 +801,10 @@ declare_class!(
impl WinitView {
pub(super) fn new(
app_state: &Rc<AppState>,
window: &WinitWindow,
accepts_first_mouse: bool,
option_as_alt: OptionAsAlt,
mtm: MainThreadMarker,
) -> Retained<Self> {
let mtm = MainThreadMarker::from(window);
let this = mtm.alloc().set_ivars(ViewState {
app_state: Rc::clone(app_state),
cursor_state: Default::default(),
Expand All @@ -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<Self> = 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<WinitWindow> {
// 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) {
Expand Down
31 changes: 24 additions & 7 deletions src/platform_impl/apple/appkit/window_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down

0 comments on commit 3657506

Please sign in to comment.