[PATCH v2 0/5] MR5604: Draft: winemac.drv: Some dock icon hiding improvements
The most important thing here is a workaround for an OS bug: rapidly hiding and showing the dock icon can result in multiple icons for the same app, and those icons will ignore NSApp.applicationIconImage. Making sure transitions happen no less than 1 second apart from one another seems to fix it. *Edit:* v2 update: 0.5 seconds is not always long enough. A good test case is the Notepad++ installer: after you select a language, it rapidly hides and shows some windows, and without a longer delay the icon will wind up with the default system terminal-looking icon. One second is probably overkill, but this ought to be a somewhat uncommon case. Also, hiding a dock icon will deactivate an app. That's not desirable if it still has visible windows, so attempt to force a reactivation. That's often not successful on Sonoma due to the cooperative app activation heuristics, but it's the best we can do, and it should be a rare case. Also, there is some logic in -transformProcessToForeground: that we often want to happen regardless of whether the app is getting a dock icon (e.g. App Nap and activation). So I've renamed that function to more accurately communicate what it does, and consolidate it with -tryToActivateIgnoringOtherApps:. -- v2: winemac.drv: Consolidate foregrounding and dock icon behavior. winemac.drv: Remove incorrect documentation for orderBelow: and rename a parameter. winemac.drv: Reactivate the app if needed after hiding its dock icon. winemac.drv: Enforce a delay between dock icon hides and shows. https://gitlab.winehq.org/wine/wine/-/merge_requests/5604
From: Tim Clem <tclem(a)codeweavers.com> --- dlls/winemac.drv/cocoa_app.m | 129 +++++++++++++++++------------------ 1 file changed, 64 insertions(+), 65 deletions(-) diff --git a/dlls/winemac.drv/cocoa_app.m b/dlls/winemac.drv/cocoa_app.m index 1a8991b60f5..ee0add686a5 100644 --- a/dlls/winemac.drv/cocoa_app.m +++ b/dlls/winemac.drv/cocoa_app.m @@ -241,16 +241,73 @@ - (void) dealloc [super dealloc]; } + - (void) setDefaultMenus + { + mainMenu = [[[NSMenu alloc] init] autorelease]; + + // Application menu + submenu = [[[NSMenu alloc] initWithTitle:WineLocalizedString(STRING_MENU_WINE)] autorelease]; + bundleName = [[NSBundle mainBundle] objectForInfoDictionaryKey:(NSString*)kCFBundleNameKey]; + + if ([bundleName length]) + title = [NSString stringWithFormat:WineLocalizedString(STRING_MENU_ITEM_HIDE_APPNAME), bundleName]; + else + title = WineLocalizedString(STRING_MENU_ITEM_HIDE); + item = [submenu addItemWithTitle:title action:@selector(hide:) keyEquivalent:@""]; + + item = [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_HIDE_OTHERS) + action:@selector(hideOtherApplications:) + keyEquivalent:@"h"]; + [item setKeyEquivalentModifierMask:NSEventModifierFlagCommand | NSEventModifierFlagOption]; + + item = [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_SHOW_ALL) + action:@selector(unhideAllApplications:) + keyEquivalent:@""]; + + [submenu addItem:[NSMenuItem separatorItem]]; + + if ([bundleName length]) + title = [NSString stringWithFormat:WineLocalizedString(STRING_MENU_ITEM_QUIT_APPNAME), bundleName]; + else + title = WineLocalizedString(STRING_MENU_ITEM_QUIT); + item = [submenu addItemWithTitle:title action:@selector(terminate:) keyEquivalent:@"q"]; + [item setKeyEquivalentModifierMask:NSEventModifierFlagCommand | NSEventModifierFlagOption]; + item = [[[NSMenuItem alloc] init] autorelease]; + [item setTitle:WineLocalizedString(STRING_MENU_WINE)]; + [item setSubmenu:submenu]; + [mainMenu addItem:item]; + + // Window menu + submenu = [[[NSMenu alloc] initWithTitle:WineLocalizedString(STRING_MENU_WINDOW)] autorelease]; + [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_MINIMIZE) + action:@selector(performMiniaturize:) + keyEquivalent:@""]; + [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_ZOOM) + action:@selector(performZoom:) + keyEquivalent:@""]; + item = [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_ENTER_FULL_SCREEN) + action:@selector(toggleFullScreen:) + keyEquivalent:@"f"]; + [item setKeyEquivalentModifierMask:NSEventModifierFlagCommand | + NSEventModifierFlagOption | + NSEventModifierFlagControl]; + [submenu addItem:[NSMenuItem separatorItem]]; + [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_BRING_ALL_TO_FRONT) + action:@selector(arrangeInFront:) + keyEquivalent:@""]; + item = [[[NSMenuItem alloc] init] autorelease]; + [item setTitle:WineLocalizedString(STRING_MENU_WINDOW)]; + [item setSubmenu:submenu]; + [mainMenu addItem:item]; + + [NSApp setMainMenu:mainMenu]; + [NSApp setWindowsMenu:submenu]; + } + - (void) transformProcessToForeground:(BOOL)activateIfTransformed { if ([NSApp activationPolicy] != NSApplicationActivationPolicyRegular) { - NSMenu* mainMenu; - NSMenu* submenu; - NSString* bundleName; - NSString* title; - NSMenuItem* item; - [NSApp setActivationPolicy:NSApplicationActivationPolicyRegular]; if (activateIfTransformed) @@ -264,65 +321,7 @@ - (void) transformProcessToForeground:(BOOL)activateIfTransformed } #endif - mainMenu = [[[NSMenu alloc] init] autorelease]; - - // Application menu - submenu = [[[NSMenu alloc] initWithTitle:WineLocalizedString(STRING_MENU_WINE)] autorelease]; - bundleName = [[NSBundle mainBundle] objectForInfoDictionaryKey:(NSString*)kCFBundleNameKey]; - - if ([bundleName length]) - title = [NSString stringWithFormat:WineLocalizedString(STRING_MENU_ITEM_HIDE_APPNAME), bundleName]; - else - title = WineLocalizedString(STRING_MENU_ITEM_HIDE); - item = [submenu addItemWithTitle:title action:@selector(hide:) keyEquivalent:@""]; - - item = [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_HIDE_OTHERS) - action:@selector(hideOtherApplications:) - keyEquivalent:@"h"]; - [item setKeyEquivalentModifierMask:NSEventModifierFlagCommand | NSEventModifierFlagOption]; - - item = [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_SHOW_ALL) - action:@selector(unhideAllApplications:) - keyEquivalent:@""]; - - [submenu addItem:[NSMenuItem separatorItem]]; - - if ([bundleName length]) - title = [NSString stringWithFormat:WineLocalizedString(STRING_MENU_ITEM_QUIT_APPNAME), bundleName]; - else - title = WineLocalizedString(STRING_MENU_ITEM_QUIT); - item = [submenu addItemWithTitle:title action:@selector(terminate:) keyEquivalent:@"q"]; - [item setKeyEquivalentModifierMask:NSEventModifierFlagCommand | NSEventModifierFlagOption]; - item = [[[NSMenuItem alloc] init] autorelease]; - [item setTitle:WineLocalizedString(STRING_MENU_WINE)]; - [item setSubmenu:submenu]; - [mainMenu addItem:item]; - - // Window menu - submenu = [[[NSMenu alloc] initWithTitle:WineLocalizedString(STRING_MENU_WINDOW)] autorelease]; - [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_MINIMIZE) - action:@selector(performMiniaturize:) - keyEquivalent:@""]; - [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_ZOOM) - action:@selector(performZoom:) - keyEquivalent:@""]; - item = [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_ENTER_FULL_SCREEN) - action:@selector(toggleFullScreen:) - keyEquivalent:@"f"]; - [item setKeyEquivalentModifierMask:NSEventModifierFlagCommand | - NSEventModifierFlagOption | - NSEventModifierFlagControl]; - [submenu addItem:[NSMenuItem separatorItem]]; - [submenu addItemWithTitle:WineLocalizedString(STRING_MENU_ITEM_BRING_ALL_TO_FRONT) - action:@selector(arrangeInFront:) - keyEquivalent:@""]; - item = [[[NSMenuItem alloc] init] autorelease]; - [item setTitle:WineLocalizedString(STRING_MENU_WINDOW)]; - [item setSubmenu:submenu]; - [mainMenu addItem:item]; - - [NSApp setMainMenu:mainMenu]; - [NSApp setWindowsMenu:submenu]; + [self setDefaultMenus]; [NSApp setApplicationIconImage:self.applicationIcon]; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5604
From: Tim Clem <tclem(a)codeweavers.com> Working around an OS bug: rapidly hiding and showing the dock icon can result in multiple icons for the same app, and those icons will ignore NSApp.applicationIconImage. Making sure transitions happen no less than 1 second apart from one another seems to fix it. --- dlls/winemac.drv/cocoa_app.h | 4 +++ dlls/winemac.drv/cocoa_app.m | 66 +++++++++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/dlls/winemac.drv/cocoa_app.h b/dlls/winemac.drv/cocoa_app.h index 4db5b511c60..d51dca2f2e3 100644 --- a/dlls/winemac.drv/cocoa_app.h +++ b/dlls/winemac.drv/cocoa_app.h @@ -125,6 +125,10 @@ @interface WineApplicationController : NSObject <NSApplicationDelegate> NSImage* applicationIcon; + NSTimeInterval lastDockIconVisibilityChangeTime; + BOOL hasPendingDockIconVisibilityChange; + BOOL pendingDockIconShow; + BOOL beenActive; NSMutableSet* windowsBeingDragged; diff --git a/dlls/winemac.drv/cocoa_app.m b/dlls/winemac.drv/cocoa_app.m index ee0add686a5..a86c58f4a30 100644 --- a/dlls/winemac.drv/cocoa_app.m +++ b/dlls/winemac.drv/cocoa_app.m @@ -304,11 +304,63 @@ - (void) setDefaultMenus [NSApp setWindowsMenu:submenu]; } + - (void) doPendingDockIconChange + { + if (!hasPendingDockIconVisibilityChange) return; + + hasPendingDockIconVisibilityChange = NO; + lastDockIconVisibilityChangeTime = [[NSProcessInfo processInfo] systemUptime]; + + if (pendingDockIconShow) + { + if (NSApp.activationPolicy != NSApplicationActivationPolicyRegular) + { + NSApp.activationPolicy = NSApplicationActivationPolicyRegular; + + /* This seems to have no effect until we actually have a dock + icon, so we can't set it earlier. */ + NSApp.applicationIconImage = self.applicationIcon; + + /* We can't take over the menu bar until we have a dock icon, so + there's no point in doing this if we don't. */ + [self setDefaultMenus]; + } + } + else if(NSApp.activationPolicy != NSApplicationActivationPolicyAccessory) + { + NSApp.activationPolicy = NSApplicationActivationPolicyAccessory; + } + } + + - (void) showDockIcon:(BOOL)show + { + static const NSTimeInterval minTimeBetweenChanges = 1.0; + NSTimeInterval timeSinceLastChange; + + pendingDockIconShow = show; + + /* Nothing to do if there's already a scheduled change. */ + if (hasPendingDockIconVisibilityChange) + return; + + hasPendingDockIconVisibilityChange = YES; + timeSinceLastChange = [[NSProcessInfo processInfo] systemUptime] - lastDockIconVisibilityChangeTime; + if (lastDockIconVisibilityChangeTime == 0 || timeSinceLastChange >= minTimeBetweenChanges) + [self doPendingDockIconChange]; + else + { + NSTimeInterval timeToWaitForNextChange = minTimeBetweenChanges - timeSinceLastChange; + [self performSelector:@selector(doPendingDockIconChange) withObject:nil afterDelay:timeToWaitForNextChange]; + } + } + - (void) transformProcessToForeground:(BOOL)activateIfTransformed { + /* activationPolicy may not be exactly accurate if there's a pending + hide/show of the dock icon, but it's not a big deal here. */ if ([NSApp activationPolicy] != NSApplicationActivationPolicyRegular) { - [NSApp setActivationPolicy:NSApplicationActivationPolicyRegular]; + [self showDockIcon:YES]; if (activateIfTransformed) [self tryToActivateIgnoringOtherApps:YES]; @@ -320,10 +372,6 @@ - (void) transformProcessToForeground:(BOOL)activateIfTransformed reason:@"Running Windows program"] retain]; // intentional leak } #endif - - [self setDefaultMenus]; - - [NSApp setApplicationIconImage:self.applicationIcon]; } } @@ -1358,16 +1406,18 @@ - (void) maybeHideDockIconDueToWindowOrderingOut:(NSWindow *)window return; } - if ([NSApp activationPolicy] == NSApplicationActivationPolicyRegular && - ![self shouldHaveDockIconAfterWindowOrdersOut:window + if (![self shouldHaveDockIconAfterWindowOrdersOut:window anyWindowIsVisible:&anyVisibleWindows]) { /* Before macOS 12 Monterey, hiding the dock icon while there are visible windows makes those windows disappear until they are programmatically ordered back in. So we don't do that transition (which should be rather uncommon) on older OSes. */ + /* We may already not have a dock icon, but -showDockIcon: queues + transitions, so we should call it regardless of the current value + of NSApp.activationPolicy. */ if (isMontereyOrLater || !anyVisibleWindows) - NSApp.activationPolicy = NSApplicationActivationPolicyAccessory; + [self showDockIcon:NO]; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5604
From: Tim Clem <tclem(a)codeweavers.com> --- dlls/winemac.drv/cocoa_app.m | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/dlls/winemac.drv/cocoa_app.m b/dlls/winemac.drv/cocoa_app.m index a86c58f4a30..dd9ae15ab15 100644 --- a/dlls/winemac.drv/cocoa_app.m +++ b/dlls/winemac.drv/cocoa_app.m @@ -328,7 +328,22 @@ - (void) doPendingDockIconChange } else if(NSApp.activationPolicy != NSApplicationActivationPolicyAccessory) { + BOOL wasActive = [NSApp isActive]; NSApp.activationPolicy = NSApplicationActivationPolicyAccessory; + + /* Hiding the dock icon deactivates the app. Reactivate if we + were active and we still have visible windows. */ + if (wasActive) + { + for (NSWindow *w in [NSApp windows]) + { + if (w.isVisible) + { + [self tryToActivateIgnoringOtherApps:YES]; + break; + } + } + } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5604
From: Tim Clem <tclem(a)codeweavers.com> The documentation referred to a return value but the method is void. Also the 'activate' parameter was basically an override; the app would activate regardless of its value if unless the window has the preventsAppActivation flag. --- dlls/winemac.drv/cocoa_window.m | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index f5b3875e909..2358cc70118 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -1741,9 +1741,8 @@ - (void) getSiblingWindowsForWindow:(WineWindow*)other ancestor:(WineWindow**)an *ancestorOfOther = otherAncestors.lastObject;; } - /* Returns whether or not the window was ordered in, which depends on if - its frame intersects any screen. */ - - (void) orderBelow:(WineWindow*)prev orAbove:(WineWindow*)next activate:(BOOL)activate + + - (void) orderBelow:(WineWindow*)prev orAbove:(WineWindow*)next forceActivate:(BOOL)activate { WineApplicationController* controller = [WineApplicationController sharedController]; if (![self isMiniaturized]) @@ -2514,7 +2513,7 @@ - (void) makeKeyAndOrderFront:(id)sender { if ([self isMiniaturized]) [self deminiaturize:nil]; - [self orderBelow:nil orAbove:nil activate:NO]; + [self orderBelow:nil orAbove:nil forceActivate:NO]; [[self ancestorWineWindow] postBroughtForwardEvent]; if (![self isKeyWindow] && !self.disabled && !self.noForeground) @@ -3433,7 +3432,7 @@ void macdrv_order_cocoa_window(macdrv_window w, macdrv_window p, OnMainThreadAsync(^{ [window orderBelow:prev orAbove:next - activate:activate]; + forceActivate:activate]; }); [window.queue discardEventsMatchingMask:event_mask_for_type(WINDOW_BROUGHT_FORWARD) forWindow:window]; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5604
From: Tim Clem <tclem(a)codeweavers.com> -transformProcessToForeground: and -tryToActivateIgnoringOtherApps: are effectively a pair. Moreover -transformProcess does some work (handling App Nap and sometimes activating the app) that we may want to happen regardless of whether the app has a dock icon. -appDidShowUIAndShouldActivate:dockIconAction: consolidates all that logic, and makes the intended behavior by callers more clear. -tryToActivateIgnoringOtherApps: is no longer exposed in the header. --- dlls/winemac.drv/cocoa_app.h | 10 ++++++++-- dlls/winemac.drv/cocoa_app.m | 28 ++++++++++++++-------------- dlls/winemac.drv/cocoa_window.m | 20 ++++++++++++-------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/dlls/winemac.drv/cocoa_app.h b/dlls/winemac.drv/cocoa_app.h index d51dca2f2e3..b33e2f43e8d 100644 --- a/dlls/winemac.drv/cocoa_app.h +++ b/dlls/winemac.drv/cocoa_app.h @@ -66,6 +66,12 @@ WineApplicationEventWakeQuery, }; +typedef enum { + WineApplicationDockIconActionHide, + WineApplicationDockIconActionShow, + WineApplicationDockIconActionNoChange +} WineApplicationDockIconAction; + @class WineEventQueue; @class WineWindow; @@ -144,8 +150,8 @@ @interface WineApplicationController : NSObject <NSApplicationDelegate> + (WineApplicationController*) sharedController; - - (void) transformProcessToForeground:(BOOL)activateIfTransformed; - - (void) tryToActivateIgnoringOtherApps:(BOOL)ignore; + - (void) appDidShowUIAndShouldActivate:(BOOL)activate + dockIconAction:(WineApplicationDockIconAction)dockIconAction; - (BOOL) registerEventQueue:(WineEventQueue*)queue; - (void) unregisterEventQueue:(WineEventQueue*)queue; diff --git a/dlls/winemac.drv/cocoa_app.m b/dlls/winemac.drv/cocoa_app.m index dd9ae15ab15..cf0759d0310 100644 --- a/dlls/winemac.drv/cocoa_app.m +++ b/dlls/winemac.drv/cocoa_app.m @@ -369,25 +369,25 @@ - (void) showDockIcon:(BOOL)show } } - - (void) transformProcessToForeground:(BOOL)activateIfTransformed + - (void) appDidShowUIAndShouldActivate:(BOOL)activate + dockIconAction:(WineApplicationDockIconAction)dockIconAction; { - /* activationPolicy may not be exactly accurate if there's a pending - hide/show of the dock icon, but it's not a big deal here. */ - if ([NSApp activationPolicy] != NSApplicationActivationPolicyRegular) + if (dockIconAction != WineApplicationDockIconActionNoChange) { - [self showDockIcon:YES]; + [self showDockIcon:dockIconAction == WineApplicationDockIconActionShow + andActivate:activate]; + } - if (activateIfTransformed) - [self tryToActivateIgnoringOtherApps:YES]; + if (activate) + [self tryToActivateIgnoringOtherApps:YES]; #if defined(MAC_OS_X_VERSION_10_9) && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 - if (!enable_app_nap && [NSProcessInfo instancesRespondToSelector:@selector(beginActivityWithOptions:reason:)]) - { - [[[NSProcessInfo processInfo] beginActivityWithOptions:NSActivityUserInitiatedAllowingIdleSystemSleep - reason:@"Running Windows program"] retain]; // intentional leak - } -#endif + if (!enable_app_nap && [NSProcessInfo instancesRespondToSelector:@selector(beginActivityWithOptions:reason:)]) + { + [[[NSProcessInfo processInfo] beginActivityWithOptions:NSActivityUserInitiatedAllowingIdleSystemSleep + reason:@"Running Windows program"] retain]; // intentional leak } +#endif } - (BOOL) waitUntilQueryDone:(int*)done timeout:(NSDate*)timeout processEvents:(BOOL)processEvents @@ -948,7 +948,7 @@ - (BOOL) setMode:(CGDisplayModeRef)mode forDisplay:(CGDirectDisplayID)displayID if (!modes.count) return FALSE; - [self transformProcessToForeground:YES]; + [self appDidShowUIAndShouldActivate:YES dockIconAction:WineApplicationDockIconActionShow]; BOOL active = [NSApp isActive]; diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 2358cc70118..112bfe6fb76 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -1239,7 +1239,8 @@ - (void) setWindowFeatures:(const struct macdrv_window_features*)wf if (neededDockIcon && !needsDockIcon) [[WineApplicationController sharedController] maybeHideDockIconDueToWindowOrderingOut:nil]; else if(!neededDockIcon && needsDockIcon) - [[WineApplicationController sharedController] transformProcessToForeground:!self.preventsAppActivation]; + [[WineApplicationController sharedController] appDidShowUIAndShouldActivate:!self.preventsAppActivation + dockIconAction:WineApplicationDockIconActionShow]; } } @@ -1753,15 +1754,16 @@ - (void) orderBelow:(WineWindow*)prev orAbove:(WineWindow*)next forceActivate:(B WineWindow* child; if (!eager_dock_icon_hiding || self.needsDockIcon) - [controller transformProcessToForeground:!self.preventsAppActivation]; + [controller appDidShowUIAndShouldActivate:activate || !self.preventsAppActivation + dockIconAction:WineApplicationDockIconActionShow]; + else + [controller appDidShowUIAndShouldActivate:activate || !self.preventsAppActivation + dockIconAction:WineApplicationDockIconActionNoChange]; if ([NSApp isHidden]) [NSApp unhide:nil]; wasVisible = [self isVisible]; - if (activate) - [controller tryToActivateIgnoringOtherApps:YES]; - NSDisableScreenUpdates(); if ([self becameEligibleParentOrChild]) @@ -2127,9 +2129,11 @@ - (void) makeFocused:(BOOL)activate WineApplicationController *controller = [WineApplicationController sharedController]; if (!eager_dock_icon_hiding || self.needsDockIcon) - [controller transformProcessToForeground:YES]; - - [controller tryToActivateIgnoringOtherApps:YES]; + [controller appDidShowUIAndShouldActivate:activate + dockIconAction:WineApplicationDockIconActionShow]; + else + [controller appDidShowUIAndShouldActivate:activate + dockIconAction:WineApplicationDockIconActionNoChange]; } causing_becomeKeyWindow = self; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5604
participants (2)
-
Tim Clem -
Tim Clem (＠tclem)