-
-
Notifications
You must be signed in to change notification settings - Fork 339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix high CPU usage #1481 #1484
fix high CPU usage #1481 #1484
Changes from 11 commits
d058db6
5d33d3a
c68b298
53b3fe3
43d44d2
4093f57
8e6a973
c6f656f
0595767
1bed0b3
50eaed6
41a3883
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -122,10 +122,15 @@ func CGSCopyWindowsWithOptionsAndTags(_ cid: CGSConnectionID, _ owner: UInt32, _ | |
func CGSManagedDisplayGetCurrentSpace(_ cid: CGSConnectionID, _ displayUuid: CFString) -> CGSSpaceID | ||
|
||
// adds the provided windows to the provided spaces | ||
// * macOS 10.10+ | ||
// * macOS 10.10-12.2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RIP 😄 |
||
@_silgen_name("CGSAddWindowsToSpaces") | ||
func CGSAddWindowsToSpaces(_ cid: CGSConnectionID, _ windows: NSArray, _ spaces: NSArray) -> Void | ||
|
||
// Move the given windows (CGWindowIDs) to the given space (CGSSpaceID) | ||
// * macOS 10.10+ | ||
@_silgen_name("CGSMoveWindowsToManagedSpace") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this work exactly like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This api will move the window from the original space to another space. It means the window will disappear from the original space and show on another space. I did not test the scenarios you mentioned. I just use this api to move the invisible window to the space we want to. I will do some tests. By the way, I can not do the tests for multiple monitors, because I only have one monitor.😂 |
||
func CGSMoveWindowsToManagedSpace(_ cid: CGSConnectionID, _ windows: NSArray, _ space: CGSSpaceID) -> Void | ||
|
||
// remove the provided windows from the provided spaces | ||
// * macOS 10.10+ | ||
@_silgen_name("CGSRemoveWindowsFromSpaces") | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -14,10 +14,19 @@ class Applications { | |||
_ = group.wait(wallTimeout: .now() + .seconds(2)) | ||||
} | ||||
|
||||
static func addOtherSpaceWindows(_ windowsOnlyOnOtherSpaces: [CGWindowID]) { | ||||
for app in list { | ||||
app.wasLaunchedBeforeAltTab = true | ||||
guard app.runningApplication.isFinishedLaunching else { continue } | ||||
app.addOtherSpaceWindows(windowsOnlyOnOtherSpaces) | ||||
} | ||||
} | ||||
|
||||
static func initialDiscovery() { | ||||
addInitialRunningApplications() | ||||
addInitialRunningApplicationsWindows() | ||||
WorkspaceEvents.observeRunningApplications() | ||||
WorkspaceEvents.registerFrontAppChangeNote() | ||||
} | ||||
|
||||
static func addInitialRunningApplications() { | ||||
|
@@ -31,17 +40,18 @@ class Applications { | |||
let windowsOnOtherSpaces = Spaces.windowsInSpaces(otherSpaces) | ||||
let windowsOnlyOnOtherSpaces = Array(Set(windowsOnOtherSpaces).subtracting(windowsOnCurrentSpace)) | ||||
if windowsOnlyOnOtherSpaces.count > 0 { | ||||
// on initial launch, we use private APIs to bring windows from other spaces into the current space, observe them, then remove them from the current space | ||||
CGSAddWindowsToSpaces(cgsMainConnectionId, windowsOnlyOnOtherSpaces as NSArray, [Spaces.currentSpaceId]) | ||||
Applications.observeNewWindowsBlocking() | ||||
CGSRemoveWindowsFromSpaces(cgsMainConnectionId, windowsOnlyOnOtherSpaces as NSArray, [Spaces.currentSpaceId]) | ||||
// Currently we add those window in other space without AXUIElement init | ||||
// We don't need to get the AXUIElement until we focus these windows. | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're correct that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see my follow-up here: #1324 (comment) |
||||
// when we need to focus these windows, we use the helper window to take us to that space, | ||||
// then get the AXUIElement, and finally focus that window. | ||||
Applications.addOtherSpaceWindows(windowsOnlyOnOtherSpaces) | ||||
} | ||||
} | ||||
} | ||||
|
||||
static func addRunningApplications(_ runningApps: [NSRunningApplication], _ wasLaunchedBeforeAltTab: Bool = false) { | ||||
runningApps.forEach { | ||||
if isActualApplication($0) { | ||||
if isActualApplication($0, wasLaunchedBeforeAltTab) { | ||||
Applications.list.append(Application($0, wasLaunchedBeforeAltTab)) | ||||
} | ||||
} | ||||
|
@@ -101,10 +111,37 @@ class Applications { | |||
} | ||||
} | ||||
|
||||
private static func isActualApplication(_ app: NSRunningApplication) -> Bool { | ||||
private static func isActualApplication(_ app: NSRunningApplication, _ wasLaunchedBeforeAltTab: Bool = false) -> Bool { | ||||
// an app can start with .activationPolicy == .prohibited, then transition to != .prohibited later | ||||
// an app can be both activationPolicy == .accessory and XPC (e.g. com.apple.dock.etci) | ||||
return (isNotXpc(app) || isAndroidEmulator(app)) && !app.processIdentifier.isZombie() | ||||
return isAnWindowApplication(app, wasLaunchedBeforeAltTab) && (isNotXpc(app) || isAndroidEmulator(app)) && !app.processIdentifier.isZombie() | ||||
} | ||||
|
||||
private static func isAnWindowApplication(_ app: NSRunningApplication, _ wasLaunchedBeforeAltTab: Bool = false) -> Bool { | ||||
if (wasLaunchedBeforeAltTab) { | ||||
// For wasLaunchedBeforeAltTab=true, we assume that those apps are all launched, if they are programs with windows. | ||||
// Even if it has 0 windows at this point, axUiElement.windows() will not throw an exception. If they are programs without windows, then axUiElement.windows() will throw an exception. | ||||
// Here I consider there is an edge case where AltTab is starting up and this program has been loading, then it is possible that axUiElement.windows() will throw an exception. | ||||
// I'm not quite sure if this happens, but even if it does, then after restarting this application, AltTab captures its window without any problem. I think this happens rarely. | ||||
let allWindows = CGWindow.windows(.optionAll) | ||||
guard let winApp = (allWindows.first { app.processIdentifier == $0.ownerPID() | ||||
&& $0.isNotMenubarOrOthers() | ||||
&& $0.id() != nil | ||||
&& $0.bounds() != nil | ||||
&& CGRect(dictionaryRepresentation: $0.bounds()!)!.width > 0 | ||||
&& CGRect(dictionaryRepresentation: $0.bounds()!)!.height > 0 | ||||
}) else { | ||||
Comment on lines
+127
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said in the other ticket, you're re-implementing a mirror to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I add these codes just to filter those apps which have no windows(e.g. com.apple.universalcontroller). This is for those apps launched before AltTab. At that time, we haven't go to |
||||
return false | ||||
} | ||||
return true | ||||
} else { | ||||
// Because we only add the application when we receive the didActivateApplicationNotification. | ||||
// So here is actually the handling for the case wasLaunchedBeforeAltTab=false. For applications where wasLaunchedBeforeAltTab=true, the majority of isActive is false. | ||||
// The reason for not using axUiElement.windows() here as a way to determine if it is a window application is that | ||||
// When we receive the didActivateApplicationNotification notification, the application may still be loading and axUiElement.windows() will throw an exception | ||||
// So we use isActive to determine if it is a window application, even if the application is not frontmost, isActive is still true at this time | ||||
return true; | ||||
} | ||||
} | ||||
|
||||
private static func isNotXpc(_ app: NSRunningApplication) -> Bool { | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import Cocoa | ||
/** | ||
* we use this window to help us switch to another space | ||
*/ | ||
class HelperWindow: NSWindow { | ||
var canBecomeKey_ = true | ||
override var canBecomeKey: Bool { canBecomeKey_ } | ||
convenience init() { | ||
self.init(contentRect: .zero, styleMask: [.borderless], backing: .buffered, defer: false) | ||
setupWindow() | ||
} | ||
|
||
private func setupWindow() { | ||
isReleasedWhenClosed = false | ||
hidesOnDeactivate = false | ||
title = "Helper Window" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this. I just fullscreen'ed Notes.app, and AltTab shows the same thing I see.
Could you share screenshots to illustrate what you mean by "toolbar window and a main window", and generally what problem this new method fixes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before adding the
fullScreenshot
methodAfter adding the
fullScreenshot
methodMaybe it depends on machine model. I'm using MBP 16' with Apple Silicon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, it does it on my machine too (still on Catalina). Very cool then! I wonder how many apps really benefit from that nuance though. It's a cool trick, but in the case of Notes.app, it's not very necessary. Maybe some other apps have an important UI element that would get lost though. I worry a bit about the complexity/performance/potential-failure-points of created the composition vs showing the simple image, though. I think it would probably be worth it to first find an app that truly benefits from this refinement before putting it in. What do you think? Especially given that in order to do this trick, we have to rely on CGS API, which has all the problems I mentioned in other comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I also took into account the performance issues. In fact, this small issue has little impact, I just made it a little bit optimized as free time. Reminders.app, Hopper Disassembler.app, Xcode.app, Microsoft Word.app, Calendar.app also have this problem as I known.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I tried to use CGWindowListCreateImageFromArray, but it didn't work at all. Maybe I'm not using it the right way.