fix: resolve UI lag and high CPU usage during drag/resize#109
Merged
pablopunk merged 3 commits intopablopunk:mainfrom Mar 29, 2026
Merged
fix: resolve UI lag and high CPU usage during drag/resize#109pablopunk merged 3 commits intopablopunk:mainfrom
pablopunk merged 3 commits intopablopunk:mainfrom
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces throttled/batched mouse updates and pending-state in MouseTracker with final-flush behavior; reduces redundant move/resize calls via last-applied caches; simplifies input/event handling and enums. WindowManager is condensed, adds Changes
Sequence Diagram(s)sequenceDiagram
participant Mouse as Mouse Events
participant Tracker as MouseTracker
participant WindowMgr as WindowManager
participant System as Accessibility/OS
Mouse->>Tracker: mouseMoved(location)
Tracker->>Tracker: set pendingMouseLocation, update lastUpdateTime
alt throttle interval elapsed
Tracker->>WindowMgr: compute origin/size from pending location
WindowMgr->>System: request AX/CG move or resize
System-->>WindowMgr: ack
WindowMgr-->>Tracker: report applied origin/size
else within interval
Tracker-->>Tracker: defer until flush
end
Mouse->>Tracker: stopTracking
Tracker->>Tracker: flushPendingMouseUpdate (final)
Tracker->>WindowMgr: final move/resize (if not approx-equal)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
Swift Shift/src/Manager/WindowManager.swift (1)
3-3: Single-line struct reduces readability.While functionally correct, condensing the struct to one line makes it harder to scan property names. Consider expanding for maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Swift` Shift/src/Manager/WindowManager.swift at line 3, The one-line declaration of the WindowBounds struct (symbol: WindowBounds with properties topLeft, topRight, bottomLeft, bottomRight) reduces readability; expand it into a multi-line struct declaration with each property on its own line and proper indentation (e.g., declare struct WindowBounds { let topLeft: NSPoint; let topRight: NSPoint; let bottomLeft: NSPoint; let bottomRight: NSPoint }) so property names are easy to scan and maintain, and consider adding a brief comment or initializer if needed for clarity.Swift Shift/src/Manager/MouseTracker.swift (1)
120-124: Clarify the key code range intent with accurate documentation.The range
36...126does not exclude Escape (53) or represent "Return through Power key" (126 is the Up arrow key). This range actually includes modifiers (Command, Shift, Option, Control), function keys, arrows, and navigation keys—essentially all non-typing keys (letters/numbers are 0-35). The intent appears to be detecting any key press that should pause window tracking, not arbitrary. However, the purpose should be documented in code. Consider adding a comment explaining that the range targets all keys except regular typing keys, or use a named constant with clear intent (e.g.,let nonTypingKeyCodes = Set<UInt16>(36...126) // All keys except letter/number typing). Note: ShortcutsManager.swift has a similar range with an inaccurate comment stating "excluding modifiers"—correct that as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Swift` Shift/src/Manager/MouseTracker.swift around lines 120 - 124, The checkForKeyPresses() function currently uses the opaque range 36...126 which incorrectly suggests it excludes modifiers and mislabels keys — update the code to introduce a named constant (e.g., nonTypingKeyCodes or nonAlphanumericKeyRange) with a clarifying comment that documents the intent (e.g., "keys intended to pause tracking: non-alphanumeric/navigation/modifier keys; this range includes Escape, arrows, function and modifier keys") and replace the magic literal in checkForKeyPresses() with that constant; also fix the inaccurate comment in ShortcutsManager.swift to reflect that the range includes modifiers and navigation keys rather than excluding them so future readers understand the exact behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Swift` Shift/src/Manager/MouseTracker.swift:
- Line 71: The timer closure force-unwraps self (`self!`) which can crash if
MouseTracker is deallocated; update the scheduledTimer callback to safely unwrap
the weak self (e.g., use `guard let self = self else { return }`) and then call
stopTracking(for: self.currentAction), referencing the existing
trackingTimer/trackingTimeout, stopTracking(_:), currentAction and MouseTracker
identifiers.
- Line 20: handleSpaceChange currently forces tracking to resume by doing
isTracking = false; isTracking = true; change it to preserve intentional pauses:
detect whether tracking was actively running before the space change (e.g., save
a previousIsTracking boolean) and only restart tracking if it was running and
not explicitly paused; alternatively, call the existing
pauseTracking()/resumeTracking() helpers and respect any paused flag set by
pauseTracking() so the handler does not override an intentional pause (update
handleSpaceChange to consult pauseTracking()/isPaused or previousIsTracking
instead of unconditionally toggling isTracking).
In `@Swift` Shift/src/Manager/WindowManager.swift:
- Around line 38-43: The getWindow(from:) function force-casts the parent
attribute (p as! AXUIElement) which can crash if kAXParentAttribute returns nil
or an unexpected type; change this to safely unwrap and type-check the parent
(e.g., guard/if let pElem = p as? AXUIElement) before recursing, and return nil
or continue gracefully when the parent is not an AXUIElement so malformed AX
hierarchies cannot crash the app.
- Around line 27-36: getTopWindowAtCursorUsingCGWindowList currently finds a
CoreGraphics window whose bounds contain the cursor but then returns wList.first
from the app's AX windows (kAXWindowsAttribute), which can be the wrong window
for multi-window apps; instead iterate the returned AXUIElement windows (wList)
and for each call AXUIElementCopyAttributeValue for kAXPositionAttribute and
kAXSizeAttribute (or kAXFrame if available), extract AXValue into CGPoint/CGSize
via AXValueGetValue, build a CGRect and check CGRect.contains(mouseLocation) and
return the matching AXUIElement; if none match, fall back to wList.first as
before and keep existing calls to getNSApplication and
PreferencesManager.isAppIgnored to skip ignored apps.
---
Nitpick comments:
In `@Swift` Shift/src/Manager/MouseTracker.swift:
- Around line 120-124: The checkForKeyPresses() function currently uses the
opaque range 36...126 which incorrectly suggests it excludes modifiers and
mislabels keys — update the code to introduce a named constant (e.g.,
nonTypingKeyCodes or nonAlphanumericKeyRange) with a clarifying comment that
documents the intent (e.g., "keys intended to pause tracking:
non-alphanumeric/navigation/modifier keys; this range includes Escape, arrows,
function and modifier keys") and replace the magic literal in
checkForKeyPresses() with that constant; also fix the inaccurate comment in
ShortcutsManager.swift to reflect that the range includes modifiers and
navigation keys rather than excluding them so future readers understand the
exact behavior.
In `@Swift` Shift/src/Manager/WindowManager.swift:
- Line 3: The one-line declaration of the WindowBounds struct (symbol:
WindowBounds with properties topLeft, topRight, bottomLeft, bottomRight) reduces
readability; expand it into a multi-line struct declaration with each property
on its own line and proper indentation (e.g., declare struct WindowBounds { let
topLeft: NSPoint; let topRight: NSPoint; let bottomLeft: NSPoint; let
bottomRight: NSPoint }) so property names are easy to scan and maintain, and
consider adding a brief comment or initializer if needed for clarity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42a3a4e8-c5d7-410d-ad05-dd9253ffc2ff
📒 Files selected for processing (2)
Swift Shift/src/Manager/MouseTracker.swiftSwift Shift/src/Manager/WindowManager.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Swift` Shift/src/Manager/MouseTracker.swift:
- Line 33: forceResetTracking currently only updates initialMouseLocation which
leaves initialWindowLocation and windowSize stale and causes jumps; update
forceResetTracking so that when currentAction != .none you also read the current
window geometry from the tracked window (e.g. use the window/frame accessor used
elsewhere in this file) and assign initialWindowLocation = currentFrame.origin
and windowSize = currentFrame.size (guard/unwrap the tracked window the same way
other methods do) so subsequent deltas are applied against up-to-date baselines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65ebd748-fdbf-47ba-b783-311b48cb482b
📒 Files selected for processing (2)
Swift Shift/src/Manager/MouseTracker.swiftSwift Shift/src/Manager/WindowManager.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Swift Shift/src/Manager/WindowManager.swift
pablopunk
reviewed
Mar 25, 2026
| var r: AnyObject?; AXUIElementCopyAttributeValue(element, kAXRoleAttribute as CFString, &r) | ||
| if r as? String == kAXWindowRole { return element } | ||
| var p: AnyObject?; AXUIElementCopyAttributeValue(element, kAXParentAttribute as CFString, &p) | ||
| if let parent = p as? AXUIElement { return getWindow(from: parent) } |
Owner
There was a problem hiding this comment.
I think coderabbit was wrong here, I clone it locally and Xcode does not build. Let's change it back:
if let parent = p as? AXUIElement { ... }to:
if let parent = p { return getWindow(from: parent as! AXUIElement) }
pablopunk
requested changes
Mar 25, 2026
Owner
pablopunk
left a comment
pablopunk
left a comment
There was a problem hiding this comment.
Thanks for the PR! I have not run into CPU issues so I can't really tell if this solves the issue or not, but the solution makes sense to me. I left a small comment; after that, ready to merge
gaorfg-bit
pushed a commit
to gaorfg-bit/SwiftShift
that referenced
this pull request
Mar 25, 2026
pablopunk
approved these changes
Mar 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements 120Hz event coalescing and AX call deduplication to fix performance issues on high-refresh rate displays and gaming mice.
Summary by CodeRabbit
Bug Fixes
Performance