Feature/fuzzy stacked win detection

[AdamWagner]

Support iTerm (and other apps that constrict window dimensions) by rounding window frames to (configurable) fuzzFactor before equality comparison.

Addresses need in #32.

Built on top of #36

[Coobaha]

hey @AdamWagner i tested this branch, here is my diff to make it work

fixes fuzz rounding and clicks were registered only on top of the icon, probably magic offset should be calculated from config variables.

Index: stackline/window.lua
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- stackline/window.lua	(revision 3876f8de906e089f74f9a968dcd5fefeb1020195)
+++ stackline/window.lua	(date 1600054643343)
@@ -361,11 +361,11 @@
 
     local fuzzFactor = stackline.config:get('frameFuzz')
     local roundToFuzzFactor = u.partial(u.roundToNearest, fuzzFactor)
+	print(hsWin:title())
 
     local f = hsWin:frame():floor():gettable()
     local ff = u.map(f, roundToFuzzFactor)
 
-    print(hsWin:title(), 'round to neartest 5:', u.roundToNearest(5, 33))
     u.p(f)
     u.p(ff)
 
Index: lib/utils.lua
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lib/utils.lua	(revision 3876f8de906e089f74f9a968dcd5fefeb1020195)
+++ lib/utils.lua	(date 1600055571234)
@@ -366,7 +366,10 @@
     end
 end -- }}}
 
-function utils.roundToNearest(roundNum, num) -- {{{
+function utils.roundToNearest(num, roundNum) -- {{{
+    if num < roundNum then
+        return 0
+    end
     if num % roundNum >= roundNum / 2 then
         return num - num % roundNum + roundNum
     else
Index: stackline/stackline.lua
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- stackline/stackline.lua	(revision 3876f8de906e089f74f9a968dcd5fefeb1020195)
+++ stackline/stackline.lua	(date 1600055571237)
@@ -42,7 +42,7 @@
         showIcons = true,
         enableTmpFixForHsBug = true,
 
-        -- Window frames are rounded to fuzzFactor before equality comparison. 
+        -- Window frames are rounded to fuzzFactor before equality comparison.
         -- ~30 is needed to support stacks w/ iTerm
         frameFuzz = 30,
     }
@@ -106,12 +106,4 @@
     stackline.refreshClickTracker()
 end):start() -- }}}
 
--- Delayed start (stackline module needs to be loaded globally before it can reference its own methods)
--- TODO: Add instructions to README.md to call stackline:start(userPrefs) from init.lua, and remove this.
-hs.timer.doUntil(function() -- {{{
-    return stackline.manager
-end, function()
-    stackline.start()
-end, 0.1) -- }}}
-
 return stackline
Index: stackline/stack.lua
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- stackline/stack.lua	(revision 3876f8de906e089f74f9a968dcd5fefeb1020195)
+++ stackline/stack.lua	(date 1600055571228)
@@ -20,7 +20,7 @@
     end, -- }}}
 
     frame = function(self) -- {{{
-        -- All stacked windows have the same dimensions, 
+        -- All stacked windows have the same dimensions,
         -- so the 1st Hs window's frame is ~= to the stack's frame
         -- TODO: Incorrect when the 1st window has min-size < stack width. See ./query.lua:104
         return self.windows[1]._win:frame()
@@ -69,10 +69,12 @@
     end, -- }}}
 
     getWindowByPoint = function(self, point) -- {{{
+		local p = hs.geometry.copy(point)
+		p.y = point.y - 22
         local foundWin = u.filter(self.windows, function(w)
             local indicatorEls = w.indicator:canvasElements()
             local wFrame = hs.geometry.rect(indicatorEls[1].frame)
-            return point:inside(wFrame)
+            return p:inside(wFrame)
         end)
 
         if #foundWin > 0 then

[AdamWagner]

Thanks @Coobaha !

I have 3 questions about the diff. I'll start with the trickiest one :-)

…fixes fuzz rounding

Function definition

The diff swaps argument order in the function definition, but not at the call location. This change makes the number to round the first argument, and the amount to round by the second argument.

I take responsibility for the crappy argument naming here :( The purpose of the args was not obvious. But since you say this diff works for you… I'm very curious to know how that's possible :-)

-- lib/utils.lua:366
-function utils.roundToNearest(roundNum, num) -- {{{
+function utils.roundToNearest(num, roundNum) -- {{{
+    if num < roundNum then
+        return 0
+    end
     if num % roundNum >= roundNum / 2 then
         return num - num % roundNum + roundNum
     else

Call location

In window.lua, u.roundToNearest() is still called with fuzzFactor as the 1st arg, not the 2nd as it is redefined above.

local roundToFuzzFactor = u.partial(u.roundToNearest, fuzzFactor)

Here we create a new function by partially applying fuzzFactor (the amount to round by). But since the diff above changes u.roundToNearest to take the number to round as the first argument, I don't see how this would work (?)

local ff = u.map(f, roundToFuzzFactor)

Here we apply our new function to make a 'fuzzy frame' by rounding each value in the window.frame table (e.g., { h = 1012, w = 1078, x = 45, y = 68 }) by the fuzz factor. Again, the changes in the diff mean that each window frame value would be interpreted as the amount to round by, not the number to round.

Maybe I'm missing something? Or there are changes that didn't get included in the diff for some reason?

---

…clicks were registered only on top of the icon

This second question is a much simpler, I think :-)

local p = hs.geometry.copy(point)
+		p.y = point.y - 22

Is this change related to fuzzy window matching? Or, does make the clickable area of the window indicators bigger, so that it's easier to focus a window by clicking its icon?

If it's related to fuzzy window matching, then I don't understand it.

If it makes the clickable area of the window indicators bigger – then that's a neat idea! It's not essential for making stacklinework with apps like iTerm, though, right? Regardless, I've also wished that the click targets were bigger, so thanks for this!

I don't quite understand why shifting point.y down by 22 would make the click target bigger overall, though.

---

probably magic offset should be calculated from config variables

Made it to question number three!

I totally agree that fuzzyFactor should be configurable. For fuzzy window matching, the fuzzFactor is read from config.fuzzFactor. There isn't yet a good way to pass configuration into the stackline constructor (this will come in #33), but it's at least on the path to being easily configurable.

But I think maybe you were referring to the p.y = point.y - 22 in getWindowByPoint(), which (again, I think) makes the indicator click targets bigger? If so, I'm not sure I agree that 'click-target-embiggening' needs to be configurable. People probably just want to be able to roughly point at an indicator & have the expected window focused, right?

---

… I'm really sorry if I've just totally misunderstand a lot of things in your diff, but I'd like to understand :)

[Coobaha]

hey @AdamWagner my diff is very random, i was hacking around to make it work without really focusing on proper code :) Please pardon me if it confused you!

I will test first case and will tell exact reasons why i swapped it soon.

Regarding second question: basically icons very capturing clicks only when top part of icon was clicked. But configurable and magic number I only meant my -22px offset to shift clickable area. I bet that it can be calculated from other variables and logic of placing click indicators.

[AdamWagner]

@Coobaha I totally get it. Sorry for the homework assignment ;-)

The thing is, though, you shouldn't need to make any changes. It should "just work", hehe. So if it doesn't, then I've probably made a mistake somewhere.

Regarding second question: basically icons very capturing clicks only when top part of icon was clicked. But configurable and magic number I only meant my -22px offset to shift clickable area. I bet that it can be calculated from other variables and logic of placing click indicators.

Odd, I cannot reproduce this behavior. When I click the bottom 1 - 2 pixels of an icon, the expected window focuses. Maybe we can revisit this icon-clicking issue after this branch is merged into master just to be sure we're testing the same code? This PR is currently up-to-date with my local version, btw.

[Coobaha]

So issue with stack detection is happening when yabai space has padding

Output with padding

2020-09-19 07:16:53: Default (bash)
2020-09-19 07:16:53: {
 h = 1392,
 w = 2538,
 x = 10,
 y = 32
}
2020-09-19 07:16:53: {
 h = 1380,
 w = 2550,
 x = 0,
 y = 30
}
2020-09-19 07:16:53: EventViewer
2020-09-19 07:16:53: {
h = 1398,
w = 2540,
x = 10,
y = 32
}
2020-09-19 07:16:53: {
h = 1410,
w = 2550,
x = 0,
y = 30
}

Output without padding

2020-09-19 07:18:57: Default (bash)
2020-09-19 07:18:57: {
  h = 1417,
  w = 2560,
  x = 0,
  y = 23
}
2020-09-19 07:18:57: {
  h = 1410,
  w = 2550,
  x = 0,
  y = 30
}
2020-09-19 07:18:57: EventViewer
2020-09-19 07:18:57: {
  h = 1417,
  w = 2560,
  x = 0,
  y = 23
}
2020-09-19 07:18:57: {
  h = 1410,
  w = 2550,
  x = 0,
  y = 30
}

iTerm height is different in this case 😢

This is how clicking behaves for me in your branch

[Coobaha]

Maybe stackId should be x|y only? Not sure if colliding stacks (when xy is equal but heights or widths are different) is possible or needed to be treated uniquely in this case..

[AdamWagner]

Thanks for the update @Coobaha

Using x|y would mostly work, but breaks when using zoom-parent on a window near a stack. (Zoom parent can make the top-left corner of a window coincident with a stack of which it is not a member). If you don't use zooming, this wouldn't be an issue. Maybe using just x|y could be a config option, then, but I don't want to make it the default.

So, I totally rewrote how windows are grouped into stacks today.

And then scrapped it ;-)

The experimental change made everything way more complicated and led to much worse results (and non-deterministic bugs). Somewhat frustrated, I took a long shot by increasing the frameFuzz value to 200 (from a mere 15) and making the rounding function more simple/predictable. Surprisingly, the results from this were way better. It's pretty resilient to minor differences in window size (the "iTerm" issue), and doesn't seem to cause any negative side effects (that I've seen).

The change is live here if you want to check it out.

---

PS – thanks for sharing the screen recording of clicking. Am I interpreting it correctly that you're able to click anywhere on an icon to focus a window now?

[Coobaha]

with my changes (-22px) yea, click works just fine. Gif is without offset logic. I was clicking 2 and 4 icons top part, when 1 and 3 icon were clicked clicks were captured by 2 and 4 icons

I will check the recent changes later, kudos for that 👍

[AdamWagner]

@Coobaha any feedback on the updated fuzzy frame matching?
FYI, I submitted an FR for yabai to provide the ability to query stacks directly. If added, this would resolve this challenge once and for all ;-)
koekeishiya/yabai#672

[AdamWagner]

@Coobaha Just following up to see if you've had a chance to test out the latest version.

And no worries if you have other things going on … I'm just checking. I think the basic changes here will be merged as part of #45 anyway.

[Coobaha]

Hey, I tested it around when you've made the changes but forgot to reply back 🙈 It was working fine, but I was not using stacks/stackline recently - so my testing was not that deep.

[AdamWagner]

Great to hear, thanks @Coobaha !

[Coobaha]

@AdamWagner sorry for pinging you back, but i found the root cause of wrong indicator positions/clicks - it depends on menubar. If it is hidden in settings - everything works as expected. If visible - extra offset is added. Please check attached video

Kapture.2021-02-07.at.13.55.23.mp4

[AdamWagner]

@Coobaha I'm bummed that I missed your original comment… but happy that the fix was less than a 1 line change!

See: #80

This is merged to main now. Sorry for not fixing this sooner!