ianyh / Amethyst

Automatic tiling window manager for macOS à la xmonad.
https://ianyh.com/amethyst/
MIT License
14.87k stars 487 forks source link

WindowManager.alternateWindowForScreenAtPoint fails to consider peeking-based overlap #689

Open ianfixes opened 6 years ago

ianfixes commented 6 years ago

System

What's the problem?

Exposed in #685 : with window peeking now (re)enabled, windows that are larger than their frame can be moved to overlap other windows. This makes a pre-existing bug more frequent, namely that dragging a window within its (apparent) frame can cause an unexpected swap. This is because the mouse may actually be in another window's frame due to peeking.

How can it be reproduced?

  1. Consider a screen that's 1200px wide, and two windows A and B. Let's say each window has a 600px minimum width.
    |--- screen --|
    :             :
    :             :
    +------Y------+
    |//////|\\\\\\|
    |//A///|\\\B\\|
    |//////|\\\\\\|
    +------|------+
  2. Arrange them in Tall layout, and drag the window on the left to 1000px width. The window on the right shrinks to its 600px minimum, and is reflowed with 200px on screen and 400px off screen.
--- screen --

: : +----------Y--+----+ |//////////|\|| |////A/////|\B|| |//////////|\|````| +----------|--+----+

3. Focus the window on the right.  It peeks, overlapping the window on the left by 400px (denoted by `X`s).  Note that the visible boundary between the windows has moved to the left but the frame divider position `Y` (according to Amethyst) has not.
--- screen --

: : +------+---Y--+ |//////|XXX\| |////A/|XXX\B| |//////|XXX\| +------+---|--+

4. Put your mouse cursor in the center of the title bar of the window on the right (which should be over the overlapped region) and move the window on the right downward by 2 px.  Make sure you move it, don't resize it.
--- screen --

: drag -+ : : down | : : V : +------+-:-Y--+ |//////|XXX\| |////A/|XXX\B| |//////|XXX\| +------+---|--+



### Expected behavior

Dragged window reflows to the peeked position

### Actual behavior

Dragged window swaps with window on the left, because technically you dragged it within the bounds of the other frame

## Suggested fix

The problematic function is `WindowManager.alternateWindowForScreenAtPoint`.  It does not understand the overlapping that peeking causes, because it only considers where windows actually _are_ and not where the layout manager says that windows _should be_.  What other source can I use to get this information (i.e., given a point on the screen, what window's frame is that)?

[Trello Card](https://trello.com/c/Luy6PMCs/312-amethyst-windowmanageralternatewindowforscreenatpoint-fails-to-consider-peeking-based-overlap)
ianfixes commented 6 years ago

Thinking more about this, I wonder if my suggested fix is wrong. In other words, I think things are indeed working as expected: the part of the peeked window that you drag is indeed being dragged "outside" of its frame -- that's due to the peeking behavior.


|--- screen --|
:             :
:  drag -+    :
:  down  |    :
:        V    :
+------+-:-Y--+
|//////|XXX\\\|
|////A/|XXX\B\|
|//////|XXX\\\|
+------+---|--+

(note that where I indicated "drag down", 
it is indeed _outside_ of the frame 
designated for the focused window).

So I think the proper fix might be more along the lines of "if the dragged mouse cursor leaves the frame where the focused window had been placed -- peeking included -- then swap it".

The one edge case I can think of is when the window becomes focused via a drag event; this could cause an un-peeked window to peek, which would change the location you'd have to drag to in order to complete a swap.

ianyh commented 6 years ago

Still thinking about this, but how would another window grab focus during a drag event?

ianfixes commented 6 years ago

In the example above, if window A was focused (meaning B was shoved partially off-screen), you can then start dragging window B -- it will focus and then peek. This has the effect of changing the part of window B that your cursor is holding, even though your cursor position on screen doesn't change.

The current implementation of that edge case is proper behavior -- you would only have to drag window B until your cursor enters the frame for window A. But in the behavior I just described (drag outside of peeked window B's boundary), it would mean you have to drag much further -- you must pass the boundary of B's peeked position.

I feel like I'm doing a terrible job of explaining this. But the good news is that I think I'm describing a fix for something that maybe nobody else will notice.

Should I record some gifs to make this conversation easier?

ianyh commented 6 years ago

Do you think there is an immediate change necessary for this? I'm wondering if we should hold off on it for the next version since there's still some open questions.

ianyh commented 6 years ago

Oh, and gifs would be great if that's possible.

ianfixes commented 6 years ago

aug-31-2018 08-55-55 amethyst stuff

here are the 2 quirks

ianfixes commented 6 years ago

This issue's title describes the first quirk shown in this gif. It comes down to the fact that if your eyes follow the movement of the settings window, it looks like you've dragged it enough. But Amethyst is looking at the movement of the mouse pointer, which is easily overshadowed by the peeking of an entire window.

I'm not sure if anything can resolve this, short of storing the distances of the peek movement in the mouse state and factoring that in to a swap decision. (And that option sounds like a can of worms that might result in really obscure and weird behavior down the road.)

So for now I'm just reporting it.