-
-
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 1 commit
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,6 +14,14 @@ 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() | ||||
|
@@ -32,10 +40,11 @@ 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) | ||||
} | ||||
} | ||||
} | ||||
|
@@ -115,7 +124,13 @@ class Applications { | |||
// 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.title() != nil}) else { | ||||
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 | ||||
|
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" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,44 @@ class Window { | |
debugPrint("Adding app-window", title ?? "nil", application.runningApplication.bundleIdentifier ?? "nil") | ||
} | ||
|
||
// we init a window when this window is not at current active space | ||
init(_ application: Application, _ cgWindow: CGWindow) { | ||
self.application = application | ||
self.title = bestEffortTitle(cgWindow.title()) | ||
self.cgWindowId = cgWindow.id()! | ||
let bounds = CGRect(dictionaryRepresentation: cgWindow.bounds()!) | ||
self.size = bounds!.size | ||
self.position = bounds!.origin | ||
updatesWindowSpace() | ||
if CGSSpaceGetType(cgsMainConnectionId, self.spaceId) == .fullscreen { | ||
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. Another worry with using CGS APIs instead of AX APIs is that I don't know if there are delays when calling it. AX has lots of problems like it will block for a long time if the app is busy or the system is busy. We have systems to retry on a loop to eventually get the info. It tooks years and is pretty elaborate and deals with all sorts of edge-cases. Here we are making a simple/naive call to some CGS API. We are making the call on the main thread, which means that any delay from that API will make AltTab UI stutter/freeze. The AX calls are always happening on another thread so that AltTab never freezes. It's a nightmare of concurrent programming but it was the only way to get rid of lag/freeze/unresponsiveness, and it's the reason AltTab is (mostly) very responsive, regardless of how laggy the OS is at responding to its APIs. |
||
self.isFullscreen = true | ||
} else { | ||
self.isFullscreen = false | ||
} | ||
self.isMinimized = false | ||
if !Preferences.hideThumbnails { | ||
refreshThumbnail() | ||
} | ||
application.removeWindowslessAppWindow() | ||
debugPrint("Adding app-window", title ?? "nil", application.runningApplication.bundleIdentifier ?? "nil") | ||
} | ||
|
||
func getAxUiElementAndObserveEvents() throws { | ||
guard let windows = try self.application.axUiElement?.windows(), windows.count > 0 else { | ||
debugPrint("try to getAxUiElementAndObserveEvents nil", self.application.runningApplication.bundleIdentifier, self.title, self.spaceId) | ||
return | ||
} | ||
for win in windows { | ||
if try cgWindowId == win.cgWindowId() { | ||
self.axUiElement = win | ||
self.isMinimized = try self.axUiElement.isMinimized() | ||
self.observeEvents() | ||
return | ||
} | ||
} | ||
debugPrint("try to getAxUiElementAndObserveEvents failed", self.application.runningApplication.bundleIdentifier, self.title, self.spaceId) | ||
} | ||
|
||
deinit { | ||
debugPrint("Deinit window", title ?? "nil", application.runningApplication.bundleIdentifier ?? "nil") | ||
} | ||
|
@@ -83,13 +121,13 @@ class Window { | |
} | ||
|
||
func refreshThumbnail() { | ||
guard let cgImage = cgWindowId.screenshot() else { return } | ||
guard let cgImage = cgWindowId.screenshot(self) else { return } | ||
thumbnail = NSImage(cgImage: cgImage, size: NSSize(width: cgImage.width, height: cgImage.height)) | ||
thumbnailFullSize = thumbnail!.size | ||
} | ||
|
||
func close() { | ||
if isWindowlessApp { return } | ||
if isWindowlessApp || self.axUiElement == nil { return } | ||
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 mean we can't close a window if we haven't visited its Space? That not ideal... 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. It's a pity that HyperSwitch also can't do that either. 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 just tested and it can. I did this to test:
How can HS close the window without the AX reference? 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. On macOS 12.3.1, I did the steps same to you, but HyperSwitch just took me to that space and didn't close the window. Maybe the private APIs they used also broken. |
||
BackgroundWork.accessibilityCommandsQueue.asyncWithCap { [weak self] in | ||
guard let self = self else { return } | ||
if self.isFullscreen { | ||
|
@@ -102,7 +140,7 @@ class Window { | |
} | ||
|
||
func minDemin() { | ||
if isWindowlessApp { return } | ||
if isWindowlessApp || self.axUiElement == nil { return } | ||
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. See comment above |
||
BackgroundWork.accessibilityCommandsQueue.asyncWithCap { [weak self] in | ||
guard let self = self else { return } | ||
if self.isFullscreen { | ||
|
@@ -119,7 +157,7 @@ class Window { | |
} | ||
|
||
func toggleFullscreen() { | ||
if isWindowlessApp { return } | ||
if isWindowlessApp || self.axUiElement == nil { return } | ||
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. See comment above |
||
BackgroundWork.accessibilityCommandsQueue.asyncWithCap { [weak self] in | ||
guard let self = self else { return } | ||
self.axUiElement.setAttribute(kAXFullscreenAttribute, !self.isFullscreen) | ||
|
@@ -145,13 +183,43 @@ class Window { | |
} | ||
} | ||
|
||
func gotoSpace(_ spaceId: CGSSpaceID) { | ||
CGSMoveWindowsToManagedSpace(cgsMainConnectionId, [App.app.helperWindow.windowNumber] as NSArray, spaceId) | ||
App.app.showHelperWindow() | ||
} | ||
|
||
func focus() { | ||
if isWindowlessApp { | ||
if let bundleID = application.runningApplication.bundleIdentifier { | ||
NSWorkspace.shared.launchApplication(withBundleIdentifier: bundleID, additionalEventParamDescriptor: nil, launchIdentifier: nil) | ||
} else { | ||
application.runningApplication.activate(options: .activateIgnoringOtherApps) | ||
} | ||
} else if self.axUiElement == nil { | ||
// the window is in other space, we can not get AXUIElement before we go to it's space | ||
// when we want to focus the window, it means we want to go to the space and focus the window | ||
// so we just go to that space by using helperWindow, then we get AXUIElement and focus the window | ||
gotoSpace(self.spaceId) | ||
do { | ||
retryAxCallUntilTimeout { [weak self] in | ||
guard let self = self else { return } | ||
try self.getAxUiElementAndObserveEvents() | ||
if self.axUiElement == nil { | ||
// retry until we get axUiElement | ||
throw AxError.runtimeError | ||
} | ||
BackgroundWork.accessibilityCommandsQueue.asyncWithCap { [weak self] in | ||
guard let self = self else { return } | ||
var psn = ProcessSerialNumber() | ||
GetProcessForPID(self.application.pid, &psn) | ||
_SLPSSetFrontProcessWithOptions(&psn, self.cgWindowId, .userGenerated) | ||
self.makeKeyWindow(psn) | ||
self.axUiElement.focusWindow() | ||
} | ||
} | ||
} catch { | ||
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 don't think exceptions can bubble up to here. 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. In fact, during the testing process, it was found that this happens. Because of the animation of the space switch, it causes us not to get the |
||
debugPrint("can not setUpMissingInfoForCurrentWindow for", self.application.runningApplication.bundleIdentifier) | ||
} | ||
} else { | ||
// macOS bug: when switching to a System Preferences window in another space, it switches to that space, | ||
// but quickly switches back to another window in that space | ||
|
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.