From 1ddceeb0630994e4960493b1fe92bc72f8c5a54b Mon Sep 17 00:00:00 2001 From: Francesca Plebani Date: Fri, 10 Jan 2020 16:02:42 -0800 Subject: [PATCH] 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 Co-authored-by: Bogaevsky --- CHANGELOG.md | 3 +- src/platform_impl/macos/activation_hack.rs | 208 +++++++++++++++++++++ src/platform_impl/macos/app.rs | 27 ++- src/platform_impl/macos/app_delegate.rs | 113 +++++------ src/platform_impl/macos/mod.rs | 1 + src/platform_impl/macos/monitor.rs | 7 +- src/platform_impl/macos/util/mod.rs | 18 +- src/platform_impl/macos/window.rs | 6 +- 8 files changed, 299 insertions(+), 84 deletions(-) create mode 100644 src/platform_impl/macos/activation_hack.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ab17da07..dec9f155 100644 --- a/CHANGELOG.md +++ b/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". diff --git a/src/platform_impl/macos/activation_hack.rs b/src/platform_impl/macos/activation_hack.rs new file mode 100644 index 00000000..6cf1960c --- /dev/null +++ b/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"); + } + } +} diff --git a/src/platform_impl/macos/app.rs b/src/platform_impl/macos/app.rs index fba48ae5..4cec9512 100644 --- a/src/platform_impl/macos/app.rs +++ b/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); diff --git a/src/platform_impl/macos/app_delegate.rs b/src/platform_impl/macos/app_delegate.rs index c0c3a121..9263cc12 100644 --- a/src/platform_impl/macos/app_delegate.rs +++ b/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 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`"); +extern "C" fn did_resign_active(this: &Object, _: Sel, _: id) { + trace!("Triggered `applicationDidResignActive`"); + unsafe { + activation_hack::refocus(this); + } + trace!("Completed `applicationDidResignActive`"); } diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index c26385da..8b5ee6a5 100644 --- a/src/platform_impl/macos/mod.rs +++ b/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; diff --git a/src/platform_impl/macos/monitor.rs b/src/platform_impl/macos/monitor.rs index d9f0baa7..817d38ee 100644 --- a/src/platform_impl/macos/monitor.rs +++ b/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); diff --git a/src/platform_impl/macos/util/mod.rs b/src/platform_impl/macos/util/mod.rs index 3a454cbb..39a97c98 100644 --- a/src/platform_impl/macos/util/mod.rs +++ b/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 { + 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 _) diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index d32da658..b77fc907 100644 --- a/src/platform_impl/macos/window.rs +++ b/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 {