Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

placemouse patch crashing with multiple monitors #27

Closed
jzbor opened this issue Apr 14, 2021 · 11 comments
Closed

placemouse patch crashing with multiple monitors #27

jzbor opened this issue Apr 14, 2021 · 11 comments

Comments

@jzbor
Copy link

jzbor commented Apr 14, 2021

I have the suspicion that there is a but somewhere in the placemouse patch. Sometimes dwm randomly crashes, when I try to move a window between monitors. It is very hard to reproduce, so I have no idea what exactly causes the crash. Just thought I might put it here if somebody encounters similar problems or has an idea of what the problem might be.

I can reproduce it most of the time with two windows on the first monitor and then moving one to the second monitor. If I then let go the mouse button, when the window is almost fully on the next monitor and only about 20/30 pixels still on the first monitor it doesn't snap in as it should. If I then try to move it again dwm crashes. If I use the top bar to change the tag it just attaches to that new tag, without crashing.

@jzbor
Copy link
Author

jzbor commented Apr 14, 2021

Ok I managed to narrow it down. The bug only occurs, when there is no other window already on the second monitor. I think this is also why switching to another tag (with a window already present on it) fixes it.

EDIT: Ok nvm now I can't reproduce it anymore...

@bakkeby
Copy link
Owner

bakkeby commented Apr 14, 2021

Hmm, that is interesting. Could it be that the window is on the other monitor, but the mouse is not (hence the selected monitor is still the same)?

I'll see if I can reproduce this.

@jzbor
Copy link
Author

jzbor commented Apr 14, 2021

I forgot to mention that I am using placemousemode = 1. Most of the times I could reproduce it the mouse was also on the new monitor, but maybe it is a bug with one of the frames in between which only comes into affect after releasing the mouse.

I tried to track the issue down and made the following changes, as I thought it might be something with the mouse position being taken accidentally instead of that of the window:

diff --git a/moonwm.c b/moonwm.c
index b91144e..7006aed 100644
--- a/moonwm.c
+++ b/moonwm.c
@@ -1975,6 +1975,8 @@ placemouse(const Arg *arg)
 		return;
 	restack(selmon);
 	prevr = c;
+    px = c->x;
+    py = c->y;
 	if (XGrabPointer(dpy, root, False, MOUSEMASK, GrabModeAsync, GrabModeAsync,
 		None, cursor[CurMove]->cursor, CurrentTime) != GrabSuccess)
 		return;
@@ -2071,7 +2073,7 @@ placemouse(const Arg *arg)
 	} while (ev.type != ButtonRelease);
 	XUngrabPointer(dpy, CurrentTime);
 
-	if ((m = recttomon(ev.xmotion.x, ev.xmotion.y, 1, 1)) && m != c->mon) {
+	if ((m = recttomon(px, py, 1, 1)) && m != c->mon) {
 		detach(c);
 		detachstack(c);
 		arrangemon(c->mon);

Sadly they didn't help, but I don't see why these should track ev instead of px/py...

EDIT: Updated diff

@jzbor
Copy link
Author

jzbor commented Apr 14, 2021

Some other suspicion I had about where the problem might be:

			if (!r || r == c)
				break;

This would return on an empty monitor. Is it possible that this causes missing out on important calls after that? Especially attach() or detach() might cause the problem...

@bakkeby
Copy link
Owner

bakkeby commented Apr 14, 2021

Hi @jzbor,

I believe the issue was two-fold.

If you are on tag 1 on the main monitor and on tag 4 on the adjacent monitor and you move a window across then the client is added on that monitor, but the client's tags remained unchanged (i.e. it remains on tag 1).

When the arrange happens the client is not moved out of the way because we are calling arrangemon instead of arrange, this means that the window remains in place although it is not actually shown as far as dwm is aware.

But because it is still visible when you click on it again it will find the window, so the moveorplace is called which had a check if selmon->sel->isfloating, but in this case selmon->sel would be null hence the crash.

I pushed a proposed fix for this.

@jzbor
Copy link
Author

jzbor commented Apr 14, 2021

Nice work! Thanks so much! That seems to have fixed it.

Just out of interest/for learning: are the changes I made above still valid? Was it intended the way it was or is it completely irrelevant?

I find it really hard sometimes to debug dwm, because there is not logging etc build in (maybe I should add some basic debug logging) so I think its really impressive how fast you found the problem!

@jzbor
Copy link
Author

jzbor commented Apr 14, 2021

Another question: How would this line have to look in order to use the same tags as on the previous monitor? Would it involve additional changes?

        c->tags = m->tagset[m->seltags];

@bakkeby
Copy link
Owner

bakkeby commented Apr 14, 2021

If you wanted to keep the same tags as on the previous monitor then you'd delete that line and call arrange instead of arrangemon further down. The feel will be a bit jarring as the window simply just disappear once you release the mouse button, and the tag may change to be marked as occupied if it was previously empty.

@bakkeby
Copy link
Owner

bakkeby commented Apr 14, 2021

I find it really hard sometimes to debug dwm, because there is not logging etc build in (maybe I should add some basic debug logging)

Yes it can be tricky, especially if dwm crashes. I know it is not really best or even good practice, but I do add temporary print statements in the code of interest if I need to work out how far it gets and it is not obvious where it might go wront.

@jzbor
Copy link
Author

jzbor commented Apr 14, 2021

Thanks for your help. On my build I will add an option for moving with keeping the tags and add a separate keybinding, because I like to have certain programs like Spotify or Discord on their designated tags. Also I am closing the issue, because your patch did resolve it.

@jzbor jzbor closed this as completed Apr 14, 2021
@bakkeby
Copy link
Owner

bakkeby commented Apr 14, 2021

Thank you for reporting the issue, it will save time for other people trying out the patch later.

bakkeby added a commit to bakkeby/dwm-flexipatch that referenced this issue Apr 14, 2021
… tags and there is no selected client on that monitor / tag

This is ref. bakkeby/patches#27
SunJukebox pushed a commit to SunJukebox/dwm that referenced this issue Aug 6, 2022
… tags and there is no selected client on that monitor / tag

This is ref. bakkeby/patches#27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants