fix issue #1249 #1079 #1392 #1437
Conversation
I am able to reproduce the issue by reinstall JetBrains AppCode. The first time I open AppCode after reinstall it, AltTab never show AppCode window. I debug the code on my Mac, I find that AltTab can get the event of app launched but never get the event of finishedLaunching. So I just remove the code runningApplication.isFinishedLaunching. After that, AltTab work well and the issue is fixed.
|
@metacodes thanks for sharing this The check is there for a reason: so that we know that the app is finished launching. This is the signal for us to ask for its windows and subscribe to its events. It's likely that removing this check would break things. Could you please look at the git history for examples of issues the check was mitigating? I advice you test with a variety of apps like Gimps for example which is slow to launch. See if your change has not broken things there. Lastly, an app should definitely send this event, so these apps should probably work on the issue instead of us covering up / working around their bugs. That being said AltTab has been pragmatic often so that people have a good UX despite buggy apps. |
|
@lwouis That makes sense. I have look at the git history and I found that you added these code to solve some background processes. But in the function observeNewWindows there also has runningApplication.isFinishedLaunching, so I think maybe remove the code runningApplication.isFinishedLaunching in the function addAndObserveWindows is safe. We just need to run observeEvents() to work around the issue they never send the finishedLaunching event. I try to test with a variety of apps. You said Gimps. Do you mean this app(https://www.gimp.org)? I have already test it, it seems work fine. I will test this code for a few days to see if the change has not broken things. I will report the result after that. |
|
It's possible that checking for So yes, maybe your removal of the check in that specific spot is safe. However, better test it carefully, as it could break the app for everyone. Regarding testing, yes I was referring to Gimp, sorry about the typo. Furthermore, I looked into past tickets, and here are some apps I think are worth testing with your change:
|
|
@lwouis Currently, I have tested Bear.app, Protege, Books.app. Only Bear.app has some problems, sometimes AltTab still can not show Bear's window. But AltTab received the event isFinishedLaunching. The problem is the axWindows_.count gets 0. When I add sleep(3) before that, axWindows_.count will get the right count of windows and AltTab shows Bear's window. I guess there are some problems with the function retryAxCallUntilTimeout, but I don't know how to solve this. Could you give me some advice? |
workaround: sometimes some apps are launched and are actived but OS returns the windows.count == 0. we wait for a moment. For example: Bear.app. At this scenario, AXUIElementCopyAttributeValue(self, kAXWindowsAttribute, &value) returns success but AXUIElementCopyAttributeValue(self, kAXPositionAttribute, &value) and AXUIElementCopyAttributeValue(self, kAXSizeAttribute, &value) return attributeUnsupported. After we wait for a moment, all things run well.
|
@lwouis I have added some codes to fix Bear.app's issue. Its scenario is different from other apps(Octave.app, Protege.app, Books.app, Firefox.app, JetBrains' app). You can find more details about this issue in my commit comments. For now, these apps(Gimp.app, Bear.app, Octave.app, Protege.app, Books.app, Firefox.app, JetBrains' app) work well after testing. And it seems no impact to other apps which are work well. |
|
#1439 I have also tested Sublime Merge.app . Everything works well. |
|
After a week of testing, everything is fine. As Apple says that some applications do not post this notification and so are never reported as finished launching, I think it's reasonable to remove the check of isFinishedLaunching before we create a new observer that can receive notifications from the specified application. |
|
I'll try and test/merge this as soon as possible. I'm struggling to find time these days (see #1179). Please forgive the delays. Hopefully i can finish this soon |
| @@ -117,6 +117,11 @@ class Application: NSObject { | |||
| if group == nil && !self.wasLaunchedBeforeAltTab && CGSSpaceGetType(cgsMainConnectionId, Spaces.currentSpaceId) == .fullscreen { | |||
| throw AxError.runtimeError | |||
| } | |||
| // workaround: sometimes some apps are launched and are actived but OS returns the windows.count == 0. we wait for a moment | |||
| // for example: Bear.app | |||
| if group == nil && !self.wasLaunchedBeforeAltTab && self.runningApplication.isActive { | |||
There was a problem hiding this comment.
I'm wondering if we need self.runningApplication.isActive here. An app could be launching in the background, and still benefit from these retries, no?
The way I think of it is: an app may launch but not have its windows exposed instantly. In that case we want to retry until the 5s timeout.
Note that it may be bad to waste CPU/battery on retrying for 5s, but the case of an app that launches but has no window to show seems very unlikely. So in most cases, I expect the retries to be a good thing, and make the windows visible within a short delay. Whereas the cases where the app actually has no window to show and waste 5s of CPU/battery should be rare.
I'm thinking that we may simplify both if above:
// workaround: opening an app while the active app is fullscreen; we wait out the space transition animation
if group == nil && !self.wasLaunchedBeforeAltTab && CGSSpaceGetType(cgsMainConnectionId, Spaces.currentSpaceId) == .fullscreen {
throw AxError.runtimeError
}
// workaround: sometimes some apps are launched and are actived but OS returns the windows.count == 0. we wait for a moment
// for example: Bear.app
if group == nil && !self.wasLaunchedBeforeAltTab && self.runningApplication.isActive {
throw AxError.runtimeError
}with a single:
// workaround: some apps launch but have no window ready instantly. It's very unlikely an app would launch with no window
// so we retry until timeout, in those rare cases (e.g. Bear.app)
if group == nil && !self.wasLaunchedBeforeAltTab {
throw AxError.runtimeError
}I think the reason why an app is not launching with windows is irrelevant in the end. We should just retry, and it's very unlikely there are really no window. And worst case scenario, we wasted 5s of retrying.
What do you think?
Beyond that, the PR looks solid to me, and once we agree on what to do here, I'll merge and release
There was a problem hiding this comment.
I agree with you. I have also done some tests base on your changes. But there are two problem.
Firstly, the variable wasLaunchedBeforeAltTab is not initialized correctly when AltTab was first launched. The value should be true for all of the apps launched before AltTab. But I found that the value always false. That cause AltTab throws lots of AxError.runtimeError when it was launching and the CPU usage was very high(20%~50%). I will add some code to fix this issue.
Secondly, there are some apps that have menu bar only and launched without window. In my case, it's a screenshot app called iShot.app. The app launches but has no window. When I use it to capture the screen, the AltTab throws lots of AxError.runtimeError.
Base on that two scenario mentioned above, I think we can't simplify both if with a single one although it's very unlikely there are really no window. We need to make sure that the app really has window, so that the retries will be a good thing.
There was a problem hiding this comment.
I have already fixed the issue that the variable wasLaunchedBeforeAltTab is not initialized correctly.
There was a problem hiding this comment.
We need to make sure that the app really has window, so that the retries will be a good thing.
there is no way to know that tough. In your example, iShot could have no window 99% of the time, but still have a Preferences window that the user can display.
when an app just launched, there is no possible way to know if that app will eventually show a window. It could take 10min to do so, if it's a mathematical simulation for example.
That's why alttab tries to get window on launch, but then also subscribe to notifications for when the app creates windows later.
The tradeoff here is: we don't retry but risk missing late windows that come after launch (but as part of the launch, so without a notification) VS retrying to ensure we get those but at the cost of CPU/resources
I don't think there is any way to win in both sides here
the current situation is that alttab ignores window that are not there at launch and not part of a later notification. What Bear.app does is a bug in my book, and AltTab retrying to get its windows is a compromise to help users but as comes at the cost of potential retries for every other app.
We could keep the status quo here to be honest, and these apps fix themselves, instead of penalizing every alttab users with more resource usage.
There was a problem hiding this comment.
You are right. I accept the retry option. In my mind, I just want to reduce the case we retry so that we can reduce CPU usage. But if we add isActive check here, we may miss some windows as some scenarios that we did not expect.
So I accept your changes here.
Do I need to change the code? Or you can change and merge it directly?
There was a problem hiding this comment.
We could keep the status quo here to be honest, and these apps fix themselves, instead of penalizing every alttab users with more resource usage.
Frankly, I came to modify the code because the window loss problem has been bothering me for a long time, and I tried to find other alternative applications, but nothing met my needs, so I finally took the plunge and learned Swift and try to solve the problem.
I don't think these problematic apps will be fixed in future versions. Because for them, they probably won't be aware of these problems without AltTab users giving them feedback, and even if they do, they'll probably think it's not a functional problem and ignore it. For AltTab users, they don't think it's a problem with other apps, they probably think it's a functional bug in AltTab.
So personally, I'd like to see a compromise solution to solve these app bugs, maybe even OS bugs, although these compromise solutions will have some maintenance costs and even negative effects in the future. It depends on how we balance the two. But for now we can control the scope of this negative impact, such as the processing of specific applications, although these special codes are annoying in terms of engineering structure and code aesthetics, and these codes will also incur some maintenance costs in the future.
|
I've embedded these commits in the next release. I committed them slightly altered, but with your author name/email, so you're properly credited. You will also be listed as a contributor on the website and in the app 👍 Closing this PR |
|
Cool 🚀 |
|
@metacodes it seems that my worries about CPU usage were founded :/ I woke up to these tickets, after yesterday new release: |
|
@lwouis sorry about that.😭 I will try to solve this issue as soon as possible. |
|
@metacodes no worries! This is volunteer work. We don't have a QA team to avoid these scenarios. It's unfortunate, but it is expected given the limited resources we have. So no need to feel guilty or anything. Now if you find a mitigation for this, it's a win for everyone ;) |
|
@lwouis I found that the high cpu usage was due to the fact that we kept trying to subscribe to notifications for certain processes, but they were background programs with no windows. (e.g. com.apple.universalcontrol, org.mozilla.plugincontainer)
For each such process, AltTab would spend 120s to retry. The whole process is actually doing useless work. But for those programs that take a long time to load, this retry is necessary again. I think it is not necessary to keep retrying all the time, we can let the program take a break and retry again, although in rare cases there may be a slight delay in the creation of the window. Retries don't happen for most apps, it happens only for those apps which takes for a long time to launch(e.g. JetBrains, Bear.app, Firefox.app etc.). So I add func retryAxCallUntilTimeout_(_ group: DispatchGroup?, _ timeoutInSeconds: Double, _ fn: @escaping ( ) throws -> Void, _ startTime: DispatchTime = DispatchTime.now()) {
do {
try fn()
group?.leave()
} catch {
Thread.sleep(forTimeInterval: 1) // sleep for 1s to reduce CPU usage
let timePassedInSeconds = Double(DispatchTime.now().uptimeNanoseconds - startTime.uptimeNanoseconds) / 1_000_000_000
if timePassedInSeconds < timeoutInSeconds {
BackgroundWork.axCallsQueue.asyncAfter(deadline: .now() + .milliseconds(10)) {
retryAxCallUntilTimeout_(group, timeoutInSeconds, fn, startTime)
}
}
}
}What do you think? EDIT: I removed func retryAxCallUntilTimeout_(_ group: DispatchGroup?, _ timeoutInSeconds: Double, _ fn: @escaping ( ) throws -> Void, _ startTime: DispatchTime = DispatchTime.now()) {
do {
try fn()
group?.leave()
} catch {
let timePassedInSeconds = Double(DispatchTime.now().uptimeNanoseconds - startTime.uptimeNanoseconds) / 1_000_000_000
if timePassedInSeconds < timeoutInSeconds {
BackgroundWork.axCallsQueue.asyncAfter(deadline: .now() + .milliseconds(1000)) {
retryAxCallUntilTimeout_(group, timeoutInSeconds, fn, startTime)
}
}
}
} |
|
Increasing the time between retries mitigates the issue, but is not really addressing the issue directly. As i understand you, it seems that |
@lwouis I've considered it, but currently these processes return the value |
|
@lwouis I have consulted other window management tools and found some useful methods. I referenced the codes and submitted a new PR(#1484).
This discussion koekeishiya/yabai#439 (comment) |
|
Some |
I found it interesting that Apple's default application switcher doesn't show these windows either. |
I am able to reproduce the issue by reinstalling JetBrains AppCode. The first time I open AppCode after reinstalling it, AltTab never shows AppCode window. I debug the code on my Mac, I find that AltTab can get the event of app launched but never get the event of finishedLaunching. So I just remove the code runningApplication.isFinishedLaunching. After that, AltTab work well and the issue is fixed.
The following picture is the debug log when I reproduce this issue:
Thank you for opening a PR! Please make sure that: