From bca57ed0b4ff0519ccafb00a7d4fe88b66ce596e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 8 Jul 2023 22:36:42 +0300 Subject: [PATCH] Stop using `&mut` in Objective-C delegate methods (#2925) * Make iOS declared classes not use &mut * Prepare `init` methods for not having access to &mut self * Prepare WinitWindow methods for not having access to &mut self * Convert a bit of WinitView's to use interior mutability * Convert a bit more of WinitView's to use interior mutability * Convert the rest of WinitView to use interior mutability * Use interior mutability instead of a Mutex for the CursorState * Use interior mutability in WinitWindowDelegate --- src/platform_impl/ios/ffi.rs | 4 + src/platform_impl/ios/view.rs | 139 +++++----- src/platform_impl/ios/window.rs | 8 +- src/platform_impl/macos/app_delegate.rs | 8 +- src/platform_impl/macos/appkit/responder.rs | 3 +- src/platform_impl/macos/appkit/view.rs | 2 +- src/platform_impl/macos/util/async.rs | 5 +- src/platform_impl/macos/view.rs | 273 +++++++++++--------- src/platform_impl/macos/window.rs | 269 ++++++++++--------- src/platform_impl/macos/window_delegate.rs | 92 ++++--- 10 files changed, 426 insertions(+), 377 deletions(-) diff --git a/src/platform_impl/ios/ffi.rs b/src/platform_impl/ios/ffi.rs index 746a5d99..b684de50 100644 --- a/src/platform_impl/ios/ffi.rs +++ b/src/platform_impl/ios/ffi.rs @@ -70,6 +70,10 @@ impl From for Idiom { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct UIRectEdge(NSUInteger); +impl UIRectEdge { + pub(crate) const NONE: Self = Self(0); +} + unsafe impl Encode for UIRectEdge { const ENCODING: Encoding = NSUInteger::ENCODING; } diff --git a/src/platform_impl/ios/view.rs b/src/platform_impl/ios/view.rs index 199daba7..f52c11de 100644 --- a/src/platform_impl/ios/view.rs +++ b/src/platform_impl/ios/view.rs @@ -1,5 +1,8 @@ #![allow(clippy::unnecessary_cast)] +use std::cell::Cell; +use std::ptr::NonNull; +use objc2::declare::{Ivar, IvarDrop}; use objc2::foundation::{CGFloat, CGRect, MainThreadMarker, NSObject, NSSet}; use objc2::rc::{Id, Shared}; use objc2::runtime::Class; @@ -260,12 +263,16 @@ impl WinitView { } } +struct ViewControllerState { + prefers_status_bar_hidden: Cell, + prefers_home_indicator_auto_hidden: Cell, + supported_orientations: Cell, + preferred_screen_edges_deferring_system_gestures: Cell, +} + declare_class!( pub(crate) struct WinitViewController { - _prefers_status_bar_hidden: bool, - _prefers_home_indicator_auto_hidden: bool, - _supported_orientations: UIInterfaceOrientationMask, - _preferred_screen_edges_deferring_system_gestures: UIRectEdge, + state: IvarDrop>, } unsafe impl ClassType for WinitViewController { @@ -274,88 +281,87 @@ declare_class!( const NAME: &'static str = "WinitUIViewController"; } + unsafe impl WinitViewController { + #[sel(init)] + fn init(&mut self) -> Option> { + let this: Option<&mut Self> = unsafe { msg_send![super(self), init] }; + this.map(|this| { + // These are set in WinitViewController::new, it's just to set them + // to _something_. + Ivar::write( + &mut this.state, + Box::new(ViewControllerState { + prefers_status_bar_hidden: Cell::new(false), + prefers_home_indicator_auto_hidden: Cell::new(false), + supported_orientations: Cell::new(UIInterfaceOrientationMask::All), + preferred_screen_edges_deferring_system_gestures: Cell::new( + UIRectEdge::NONE, + ), + }), + ); + NonNull::from(this) + }) + } + } + unsafe impl WinitViewController { #[sel(shouldAutorotate)] fn should_autorotate(&self) -> bool { true } - } - unsafe impl WinitViewController { #[sel(prefersStatusBarHidden)] fn prefers_status_bar_hidden(&self) -> bool { - *self._prefers_status_bar_hidden - } - - #[sel(setPrefersStatusBarHidden:)] - fn set_prefers_status_bar_hidden(&mut self, val: bool) { - *self._prefers_status_bar_hidden = val; - self.setNeedsStatusBarAppearanceUpdate(); + self.state.prefers_status_bar_hidden.get() } #[sel(prefersHomeIndicatorAutoHidden)] fn prefers_home_indicator_auto_hidden(&self) -> bool { - *self._prefers_home_indicator_auto_hidden - } - - #[sel(setPrefersHomeIndicatorAutoHidden:)] - fn set_prefers_home_indicator_auto_hidden(&mut self, val: bool) { - *self._prefers_home_indicator_auto_hidden = val; - let os_capabilities = app_state::os_capabilities(); - if os_capabilities.home_indicator_hidden { - self.setNeedsUpdateOfHomeIndicatorAutoHidden(); - } else { - os_capabilities.home_indicator_hidden_err_msg("ignoring") - } + self.state.prefers_home_indicator_auto_hidden.get() } #[sel(supportedInterfaceOrientations)] fn supported_orientations(&self) -> UIInterfaceOrientationMask { - *self._supported_orientations - } - - #[sel(setSupportedInterfaceOrientations:)] - fn set_supported_orientations(&mut self, val: UIInterfaceOrientationMask) { - *self._supported_orientations = val; - UIViewController::attemptRotationToDeviceOrientation(); + self.state.supported_orientations.get() } #[sel(preferredScreenEdgesDeferringSystemGestures)] fn preferred_screen_edges_deferring_system_gestures(&self) -> UIRectEdge { - *self._preferred_screen_edges_deferring_system_gestures + self.state + .preferred_screen_edges_deferring_system_gestures + .get() } - - #[sel(setPreferredScreenEdgesDeferringSystemGestures:)] - fn set_preferred_screen_edges_deferring_system_gestures(&mut self, val: UIRectEdge) { - *self._preferred_screen_edges_deferring_system_gestures = val; - let os_capabilities = app_state::os_capabilities(); - if os_capabilities.defer_system_gestures { - self.setNeedsUpdateOfScreenEdgesDeferringSystemGestures(); - } else { - os_capabilities.defer_system_gestures_err_msg("ignoring") - } - } - } -); - -extern_methods!( - #[allow(non_snake_case)] - unsafe impl WinitViewController { - #[sel(setPrefersStatusBarHidden:)] - pub(crate) fn setPrefersStatusBarHidden(&self, flag: bool); - - #[sel(setSupportedInterfaceOrientations:)] - pub(crate) fn setSupportedInterfaceOrientations(&self, val: UIInterfaceOrientationMask); - - #[sel(setPrefersHomeIndicatorAutoHidden:)] - pub(crate) fn setPrefersHomeIndicatorAutoHidden(&self, val: bool); - - #[sel(setPreferredScreenEdgesDeferringSystemGestures:)] - pub(crate) fn setPreferredScreenEdgesDeferringSystemGestures(&self, val: UIRectEdge); } ); impl WinitViewController { + pub(crate) fn set_prefers_status_bar_hidden(&self, val: bool) { + self.state.prefers_status_bar_hidden.set(val); + self.setNeedsStatusBarAppearanceUpdate(); + } + + pub(crate) fn set_prefers_home_indicator_auto_hidden(&self, val: bool) { + self.state.prefers_home_indicator_auto_hidden.set(val); + let os_capabilities = app_state::os_capabilities(); + if os_capabilities.home_indicator_hidden { + self.setNeedsUpdateOfHomeIndicatorAutoHidden(); + } else { + os_capabilities.home_indicator_hidden_err_msg("ignoring") + } + } + + pub(crate) fn set_preferred_screen_edges_deferring_system_gestures(&self, val: UIRectEdge) { + self.state + .preferred_screen_edges_deferring_system_gestures + .set(val); + let os_capabilities = app_state::os_capabilities(); + if os_capabilities.defer_system_gestures { + self.setNeedsUpdateOfScreenEdgesDeferringSystemGestures(); + } else { + os_capabilities.defer_system_gestures_err_msg("ignoring") + } + } + pub(crate) fn set_supported_interface_orientations( &self, mtm: MainThreadMarker, @@ -378,7 +384,8 @@ impl WinitViewController { | UIInterfaceOrientationMask::PortraitUpsideDown } }; - self.setSupportedInterfaceOrientations(mask); + self.state.supported_orientations.set(mask); + UIViewController::attemptRotationToDeviceOrientation(); } pub(crate) fn new( @@ -390,13 +397,15 @@ impl WinitViewController { let this: Id = unsafe { msg_send_id![msg_send_id![Self::class(), alloc], init] }; - this.setPrefersStatusBarHidden(platform_attributes.prefers_status_bar_hidden); + this.set_prefers_status_bar_hidden(platform_attributes.prefers_status_bar_hidden); this.set_supported_interface_orientations(mtm, platform_attributes.valid_orientations); - this.setPrefersHomeIndicatorAutoHidden(platform_attributes.prefers_home_indicator_hidden); + this.set_prefers_home_indicator_auto_hidden( + platform_attributes.prefers_home_indicator_hidden, + ); - this.setPreferredScreenEdgesDeferringSystemGestures( + this.set_preferred_screen_edges_deferring_system_gestures( platform_attributes .preferred_screen_edges_deferring_system_gestures .into(), diff --git a/src/platform_impl/ios/window.rs b/src/platform_impl/ios/window.rs index 222c7a15..7e0cbdcb 100644 --- a/src/platform_impl/ios/window.rs +++ b/src/platform_impl/ios/window.rs @@ -23,7 +23,6 @@ use crate::{ platform_impl::platform::{ app_state, event_loop::{EventProxy, EventWrapper}, - ffi::UIRectEdge, monitor, EventLoopWindowTarget, Fullscreen, MonitorHandle, }, window::{ @@ -538,17 +537,16 @@ impl Inner { pub fn set_prefers_home_indicator_hidden(&self, hidden: bool) { self.view_controller - .setPrefersHomeIndicatorAutoHidden(hidden); + .set_prefers_home_indicator_auto_hidden(hidden); } pub fn set_preferred_screen_edges_deferring_system_gestures(&self, edges: ScreenEdge) { - let edges: UIRectEdge = edges.into(); self.view_controller - .setPreferredScreenEdgesDeferringSystemGestures(edges); + .set_preferred_screen_edges_deferring_system_gestures(edges.into()); } pub fn set_prefers_status_bar_hidden(&self, hidden: bool) { - self.view_controller.setPrefersStatusBarHidden(hidden); + self.view_controller.set_prefers_status_bar_hidden(hidden); } } diff --git a/src/platform_impl/macos/app_delegate.rs b/src/platform_impl/macos/app_delegate.rs index 0c9eff17..7a57bf8c 100644 --- a/src/platform_impl/macos/app_delegate.rs +++ b/src/platform_impl/macos/app_delegate.rs @@ -1,3 +1,5 @@ +use std::ptr::NonNull; + use objc2::foundation::NSObject; use objc2::rc::{Id, Shared}; use objc2::runtime::Object; @@ -21,18 +23,18 @@ declare_class!( unsafe impl ApplicationDelegate { #[sel(initWithActivationPolicy:defaultMenu:activateIgnoringOtherApps:)] - fn init( + unsafe fn init( &mut self, activation_policy: NSApplicationActivationPolicy, default_menu: bool, activate_ignoring_other_apps: bool, - ) -> Option<&mut Self> { + ) -> Option> { let this: Option<&mut Self> = unsafe { msg_send![super(self), init] }; this.map(|this| { *this.activation_policy = activation_policy; *this.default_menu = default_menu; *this.activate_ignoring_other_apps = activate_ignoring_other_apps; - this + NonNull::from(this) }) } diff --git a/src/platform_impl/macos/appkit/responder.rs b/src/platform_impl/macos/appkit/responder.rs index a1ae1877..8aa49a3d 100644 --- a/src/platform_impl/macos/appkit/responder.rs +++ b/src/platform_impl/macos/appkit/responder.rs @@ -17,8 +17,7 @@ extern_class!( extern_methods!( unsafe impl NSResponder { - // TODO: Allow "immutably" on main thread #[sel(interpretKeyEvents:)] - pub unsafe fn interpretKeyEvents(&mut self, events: &NSArray); + pub unsafe fn interpretKeyEvents(&self, events: &NSArray); } ); diff --git a/src/platform_impl/macos/appkit/view.rs b/src/platform_impl/macos/appkit/view.rs index b72b712d..f883c3e4 100644 --- a/src/platform_impl/macos/appkit/view.rs +++ b/src/platform_impl/macos/appkit/view.rs @@ -66,7 +66,7 @@ extern_methods!( pub fn setWantsLayer(&self, wants_layer: bool); #[sel(setPostsFrameChangedNotifications:)] - pub fn setPostsFrameChangedNotifications(&mut self, value: bool); + pub fn setPostsFrameChangedNotifications(&self, value: bool); #[sel(removeTrackingRect:)] pub fn removeTrackingRect(&self, tag: NSTrackingRectTag); diff --git a/src/platform_impl/macos/util/async.rs b/src/platform_impl/macos/util/async.rs index b8e781d1..494e2b6c 100644 --- a/src/platform_impl/macos/util/async.rs +++ b/src/platform_impl/macos/util/async.rs @@ -2,7 +2,7 @@ use std::ops::Deref; use dispatch::Queue; use objc2::foundation::{is_main_thread, CGFloat, NSPoint, NSSize, NSString}; -use objc2::rc::{autoreleasepool, Id}; +use objc2::rc::autoreleasepool; use crate::{ dpi::{LogicalPosition, LogicalSize}, @@ -209,8 +209,7 @@ pub(crate) fn set_ime_cursor_area_sync( ) { let window = MainThreadSafe(window); run_on_main(move || { - // TODO(madsmtm): Remove the need for this - unsafe { Id::from_shared(window.view()) }.set_ime_cursor_area(logical_spot, size); + window.view().set_ime_cursor_area(logical_spot, size); }); } diff --git a/src/platform_impl/macos/view.rs b/src/platform_impl/macos/view.rs index d5375b26..d1fd18ce 100644 --- a/src/platform_impl/macos/view.rs +++ b/src/platform_impl/macos/view.rs @@ -1,12 +1,11 @@ #![allow(clippy::unnecessary_cast)] -use std::{ - boxed::Box, - collections::{HashMap, VecDeque}, - os::raw::*, - ptr, str, - sync::Mutex, -}; +use std::boxed::Box; +use std::cell::{Cell, RefCell}; +use std::collections::{HashMap, VecDeque}; +use std::os::raw::*; +use std::ptr::{self, NonNull}; +use std::str; use objc2::declare::{Ivar, IvarDrop}; use objc2::foundation::{ @@ -41,9 +40,9 @@ use crate::{ }; #[derive(Debug)] -pub struct CursorState { - pub visible: bool, - pub(super) cursor: Id, +struct CursorState { + visible: bool, + cursor: Id, } impl Default for CursorState { @@ -55,8 +54,9 @@ impl Default for CursorState { } } -#[derive(Debug, Eq, PartialEq, Clone, Copy)] +#[derive(Debug, Eq, PartialEq, Clone, Copy, Default)] enum ImeState { + #[default] /// The IME events are disabled, so only `ReceivedCharacter` is being sent to the user. Disabled, @@ -88,7 +88,7 @@ impl ModLocationMask { } } -pub fn key_to_modifier(key: &Key) -> ModifiersState { +fn key_to_modifier(key: &Key) -> ModifiersState { match key { Key::Alt => ModifiersState::ALT, Key::Control => ModifiersState::CONTROL, @@ -118,26 +118,28 @@ fn get_left_modifier_code(key: &Key) -> KeyCode { } } -#[derive(Debug)] -pub(super) struct ViewState { - pub cursor_state: Mutex, - ime_position: LogicalPosition, - ime_size: LogicalSize, - pub(super) modifiers: Modifiers, - phys_modifiers: HashMap, - tracking_rect: Option, - // phys_modifiers: HashSet, - ime_state: ImeState, - input_source: String, +#[derive(Debug, Default)] +struct ViewState { + cursor_state: RefCell, + ime_position: Cell>, + ime_size: Cell>, + modifiers: Cell, + phys_modifiers: RefCell>, + tracking_rect: Cell>, + ime_state: Cell, + input_source: RefCell, /// True iff the application wants IME events. /// /// Can be set using `set_ime_allowed` - ime_allowed: bool, + ime_allowed: Cell, /// True if the current key event should be forwarded /// to the application, even during IME - forward_key_to_app: bool, + forward_key_to_app: Cell, + + marked_text: RefCell>, + accepts_first_mouse: bool, } declare_class!( @@ -146,9 +148,7 @@ declare_class!( pub(super) struct WinitView { // Weak reference because the window keeps a strong reference to the view _ns_window: IvarDrop>>, - pub(super) state: IvarDrop>, - marked_text: IvarDrop>, - accepts_first_mouse: bool, + state: IvarDrop>, } unsafe impl ClassType for WinitView { @@ -158,24 +158,16 @@ declare_class!( unsafe impl WinitView { #[sel(initWithId:acceptsFirstMouse:)] - fn init_with_id( + unsafe fn init_with_id( &mut self, window: &WinitWindow, accepts_first_mouse: bool, - ) -> Option<&mut Self> { + ) -> Option> { let this: Option<&mut Self> = unsafe { msg_send![super(self), init] }; this.map(|this| { let state = ViewState { - cursor_state: Default::default(), - ime_position: LogicalPosition::new(0.0, 0.0), - ime_size: Default::default(), - modifiers: Default::default(), - phys_modifiers: Default::default(), - tracking_rect: None, - ime_state: ImeState::Disabled, - input_source: String::new(), - ime_allowed: false, - forward_key_to_app: false, + accepts_first_mouse, + ..Default::default() }; Ivar::write( @@ -183,8 +175,6 @@ declare_class!( Box::new(WeakId::new(&window.retain())), ); Ivar::write(&mut this.state, Box::new(state)); - Ivar::write(&mut this.marked_text, NSMutableAttributedString::new()); - Ivar::write(&mut this.accepts_first_mouse, accepts_first_mouse); this.setPostsFrameChangedNotifications(true); @@ -204,15 +194,15 @@ declare_class!( ]; } - this.state.input_source = this.current_input_source(); - this + *this.state.input_source.borrow_mut() = this.current_input_source(); + NonNull::from(this) }) } } unsafe impl WinitView { #[sel(viewDidMoveToWindow)] - fn view_did_move_to_window(&mut self) { + fn view_did_move_to_window(&self) { trace_scope!("viewDidMoveToWindow"); if let Some(tracking_rect) = self.state.tracking_rect.take() { self.removeTrackingRect(tracking_rect); @@ -220,11 +210,11 @@ declare_class!( let rect = self.visibleRect(); let tracking_rect = self.add_tracking_rect(rect, false); - self.state.tracking_rect = Some(tracking_rect); + self.state.tracking_rect.set(Some(tracking_rect)); } #[sel(frameDidChange:)] - fn frame_did_change(&mut self, _event: &NSEvent) { + fn frame_did_change(&self, _event: &NSEvent) { trace_scope!("frameDidChange:"); if let Some(tracking_rect) = self.state.tracking_rect.take() { self.removeTrackingRect(tracking_rect); @@ -232,7 +222,7 @@ declare_class!( let rect = self.visibleRect(); let tracking_rect = self.add_tracking_rect(rect, false); - self.state.tracking_rect = Some(tracking_rect); + self.state.tracking_rect.set(Some(tracking_rect)); // Emit resize event here rather than from windowDidResize because: // 1. When a new window is created as a tab, the frame size may change without a window resize occurring. @@ -243,7 +233,7 @@ declare_class!( } #[sel(drawRect:)] - fn draw_rect(&mut self, rect: NSRect) { + 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()`. @@ -276,7 +266,7 @@ declare_class!( fn reset_cursor_rects(&self) { trace_scope!("resetCursorRects"); let bounds = self.bounds(); - let cursor_state = self.state.cursor_state.lock().unwrap(); + let cursor_state = self.state.cursor_state.borrow(); // We correctly invoke `addCursorRect` only from inside `resetCursorRects` if cursor_state.visible { self.addCursorRect(bounds, &cursor_state.cursor); @@ -290,13 +280,13 @@ declare_class!( #[sel(hasMarkedText)] fn has_marked_text(&self) -> bool { trace_scope!("hasMarkedText"); - self.marked_text.len_utf16() > 0 + self.state.marked_text.borrow().len_utf16() > 0 } #[sel(markedRange)] fn marked_range(&self) -> NSRange { trace_scope!("markedRange"); - let length = self.marked_text.len_utf16(); + let length = self.state.marked_text.borrow().len_utf16(); if length > 0 { NSRange::new(0, length) } else { @@ -312,7 +302,7 @@ declare_class!( #[sel(setMarkedText:selectedRange:replacementRange:)] fn set_marked_text( - &mut self, + &self, string: &NSObject, _selected_range: NSRange, _replacement_range: NSRange, @@ -339,19 +329,19 @@ declare_class!( }; // Update marked text. - *self.marked_text = marked_text; + *self.state.marked_text.borrow_mut() = marked_text; // Notify IME is active if application still doesn't know it. - if self.state.ime_state == ImeState::Disabled { - self.state.input_source = self.current_input_source(); + if self.state.ime_state.get() == ImeState::Disabled { + *self.state.input_source.borrow_mut() = self.current_input_source(); self.queue_event(WindowEvent::Ime(Ime::Enabled)); } if self.hasMarkedText() { - self.state.ime_state = ImeState::Preedit; + self.state.ime_state.set(ImeState::Preedit); } else { // In case the preedit was cleared, set IME into the Ground state. - self.state.ime_state = ImeState::Ground; + self.state.ime_state.set(ImeState::Ground); } // Empty string basically means that there's no preedit, so indicate that by sending @@ -367,9 +357,9 @@ declare_class!( } #[sel(unmarkText)] - fn unmark_text(&mut self) { + fn unmark_text(&self) { trace_scope!("unmarkText"); - *self.marked_text = NSMutableAttributedString::new(); + *self.state.marked_text.borrow_mut() = NSMutableAttributedString::new(); let input_context = self.inputContext().expect("input context"); input_context.discardMarkedText(); @@ -377,7 +367,7 @@ declare_class!( self.queue_event(WindowEvent::Ime(Ime::Preedit(String::new(), None))); if self.is_ime_enabled() { // Leave the Preedit self.state - self.state.ime_state = ImeState::Ground; + self.state.ime_state.set(ImeState::Ground); } else { warn!("Expected to have IME enabled when receiving unmarkText"); } @@ -416,14 +406,14 @@ declare_class!( let content_rect = window.contentRectForFrameRect(window.frame()); let base_x = content_rect.origin.x as f64; let base_y = (content_rect.origin.y + content_rect.size.height) as f64; - let x = base_x + self.state.ime_position.x; - let y = base_y - self.state.ime_position.y; - let LogicalSize { width, height } = self.state.ime_size; + let x = base_x + self.state.ime_position.get().x; + let y = base_y - self.state.ime_position.get().y; + let LogicalSize { width, height } = self.state.ime_size.get(); NSRect::new(NSPoint::new(x as _, y as _), NSSize::new(width, height)) } #[sel(insertText:replacementRange:)] - fn insert_text(&mut self, string: &NSObject, _replacement_range: NSRange) { + fn insert_text(&self, string: &NSObject, _replacement_range: NSRange) { trace_scope!("insertText:replacementRange:"); // SAFETY: This method is guaranteed to get either a `NSString` or a `NSAttributedString`. @@ -443,45 +433,49 @@ declare_class!( if self.hasMarkedText() && self.is_ime_enabled() && !is_control { self.queue_event(WindowEvent::Ime(Ime::Preedit(String::new(), None))); self.queue_event(WindowEvent::Ime(Ime::Commit(string))); - self.state.ime_state = ImeState::Commited; + self.state.ime_state.set(ImeState::Commited); } } // Basically, we're sent this message whenever a keyboard event that doesn't generate a "human // readable" character happens, i.e. newlines, tabs, and Ctrl+C. #[sel(doCommandBySelector:)] - fn do_command_by_selector(&mut self, _command: Sel) { + fn do_command_by_selector(&self, _command: Sel) { trace_scope!("doCommandBySelector:"); // We shouldn't forward any character from just commited text, since we'll end up sending // it twice with some IMEs like Korean one. We'll also always send `Enter` in that case, // which is not desired given it was used to confirm IME input. - if self.state.ime_state == ImeState::Commited { + if self.state.ime_state.get() == ImeState::Commited { return; } - self.state.forward_key_to_app = true; + self.state.forward_key_to_app.set(true); - if self.hasMarkedText() && self.state.ime_state == ImeState::Preedit { + if self.hasMarkedText() && self.state.ime_state.get() == ImeState::Preedit { // Leave preedit so that we also report the key-up for this key. - self.state.ime_state = ImeState::Ground; + self.state.ime_state.set(ImeState::Ground); } } } unsafe impl WinitView { #[sel(keyDown:)] - fn key_down(&mut self, event: &NSEvent) { + fn key_down(&self, event: &NSEvent) { trace_scope!("keyDown:"); - let input_source = self.current_input_source(); - if self.state.input_source != input_source && self.is_ime_enabled() { - self.state.ime_state = ImeState::Disabled; - self.state.input_source = input_source; - self.queue_event(WindowEvent::Ime(Ime::Disabled)); + { + let mut prev_input_source = self.state.input_source.borrow_mut(); + let current_input_source = self.current_input_source(); + if *prev_input_source != current_input_source && self.is_ime_enabled() { + *prev_input_source = current_input_source; + drop(prev_input_source); + self.state.ime_state.set(ImeState::Disabled); + self.queue_event(WindowEvent::Ime(Ime::Disabled)); + } } // Get the characters from the event. - let old_ime_state = self.state.ime_state; - self.state.forward_key_to_app = false; + let old_ime_state = self.state.ime_state.get(); + self.state.forward_key_to_app.set(false); let event = replace_event(event, self.window().option_as_alt()); // The `interpretKeyEvents` function might call @@ -490,31 +484,31 @@ declare_class!( // we must send the `KeyboardInput` event during IME if it triggered // `doCommandBySelector`. (doCommandBySelector means that the keyboard input // is not handled by IME and should be handled by the application) - if self.state.ime_allowed { + if self.state.ime_allowed.get() { let events_for_nsview = NSArray::from_slice(&[event.copy()]); unsafe { self.interpretKeyEvents(&events_for_nsview) }; // If the text was commited we must treat the next keyboard event as IME related. - if self.state.ime_state == ImeState::Commited { + if self.state.ime_state.get() == ImeState::Commited { // Remove any marked text, so normal input can continue. - *self.marked_text = NSMutableAttributedString::new(); + *self.state.marked_text.borrow_mut() = NSMutableAttributedString::new(); } } self.update_modifiers(&event, false); - let had_ime_input = match self.state.ime_state { + let had_ime_input = match self.state.ime_state.get() { ImeState::Commited => { // Allow normal input after the commit. - self.state.ime_state = ImeState::Ground; + self.state.ime_state.set(ImeState::Ground); true } ImeState::Preedit => true, // `key_down` could result in preedit clear, so compare old and current state. - _ => old_ime_state != self.state.ime_state, + _ => old_ime_state != self.state.ime_state.get(), }; - if !had_ime_input || self.state.forward_key_to_app { + if !had_ime_input || self.state.forward_key_to_app.get() { let key_event = create_key_event(&event, true, event.is_a_repeat(), None); self.queue_event(WindowEvent::KeyboardInput { device_id: DEVICE_ID, @@ -525,14 +519,17 @@ declare_class!( } #[sel(keyUp:)] - fn key_up(&mut self, event: &NSEvent) { + fn key_up(&self, event: &NSEvent) { trace_scope!("keyUp:"); let event = replace_event(event, self.window().option_as_alt()); self.update_modifiers(&event, false); // We want to send keyboard input when we are currently in the ground state. - if matches!(self.state.ime_state, ImeState::Ground | ImeState::Disabled) { + if matches!( + self.state.ime_state.get(), + ImeState::Ground | ImeState::Disabled + ) { self.queue_event(WindowEvent::KeyboardInput { device_id: DEVICE_ID, event: create_key_event(&event, false, false, None), @@ -542,7 +539,7 @@ declare_class!( } #[sel(flagsChanged:)] - fn flags_changed(&mut self, ns_event: &NSEvent) { + fn flags_changed(&self, ns_event: &NSEvent) { trace_scope!("flagsChanged:"); self.update_modifiers(ns_event, true); @@ -573,7 +570,7 @@ declare_class!( // Allows us to receive Cmd-. (the shortcut for closing a dialog) // https://bugs.eclipse.org/bugs/show_bug.cgi?id=300620#c6 #[sel(cancelOperation:)] - fn cancel_operation(&mut self, _sender: *const Object) { + fn cancel_operation(&self, _sender: *const Object) { trace_scope!("cancelOperation:"); let event = NSApp() @@ -591,42 +588,42 @@ declare_class!( } #[sel(mouseDown:)] - fn mouse_down(&mut self, event: &NSEvent) { + fn mouse_down(&self, event: &NSEvent) { trace_scope!("mouseDown:"); self.mouse_motion(event); self.mouse_click(event, ElementState::Pressed); } #[sel(mouseUp:)] - fn mouse_up(&mut self, event: &NSEvent) { + fn mouse_up(&self, event: &NSEvent) { trace_scope!("mouseUp:"); self.mouse_motion(event); self.mouse_click(event, ElementState::Released); } #[sel(rightMouseDown:)] - fn right_mouse_down(&mut self, event: &NSEvent) { + fn right_mouse_down(&self, event: &NSEvent) { trace_scope!("rightMouseDown:"); self.mouse_motion(event); self.mouse_click(event, ElementState::Pressed); } #[sel(rightMouseUp:)] - fn right_mouse_up(&mut self, event: &NSEvent) { + fn right_mouse_up(&self, event: &NSEvent) { trace_scope!("rightMouseUp:"); self.mouse_motion(event); self.mouse_click(event, ElementState::Released); } #[sel(otherMouseDown:)] - fn other_mouse_down(&mut self, event: &NSEvent) { + fn other_mouse_down(&self, event: &NSEvent) { trace_scope!("otherMouseDown:"); self.mouse_motion(event); self.mouse_click(event, ElementState::Pressed); } #[sel(otherMouseUp:)] - fn other_mouse_up(&mut self, event: &NSEvent) { + fn other_mouse_up(&self, event: &NSEvent) { trace_scope!("otherMouseUp:"); self.mouse_motion(event); self.mouse_click(event, ElementState::Released); @@ -635,22 +632,22 @@ declare_class!( // No tracing on these because that would be overly verbose #[sel(mouseMoved:)] - fn mouse_moved(&mut self, event: &NSEvent) { + fn mouse_moved(&self, event: &NSEvent) { self.mouse_motion(event); } #[sel(mouseDragged:)] - fn mouse_dragged(&mut self, event: &NSEvent) { + fn mouse_dragged(&self, event: &NSEvent) { self.mouse_motion(event); } #[sel(rightMouseDragged:)] - fn right_mouse_dragged(&mut self, event: &NSEvent) { + fn right_mouse_dragged(&self, event: &NSEvent) { self.mouse_motion(event); } #[sel(otherMouseDragged:)] - fn other_mouse_dragged(&mut self, event: &NSEvent) { + fn other_mouse_dragged(&self, event: &NSEvent) { self.mouse_motion(event); } @@ -672,7 +669,7 @@ declare_class!( } #[sel(scrollWheel:)] - fn scroll_wheel(&mut self, event: &NSEvent) { + fn scroll_wheel(&self, event: &NSEvent) { trace_scope!("scrollWheel:"); self.mouse_motion(event); @@ -767,7 +764,7 @@ declare_class!( } #[sel(pressureChangeWithEvent:)] - fn pressure_change_with_event(&mut self, event: &NSEvent) { + fn pressure_change_with_event(&self, event: &NSEvent) { trace_scope!("pressureChangeWithEvent:"); self.mouse_motion(event); @@ -791,7 +788,7 @@ declare_class!( #[sel(acceptsFirstMouse:)] fn accepts_first_mouse(&self, _event: &NSEvent) -> bool { trace_scope!("acceptsFirstMouse:"); - *self.accepts_first_mouse + self.state.accepts_first_mouse } } ); @@ -841,7 +838,7 @@ impl WinitView { } fn is_ime_enabled(&self) -> bool { - !matches!(self.state.ime_state, ImeState::Disabled) + !matches!(self.state.ime_state.get(), ImeState::Disabled) } fn current_input_source(&self) -> String { @@ -852,43 +849,68 @@ impl WinitView { .unwrap_or_else(String::new) } - pub(super) fn set_ime_allowed(&mut self, ime_allowed: bool) { - if self.state.ime_allowed == ime_allowed { + pub(super) fn set_cursor_icon(&self, icon: Id) { + let mut cursor_state = self.state.cursor_state.borrow_mut(); + cursor_state.cursor = icon; + } + + /// Set whether the cursor should be visible or not. + /// + /// Returns whether the state changed. + pub(super) fn set_cursor_visible(&self, visible: bool) -> bool { + let mut cursor_state = self.state.cursor_state.borrow_mut(); + if visible != cursor_state.visible { + cursor_state.visible = visible; + true + } else { + false + } + } + + pub(super) fn set_ime_allowed(&self, ime_allowed: bool) { + if self.state.ime_allowed.get() == ime_allowed { return; } - self.state.ime_allowed = ime_allowed; - if self.state.ime_allowed { + self.state.ime_allowed.set(ime_allowed); + if self.state.ime_allowed.get() { return; } // Clear markedText - *self.marked_text = NSMutableAttributedString::new(); + *self.state.marked_text.borrow_mut() = NSMutableAttributedString::new(); - if self.state.ime_state != ImeState::Disabled { - self.state.ime_state = ImeState::Disabled; + if self.state.ime_state.get() != ImeState::Disabled { + self.state.ime_state.set(ImeState::Disabled); self.queue_event(WindowEvent::Ime(Ime::Disabled)); } } pub(super) fn set_ime_cursor_area( - &mut self, + &self, position: LogicalPosition, size: LogicalSize, ) { - self.state.ime_position = position; - self.state.ime_size = size; + self.state.ime_position.set(position); + self.state.ime_size.set(size); let input_context = self.inputContext().expect("input context"); input_context.invalidateCharacterCoordinates(); } - // Update `state.modifiers` if `event` has something different - fn update_modifiers(&mut self, ns_event: &NSEvent, is_flags_changed_event: bool) { + /// Reset modifiers and emit a synthetic ModifiersChanged event if deemed necessary. + pub(super) fn reset_modifiers(&self) { + if !self.state.modifiers.get().state().is_empty() { + self.state.modifiers.set(Modifiers::default()); + self.queue_event(WindowEvent::ModifiersChanged(self.state.modifiers.get())); + } + } + + /// Update modifiers if `event` has something different + fn update_modifiers(&self, ns_event: &NSEvent, is_flags_changed_event: bool) { use ElementState::{Pressed, Released}; let current_modifiers = event_mods(ns_event); - let prev_modifiers = self.state.modifiers; - - self.state.modifiers = current_modifiers; + let prev_modifiers = self.state.modifiers.get(); + self.state.modifiers.set(current_modifiers); // This function was called form the flagsChanged event, which is triggered // when the user presses/releases a modifier even if the same kind of modifier @@ -907,9 +929,8 @@ impl WinitView { event.location = code_to_location(keycode); let location_mask = ModLocationMask::from_location(event.location); - let phys_mod = self - .state - .phys_modifiers + let mut phys_mod_state = self.state.phys_modifiers.borrow_mut(); + let phys_mod = phys_mod_state .entry(key) .or_insert(ModLocationMask::empty()); @@ -975,6 +996,8 @@ impl WinitView { }); } + drop(phys_mod_state); + for event in events { self.queue_event(event); } @@ -984,10 +1007,10 @@ impl WinitView { return; } - self.queue_event(WindowEvent::ModifiersChanged(self.state.modifiers)); + self.queue_event(WindowEvent::ModifiersChanged(self.state.modifiers.get())); } - fn mouse_click(&mut self, event: &NSEvent, button_state: ElementState) { + fn mouse_click(&self, event: &NSEvent, button_state: ElementState) { let button = mouse_button(event); self.update_modifiers(event, false); @@ -999,7 +1022,7 @@ impl WinitView { }); } - fn mouse_motion(&mut self, event: &NSEvent) { + fn mouse_motion(&self, event: &NSEvent) { let window_point = event.locationInWindow(); let view_point = self.convertPoint_fromView(window_point, None); let view_rect = self.frame(); diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index 7e356eec..1df683fc 100644 --- a/src/platform_impl/macos/window.rs +++ b/src/platform_impl/macos/window.rs @@ -1,14 +1,11 @@ #![allow(clippy::unnecessary_cast)] -use std::{ - collections::VecDeque, - f64, ops, - os::raw::c_void, - sync::{ - atomic::{AtomicBool, Ordering}, - Mutex, MutexGuard, - }, -}; +use std::collections::VecDeque; +use std::f64; +use std::ops; +use std::os::raw::c_void; +use std::ptr::NonNull; +use std::sync::{Mutex, MutexGuard}; use raw_window_handle::{ AppKitDisplayHandle, AppKitWindowHandle, RawDisplayHandle, RawWindowHandle, @@ -43,7 +40,7 @@ use objc2::foundation::{ is_main_thread, CGFloat, NSArray, NSCopying, NSInteger, NSObject, NSPoint, NSRect, NSSize, NSString, }; -use objc2::rc::{autoreleasepool, Id, Owned, Shared}; +use objc2::rc::{autoreleasepool, Id, Shared}; use objc2::{declare_class, msg_send, msg_send_id, sel, ClassType}; use super::appkit::{ @@ -110,9 +107,7 @@ declare_class!( #[derive(Debug)] pub(crate) struct WinitWindow { // TODO: Fix unnecessary boxing here - // SAFETY: These are initialized in WinitWindow::new, right after it is created. shared_state: IvarDrop>>, - decorations: IvarDrop>, } unsafe impl ClassType for WinitWindow { @@ -120,6 +115,40 @@ declare_class!( type Super = NSWindow; } + unsafe impl WinitWindow { + #[sel(initWithContentRect:styleMask:state:)] + unsafe fn init( + &mut self, + frame: NSRect, + mask: NSWindowStyleMask, + state: *mut c_void, + ) -> Option> { + let this: Option<&mut Self> = unsafe { + msg_send![ + super(self), + initWithContentRect: frame, + styleMask: mask, + backing: NSBackingStoreType::NSBackingStoreBuffered, + defer: false, + ] + }; + + this.map(|this| { + // SAFETY: The pointer originally came from `Box::into_raw`. + Ivar::write(&mut this.shared_state, unsafe { + Box::from_raw(state as *mut Mutex) + }); + + // It is imperative to correct memory management that we + // disable the extra release that would otherwise happen when + // calling `clone` on the window. + this.setReleasedWhenClosed(false); + + NonNull::from(this) + }) + } + } + unsafe impl WinitWindow { #[sel(canBecomeMainWindow)] fn can_become_main_window(&self) -> bool { @@ -164,6 +193,8 @@ pub struct SharedState { pub(crate) resize_increments: NSSize, /// The state of the `Option` as `Alt`. pub(crate) option_as_alt: OptionAsAlt, + + decorations: bool, } impl SharedState { @@ -292,91 +323,83 @@ impl WinitWindow { masks |= NSWindowStyleMask::NSFullSizeContentViewWindowMask; } - let this: Option> = unsafe { + let state = SharedState { + resizable: attrs.resizable, + maximized: attrs.maximized, + decorations: attrs.decorations, + ..Default::default() + }; + + // Pass the state through FFI to the method declared on the class + let state_ptr: *mut c_void = Box::into_raw(Box::new(Mutex::new(state))).cast(); + let this: Option> = unsafe { msg_send_id![ msg_send_id![WinitWindow::class(), alloc], initWithContentRect: frame, styleMask: masks, - backing: NSBackingStoreType::NSBackingStoreBuffered, - defer: false, + state: state_ptr, ] }; + let this = this?; - this.map(|mut this| { - let resize_increments = match attrs - .resize_increments - .map(|i| i.to_logical::(this.scale_factor())) - { - Some(LogicalSize { width, height }) if width >= 1. && height >= 1. => { - NSSize::new(width, height) - } - _ => NSSize::new(1., 1.), - }; - - // Properly initialize the window's variables - // - // Ideally this should be done in an `init` method, - // but for convenience we do it here instead. - let state = SharedState { - resizable: attrs.resizable, - maximized: attrs.maximized, - resize_increments, - ..Default::default() - }; - Ivar::write(&mut this.shared_state, Box::new(Mutex::new(state))); - Ivar::write( - &mut this.decorations, - Box::new(AtomicBool::new(attrs.decorations)), - ); - - this.setReleasedWhenClosed(false); - this.setTitle(&NSString::from_str(&attrs.title)); - this.setAcceptsMouseMovedEvents(true); - - if attrs.content_protected { - this.setSharingType(NSWindowSharingType::NSWindowSharingNone); + let resize_increments = match attrs + .resize_increments + .map(|i| i.to_logical::(this.scale_factor())) + { + Some(LogicalSize { width, height }) if width >= 1. && height >= 1. => { + NSSize::new(width, height) } + _ => NSSize::new(1., 1.), + }; - if pl_attrs.titlebar_transparent { - this.setTitlebarAppearsTransparent(true); - } - if pl_attrs.title_hidden { - this.setTitleVisibility(NSWindowTitleVisibility::Hidden); - } - if pl_attrs.titlebar_buttons_hidden { - for titlebar_button in &[ - #[allow(deprecated)] - NSWindowButton::FullScreen, - NSWindowButton::Miniaturize, - NSWindowButton::Close, - NSWindowButton::Zoom, - ] { - if let Some(button) = this.standardWindowButton(*titlebar_button) { - button.setHidden(true); - } + this.lock_shared_state("init").resize_increments = resize_increments; + + this.setTitle(&NSString::from_str(&attrs.title)); + this.setAcceptsMouseMovedEvents(true); + + if attrs.content_protected { + this.setSharingType(NSWindowSharingType::NSWindowSharingNone); + } + + if pl_attrs.titlebar_transparent { + this.setTitlebarAppearsTransparent(true); + } + if pl_attrs.title_hidden { + this.setTitleVisibility(NSWindowTitleVisibility::Hidden); + } + if pl_attrs.titlebar_buttons_hidden { + for titlebar_button in &[ + #[allow(deprecated)] + NSWindowButton::FullScreen, + NSWindowButton::Miniaturize, + NSWindowButton::Close, + NSWindowButton::Zoom, + ] { + if let Some(button) = this.standardWindowButton(*titlebar_button) { + button.setHidden(true); } } - if pl_attrs.movable_by_window_background { - this.setMovableByWindowBackground(true); - } + } + if pl_attrs.movable_by_window_background { + this.setMovableByWindowBackground(true); + } - if !attrs.enabled_buttons.contains(WindowButtons::MAXIMIZE) { - if let Some(button) = this.standardWindowButton(NSWindowButton::Zoom) { - button.setEnabled(false); - } + if !attrs.enabled_buttons.contains(WindowButtons::MAXIMIZE) { + if let Some(button) = this.standardWindowButton(NSWindowButton::Zoom) { + button.setEnabled(false); } + } - if !pl_attrs.has_shadow { - this.setHasShadow(false); - } - if attrs.position.is_none() { - this.center(); - } + if !pl_attrs.has_shadow { + this.setHasShadow(false); + } + if attrs.position.is_none() { + this.center(); + } - this.set_option_as_alt(pl_attrs.option_as_alt); + this.set_option_as_alt(pl_attrs.option_as_alt); - Id::into_shared(this) - }) + Some(this) }) .ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?; @@ -753,9 +776,7 @@ impl WinitWindow { pub fn set_cursor_icon(&self, icon: CursorIcon) { let view = self.view(); - let mut cursor_state = view.state.cursor_state.lock().unwrap(); - cursor_state.cursor = NSCursor::from_icon(icon); - drop(cursor_state); + view.set_cursor_icon(NSCursor::from_icon(icon)); self.invalidateCursorRectsForView(&view); } @@ -777,10 +798,8 @@ impl WinitWindow { #[inline] pub fn set_cursor_visible(&self, visible: bool) { let view = self.view(); - let mut cursor_state = view.state.cursor_state.lock().unwrap(); - if visible != cursor_state.visible { - cursor_state.visible = visible; - drop(cursor_state); + let state_changed = view.set_cursor_visible(visible); + if state_changed { self.invalidateCursorRectsForView(&view); } } @@ -1097,45 +1116,44 @@ impl WinitWindow { #[inline] pub fn set_decorations(&self, decorations: bool) { - if decorations != self.decorations.load(Ordering::Acquire) { - self.decorations.store(decorations, Ordering::Release); - - let (fullscreen, resizable) = { - let shared_state_lock = self.lock_shared_state("set_decorations"); - ( - shared_state_lock.fullscreen.is_some(), - shared_state_lock.resizable, - ) - }; - - // If we're in fullscreen mode, we wait to apply decoration changes - // until we're in `window_did_exit_fullscreen`. - if fullscreen { - return; - } - - let new_mask = { - let mut new_mask = if decorations { - NSWindowStyleMask::NSClosableWindowMask - | NSWindowStyleMask::NSMiniaturizableWindowMask - | NSWindowStyleMask::NSResizableWindowMask - | NSWindowStyleMask::NSTitledWindowMask - } else { - NSWindowStyleMask::NSBorderlessWindowMask - | NSWindowStyleMask::NSResizableWindowMask - }; - if !resizable { - new_mask &= !NSWindowStyleMask::NSResizableWindowMask; - } - new_mask - }; - self.set_style_mask_sync(new_mask); + let mut shared_state_lock = self.lock_shared_state("set_decorations"); + if decorations == shared_state_lock.decorations { + return; } + + shared_state_lock.decorations = decorations; + + let fullscreen = shared_state_lock.fullscreen.is_some(); + let resizable = shared_state_lock.resizable; + + drop(shared_state_lock); + + // If we're in fullscreen mode, we wait to apply decoration changes + // until we're in `window_did_exit_fullscreen`. + if fullscreen { + return; + } + + let new_mask = { + let mut new_mask = if decorations { + NSWindowStyleMask::NSClosableWindowMask + | NSWindowStyleMask::NSMiniaturizableWindowMask + | NSWindowStyleMask::NSResizableWindowMask + | NSWindowStyleMask::NSTitledWindowMask + } else { + NSWindowStyleMask::NSBorderlessWindowMask | NSWindowStyleMask::NSResizableWindowMask + }; + if !resizable { + new_mask &= !NSWindowStyleMask::NSResizableWindowMask; + } + new_mask + }; + self.set_style_mask_sync(new_mask); } #[inline] pub fn is_decorated(&self) -> bool { - self.decorations.load(Ordering::Acquire) + self.lock_shared_state("is_decorated").decorations } #[inline] @@ -1170,8 +1188,7 @@ impl WinitWindow { #[inline] pub fn set_ime_allowed(&self, allowed: bool) { - // TODO(madsmtm): Remove the need for this - unsafe { Id::from_shared(self.view()) }.set_ime_allowed(allowed); + self.view().set_ime_allowed(allowed); } #[inline] @@ -1385,12 +1402,12 @@ impl WindowExtMacOS for WinitWindow { } fn set_option_as_alt(&self, option_as_alt: OptionAsAlt) { - let mut shared_state_lock = self.shared_state.lock().unwrap(); + let mut shared_state_lock = self.lock_shared_state("set_option_as_alt"); shared_state_lock.option_as_alt = option_as_alt; } fn option_as_alt(&self) -> OptionAsAlt { - let shared_state_lock = self.shared_state.lock().unwrap(); + let shared_state_lock = self.lock_shared_state("option_as_alt"); shared_state_lock.option_as_alt } } diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs index ff3bbc78..0b50de82 100644 --- a/src/platform_impl/macos/window_delegate.rs +++ b/src/platform_impl/macos/window_delegate.rs @@ -1,6 +1,6 @@ #![allow(clippy::unnecessary_cast)] - -use std::ptr; +use std::cell::Cell; +use std::ptr::{self, NonNull}; use objc2::declare::{Ivar, IvarDrop}; use objc2::foundation::{NSArray, NSObject, NSSize, NSString}; @@ -14,7 +14,6 @@ use super::appkit::{ use crate::{ dpi::{LogicalPosition, LogicalSize}, event::{Event, WindowEvent}, - keyboard::ModifiersState, platform_impl::platform::{ app_state::AppState, event::{EventProxy, EventWrapper}, @@ -25,24 +24,28 @@ use crate::{ window::WindowId, }; +#[derive(Debug)] +struct State { + // This is set when WindowBuilder::with_fullscreen was set, + // see comments of `window_did_fail_to_enter_fullscreen` + initial_fullscreen: Cell, + + // During `windowDidResize`, we use this to only send Moved if the position changed. + previous_position: Cell>, + + // Used to prevent redundant events. + previous_scale_factor: Cell, +} + declare_class!( #[derive(Debug)] pub(crate) struct WinitWindowDelegate { window: IvarDrop>, - // TODO: It's possible for delegate methods to be called asynchronously, - // causing data races / `RefCell` panics. - - // This is set when WindowBuilder::with_fullscreen was set, - // see comments of `window_did_fail_to_enter_fullscreen` - initial_fullscreen: bool, - - // During `windowDidResize`, we use this to only send Moved if the position changed. + // TODO: It may be possible for delegate methods to be called + // asynchronously, causing data races panics? // TODO: Remove unnecessary boxing here - previous_position: IvarDrop>>, - - // Used to prevent redundant events. - previous_scale_factor: f64, + state: IvarDrop>, } unsafe impl ClassType for WinitWindowDelegate { @@ -51,19 +54,24 @@ declare_class!( unsafe impl WinitWindowDelegate { #[sel(initWithWindow:initialFullscreen:)] - fn init_with_winit( + unsafe fn init_with_winit( &mut self, window: &WinitWindow, initial_fullscreen: bool, - ) -> Option<&mut Self> { - let this: Option<&mut Self> = unsafe { msg_send![self, init] }; + ) -> Option> { + let this: Option<&mut Self> = unsafe { msg_send![super(self), init] }; this.map(|this| { let scale_factor = window.scale_factor(); Ivar::write(&mut this.window, window.retain()); - Ivar::write(&mut this.initial_fullscreen, initial_fullscreen); - Ivar::write(&mut this.previous_position, None); - Ivar::write(&mut this.previous_scale_factor, scale_factor); + Ivar::write( + &mut this.state, + Box::new(State { + initial_fullscreen: Cell::new(initial_fullscreen), + previous_position: Cell::new(None), + previous_scale_factor: Cell::new(scale_factor), + }), + ); if scale_factor != 1.0 { this.queue_static_scale_factor_changed_event(); @@ -85,7 +93,7 @@ declare_class!( ] }; - this + NonNull::from(this) }) } } @@ -112,14 +120,14 @@ declare_class!( } #[sel(windowDidResize:)] - fn window_did_resize(&mut self, _: Option<&Object>) { + fn window_did_resize(&self, _: Option<&Object>) { trace_scope!("windowDidResize:"); // NOTE: WindowEvent::Resized is reported in frameDidChange. self.emit_move_event(); } #[sel(windowWillStartLiveResize:)] - fn window_will_start_live_resize(&mut self, _: Option<&Object>) { + fn window_will_start_live_resize(&self, _: Option<&Object>) { trace_scope!("windowWillStartLiveResize:"); let increments = self @@ -130,20 +138,20 @@ declare_class!( } #[sel(windowDidEndLiveResize:)] - fn window_did_end_live_resize(&mut self, _: Option<&Object>) { + fn window_did_end_live_resize(&self, _: Option<&Object>) { trace_scope!("windowDidEndLiveResize:"); self.window.set_resize_increments_inner(NSSize::new(1., 1.)); } // This won't be triggered if the move was part of a resize. #[sel(windowDidMove:)] - fn window_did_move(&mut self, _: Option<&Object>) { + fn window_did_move(&self, _: Option<&Object>) { trace_scope!("windowDidMove:"); self.emit_move_event(); } #[sel(windowDidChangeBackingProperties:)] - fn window_did_change_backing_properties(&mut self, _: Option<&Object>) { + fn window_did_change_backing_properties(&self, _: Option<&Object>) { trace_scope!("windowDidChangeBackingProperties:"); self.queue_static_scale_factor_changed_event(); } @@ -166,17 +174,7 @@ declare_class!( // NSWindowDelegate, and as a result a tracked modifiers state can quite // easily fall out of synchrony with reality. This requires us to emit // a synthetic ModifiersChanged event when we lose focus. - - // TODO(madsmtm): Remove the need for this unsafety - let mut view = unsafe { Id::from_shared(self.window.view()) }; - - // Both update the state and emit a ModifiersChanged event. - if !view.state.modifiers.state().is_empty() { - view.state.modifiers = ModifiersState::empty().into(); - self.queue_event(WindowEvent::ModifiersChanged( - ModifiersState::empty().into(), - )); - } + self.window.view().reset_modifiers(); self.queue_event(WindowEvent::Focused(false)); } @@ -307,9 +305,9 @@ declare_class!( /// Invoked when entered fullscreen #[sel(windowDidEnterFullScreen:)] - fn window_did_enter_fullscreen(&mut self, _: Option<&Object>) { + fn window_did_enter_fullscreen(&self, _: Option<&Object>) { trace_scope!("windowDidEnterFullScreen:"); - *self.initial_fullscreen = false; + self.state.initial_fullscreen.set(false); let mut shared_state = self.window.lock_shared_state("window_did_enter_fullscreen"); shared_state.in_fullscreen_transition = false; let target_fullscreen = shared_state.target_fullscreen.take(); @@ -358,7 +356,7 @@ declare_class!( .lock_shared_state("window_did_fail_to_enter_fullscreen"); shared_state.in_fullscreen_transition = false; shared_state.target_fullscreen = None; - if *self.initial_fullscreen { + if self.state.initial_fullscreen.get() { #[allow(clippy::let_unit_value)] unsafe { let _: () = msg_send![ @@ -448,13 +446,13 @@ impl WinitWindowDelegate { AppState::queue_event(EventWrapper::StaticEvent(event)); } - fn queue_static_scale_factor_changed_event(&mut self) { + fn queue_static_scale_factor_changed_event(&self) { let scale_factor = self.window.scale_factor(); - if scale_factor == *self.previous_scale_factor { + if scale_factor == self.state.previous_scale_factor.get() { return; }; - *self.previous_scale_factor = scale_factor; + self.state.previous_scale_factor.set(scale_factor); let wrapper = EventWrapper::EventProxy(EventProxy::DpiChangedProxy { window: self.window.clone(), suggested_size: self.view_size(), @@ -463,12 +461,12 @@ impl WinitWindowDelegate { AppState::queue_event(wrapper); } - fn emit_move_event(&mut self) { + fn emit_move_event(&self) { let rect = self.window.frame(); let x = rect.origin.x as f64; let y = util::bottom_left_to_top_left(rect); - if self.previous_position.as_deref() != Some(&(x, y)) { - *self.previous_position = Some(Box::new((x, y))); + if self.state.previous_position.get() != Some((x, y)) { + self.state.previous_position.set(Some((x, y))); let scale_factor = self.window.scale_factor(); let physical_pos = LogicalPosition::::from((x, y)).to_physical(scale_factor); self.queue_event(WindowEvent::Moved(physical_pos));