AdamWagner / stackline

Visualize yabai window stacks on macOS. Works with yabai & hammerspoon.
944 stars 47 forks source link

Clicking on other monitor is like clicking on stackline of main monitor #100

Open kiryph opened 2 years ago

kiryph commented 2 years ago

I have two screens: Screenshot 2022-03-31 at 18 30 26

+--------++----------------+
|w1      ||Indic.1 stacked |
|single  ||Indic.2 w1 & w2 |
|window  ||                |
|        |+----------------+
|        |
+--------+

When I click on the left screen in the upper left corner, it is like I would click on the right screen on the indicator icons of stackline.

I have added another log command to a relevant section of the code:

index a664063..f723d27 100644
--- a/stackline/stack.lua
+++ b/stackline/stack.lua
@@ -1,3 +1,4 @@
+local log   = hs.logger.new('window', 'info')
 local Stack = {}

 function Stack:new(stackedWindows) -- {{{
@@ -86,6 +87,7 @@ function Stack:getWindowByPoint(p)
       p = clickedScren
          and clickedScren:absoluteToLocal(p)
          or p
+      log.i('absoluteToLocal', p)
    end

    return table.unpack(

The output is

2022-03-31 18:22:13: 18:22:13     window: absoluteToLocal hs.geometry.point(23.1962890625,76.979263305664)
2022-03-31 18:22:13:           stackline: Clicked window at hs.geometry.point(-1056.8037109375,76.979263305664)

The click coordinates are modified and become coordinates of the main screen. IMHO this should not happen.

The change clickedScreen:absoluteToLocal(p) has been added in commit b02ffe763b2cdf3a8b73255937bea49f071e6bbd, an attempt to fix another issue #62

acdifran commented 2 years ago

FYI for anyone else with this issue:

To get around this for now I've commented out the logic in function Stack:getWindowByPoint(p) that handles negative coordinates.

kiryph commented 2 years ago

My fix is following diff

diff --git a/stackline/stack.lua b/stackline/stack.lua
index a664063..0977a60 100644
--- a/stackline/stack.lua
+++ b/stackline/stack.lua
@@ -66,32 +66,24 @@ function Stack:deleteAllIndicators() -- {{{
 end -- }}}

 function Stack:getWindowByPoint(p)
-   if p.x < 0 or p.y < 0 then
-      -- FIX: https://github.com/AdamWagner/stackline/issues/62
-      -- NOTE: Window indicator frame coordinates are relative to the window's screen.
-      -- So, if click point has negative X or Y vals, then convert its coordinates
-      -- to relative to the clicked screen before comparing to window indicator frames.
-      -- TODO: Clean this up after fix is confirmed
+  -- Get the screen with frame that contains point 'p'
+  local function findClickedScreen(_p) -- {{{
+     return table.unpack(
+        u.filter(hs.screen.allScreens(), function(s)
+           return _p:inside(s:frame())
+        end)
+     )
+  end -- }}}

-      -- Get the screen with frame that contains point 'p'
-      local function findClickedScreen(_p) -- {{{
-         return table.unpack(
-            u.filter(hs.screen.allScreens(), function(s)
-               return _p:inside(s:frame())
-            end)
-         )
-      end -- }}}
-
-      local clickedScren = findClickedScreen(p)
-      p = clickedScren
-         and clickedScren:absoluteToLocal(p)
-         or p
-   end
+  local clickedScreen = findClickedScreen(p)
+  p = clickedScreen
+     and clickedScreen:absoluteToLocal(p)
+     or p

    return table.unpack(
          u.filter(self.windows, function(w)
           local indicatorFrame = w.indicator and w.indicator:canvasElements()[1].frame
-          if not indicatorFrame then return false end
+          if not indicatorFrame or w.screen:id() ~= clickedScreen:id() then return false end
           return p:inside(indicatorFrame) -- NOTE: frame *must* be a hs.geometry.rect instance
       end)
    )

Two aspects

  1. Determine current screen, skip indicator windows which are not on the current screen:
    return table.unpack(
          u.filter(self.windows, function(w)
           local indicatorFrame = w.indicator and w.indicator:canvasElements()[1].frame
    -          if not indicatorFrame then return false end
    +          if not indicatorFrame or w.screen:id() ~= clickedScreen:id() then return false end
           return p:inside(indicatorFrame) -- NOTE: frame *must* be a hs.geometry.rect instance
       end)
    )
  2. If you have three displays Screenshot 2022-07-26 at 09 42 55 the criteria that only negative coordinates have to be transformed with absoluteToLocal(p) is not true.

    Also positive coordinates can be outside of the primary display. Here my geometry according to yabai -m query --displays:

    [{
    "id":722478165,
    "uuid":"960C98A9-3C2B-9F9D-9C66-B3B7B13CF295",
    "index":1,
    "frame":{
        "x":0.0000,
        "y":0.0000,
        "w":2560.0000,
        "h":1440.0000
    },
    "spaces":[1, 2, 3, 4, 5, 6]
    },{
    "id":722474380,
    "uuid":"C5BC41F5-4943-CAE6-0B3B-6C862C1FCE6B",
    "index":3,
    "frame":{
        "x":-2560.0000,
        "y":360.0000,
        "w":2560.0000,
        "h":1080.0000
    },
    "spaces":[12, 13, 14, 15, 16]
    },{
    "id":4128835,
    "uuid":"000010AC-0000-A06E-3137-4C5500000000",
    "index":2,
    "frame":{
        "x":2560.0000,
        "y":0.0000,
        "w":1080.0000,
        "h":1920.0000
    },
    "spaces":[7, 8, 9, 10, 11]
    }]
danielbernalo commented 9 months ago

My fix is following diff

diff --git a/stackline/stack.lua b/stackline/stack.lua
index a664063..0977a60 100644
--- a/stackline/stack.lua
+++ b/stackline/stack.lua
@@ -66,32 +66,24 @@ function Stack:deleteAllIndicators() -- {{{
 end -- }}}

 function Stack:getWindowByPoint(p)
-   if p.x < 0 or p.y < 0 then
-      -- FIX: https://github.com/AdamWagner/stackline/issues/62
-      -- NOTE: Window indicator frame coordinates are relative to the window's screen.
-      -- So, if click point has negative X or Y vals, then convert its coordinates
-      -- to relative to the clicked screen before comparing to window indicator frames.
-      -- TODO: Clean this up after fix is confirmed
+  -- Get the screen with frame that contains point 'p'
+  local function findClickedScreen(_p) -- {{{
+     return table.unpack(
+        u.filter(hs.screen.allScreens(), function(s)
+           return _p:inside(s:frame())
+        end)
+     )
+  end -- }}}

-      -- Get the screen with frame that contains point 'p'
-      local function findClickedScreen(_p) -- {{{
-         return table.unpack(
-            u.filter(hs.screen.allScreens(), function(s)
-               return _p:inside(s:frame())
-            end)
-         )
-      end -- }}}
-
-      local clickedScren = findClickedScreen(p)
-      p = clickedScren
-         and clickedScren:absoluteToLocal(p)
-         or p
-   end
+  local clickedScreen = findClickedScreen(p)
+  p = clickedScreen
+     and clickedScreen:absoluteToLocal(p)
+     or p

    return table.unpack(
          u.filter(self.windows, function(w)
           local indicatorFrame = w.indicator and w.indicator:canvasElements()[1].frame
-          if not indicatorFrame then return false end
+          if not indicatorFrame or w.screen:id() ~= clickedScreen:id() then return false end
           return p:inside(indicatorFrame) -- NOTE: frame *must* be a hs.geometry.rect instance
       end)
    )

Two aspects

  1. Determine current screen, skip indicator windows which are not on the current screen:
    return table.unpack(
          u.filter(self.windows, function(w)
           local indicatorFrame = w.indicator and w.indicator:canvasElements()[1].frame
-          if not indicatorFrame then return false end
+          if not indicatorFrame or w.screen:id() ~= clickedScreen:id() then return false end
           return p:inside(indicatorFrame) -- NOTE: frame *must* be a hs.geometry.rect instance
       end)
    )
  1. If you have three displays Screenshot 2022-07-26 at 09 42 55 the criteria that only negative coordinates have to be transformed with absoluteToLocal(p) is not true. Also positive coordinates can be outside of the primary display. Here my geometry according to yabai -m query --displays:
[{
  "id":722478165,
  "uuid":"960C98A9-3C2B-9F9D-9C66-B3B7B13CF295",
  "index":1,
  "frame":{
      "x":0.0000,
      "y":0.0000,
      "w":2560.0000,
      "h":1440.0000
  },
  "spaces":[1, 2, 3, 4, 5, 6]
},{
  "id":722474380,
  "uuid":"C5BC41F5-4943-CAE6-0B3B-6C862C1FCE6B",
  "index":3,
  "frame":{
      "x":-2560.0000,
      "y":360.0000,
      "w":2560.0000,
      "h":1080.0000
  },
  "spaces":[12, 13, 14, 15, 16]
},{
  "id":4128835,
  "uuid":"000010AC-0000-A06E-3137-4C5500000000",
  "index":2,
  "frame":{
      "x":2560.0000,
      "y":0.0000,
      "w":1080.0000,
      "h":1920.0000
  },
  "spaces":[7, 8, 9, 10, 11]
}]

Please review a typo (clickedScren) !!.. Its work to me!! thanks

if not indicatorFrame or w.screen:id() ~= clickedScren:id() then return false end