Refactor macOS to use new objc2 features (#2465)

* Remove UnownedWindow::inner_rect * Refactor custom view to use much less `unsafe` The compiler fence is safe to get rid of now since `interpretKeyEvents` takes `&mut self` * Refactor Window to use much less unsafe * Refactor NSApplication usage to have much less unsafe * Remove cocoa dependency * Enable `deny(unsafe_op_in_unsafe_fn)` on macOS Also re-enable clippy `let_unit_value` lint * Remove #[macro_use] on macOS * Refactor window delegate to use much less unsafe
rust-windowing · Sep 8, 2022 · 340f951 · 340f951
1 parent 05dd31b
commit 340f951
Show file tree

Hide file tree

Showing 34 changed files with 2,729 additions and 2,308 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -65,11 +65,8 @@ ndk-glue = "0.7.0"
 objc = { version = "=0.3.0-beta.3", package = "objc2" }
 
 [target.'cfg(target_os = "macos")'.dependencies]
-# Branch: objc2
-# TODO: Use non-git versions before we release
-cocoa = { git = "https://github.com/madsmtm/core-foundation-rs.git", rev = "c770e620ba0766fc1d2a9f83327b0fee905eb5cb" }
-core-foundation = { git = "https://github.com/madsmtm/core-foundation-rs.git", rev = "c770e620ba0766fc1d2a9f83327b0fee905eb5cb" }
-core-graphics = { git = "https://github.com/madsmtm/core-foundation-rs.git", rev = "c770e620ba0766fc1d2a9f83327b0fee905eb5cb" }
+core-foundation = "0.9.3"
+core-graphics = "0.22.3"
 dispatch = "0.2.0"
 
 [target.'cfg(target_os = "windows")'.dependencies.windows-sys]

diff --git a/src/lib.rs b/src/lib.rs
@@ -145,7 +145,7 @@ extern crate log;
 extern crate serde;
 #[macro_use]
 extern crate bitflags;
-#[cfg(any(target_os = "macos", target_os = "ios"))]
+#[cfg(target_os = "ios")]
 #[macro_use]
 extern crate objc;
 #[cfg(target_os = "macos")]

diff --git a/src/platform/macos.rs b/src/platform/macos.rs
@@ -1,3 +1,4 @@
+use objc2::rc::Id;
 use std::os::raw::c_void;
 
 use crate::{
@@ -240,7 +241,7 @@ impl MonitorHandleExtMacOS for MonitorHandle {
     }
 
     fn ns_screen(&self) -> Option<*mut c_void> {
-        self.inner.ns_screen().map(|s| s as *mut c_void)
+        self.inner.ns_screen().map(|s| Id::as_ptr(&s) as _)
     }
 }
 

diff --git a/src/platform_impl/macos/app.rs b/src/platform_impl/macos/app.rs
@@ -1,14 +1,10 @@
 use std::collections::VecDeque;
 
-use cocoa::{
-    appkit::{self, NSEvent},
-    base::id,
-};
 use objc2::foundation::NSObject;
-use objc2::{declare_class, ClassType};
+use objc2::{declare_class, msg_send, ClassType};
 
-use super::appkit::{NSApplication, NSResponder};
-use super::{app_state::AppState, event::EventWrapper, util, DEVICE_ID};
+use super::appkit::{NSApplication, NSEvent, NSEventModifierFlags, NSEventType, NSResponder};
+use super::{app_state::AppState, event::EventWrapper, DEVICE_ID};
 use crate::event::{DeviceEvent, ElementState, Event};
 
 declare_class!(
@@ -25,37 +21,33 @@ declare_class!(
         // Overriding `sendEvent:` like this fixes that. (https://stackoverflow.com/a/15294196)
         // Fun fact: Firefox still has this bug! (https://bugzilla.mozilla.org/show_bug.cgi?id=1299553)
         #[sel(sendEvent:)]
-        fn send_event(&self, event: id) {
-            unsafe {
-                // For posterity, there are some undocumented event types
-                // (https://github.com/servo/cocoa-rs/issues/155)
-                // but that doesn't really matter here.
-                let event_type = event.eventType();
-                let modifier_flags = event.modifierFlags();
-                if event_type == appkit::NSKeyUp
-                    && util::has_flag(
-                        modifier_flags,
-                        appkit::NSEventModifierFlags::NSCommandKeyMask,
-                    )
-                {
-                    let key_window: id = msg_send![self, keyWindow];
-                    let _: () = msg_send![key_window, sendEvent: event];
-                } else {
-                    maybe_dispatch_device_event(event);
-                    let _: () = msg_send![super(self), sendEvent: event];
+        fn send_event(&self, event: &NSEvent) {
+            // For posterity, there are some undocumented event types
+            // (https://github.com/servo/cocoa-rs/issues/155)
+            // but that doesn't really matter here.
+            let event_type = event.type_();
+            let modifier_flags = event.modifierFlags();
+            if event_type == NSEventType::NSKeyUp
+                && modifier_flags.contains(NSEventModifierFlags::NSCommandKeyMask)
+            {
+                if let Some(key_window) = self.keyWindow() {
+                    unsafe { key_window.sendEvent(event) };
                 }
+            } else {
+                maybe_dispatch_device_event(event);
+                unsafe { msg_send![super(self), sendEvent: event] }
             }
         }
     }
 );
 
-unsafe fn maybe_dispatch_device_event(event: id) {
-    let event_type = event.eventType();
+fn maybe_dispatch_device_event(event: &NSEvent) {
+    let event_type = event.type_();
     match event_type {
-        appkit::NSMouseMoved
-        | appkit::NSLeftMouseDragged
-        | appkit::NSOtherMouseDragged
-        | appkit::NSRightMouseDragged => {
+        NSEventType::NSMouseMoved
+        | NSEventType::NSLeftMouseDragged
+        | NSEventType::NSOtherMouseDragged
+        | NSEventType::NSRightMouseDragged => {
             let mut events = VecDeque::with_capacity(3);
 
             let delta_x = event.deltaX() as f64;
@@ -92,7 +84,9 @@ unsafe fn maybe_dispatch_device_event(event: id) {
 
             AppState::queue_events(events);
         }
-        appkit::NSLeftMouseDown | appkit::NSRightMouseDown | appkit::NSOtherMouseDown => {
+        NSEventType::NSLeftMouseDown
+        | NSEventType::NSRightMouseDown
+        | NSEventType::NSOtherMouseDown => {
             let mut events = VecDeque::with_capacity(1);
 
             events.push_back(EventWrapper::StaticEvent(Event::DeviceEvent {
@@ -105,7 +99,7 @@ unsafe fn maybe_dispatch_device_event(event: id) {
 
             AppState::queue_events(events);
         }
-        appkit::NSLeftMouseUp | appkit::NSRightMouseUp | appkit::NSOtherMouseUp => {
+        NSEventType::NSLeftMouseUp | NSEventType::NSRightMouseUp | NSEventType::NSOtherMouseUp => {
             let mut events = VecDeque::with_capacity(1);
 
             events.push_back(EventWrapper::StaticEvent(Event::DeviceEvent {

diff --git a/src/platform_impl/macos/app_delegate.rs b/src/platform_impl/macos/app_delegate.rs
@@ -1,10 +1,10 @@
-use cocoa::appkit::NSApplicationActivationPolicy;
 use objc2::foundation::NSObject;
 use objc2::rc::{Id, Shared};
 use objc2::runtime::Object;
-use objc2::{declare_class, ClassType};
+use objc2::{declare_class, msg_send, msg_send_id, ClassType};
 
 use super::app_state::AppState;
+use super::appkit::NSApplicationActivationPolicy;
 
 declare_class!(
     #[derive(Debug)]

diff --git a/src/platform_impl/macos/app_state.rs b/src/platform_impl/macos/app_state.rs
@@ -2,7 +2,6 @@ use std::{
     cell::{RefCell, RefMut},
     collections::VecDeque,
     fmt::{self, Debug},
-    hint::unreachable_unchecked,
     mem,
     rc::{Rc, Weak},
     sync::{
@@ -12,27 +11,22 @@ use std::{
     time::Instant,
 };
 
-use cocoa::{
-    appkit::{NSApp, NSApplication, NSApplicationActivationPolicy, NSWindow},
-    base::{id, nil},
-    foundation::NSSize,
-};
-use objc::foundation::is_main_thread;
-use objc::rc::autoreleasepool;
-use objc::runtime::Bool;
+use objc2::foundation::{is_main_thread, NSSize};
+use objc2::rc::autoreleasepool;
 use once_cell::sync::Lazy;
 
+use super::appkit::{NSApp, NSApplication, NSApplicationActivationPolicy, NSEvent};
 use crate::{
     dpi::LogicalSize,
     event::{Event, StartCause, WindowEvent},
     event_loop::{ControlFlow, EventLoopWindowTarget as RootWindowTarget},
     platform_impl::platform::{
         event::{EventProxy, EventWrapper},
-        event_loop::{post_dummy_event, PanicInfo},
+        event_loop::PanicInfo,
         menu,
         observer::{CFRunLoopGetMain, CFRunLoopWakeUp, EventLoopWaker},
-        util::{IdRef, Never},
-        window::get_window_id,
+        util::Never,
+        window::WinitWindow,
     },
     window::WindowId,
 };
@@ -44,7 +38,7 @@ impl<'a, Never> Event<'a, Never> {
         self.map_nonuser_event()
             // `Never` can't be constructed, so the `UserEvent` variant can't
             // be present here.
-            .unwrap_or_else(|_| unsafe { unreachable_unchecked() })
+            .unwrap_or_else(|_| unreachable!())
     }
 }
 
@@ -217,14 +211,14 @@ impl Handler {
     fn handle_scale_factor_changed_event(
         &self,
         callback: &mut Box<dyn EventHandler + 'static>,
-        ns_window: IdRef,
+        window: &WinitWindow,
         suggested_size: LogicalSize<f64>,
         scale_factor: f64,
     ) {
         let mut size = suggested_size.to_physical(scale_factor);
         let new_inner_size = &mut size;
         let event = Event::WindowEvent {
-            window_id: WindowId(get_window_id(*ns_window)),
+            window_id: WindowId(window.id()),
             event: WindowEvent::ScaleFactorChanged {
                 scale_factor,
                 new_inner_size,
@@ -236,26 +230,26 @@ impl Handler {
         let physical_size = *new_inner_size;
         let logical_size = physical_size.to_logical(scale_factor);
         let size = NSSize::new(logical_size.width, logical_size.height);
-        unsafe { NSWindow::setContentSize_(*ns_window, size) };
+        window.setContentSize(size);
     }
 
     fn handle_proxy(&self, proxy: EventProxy, callback: &mut Box<dyn EventHandler + 'static>) {
         match proxy {
             EventProxy::DpiChangedProxy {
-                ns_window,
+                window,
                 suggested_size,
                 scale_factor,
             } => self.handle_scale_factor_changed_event(
                 callback,
-                ns_window,
+                &window,
                 suggested_size,
                 scale_factor,
             ),
         }
     }
 }
 
-pub enum AppState {}
+pub(crate) enum AppState {}
 
 impl AppState {
     pub fn set_callback<T>(callback: Weak<Callback<T>>, window_target: Rc<RootWindowTarget<T>>) {
@@ -278,18 +272,16 @@ impl AppState {
     }
 
     pub fn launched(activation_policy: NSApplicationActivationPolicy, create_default_menu: bool) {
-        unsafe {
-            let ns_app = NSApp();
+        let app = NSApp();
+        // We need to delay setting the activation policy and activating the app
+        // until `applicationDidFinishLaunching` has been called. Otherwise the
+        // menu bar is initially unresponsive on macOS 10.15.
+        app.setActivationPolicy(activation_policy);
 
-            // We need to delay setting the activation policy and activating the app
-            // until `applicationDidFinishLaunching` has been called. Otherwise the
-            // menu bar is initially unresponsive on macOS 10.15.
-            ns_app.setActivationPolicy_(activation_policy);
+        window_activation_hack(&app);
+        // TODO: Consider allowing the user to specify they don't want their application activated
+        app.activateIgnoringOtherApps(true);
 
-            window_activation_hack(ns_app);
-            // TODO: Consider allowing the user to specify they don't want their application activated
-            ns_app.activateIgnoringOtherApps_(Bool::YES.as_raw());
-        };
         HANDLER.set_ready();
         HANDLER.waker().start();
         if create_default_menu {
@@ -397,15 +389,12 @@ impl AppState {
         HANDLER.set_in_callback(false);
 
         if HANDLER.should_exit() {
-            unsafe {
-                let app: id = NSApp();
-
-                autoreleasepool(|_| {
-                    let _: () = msg_send![app, stop: nil];
-                    // To stop event loop immediately, we need to post some event here.
-                    post_dummy_event(app);
-                });
-            };
+            let app = NSApp();
+            autoreleasepool(|_| {
+                app.stop(None);
+                // To stop event loop immediately, we need to post some event here.
+                app.postEvent_atStart(&NSEvent::dummy(), true);
+            });
         }
         HANDLER.update_start_time();
         match HANDLER.get_old_and_new_control_flow() {
@@ -426,25 +415,17 @@ impl AppState {
 ///
 /// If this becomes too bothersome to maintain, it can probably be removed
 /// without too much damage.
-unsafe fn window_activation_hack(ns_app: id) {
-    // Get the application's windows
+fn window_activation_hack(app: &NSApplication) {
     // TODO: Proper ordering of the windows
-    let ns_windows: id = msg_send![ns_app, windows];
-    let ns_enumerator: id = msg_send![ns_windows, objectEnumerator];
-    loop {
-        // Enumerate over the windows
-        let ns_window: id = msg_send![ns_enumerator, nextObject];
-        if ns_window == nil {
-            break;
-        }
-        // And call `makeKeyAndOrderFront` if it was called on the window in `UnownedWindow::new`
+    app.windows().into_iter().for_each(|window| {
+        // Call `makeKeyAndOrderFront` if it was called on the window in `WinitWindow::new`
         // This way we preserve the user's desired initial visiblity status
         // TODO: Also filter on the type/"level" of the window, and maybe other things?
-        if Bool::from_raw(ns_window.isVisible()).as_bool() {
+        if window.isVisible() {
             trace!("Activating visible window");
-            ns_window.makeKeyAndOrderFront_(nil);
+            window.makeKeyAndOrderFront(None);
         } else {
             trace!("Skipping activating invisible window");
         }
-    }
+    })
 }