Skip to content
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

feat: efficient current Dock item detection and improved window management #254

Merged
merged 44 commits into from
Sep 4, 2024

Conversation

ShlomoCode
Copy link
Contributor

@ShlomoCode ShlomoCode commented Aug 14, 2024

Describe your changes

  • Get current dock item element by macOS Dock's detection (kAXSelectedChildrenChangedNotification and kAXSelectedChildrenAttribute) instead of calculating mouse position.
  • Get rid of listening for mouse events, reducing CPU usage to 0% when not interacting\hovering with the Dock.
  • Store windows in cache (SpaceWindowCacheManager.swift) by PID instead of BundleID, to deal with apps like scrcpy that not having a BundleID.
  • Adds additional logic for better window state management.
  • Addresses window switcher update inconsistencies.
  • Introduces window fade out animations and configuration options.

Ported from #214.

Related issue number(s) and link(s)

closes #190
closes #19
closes #243
closes #30
closes #54
closes #212
closes #277

Checklist before requesting a review

  • I have performed a self-review of my code
  • If this change affects core functionality, I have added a description highlighting the changes
  • I have followed conventional commit guidelines

Core Functionality Changes

If this PR modifies core functionality, please provide a brief description of the changes and their impact below:

Fixes some bugs, and significantly improves performance.

@ShlomoCode ShlomoCode force-pushed the perf branch 2 times, most recently from 72d4a3d to 2bece81 Compare August 17, 2024 21:22
@ShlomoCode ShlomoCode changed the title WORK IN PROGRESS perf: remove mouse event listening Aug 17, 2024
@ShlomoCode ShlomoCode force-pushed the perf branch 8 times, most recently from 64047ce to 83f9fe8 Compare August 20, 2024 00:56
@ShlomoCode ShlomoCode changed the title perf: remove mouse event listening perf: remove mouse listening Aug 20, 2024
@ShlomoCode ShlomoCode changed the title perf: remove mouse listening perf: remove global listener for mouse movements\clicks Aug 20, 2024
@ShlomoCode ShlomoCode changed the title perf: remove global listener for mouse movements\clicks perf: remove the global listener for mouse movements\clicks Aug 20, 2024
@ShlomoCode
Copy link
Contributor Author

@ejbills
Copy link
Owner

ejbills commented Aug 21, 2024

@ejbills I'd appreciate help with https://github.com/ejbills/DockDoor/pull/254/files#diff-017b6b046f3844ad59afc9f7acd992ad75e9b77ec27f24b4186d13130fc73f20R85

Took a look, I think we can just store the previous app name string and only allow the hide action when it differs. I believe that is what happened previously.

Can I push to this PR, or are you working on anything?

@ShlomoCode
Copy link
Contributor Author

You can push. You'll probably want to use DockObserver's lastAppUnderMouse.

@ShlomoCode
Copy link
Contributor Author

ShlomoCode commented Aug 22, 2024

I push some changes, Looks better now, But it seems like quick mouse movement doesn't trigger SwiftUI's .onHover 🤔

@ejbills
Copy link
Owner

ejbills commented Aug 22, 2024

You can push. You'll probably want to use DockObserver's lastAppUnderMouse.

I just sat down to work on this, did you already do it? The link goes to a different part of the code now, I can't find the TODO.

@ejbills
Copy link
Owner

ejbills commented Aug 22, 2024

I push some changes, Looks better now, But it seems like quick mouse movement doesn't trigger SwiftUI's .onHover 🤔

I think it's all working on my end, what isn't working for you?

@ShlomoCode
Copy link
Contributor Author

You can push. You'll probably want to use DockObserver's lastAppUnderMouse.

I just sat down to work on this, did you already do it? The link goes to a different part of the code now, I can't find the TODO.

Yes i did it

@hasansultan92
Copy link
Contributor

I assume this is not expected behavior when triggering the window switcher. The same windows are recognized in the dock tho🤔
image

- Improve clarity for mouse distance threshold logic
- Prevent accidental duplicate window display during fast mouse movements
@ejbills
Copy link
Owner

ejbills commented Sep 2, 2024

@ShlomoCode I have solved all issues.

  • Hovering back onto an application does not hide the window.
  • Quickly passing over a view no longer incorrectly displays a lingering window.
  • Lingering windows get cleaned up with a temporary mouse polling timer, resolving issues where the mouse does not enter the window preview at all.

I have played around with this, and I think it irons out all the issues we were dealing with. Please give it a test, and let me know what you think. I left some comments in the WindowDismissalContainer to explain this logic.

I am opening this for review in light of this, great work to both of us! :)

@ejbills ejbills added the enhancement New feature or request label Sep 2, 2024
@ejbills ejbills marked this pull request as ready for review September 2, 2024 23:51
@ejbills ejbills changed the title perf: remove the global listener for mouse movements\clicks feat: Optimize Dock item detection and improve window management Sep 3, 2024
@ShlomoCode ShlomoCode changed the title feat: Optimize Dock item detection and improve window management feat: optimized Dock item detection and improved window management Sep 3, 2024
@ShlomoCode ShlomoCode changed the title feat: optimized Dock item detection and improved window management feat: efficient current Dock item detection and improved window management Sep 3, 2024
@ShlomoCode
Copy link
Contributor Author

@ejbills Sounds good! I'll give it a spin and come back

@ShlomoCode
Copy link
Contributor Author

It seems that when there are several open apps adjacent to each other in the Dock, the issue still exists

CleanShot.2024-09-03.at.21.27.27.mp4

@ejbills
Copy link
Owner

ejbills commented Sep 3, 2024

It seems that when there are several open apps adjacent to each other in the Dock, the issue still exists

This is the case of point 3 on this comment: #254 (comment)

Lingering windows get cleaned up with a temporary mouse polling timer, resolving issues where the mouse does not enter the window preview at all.

The dead zone for cleaning up lingering windows is 800 px. If you move your mouse away while a window is lingering, it will hide after the distance is > 800. I actually think this is too large of an area, what do you think is a reasonable area for this? (You can play around with it on line 59 of WindowDismissalContainer.swift)

@ejbills
Copy link
Owner

ejbills commented Sep 4, 2024

I have tested this all day with no issues, I will be merging. Again, thanks a ton @ShlomoCode, this is a huge and very beneficial PR for DockDoor. Great work.

@ejbills ejbills merged commit dec7ed1 into ejbills:main Sep 4, 2024
2 checks passed
@ShlomoCode ShlomoCode deleted the perf branch September 4, 2024 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment