diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e62cb68..bda66817 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- On macOS, wait with activating the application until the application has initialized. - On macOS, fix creating new windows when the application has a main menu. - On Windows, fix fractional deltas for mouse wheel device events. - On macOS, fix segmentation fault after dropping the main window. diff --git a/src/platform_impl/macos/activation_hack.rs b/src/platform_impl/macos/activation_hack.rs deleted file mode 100644 index 6cf1960c..00000000 --- a/src/platform_impl/macos/activation_hack.rs +++ /dev/null @@ -1,208 +0,0 @@ -// 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 4cec9512..b5a5582c 100644 --- a/src/platform_impl/macos/app.rs +++ b/src/platform_impl/macos/app.rs @@ -2,14 +2,14 @@ use std::collections::VecDeque; use cocoa::{ appkit::{self, NSEvent}, - base::{id, nil}, + base::id, }; use objc::{ declare::ClassDecl, runtime::{Class, Object, Sel}, }; -use super::{activation_hack, app_state::AppState, event::EventWrapper, util, DEVICE_ID}; +use super::{app_state::AppState, event::EventWrapper, util, DEVICE_ID}; use crate::event::{DeviceEvent, ElementState, Event}; pub struct AppClass(pub *const Class); @@ -49,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(this, event); + maybe_dispatch_device_event(event); let superclass = util::superclass(this); let _: () = msg_send![super(this, superclass), sendEvent: event]; } } } -unsafe fn maybe_dispatch_device_event(this: &Object, event: id) { +unsafe fn maybe_dispatch_device_event(event: id) { let event_type = event.eventType(); match event_type { appkit::NSMouseMoved @@ -98,21 +98,6 @@ unsafe fn maybe_dispatch_device_event(this: &Object, 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 9263cc12..df59a2a1 100644 --- a/src/platform_impl/macos/app_delegate.rs +++ b/src/platform_impl/macos/app_delegate.rs @@ -1,10 +1,9 @@ -use super::{activation_hack, app_state::AppState}; +use super::app_state::AppState; use cocoa::base::id; use objc::{ declare::ClassDecl, runtime::{Class, Object, Sel}, }; -use std::os::raw::c_void; pub struct AppDelegateClass(pub *const Class); unsafe impl Send for AppDelegateClass {} @@ -15,67 +14,17 @@ 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), ); - decl.add_method( - sel!(applicationDidBecomeActive:), - did_become_active as extern "C" fn(&Object, Sel, id), - ); - decl.add_method( - 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!(activationHackMouseMoved:), - activation_hack::mouse_moved as extern "C" fn(&Object, Sel, id), - ); AppDelegateClass(decl.register()) }; } -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 `applicationDidFinishLaunching`"); } - -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 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/app_state.rs b/src/platform_impl/macos/app_state.rs index 4f20e216..44525c86 100644 --- a/src/platform_impl/macos/app_state.rs +++ b/src/platform_impl/macos/app_state.rs @@ -13,10 +13,11 @@ use std::{ }; use cocoa::{ - appkit::{NSApp, NSWindow}, + appkit::{NSApp, NSApplication, NSWindow}, base::{id, nil}, foundation::{NSAutoreleasePool, NSSize}, }; +use objc::runtime::YES; use crate::{ dpi::LogicalSize, @@ -273,6 +274,12 @@ impl AppState { } pub fn launched() { + unsafe { + let ns_app = NSApp(); + window_activation_hack(ns_app); + // TODO: Consider allowing the user to specify they don't want their application activated + ns_app.activateIgnoringOtherApps_(YES); + }; HANDLER.set_ready(); HANDLER.waker().start(); // The menubar initialization should be before the `NewEvents` event, to allow overriding @@ -419,3 +426,34 @@ impl AppState { } } } + +/// A hack to make activation of multiple windows work when creating them before +/// `applicationDidFinishLaunching:` / `Event::Event::NewEvents(StartCause::Init)`. +/// +/// Alternative to this would be the user calling `window.set_visible(true)` in +/// `StartCause::Init`. +/// +/// 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 + // 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` + // 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 ns_window.isVisible() == YES { + trace!("Activating visible window"); + ns_window.makeKeyAndOrderFront_(nil); + } else { + trace!("Skipping activating invisible window"); + } + } +} diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 09b61a5d..8c22e588 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -1,6 +1,5 @@ #![cfg(target_os = "macos")] -mod activation_hack; mod app; mod app_delegate; mod app_state; diff --git a/src/platform_impl/macos/util/mod.rs b/src/platform_impl/macos/util/mod.rs index a6187a50..2ebfe56c 100644 --- a/src/platform_impl/macos/util/mod.rs +++ b/src/platform_impl/macos/util/mod.rs @@ -106,6 +106,7 @@ pub unsafe fn ns_string_id_ref(s: &str) -> IdRef { IdRef::new(NSString::alloc(nil).init_str(s)) } +#[allow(dead_code)] // In case we want to use this function in the future pub unsafe fn app_name() -> Option { let bundle: id = msg_send![class!(NSBundle), mainBundle]; let dict: id = msg_send![bundle, infoDictionary]; diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index 7d16f2fc..435aab43 100644 --- a/src/platform_impl/macos/window.rs +++ b/src/platform_impl/macos/window.rs @@ -359,7 +359,7 @@ impl UnownedWindow { let pool = unsafe { NSAutoreleasePool::new(nil) }; - let ns_app = create_app(pl_attribs.activation_policy).ok_or_else(|| { + create_app(pl_attribs.activation_policy).ok_or_else(|| { unsafe { pool.drain() }; os_error!(OsError::CreationError("Couldn't create `NSApplication`")) })?; @@ -391,7 +391,6 @@ impl UnownedWindow { ns_window.setBackgroundColor_(NSColor::clearColor(nil)); } - ns_app.activateIgnoringOtherApps_(YES); win_attribs.min_inner_size.map(|dim| { let logical_dim = dim.to_logical(scale_factor); set_min_inner_size(*ns_window, logical_dim) @@ -441,12 +440,9 @@ impl UnownedWindow { // Setting the window as key has to happen *after* we set the fullscreen // state, since otherwise we'll briefly see the window at normal size // before it transitions. - unsafe { - if visible { - window.ns_window.makeKeyAndOrderFront_(nil); - } else { - window.ns_window.makeKeyWindow(); - } + if visible { + // Tightly linked with `app_state::window_activation_hack` + unsafe { window.ns_window.makeKeyAndOrderFront_(nil) }; } if maximized {