MacOS: Only activate after the application has finished launching (#1903)

* MacOS: Only activate after the application has finished launching

This fixes the main menu not responding until you refocus, at least from what I can tell - though we might have to do something similar to https://github.com/linebender/druid/pull/994 to fix it fully?

* MacOS: Remove activation hack

* Stop unnecessarily calling `makeKeyWindow` on initially hidden windows

You can't make hidden windows the key window

* Add new, simpler activation hack

For activating multiple windows created before the application finished launching
This commit is contained in:
Mads Marquart 2021-04-29 19:49:17 +02:00 committed by GitHub
parent 45aacd8407
commit 277515636d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 50 additions and 289 deletions

View file

@ -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.

View file

@ -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");
}
}
}

View file

@ -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);

View file

@ -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`");
}

View file

@ -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");
}
}
}

View file

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

View file

@ -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<id> {
let bundle: id = msg_send![class!(NSBundle), mainBundle];
let dict: id = msg_send![bundle, infoDictionary];

View file

@ -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 {