Warping without unwanted change of the stack order

Question

Warping without unwanted change of the stack order

jdujava opened this issue a year ago · 8 comments

While using the very useful warp patch I noticed a slight inconvenience.
Consider the following scenario, in which one opens a centered floating window.

Initial focus is on the bottom-right window.

+------------+--------+
|            |        |
|            |        |
|            |        |
|            +========+
|            ∥        ∥
|            ∥  mouse ∥
+------------+========+

After opening a floating window, the mouse cursor warps to the middle.

+------------+--------+
|            |        |
|      +======+       |
|      ∥mouse ∥       |
|      ∥      ∥-------+
|      +======+       |
|            |        |
+------------+--------+

After closing a floating window, the focus stays on the window which was behind.

+============+--------+
∥            ∥        |
∥            ∥        |
∥       mouse∥        |
∥            +--------+
∥            ∥        |
∥            ∥        |
+============+--------+

Personally, I would find it desirable that the focus returns to the initial window.
If I understand correctly, this is caused by warp moving the cursor and firing the EnterNotify event, thus changing the stack order.

I tried to cook up a "fix", and first calling the warp like

...
if (m == selmon && (m->tagset[m->seltags] & m->sel->tags) && m->lt[m->sellt]->arrange != &monocle)
    warp(m->sel);
XSync(dpy, False);
while (XCheckMaskEvent(dpy, EnterWindowMask, &ev));
...

seems to work alright.

By any means I am not an expert, so could you please tell me what do you think about the "fix".
Could there be any unwanted side effects?

Thanks!

Answer 1 · 2023-05-24T15:52:25.000Z

If that fix works then great.

It is a bit unclear to me why exactly the fix would work though.

The way it is implemented (moving the if statement before the XSync and XCheckMaskEvent calls) suggests that the action of warping the cursor is somehow causing the EnterNotify event to be sent, which sounds kind of odd to me but this could still be the case.

It may also be that something is causing the EnterNotify to be sent and that moving the if statement simply causes an additional delay of a few nanoseconds; enough for the EnterNotify to come into the queue and to be captured by the XCheckMaskEvent call(s).

I tried to reproduce this in my own build but wasn't able to.

Using the focus-on-click patch would also eliminate issues like this.

Answer 2 · 2023-05-24T17:18:54.000Z

My first hacky solution was to append XSync(dpy, True) after warping the cursor directly in the warp function, which fixed the unwanted change of the stack order. That wasn't exactly valid, because it caused a greater problem.

Suppose I would like to force kill a window with the systray entry (e.g. Telegram). In cases when the warp was triggered on the close, the systray wouldn't properly delete the icon, sometimes leading to maxed out CPU by dwm for some time.

Then I came up with the fix mentioned previously, but to avoid breaking something again I wanted to consult someone who knows what they are doing 😅. So if you don't see anything wrong with the solution then amazing.

I tried to reproduce this in my own build but wasn't able to.

I have succeeded to reproduce the issue on clean build of dwm with just warp and center patch applied. I have given centered rule to pavucontrol (but any program/window would work), and bound a key via sxhkd to launch it. Then the unwanted change of focus happens as described earlier. It seems to me that the culprit shouldn't be specific to my build/machine.

Using the focus-on-click patch would also eliminate issues like this.

True, but I wanted to keep the "focus automatically following the mouse" functionality.

Anyway, thank you for taking your time and answering my questions 👍.

Answer 3 · 2023-05-25T09:08:41.000Z

I tried with just dwm and the warp patch and I was not able to reproduce the issue - at least not the way I was trying to reproduce it (open a terminal, move it to the center, change focus between the last client in the stack and the now centered floating window, then closing the floating terminal. In this case the focus remained on the last client as desired.

So I tried again following your replication steps to the point and installed warp, center, setting up the client rule and starting pavucontrol - and I was able to reproduce the issue.

The warp is indeed what is causing the focus change, but for a different reason than you might think.

Here is the scenario when the program starts:

the program creates a window
the program sends a mapping request to the X server, which forwards this to the window manager (dwm)
dwm handles the event in the maprequest function and as it is not already managing this window it forwards it to the manage function
client rules are applied; the window is floating and centered
the X and Y position are changed because the window is centered
the client is attached to the stack
the window is moved into position (XMoveResizeWindow)
the state of the window is set to normal
the currently focused client is unfocused
the new client is set as the selected client
an arrange is being triggered
- causing a restack to be triggerd
  - causing warp to be triggered
  - the cursor is warped to the center of the window position
  - an EnterNotify event is triggered for the master client, because the cursor is moved on top of it
the window is mapped / displayed (XMapWindow)
the EnterNotify is handled by enternotify and the master client gets focus
the master client is at the top of the stack
a FocusIn event is triggered for the newly created window, giving focus back to pavucontrol

When pavucontrol is closed we have that:

the next in the stack receives focus (the master client)
as the cursor is already within the borders of the master client the cursor is not warped

So the issue is really how the cursor is warped to the client before it is mapped. There are a few ways to go about fixing this.

In your case the fix works because you do the warp first, which generates the EnterNotify event because the window is not visible yet (and as such the cursor entered the master client window), then it clears out all EnterNotify events (XCheckMaskEvent).

if (m == selmon && (m->tagset[m->seltags] & m->sel->tags) && m->lt[m->sellt]->arrange != &monocle)
    warp(m->sel);
XSync(dpy, False);
while (XCheckMaskEvent(dpy, EnterWindowMask, &ev));

Answer 4 · 2023-05-25T10:29:34.000Z

Whoa, thank you very much for a thorough explanation. Makes a lot more sense (at least to the degree I am able to understand).

So the issue is really how the cursor is warped to the client before it is mapped. There are a few ways to go about fixing this.

Do you think there is an alternative fix, which would be worth the trouble (is more robust yet simple, etc..)?

Answer 5 · 2023-05-25T13:38:02.000Z

The fix you have proposed is fine as-is. I think regardless of how you fix this it is going to be hard to reason about why it is implemented like this and that unless there is half a page of comments explaining why things are done this and that way.

An alternative that would also work is in the manage function at the end we have:

    ...
    arrange(c->mon);
    XMapWindow(dpy, c->win);
    focus(NULL);
}

If the new window is floating then we do not really need to re-arrange all tiled windows.

The arrange function in this context is only called in order to call restack, which is redundant because the new window is already on top of other windows.

As such the arrange > restack function in this context is only called in order to call drawbar.

(generally I don't like how restack is used in dwm)

So we could potentially change this to something like:

    ...
    if (c->isfloating) {
        drawbar(c->mon);
    } else {
        arrange(c->mon);
    }
    XMapWindow(dpy, c->win);
    focus(NULL);
}

but you wouldn't have the mouse cursor warping to the client in this case.

So you may need to take that into account as well.

    ...
    if (c->isfloating) {
        drawbar(c->mon);
    } else {
        arrange(c->mon);
    }
    XMapWindow(dpy, c->win);
    if (c->isfloating) {
        warp(c);
    }
    focus(NULL);
}

This is starting to look pretty ugly. More so I think that you could have the same issue if you are using the deck layout (which is a monocle arrangement in the stack area).

I am thinking that if you have one client in the master area plus one client in the stack area, with focus on the master client. Open a new client that is tiled and placed in the stack area.

The arrange > restack > warp would result in the mouse cursor being moved on top of the client in the stack area, generating an EnterNotify event for that window. The new client is then mapped (shown on screen) and we give focus to this client.

When you close that client the focus reverts to the other window in the stack area, rather than the master client that initially had focus.

I haven't tested that scenario but your fix would address this one as well.

Moving the warp call out of restack and into corresponding functions like focusstack or even focus might work as well, but that is a different topic imho.

Answer 6 · 2023-05-25T14:37:33.000Z

Great, thanks again for the insightful comments. I have learned quite a bit.

I leave it to you whether this Issue should be converted to a Discussion, where probably more people can find it. Also, whether this "fix" should be included by default in the warp patch.

Answer 7 · 2023-05-25T15:57:20.000Z

I'll add this fix to the patch in this repository at least.

If you want to contribute to https://dwm.suckless.org/patches/warp/ then feel free to do so for the learning experience.

I don't remember why I added the warp patch in this repository, perhaps it was primarily to have a reference. The only difference between this and the patch on the suckless site is this bit:

... && selmon->lt[selmon->sellt] != &layouts[2])

which is checking to see if the selected layout is the monocle layout, assumed to be the 3rd in the layouts list.

I changed this to

...  && m->lt[m->sellt]->arrange != &monocle)

Answer 8 · 2023-05-25T16:43:31.000Z

If you want to contribute to https://dwm.suckless.org/patches/warp/ then feel free to do so for the learning experience.

Hopefully I will find the time to try it out 😉.