Closed jdujava closed 1 year ago
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.
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 :sweat_smile:. 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 :+1:.
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:
maprequest
function and as it is not already managing this window it forwards it to the manage
functionXMoveResizeWindow
)arrange
is being triggered
restack
to be triggerd
XMapWindow
)enternotify
and the master client gets focusWhen pavucontrol is closed we have that:
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));
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..)?
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.
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.
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)
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 :wink:.
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.
After opening a floating window, the mouse cursor warps to the middle.
After closing a floating window, the focus stays on the window which was behind.
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 theEnterNotify
event, thus changing the stack order.I tried to cook up a "fix", and first calling the
warp
likeseems 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!