macOS: Unbundled window activation hack (#1318)

* `ns_string_id_ref` convenience fn

* Unbundled app activation hack

* Greatly improved solution

* Shortened names

* Remove cruft I left here a year ago

* Doc improvements

* Use `AtomicBool` for `activationHackFlag`

* Add CHANGELOG entry

* Doc improvements

* Fix `did_finish_launching` return type + delay more

* More reliable activation checking

* Fix merge goof

* ...fix other merge goof

Co-authored-by: Freya Gentz <zegentzy@protonmail.com>
Co-authored-by: Bogaevsky <vbogaevsky@gmail.com>

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
 
+- On macOS, fix issue where unbundled applications would sometimes open without being focused.
 - On macOS, fix `run_return` does not return unless it receives a message.
 - On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop.
 - On X11, fix deadlock on window state when handling certain window events.
@@ -26,7 +27,7 @@
 - On all platforms except mobile and WASM, implement `Window::set_minimized`.
 - On X11, fix `CursorEntered` event being generated for non-winit windows.
 - On macOS, fix crash when starting maximized without decorations.
-- On macOS, fix application not to terminate on `run_return`.
+- On macOS, fix application not terminating on `run_return`.
 - On Wayland, fix cursor icon updates on window borders when using CSD.
 - On Wayland, under mutter(GNOME Wayland), fix CSD being behind the status bar, when starting window in maximized mode.
 - On Windows, theme the title bar according to whether the system theme is "Light" or "Dark".

src/platform_impl/macos/activation_hack.rs

@@ -0,0 +1,208 @@
+// Normally when you run or distribute a macOS app, it's bundled: it's in one
+// of those fun little folders that you have to right click "Show Package
+// Contents" on, and usually contains myriad delights including, but not
+// limited to, plists, icons, and of course, your beloved executable. However,
+// when you use `cargo run`, your app is unbundled - it's just a lonely, bare
+// executable.
+//
+// Apple isn't especially fond of unbundled apps, which is to say, they seem to
+// barely be supported. If you move the mouse while opening a winit window from
+// an unbundled app, the window will fail to activate and be in a grayed-out
+// uninteractable state. Switching to another app and back is the only way to
+// get the winit window into a normal state. None of this happens if the app is
+// bundled, i.e. when running via Xcode.
+//
+// To workaround this, we just switch focus to the Dock and then switch back to
+// our app. We only do this for unbundled apps, and only when they fail to
+// become active on their own.
+//
+// This solution was derived from this Godot PR:
+// https://github.com/godotengine/godot/pull/17187
+// (which appears to be based on https://stackoverflow.com/a/7602677)
+// The curious specialness of mouse motions is touched upon here:
+// https://github.com/godotengine/godot/issues/8653#issuecomment-358130512
+//
+// We omit the 2nd step of the solution used in Godot, since it appears to have
+// no effect - I speculate that it's just technical debt picked up from the SO
+// answer; the API used is fairly exotic, and was historically used for very
+// old versions of macOS that didn't support `activateIgnoringOtherApps`, i.e.
+// in previous versions of SDL:
+// https://hg.libsdl.org/SDL/file/c0bcc39a3491/src/video/cocoa/SDL_cocoaevents.m#l322
+//
+// The `performSelector` delays in the Godot solution are used for sequencing,
+// since refocusing the app will fail if the call is made before it finishes
+// unfocusing. The delays used there are much smaller than the ones in the
+// original SO answer, presumably because they found the fastest delay that
+// works reliably through trial and error. Instead of using delays, we just
+// handle `applicationDidResignActive`; despite the app not activating reliably,
+// that still triggers when we switch focus to the Dock.
+//
+// The Godot solution doesn't appear to skip the hack when an unbundled app
+// activates normally. Checking for this is difficult, since if you call
+// `isActive` too early, it will always be `NO`. Even though we receive
+// `applicationDidResignActive` when switching focus to the Dock, we never
+// receive a preceding `applicationDidBecomeActive` if the app fails to
+// activate normally. I wasn't able to find a proper point in time to perform
+// the `isActive` check, so we instead check for the cause of the quirk: if
+// any mouse motion occurs prior to us receiving `applicationDidResignActive`,
+// we assume the app failed to become active.
+//
+// Fun fact: this issue is still present in GLFW
+// (https://github.com/glfw/glfw/issues/1515)
+//
+// A similar issue was found in SDL, but the resolution doesn't seem to work
+// for us: https://bugzilla.libsdl.org/show_bug.cgi?id=3051
+
+use super::util;
+use cocoa::{
+    appkit::{NSApp, NSApplicationActivateIgnoringOtherApps},
+    base::id,
+    foundation::NSUInteger,
+};
+use objc::runtime::{Object, Sel, BOOL, NO, YES};
+use std::{
+    os::raw::c_void,
+    sync::atomic::{AtomicBool, Ordering},
+};
+
+#[derive(Debug, Default)]
+pub struct State {
+    // Indicates that the hack has either completed or been skipped.
+    activated: AtomicBool,
+    // Indicates that the mouse has moved at some point in time.
+    mouse_moved: AtomicBool,
+    // Indicates that the hack is in progress, and that we should refocus when
+    // the app resigns active.
+    needs_refocus: AtomicBool,
+}
+
+impl State {
+    pub fn name() -> &'static str {
+        "activationHackState"
+    }
+
+    pub fn new() -> *mut c_void {
+        let this = Box::new(Self::default());
+        Box::into_raw(this) as *mut c_void
+    }
+
+    pub unsafe fn free(this: *mut Self) {
+        Box::from_raw(this);
+    }
+
+    pub unsafe fn get_ptr(obj: &Object) -> *mut Self {
+        let this: *mut c_void = *(*obj).get_ivar(Self::name());
+        assert!(!this.is_null(), "`activationHackState` pointer was null");
+        this as *mut Self
+    }
+
+    pub unsafe fn set_activated(obj: &Object, value: bool) {
+        let this = Self::get_ptr(obj);
+        (*this).activated.store(value, Ordering::Release);
+    }
+
+    unsafe fn get_activated(obj: &Object) -> bool {
+        let this = Self::get_ptr(obj);
+        (*this).activated.load(Ordering::Acquire)
+    }
+
+    pub unsafe fn set_mouse_moved(obj: &Object, value: bool) {
+        let this = Self::get_ptr(obj);
+        (*this).mouse_moved.store(value, Ordering::Release);
+    }
+
+    pub unsafe fn get_mouse_moved(obj: &Object) -> bool {
+        let this = Self::get_ptr(obj);
+        (*this).mouse_moved.load(Ordering::Acquire)
+    }
+
+    pub unsafe fn set_needs_refocus(obj: &Object, value: bool) {
+        let this = Self::get_ptr(obj);
+        (*this).needs_refocus.store(value, Ordering::Release);
+    }
+
+    unsafe fn get_needs_refocus(obj: &Object) -> bool {
+        let this = Self::get_ptr(obj);
+        (*this).needs_refocus.load(Ordering::Acquire)
+    }
+}
+
+// This is the entry point for the hack - if the app is unbundled and a mouse
+// movement occurs before the app activates, it will trigger the hack. Because
+// mouse movements prior to activation are the cause of this quirk, they should
+// be a reliable way to determine if the hack needs to be performed.
+pub extern "C" fn mouse_moved(this: &Object, _: Sel, _: id) {
+    trace!("Triggered `activationHackMouseMoved`");
+    unsafe {
+        if !State::get_activated(this) {
+            // We check if `CFBundleName` is undefined to determine if the
+            // app is unbundled.
+            if let None = util::app_name() {
+                info!("App detected as unbundled");
+                unfocus(this);
+            } else {
+                info!("App detected as bundled");
+            }
+        }
+    }
+    trace!("Completed `activationHackMouseMoved`");
+}
+
+// Switch focus to the dock.
+unsafe fn unfocus(this: &Object) {
+    // We only perform the hack if the app failed to activate, since otherwise,
+    // there'd be a gross (but fast) flicker as it unfocused and then refocused.
+    // However, we only enter this function if we detect mouse movement prior
+    // to activation, so this should always be `NO`.
+    //
+    // Note that this check isn't necessarily reliable in detecting a violation
+    // of the invariant above, since it's not guaranteed that activation will
+    // resolve before this point. In other words, it can spuriously return `NO`.
+    // This is also why the mouse motion approach was chosen, since it's not
+    // obvious how to sequence this check - if someone knows how to, then that
+    // would almost surely be a cleaner approach.
+    let active: BOOL = msg_send![NSApp(), isActive];
+    if active == YES {
+        error!("Unbundled app activation hack triggered on an app that's already active; this shouldn't happen!");
+    } else {
+        info!("Performing unbundled app activation hack");
+        let dock_bundle_id = util::ns_string_id_ref("com.apple.dock");
+        let dock_array: id = msg_send![
+            class!(NSRunningApplication),
+            runningApplicationsWithBundleIdentifier: *dock_bundle_id
+        ];
+        let dock_array_len: NSUInteger = msg_send![dock_array, count];
+        if dock_array_len == 0 {
+            error!("The Dock doesn't seem to be running, so switching focus to it is impossible");
+        } else {
+            State::set_needs_refocus(this, true);
+            let dock: id = msg_send![dock_array, objectAtIndex: 0];
+            // This will trigger `applicationDidResignActive`, which will in
+            // turn call `refocus`.
+            let status: BOOL = msg_send![
+                dock,
+                activateWithOptions: NSApplicationActivateIgnoringOtherApps
+            ];
+            if status == NO {
+                error!("Failed to switch focus to Dock");
+            }
+        }
+    }
+}
+
+// Switch focus back to our app, causing the user to rejoice!
+pub unsafe fn refocus(this: &Object) {
+    if State::get_needs_refocus(this) {
+        State::set_needs_refocus(this, false);
+        let app: id = msg_send![class!(NSRunningApplication), currentApplication];
+        // Simply calling `NSApp activateIgnoringOtherApps` doesn't work. The
+        // nuanced difference isn't clear to me, but hey, I tried.
+        let success: BOOL = msg_send![
+            app,
+            activateWithOptions: NSApplicationActivateIgnoringOtherApps
+        ];
+        if success == NO {
+            error!("Failed to refocus app");
+        }
+    }
+}

src/platform_impl/macos/app.rs

@@ -2,17 +2,15 @@ use std::collections::VecDeque;
 
 use cocoa::{
     appkit::{self, NSEvent},
-    base::id,
+    base::{id, nil},
 };
 use objc::{
     declare::ClassDecl,
     runtime::{Class, Object, Sel},
 };
 
-use crate::{
-    event::{DeviceEvent, ElementState, Event},
-    platform_impl::platform::{app_state::AppState, event::EventWrapper, util, DEVICE_ID},
-};
+use super::{activation_hack, app_state::AppState, event::EventWrapper, util, DEVICE_ID};
+use crate::event::{DeviceEvent, ElementState, Event};
 
 pub struct AppClass(pub *const Class);
 unsafe impl Send for AppClass {}
@@ -51,14 +49,14 @@ extern "C" fn send_event(this: &Object, _sel: Sel, event: id) {
             let key_window: id = msg_send![this, keyWindow];
             let _: () = msg_send![key_window, sendEvent: event];
         } else {
-            maybe_dispatch_device_event(event);
+            maybe_dispatch_device_event(this, event);
             let superclass = util::superclass(this);
             let _: () = msg_send![super(this, superclass), sendEvent: event];
         }
     }
 }
 
-unsafe fn maybe_dispatch_device_event(event: id) {
+unsafe fn maybe_dispatch_device_event(this: &Object, event: id) {
     let event_type = event.eventType();
     match event_type {
         appkit::NSMouseMoved
@@ -100,6 +98,21 @@ unsafe fn maybe_dispatch_device_event(event: id) {
             }
 
             AppState::queue_events(events);
+
+            // Notify the delegate when the first mouse move occurs. This is
+            // used for the unbundled app activation hack, which needs to know
+            // if any mouse motions occurred prior to the app activating.
+            let delegate: id = msg_send![this, delegate];
+            assert_ne!(delegate, nil);
+            if !activation_hack::State::get_mouse_moved(&*delegate) {
+                activation_hack::State::set_mouse_moved(&*delegate, true);
+                let () = msg_send![
+                    delegate,
+                    performSelector: sel!(activationHackMouseMoved:)
+                    withObject: nil
+                    afterDelay: 0.0
+                ];
+            }
         }
         appkit::NSLeftMouseDown | appkit::NSRightMouseDown | appkit::NSOtherMouseDown => {
             let mut events = VecDeque::with_capacity(1);

src/platform_impl/macos/app_delegate.rs

@@ -1,10 +1,10 @@
+use super::{activation_hack, app_state::AppState};
 use cocoa::base::id;
 use objc::{
     declare::ClassDecl,
-    runtime::{Class, Object, Sel, BOOL, YES},
+    runtime::{Class, Object, Sel},
 };
-
-use crate::platform_impl::platform::app_state::AppState;
+use std::os::raw::c_void;
 
 pub struct AppDelegateClass(pub *const Class);
 unsafe impl Send for AppDelegateClass {}
@@ -15,90 +15,67 @@ lazy_static! {
         let superclass = class!(NSResponder);
         let mut decl = ClassDecl::new("WinitAppDelegate", superclass).unwrap();
 
+        decl.add_class_method(sel!(new), new as extern "C" fn(&Class, Sel) -> id);
+        decl.add_method(sel!(dealloc), dealloc as extern "C" fn(&Object, Sel));
         decl.add_method(
             sel!(applicationDidFinishLaunching:),
-            did_finish_launching as extern "C" fn(&Object, Sel, id) -> BOOL,
+            did_finish_launching as extern "C" fn(&Object, Sel, id),
         );
         decl.add_method(
             sel!(applicationDidBecomeActive:),
             did_become_active as extern "C" fn(&Object, Sel, id),
         );
         decl.add_method(
-            sel!(applicationWillResignActive:),
-            will_resign_active as extern "C" fn(&Object, Sel, id),
+            sel!(applicationDidResignActive:),
+            did_resign_active as extern "C" fn(&Object, Sel, id),
         );
+
+        decl.add_ivar::<*mut c_void>(activation_hack::State::name());
         decl.add_method(
-            sel!(applicationWillEnterForeground:),
-            will_enter_foreground as extern "C" fn(&Object, Sel, id),
-        );
-        decl.add_method(
-            sel!(applicationDidEnterBackground:),
-            did_enter_background as extern "C" fn(&Object, Sel, id),
-        );
-        decl.add_method(
-            sel!(applicationWillTerminate:),
-            will_terminate as extern "C" fn(&Object, Sel, id),
+            sel!(activationHackMouseMoved:),
+            activation_hack::mouse_moved as extern "C" fn(&Object, Sel, id),
         );
 
         AppDelegateClass(decl.register())
     };
 }
 
-extern "C" fn did_finish_launching(_: &Object, _: Sel, _: id) -> BOOL {
-    trace!("Triggered `didFinishLaunching`");
+extern "C" fn new(class: &Class, _: Sel) -> id {
+    unsafe {
+        let this: id = msg_send![class, alloc];
+        let this: id = msg_send![this, init];
+        (*this).set_ivar(
+            activation_hack::State::name(),
+            activation_hack::State::new(),
+        );
+        this
+    }
+}
+
+extern "C" fn dealloc(this: &Object, _: Sel) {
+    unsafe {
+        activation_hack::State::free(activation_hack::State::get_ptr(this));
+    }
+}
+
+extern "C" fn did_finish_launching(_: &Object, _: Sel, _: id) {
+    trace!("Triggered `applicationDidFinishLaunching`");
     AppState::launched();
-    trace!("Completed `didFinishLaunching`");
-    YES
+    trace!("Completed `applicationDidFinishLaunching`");
 }
 
-extern "C" fn did_become_active(_: &Object, _: Sel, _: id) {
-    trace!("Triggered `didBecomeActive`");
-    /*unsafe {
-        HANDLER.lock().unwrap().handle_nonuser_event(Event::Resumed)
-    }*/
-    trace!("Completed `didBecomeActive`");
+extern "C" fn did_become_active(this: &Object, _: Sel, _: id) {
+    trace!("Triggered `applicationDidBecomeActive`");
+    unsafe {
+        activation_hack::State::set_activated(this, true);
+    }
+    trace!("Completed `applicationDidBecomeActive`");
 }
 
-extern "C" fn will_resign_active(_: &Object, _: Sel, _: id) {
-    trace!("Triggered `willResignActive`");
-    /*unsafe {
-        HANDLER.lock().unwrap().handle_nonuser_event(Event::Suspended)
-    }*/
-    trace!("Completed `willResignActive`");
+extern "C" fn did_resign_active(this: &Object, _: Sel, _: id) {
+    trace!("Triggered `applicationDidResignActive`");
+    unsafe {
+        activation_hack::refocus(this);
     }
-
-extern "C" fn will_enter_foreground(_: &Object, _: Sel, _: id) {
-    trace!("Triggered `willEnterForeground`");
-    trace!("Completed `willEnterForeground`");
-}
-
-extern "C" fn did_enter_background(_: &Object, _: Sel, _: id) {
-    trace!("Triggered `didEnterBackground`");
-    trace!("Completed `didEnterBackground`");
-}
-
-extern "C" fn will_terminate(_: &Object, _: Sel, _: id) {
-    trace!("Triggered `willTerminate`");
-    /*unsafe {
-        let app: id = msg_send![class!(UIApplication), sharedApplication];
-        let windows: id = msg_send![app, windows];
-        let windows_enum: id = msg_send![windows, objectEnumerator];
-        let mut events = Vec::new();
-        loop {
-            let window: id = msg_send![windows_enum, nextObject];
-            if window == nil {
-                break
-            }
-            let is_winit_window: BOOL = msg_send![window, isKindOfClass:class!(WinitUIWindow)];
-            if is_winit_window == YES {
-                events.push(Event::WindowEvent {
-                    window_id: RootWindowId(window.into()),
-                    event: WindowEvent::Destroyed,
-                });
-            }
-        }
-        HANDLER.lock().unwrap().handle_nonuser_events(events);
-        HANDLER.lock().unwrap().terminated();
-    }*/
-    trace!("Completed `willTerminate`");
+    trace!("Completed `applicationDidResignActive`");
 }

src/platform_impl/macos/mod.rs

@@ -1,5 +1,6 @@
 #![cfg(target_os = "macos")]
 
+mod activation_hack;
 mod app;
 mod app_delegate;
 mod app_state;

src/platform_impl/macos/monitor.rs

@@ -1,15 +1,14 @@
 use std::{collections::VecDeque, fmt};
 
-use super::ffi;
+use super::{ffi, util};
 use crate::{
     dpi::{PhysicalPosition, PhysicalSize},
     monitor::{MonitorHandle as RootMonitorHandle, VideoMode as RootVideoMode},
-    platform_impl::platform::util::IdRef,
 };
 use cocoa::{
     appkit::NSScreen,
     base::{id, nil},
-    foundation::{NSString, NSUInteger},
+    foundation::NSUInteger,
 };
 use core_foundation::{
     array::{CFArrayGetCount, CFArrayGetValueAtIndex},
@@ -303,7 +302,7 @@ impl MonitorHandle {
             let uuid = ffi::CGDisplayCreateUUIDFromDisplayID(self.0);
             let screens = NSScreen::screens(nil);
             let count: NSUInteger = msg_send![screens, count];
-            let key = IdRef::new(NSString::alloc(nil).init_str("NSScreenNumber"));
+            let key = util::ns_string_id_ref("NSScreenNumber");
             for i in 0..count {
                 let screen = msg_send![screens, objectAtIndex: i as NSUInteger];
                 let device_description = NSScreen::deviceDescription(screen);

src/platform_impl/macos/util/mod.rs

@@ -8,7 +8,7 @@ use std::ops::{BitAnd, Deref};
 use cocoa::{
     appkit::{NSApp, NSWindowStyleMask},
     base::{id, nil},
-    foundation::{NSAutoreleasePool, NSRect, NSUInteger},
+    foundation::{NSAutoreleasePool, NSRect, NSString, NSUInteger},
 };
 use core_graphics::display::CGDisplay;
 use objc::runtime::{Class, Object, Sel, BOOL, YES};
@@ -91,6 +91,22 @@ pub fn bottom_left_to_top_left(rect: NSRect) -> f64 {
     CGDisplay::main().pixels_high() as f64 - (rect.origin.y + rect.size.height)
 }
 
+pub unsafe fn ns_string_id_ref(s: &str) -> IdRef {
+    IdRef::new(NSString::alloc(nil).init_str(s))
+}
+
+pub unsafe fn app_name() -> Option<id> {
+    let bundle: id = msg_send![class!(NSBundle), mainBundle];
+    let dict: id = msg_send![bundle, infoDictionary];
+    let key = ns_string_id_ref("CFBundleName");
+    let app_name: id = msg_send![dict, objectForKey:*key];
+    if app_name != nil {
+        Some(app_name)
+    } else {
+        None
+    }
+}
+
 pub unsafe fn superclass<'a>(this: &'a Object) -> &'a Class {
     let superclass: id = msg_send![this, superclass];
     &*(superclass as *const _)

src/platform_impl/macos/window.rs

@@ -36,7 +36,7 @@ use cocoa::{
         NSWindow, NSWindowButton, NSWindowStyleMask,
     },
     base::{id, nil},
-    foundation::{NSAutoreleasePool, NSDictionary, NSPoint, NSRect, NSSize, NSString},
+    foundation::{NSAutoreleasePool, NSDictionary, NSPoint, NSRect, NSSize},
 };
 use core_graphics::display::{CGDisplay, CGDisplayMode};
 use objc::{
@@ -178,7 +178,7 @@ fn create_window(
             NO,
         ));
         let res = ns_window.non_nil().map(|ns_window| {
-            let title = IdRef::new(NSString::alloc(nil).init_str(&attrs.title));
+            let title = util::ns_string_id_ref(&attrs.title);
             ns_window.setReleasedWhenClosed_(NO);
             ns_window.setTitle_(*title);
             ns_window.setAcceptsMouseMovedEvents_(YES);
@@ -946,7 +946,7 @@ impl UnownedWindow {
         unsafe {
             let screen: id = msg_send![*self.ns_window, screen];
             let desc = NSScreen::deviceDescription(screen);
-            let key = IdRef::new(NSString::alloc(nil).init_str("NSScreenNumber"));
+            let key = util::ns_string_id_ref("NSScreenNumber");
             let value = NSDictionary::valueForKey_(desc, *key);
             let display_id = msg_send![value, unsignedIntegerValue];
             RootMonitorHandle {