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

refactor: alt-tab rework #1319

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alex-ds13
Copy link
Contributor

This is a refactor of the alt-tab reconciliation. This PR gets rid of the workspace_reconciliator, hence removing another source of potential issues with WM locks.

To replace it there are two simple new functions:

  • needs_reconciliation(): checks if the window with the Show or Uncloak event is already handled by komorebi and if it is, checks if it is a window on an unfocused workspace or a hidden window on a stack container. If this is the case then it means the user alt-tabbed into some window and we need to reconcile the currently focused monitor, workspace, container and window on komorebi to the ones from the window the user just alt-tabbed into. This function returns the monitor/workspace index pair of the window in question.
  • perform_reconciliation(): this function actually performs the previously mentioned reconciliation. If the window was on a stack it makes sure to update the focused window of that stack even if that stack is on the currently focused workspace. If there was a monocle on the workspace of that window, then it toggles the monocle off so that we can give focus to the alt-tabbed window.

This also fixes 2 bugs related to the alt-tab:

  • Previously, if you alt-tabbed to an unfocused window on the currently focused workspace it would bring that window to front and put focus on it, however komorebi wouldn't know about it and would still think you had focus on the previously focused container. This is fixed with this change.
  • Previously, if you alt-tabbed to a window on another workspace that wasn't the window focused on that workspace before it would change workspaces to the correct one, however it would focus the previously focused container instead of keeping focus on the window the user just alt-tabbed into. This is fixed with this change.

I'm making this a draft since it is a big change and I'm not sure exactly of the reasons that lead you to make the workspace_reconciliator in the first place. I'm not sure if there was a specific reason to need to have the reconciliation being done on another thread with a 100ms delay with all that that entails. From my tests, this version of mine works just fine like it used to with the added benefit of the aforementioned bug fixes. I haven't encountered any issues with alt-tab yet. There was also some situations before where komorebi would think some event was an alt-tab when in fact it wasn't and it would send a notification and perform a workspace reconciliation. That also doesn't seem to be happening anymore.

In spite of my tests revealing no issues so far, I still think this should be kept as a draft for a while until at least a couple more people can test it out!

So I'm calling out for @LGUG2Z, @CtByte and any others that can help test this!

Also I've seen this discussion #1275 which is what first prompted to look into the alt-tab to fix the mentioned issue, so I would also like to ping @f34r1335 in case you are able to test this PR as well and see if it works well for you!

@LGUG2Z LGUG2Z force-pushed the master branch 7 times, most recently from 75a0506 to 7600238 Compare March 7, 2025 03:40
@LGUG2Z LGUG2Z added the 0.1.36 label Mar 9, 2025
@f34r1335
Copy link

f34r1335 commented Mar 9, 2025

Also I've seen this discussion #1275 which is what first prompted to look into the alt-tab to fix the mentioned issue, so I would also like to ping @f34r1335 in case you are able to test this PR as well and see if it works well for you!

Thanks for looking into this, I tried the latest nightly version released yesterday 0.1.35, looks like this might be for 0.1.36 so it didn't make it in yet. Will try as soon as it becomes available. Just curious if you also tested with clicking from the Windows taskbar? I'm assuming if this issue is fixed for alt+tab it would automatically fix it for clicking from the taskbar as well, but not certain.

@alex-ds13 alex-ds13 force-pushed the feat/alt-tab-rework branch 2 times, most recently from 3aa02f1 to bcf7ca5 Compare March 10, 2025 12:22
@alex-ds13
Copy link
Contributor Author

alex-ds13 commented Mar 10, 2025

Also I've seen this discussion #1275 which is what first prompted to look into the alt-tab to fix the mentioned issue, so I would also like to ping @f34r1335 in case you are able to test this PR as well and see if it works well for you!

Thanks for looking into this, I tried the latest nightly version released yesterday 0.1.35, looks like this might be for 0.1.36 so it didn't make it in yet. Will try as soon as it becomes available. Just curious if you also tested with clicking from the Windows taskbar? I'm assuming if this issue is fixed for alt+tab it would automatically fix it for clicking from the taskbar as well, but not certain.

You can use the executables from the PR build here: https://github.com/LGUG2Z/komorebi/actions/runs/13769112528?pr=1319

You can download the .exe files and replace the ones on your komorebi install folder. You probably want the one starting with komorebi-x86_64-pc-windows-msvc...

@alex-ds13 alex-ds13 force-pushed the feat/alt-tab-rework branch from bcf7ca5 to 85effe3 Compare March 10, 2025 15:57
@f34r1335
Copy link

f34r1335 commented Mar 10, 2025

You can use the executables from the PR build here: https://github.com/LGUG2Z/komorebi/actions/runs/13769112528?pr=1319

You can download the .exe files and replace the ones on your komorebi install folder. You probably want the one starting with komorebi-x86_64-pc-windows-msvc...

Thanks for the tip, I did some quick tests, seems to be working flawlessly, bravo! It also works with clicking from the taskbar as expected, and works with stackbar too, awesome.

Speaking of stackbar, let's say you have a workspace with columns layout and 4 containers, so from left to right is container 1 to 4 respectively. Using komorebic cycle-focus next, naturally it moves from container 1 to 2, 2 to 3, 3 to 4 and 4 to 1. Then run komorebic stack-all and with komorebic cycle-stack next the order is the opposite, the window goes from 1 to 4, 4 to 3, 3 to 2, 2 to 1.

Would it make sense to have it the other way? Of course the easiest thing to do currently is reverse your windows before stack-all if you want to cycle your windows in that direction specifically, just that it feels a tiny bit counterintuitive. Although I haven't tested all the other layouts, maybe the current way might benefit other layouts, any thoughts? Thanks.

@alex-ds13 alex-ds13 force-pushed the feat/alt-tab-rework branch from 85effe3 to e03eb19 Compare March 11, 2025 14:51
This commit changes the way the alt-tab reconciliation is done. It no
longer uses the `workspace_reconciliator` with the sending of
notifications. Instead there is a simple function that checks if the
shown/uncloaked window is already handled by komorebi and if it is it
will check if it is on an unfocused workspace and/or if it is an
unfocused window of a stacked container, returning the
monitor/workspace index pair of said window. If we get this pair then we
perform the reconciliation immediately by focusing the monitor,
workspace, container and window indices corresponding to that window.
@alex-ds13 alex-ds13 force-pushed the feat/alt-tab-rework branch from e03eb19 to 0578e7f Compare March 12, 2025 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants