[PATCH 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 0.5 seconds apart from one another seems to fix it. 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:. -- 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 0.5 seconds 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..e4b2577b223 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 = 0.5; + 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 not already not have a dock icon, but -showDockIcon: + queues transitions, so we should call it regardless of the 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 e4b2577b223..8d914a62d56 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 8d914a62d56..7285ade4cec 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
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=145474 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/winemac.drv/cocoa_app.m:328 error: patch failed: dlls/winemac.drv/cocoa_app.m:369 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/winemac.drv/cocoa_app.m:328 error: patch failed: dlls/winemac.drv/cocoa_app.m:369 Task: Patch failed to apply
participants (3)
-
Marvin -
Tim Clem -
Tim Clem (＠tclem)