Skip to content

Query window data entirely in hammerspoon instead of shelling out to yabai? #8

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Closed
AdamWagner opened this issue Aug 2, 2020 · 4 comments
Closed

Query window data entirely in hammerspoon instead of shelling out to yabai? #8

AdamWagner opened this issue Aug 2, 2020 · 4 comments
Labels
help wanted Help designing a solution to this issue or reviewing the PR would be appreciated!

Comments

@AdamWagner
Copy link
Owner

AdamWagner commented Aug 2, 2020 •
edited
Loading

Today I learned that hammerspoon window objects also have a frame property (alongside many others: hs.window docs).

Especially since we're we're grouping windows into stacks manually by grouping on frame.x and frame.y (due to yabai lacking a stack_id property), there's really no reason that we need to call out to yabai -m query …

(I think) this would resolve #9 .
@AdamWagner AdamWagner mentioned this issue Aug 2, 2020
Closed
@AdamWagner AdamWagner added the help wanted Help designing a solution to this issue or reviewing the PR would be appreciated! label Aug 2, 2020
AdamWagner added a commit that referenced this issue Aug 2, 2020
@AdamWagner
WIP: Working toward addressing #8 in stackline/query.lua, which repla…
665fdd3 
…ces bin/yabai-get-stacks.
AdamWagner added a commit that referenced this issue Aug 2, 2020
@AdamWagner
WIP: Working toward addressing #8 in stackline/query.lua, which repla…
6edc81b 
…ces bin/yabai-get-stacks.

The goal of this file is to eliminate the need to 'shell out' to yabai to query
window data needed to render stackline, which would address
#8. The main problem with relying
on yabai is that a 0.03s sleep is required in the yabai script to ensure that
the changes that triggered hammerspoon's window event subscriber are, in fact,
represented in the query response from yabai. There are probably secondary
downsides, such as overall performance, and specifically *yabai* performance
(I've noticed that changing focus is slower when lots of yabai queries are
happening simultaneously).

┌────────┐
│ Status │
└────────┘
We're not yet using any of the code in this file to actually render the
indiators or query ata — all of that is still achieved via the "old" methods.
However, this file IS being required by ./core.lua and runs one every window focus
event, and the resulting "stack" data is printed to the hammerspoon console.
The stack data structure differs from that used in ./stack.lua enough that it
won't work as a drop-in replacement. I think that's fine (and it wouldn't be
worth attempting to make this a non-breaking change, esp. since zero people rely
on it as of 2020-08-02.

┌──────┐
│ Next │
└──────┘
- [ ] Integrate appropriate functionality in this file into the Stack module
- [ ] Update key Stack module functions to have basic compatiblity with the new data structure
- [ ] Simplify / refine Stack functions to leverage the benefits of having access to the hs.window module for each tracked window
- [ ] Integrate appropriate functionality in this file into the Core module
- [ ] … see if there's anything left and decide where it should live

┌───────────┐
│ WIP NOTES │
└───────────┘
Much of the functionality in this file should either be integrated into
stack.lua or core.lua — I don't think a new file is needed.
Rather than calling out to the script ../bin/yabai-get-stacks, we're using
hammerspoon's mature (if complicated) hs.window.filter and hs.window modules to
achieve the same goal natively within hammerspon.
There might be other benefits in addition to fixing the problems that inspired
tracked by stackline, which will probably make it easier to implement
enhancements that we haven't even considered yet. This approach should also be
easier to maintain, *and* we get to drop the jq dependency!
@AdamWagner AdamWagner mentioned this issue Aug 4, 2020
Merged
@AdamWagner
Copy link
Owner Author

AdamWagner commented Aug 4, 2020 •
edited
Loading

This approach is looking promising.

MUCH better performance when switching the focused window within a stack. Also, tracking the state of hammerspoon window objects instead of ingesting json and building our own window objects cleans up a lot of stuff.

Also, I discovered hs.timer.delayed which, ahem: "Is a specialized timer objects to coalesce processing of unpredictable asynchronous events into a single callback". That's exactly what I had been looking for to fix the excessive re-stacking issue I had when using yabai signals, and still had to some degree when using hammerspoon's window filters.

The major problem I ran into is that the premise (query window data entirely in hammerspoon) isn't possible: There's a key bit of info that only (as far as I can tell) exists in yabai: window.stack-index. The stack index stores the position of a window within a stack, and is critical for drawing the indicators in the correct order. If anyone has an idea for how to infer this without querying yabai, I'm all ears.

Hammerspoon window filters order windows by last focused by default, which causes the indicators to appear as if they aren't changing when they're actually changing and reordering (I wasted a lot of time trying to solve the wrong problem there).

Unless we find a way to infer stack index without yabai, I'm querying yabai just once on space switch, which is better than querying it on every single window event.

@AdamWagner AdamWagner mentioned this issue Aug 5, 2020
Closed
@AdamWagner
Copy link
Owner Author

This last push improves performance & reliability, and even some new functionality:

  • Hammerspoon is now responsible for querying and processing macOS window data. Hammerspoon's ability to coalesce the swarm of asynchronous change events that were causing your fans to spin up is a major improvement over calling yabai -m query --windows --space … dozens of times a minute. The move also made it possible to take a more traditional OOP approach, which has made tracking and mutating all of this desktop state a bit simpler. Unfortunately, it's still necessary to call out yabai, as it's the the keeper of each window's stack-index — a key bit of info that, afaict, is neither available elsewhere nor inferrable. That said, yabai is invoked much less frequently in this update.
  • In addition to only updating data when it's actually necessary, special attention has been given to changing focus within a stack: the POC blew away all of the UI state and _regenerated it from scratch … every … time … a window gained/lost focus. That approach is easier to think about, but it's far too slow to be useful. In this version, indicators should be snappy when changing focus :)

There's also some fun new functionality:

  • Stack indicators are always positioned on the side of the window that's closest to the edge of the screen. This allows for tight window_gaps, — even with showIcons enabled.
  • Magic numbers are less entangled in the implementation details (though it's still pretty bad) — and a few of them have even been abstracted into easy-to-mess with configuration settings. The new config.lua file isn't that exciting yet (it's mostly boilerplate), but I think the next update will bring the much-needed configuration mojo.

@AdamWagner
Copy link
Owner Author

AdamWagner commented Aug 16, 2020 •
edited
Loading

Update 2020-08-16

Changes

  • Support stack focus "events"! Now, a stack takes on a new look when all windows become unfocused, with the last-active window distinct from the rest. This required a fair bit more complexity than expected, but is unavoidable (I think). There's a minor, barely noticeable performance hit, too (not yet a problem, tho).
  • Centralized indicator config settings & consistent "current style" retrieval. Reduced reliance on magic numbers (indicator style is more purely from user config settings now).
  • Store a reference to the stack on each window, so any window can easily call stack methods. This allowed redrawOtherAppWindows() to move into the window class, where it's less awkward.
  • Resolved bug in which unfocused same-app windows would 'flash focus' briefly

Multi-monitor support is still a ?

Stacks refresh on every space/monitor change, wasting resources shelling out to yabai & redrawing all indicators from scratch.

Instead, it might better to update our data model to store: screens[] → spaces[] → stacks[] → windows[] … and then only update on window change events.

AdamWagner added a commit that referenced this issue Aug 22, 2020
@AdamWagner
Feature query windows via hammerspoon (#18)
a6c37e7 
* format: auto-formatted per luaformat config. Functions folded with 'set fdm=marker' in vim

* WIP: Working toward addressing #8 in stackline/query.lua, which replaces bin/yabai-get-stacks.

The goal of this file is to eliminate the need to 'shell out' to yabai to query
window data needed to render stackline, which would address
#8. The main problem with relying
on yabai is that a 0.03s sleep is required in the yabai script to ensure that
the changes that triggered hammerspoon's window event subscriber are, in fact,
represented in the query response from yabai. There are probably secondary
downsides, such as overall performance, and specifically *yabai* performance
(I've noticed that changing focus is slower when lots of yabai queries are
happening simultaneously).

┌────────┐
│ Status │
└────────┘
We're not yet using any of the code in this file to actually render the
indiators or query ata — all of that is still achieved via the "old" methods.
However, this file IS being required by ./core.lua and runs one every window focus
event, and the resulting "stack" data is printed to the hammerspoon console.
The stack data structure differs from that used in ./stack.lua enough that it
won't work as a drop-in replacement. I think that's fine (and it wouldn't be
worth attempting to make this a non-breaking change, esp. since zero people rely
on it as of 2020-08-02.

┌──────┐
│ Next │
└──────┘
- [ ] Integrate appropriate functionality in this file into the Stack module
- [ ] Update key Stack module functions to have basic compatiblity with the new data structure
- [ ] Simplify / refine Stack functions to leverage the benefits of having access to the hs.window module for each tracked window
- [ ] Integrate appropriate functionality in this file into the Core module
- [ ] … see if there's anything left and decide where it should live

┌───────────┐
│ WIP NOTES │
└───────────┘
Much of the functionality in this file should either be integrated into
stack.lua or core.lua — I don't think a new file is needed.
Rather than calling out to the script ../bin/yabai-get-stacks, we're using
hammerspoon's mature (if complicated) hs.window.filter and hs.window modules to
achieve the same goal natively within hammerspon.
There might be other benefits in addition to fixing the problems that inspired
tracked by stackline, which will probably make it easier to implement
enhancements that we haven't even considered yet. This approach should also be
easier to maintain, *and* we get to drop the jq dependency!

* WIP: basic working state with lots of cruft to clean up and edge cases to fix

* Bring over some changes from #13

* Bring over indicator styling improvements from #13

* Move self.colorOpts into Window:drawIndicator() so that the latter can be called in place of Window:process() to redraw a window.

* A small bit of much-needed cleanup

* Basic functionality working well. Parameterized display options. Indicators display on left/right edge of window based on which side of the screen the window resides. Known bug: changes broke multiple stacks on the same space.

* Fixed issue where only 1 of N stacks responded to focus events. Moved all stack-is-occluded functionality from Query to StackMgr and Stack modules.

* Workaround hammerspoon bug (Hammerspoon/hammerspoon#2400) to ensure indicators update when switching between windows of the same app.

* Fold Window:getScreenSide()

* Modifying existing indicators on focus change is MUCH faster than destroying them & rendering from scratch.

* Cleanup (one of the) utils.lua files

* Add a status update to readme describing recent changes

* Included a bunch of cleanup/reorg changes missed in last commit

* Fix path to dash shell (again)

* Add hs._asm.undocumented.spaces dependency

* Remove unused StackConfig:handleSignal() method (it needs to be available outside of class)

* Cleanup comments & delete unused bluclass.lua

* Don't use hs._asm.undocumented.spaces. Uncomment window.filter.notInCurrentSpace which depends on it.'

* Fill gap left behind by hs._asm.undocumented.spaces with hs.spaces.watcher

* Increase threshold in window:getSide() to fix issue w/ large yabai padding values

* Cleanup comments in query.lua

* Removed hs._asm.undocumented.spaces from list of dependencies

* Delete unused utils, consolidate utils into single file.

* Track stack focus state, indicate last-focused window in unfocused stack, and centralize indicator config settings & retrieval.

* Fix misleading comment

* Remove garbage characters in comment

* Fix error stackline/window.lua:95: attempt to perform arithmetic on a nil value (field 'stackIdx') by checking for stackIdx > 0

* Remove completed TODO comments, try to keep TODO comments on single line to work well with leasot / rg

* Enable keybindings in init.lua by returning stackConfig & stackManager instances from main stackline.lua
Remove keybinding from config.lua & move to readme.

* Fix #21 - Limit stack left/right edge to screen boundary so it doesn't go off screen

* Update version to 0.1.50
@AdamWagner
Copy link
Owner Author

Closed by #18

# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
help wanted Help designing a solution to this issue or reviewing the PR would be appreciated!
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@AdamWagner