fvwm3 icon indicating copy to clipboard operation
fvwm3 copied to clipboard

Windows moving between desktops on KVM switch

Open hrnick opened this issue 3 years ago • 26 comments

Upfront Information

$ fvwm3 --version fvwm3 1.0.2 (released) with support for: ReadLine, XPM, PNG, Shape, XShm, SM, XRandR, XRender, XCursor, XFT

$ cat /etc/gentoo-release Gentoo Base System release 2.7

$ uname -sp Linux Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz

Actual Behaviour

I have my computer connected to a KVM switch. Whenever I switch to the other computer and back all windows end up on the current desktop and I have to manually move them back to each of their desktops respectively using MoveToDesktop.

Expected Behaviour

I would expect the windows to stay in the desktop where they were.

Enabling logging

I have ExplainWindowPlacement enabled but it doesn't show anything. Maybe this doesn't work in fvwm3? BugOpts ExplainWindowPlacement On

Nothing in ~/.fvwm/fvwm3-output.log is being generated by this event.

Steps to Reproduce

Switch to other computer connected to KVM switch and back again. This did not happen in fvwm2.

I've reduced my config to this and still have the same problem:

IgnoreModifiers L25 DeskTopSize 1x1

Key F12 A A Menu RootMenu Root c c Nop Key F11 A A Popup MoveToDesk Key 1 A 4 GotoDesk 0 0 Key 2 A 4 GotoDesk 0 1 Key 3 A 4 GotoDesk 0 2 Key 4 A 4 GotoDesk 0 3

AddToFunc MoveToDesktop

  • I MoveToDesk 0 $0
  • I Prev (!Iconic, CurrentDesk, AcceptsFocus) Focus

AddToMenu MoveToDesk

  • "Desktop &1" MoveToDesktop 0
  • "Desktop &2" MoveToDesktop 1
  • "Desktop &3" MoveToDesktop 2
  • "Desktop &4" MoveToDesktop 3

AddToMenu RootMenu

  • "URxvt" Exec urxvt
  • "Restart FVWM" Restart
  • "Quit FVWM" Quit

hrnick avatar Jan 18 '21 07:01 hrnick

Hi @hrnick

Thanks. I can't test this (I don't have a KVM switch -- unless you know of a means of simulating and reproducing this via KVM?), but my first thought is that it's probably RandR-related.

If you start fvwm3 as fvwm3 -v this will start logging to ~/.fvwm/fvwm3-output.log. In your config, you can add:

BugOpts DebugRandR On

... and then use your KVM, reproduce the problem, and attach fvwm3's log file here.

BTW, BugOpts ExplainWindowPlacement On does work, you either didn't have logging enabled in fvwm3, but also moving windows around doesn't trigger the ExplainWindowPlacement condition, that only happens when the window is mapped, or the PlaceAgain command is used.

ThomasAdam avatar Jan 18 '21 09:01 ThomasAdam

Is this of any help?

https://pastebin.com/JwJ17f0p

I started fvwm on the minimal config above (with correction for syntax errors in it) using "exec fvwm3 -v -f ~/.fvwm/debug. I started an urxvt window on each virtual desktop, switched on the KVM switch and back and all four windows were on the same desktop.

hrnick avatar Jan 18 '21 23:01 hrnick

Just let me know if there's anything you want me to try out, I'd be happy to help! I'm running Gentoo so any patch against 1.0.2 would be easy to add to /etc/portage/patches and add when compiling.

hrnick avatar Jan 23 '21 18:01 hrnick

Hi @hrnick,

Can you check to see if this is reproducible without a KVM switch? That is, are you able to use fvwm3 connected to both monitors, and then completely detach one of them via xrandr? I'm curious if that produces the same results.

From looking at the output you've provided, I can kind of see what's going on, but could really do with a usecase to reproduce this by which doesn't include me fetching a KVM switch from somewhere.

ThomasAdam avatar Jan 23 '21 18:01 ThomasAdam

Hi @ThomasAdam,

It's just one monitor that is attached to the KVM switch with my stationary Gentoo system and a docking station for my work laptop. Working from home nowadays I have a setup to use the one screen I have on my desktop for both my system and my laptop from work.

I get that. :)

hrnick avatar Jan 23 '21 18:01 hrnick

Hey @hrnick

OK, this is reproducible by me just disabling off one of my monitors via RandR.

Will get back to you in a bit with a fix.

ThomasAdam avatar Jan 23 '21 19:01 ThomasAdam

Hi @ThomasAdam,

Thanks! Just let me know and I'd be more than happy to try it out here!

hrnick avatar Jan 23 '21 19:01 hrnick

I've been running into this bug recently so I'm planning to have a look into it. As a first start, here is a recipe to reproduce that uses Xspice, so you don't need to actually have two monitors or to take your development environment out of commission in the process of testing it.

Prerequisites: xserver-xspice virt-viewer spice-vdagent (all packages in Debian stable) plus the attached minimal fvwmrc:

fvwmrc.txt

In one host terminal:

Xspice --auto --vdagent --vdagentd-exec /usr/sbin/spice-vdagentd  --port 5900 --password pick-a-password :1.0

(This creates an Xserver using display 1; if you're using that for something else pick another number and adjust the DISPLAY=:1 parts of the commands below.)

In another terminal:

remote-viewer spice://localhost:5900

and when prompted, enter the password you gave on the Xspice command line.

In the remote-viewer GUI app's menus, select View -> Displays and tick the box for "Display 2" to bring up a window for the 2nd monitor (put it to the RHS of the original window).

Finally, in a 3rd host terminal run these commands:

DISPLAY=:1 xrandr --output qxl-1 --preferred --right-of qxl-0
DISPLAY=:1 fvwm3 -f fvwm-bug-fvwmrc &
DISPLAY=:1 xterm -xrm '*Page:3' -geometry +1500+50 &
DISPLAY=:1 xterm -xrm '*Page:2' -geometry +1510+60 &
DISPLAY=:1 xterm -xrm '*Page:1' -geometry +1520+70 &
DISPLAY=:1 xterm -xrm '*Page:0' -geometry +1530+80 &

This sets up the two monitors, runs fvwm3 with a FvwmPager and 4 desktops, and starts one xterm on each desk (which you should be able to see in the Pager). Now turn off monitor 2:

DISPLAY=:1 xrandr --output qxl-1 --off

Notice that in the FvwmPager all the xterms on desks 1, 2 and 3 have disappeared.

Turn the monitor back on again:

DISPLAY=:1 xrandr --output qxl-1 --preferred --right-of qxl-0

There's only one xterm window visible on desk 0, but if you click on the FvwmPager to switch to desk 1, and then back to desk 0, when you go back all the xterm windows have now appeared on desk 0.

(Couple of cautionary notes about the test setup: (1) Xspice seems to listen on all interfaces, so the password and any firewall you have is the only thing keeping randos on the internet from connecting. (2) Mouse input to the second 'screen' window doesn't work correctly. I'll try to figure out something better, but the above is sufficient to be a reproducible test case for the fvwm bug, at least.)

pm215 avatar Jun 19 '21 13:06 pm215

Hi @pm215

Yep. The problem is that we're not updating windows correctly/enough when we detect a RandR event.

Since you've expressed an interest in this, take a look at what happens if you have a RandR setup but one of those monitors is disabled before Fvwm3 starts...

Everything will work fine. Open FvwmPager. That'll work fine, too.

Now enable the previously turned off screen. You'll note FvwmPager gets confused as to the desktopsize.

This is because I've broken this in events.c when detecting RandR changes. I don't do this enough to have noticed... Until now. It's also at this point things go awry.

I'd concentrate your efforts there.

I'm more than happy to mentor you through this, @pm215

Thanks!

ThomasAdam avatar Jun 19 '21 13:06 ThomasAdam

I've put in and/or enabled some debug logging, and so far it looks like:

  • when the XRandR events come in we do correctly spot the monitor changes, and update fvwm's monitor_q list (logging in monitor_dump_state shows the monitors listed with Disabled set appropriately and the monitor coords, width correct, and MyDisplayWidth/MyDisplayHeight also correct, ie the full both-monitors dimensions)
  • we do broadcast the information about the monitor change to FvwmPager
  • FvwmPager is handling the "Monitor" config message at least as far as receiving it goes (haven't checked whether what it does with the values is the right thing yet)

I did notice that we don't send the right size _NET_DESKTOP_GEOMETRY in ewmh.c (it's the size of one monitor, not the whole thing), but FvwmPager doesn't appear to look at that so it's probably not directly causing this bug.

pm215 avatar Jun 23 '21 20:06 pm215

Hi @pm215

Are you still looking at this? I can certainly help with this -- and I think one solution here is likely to be monitor tracking between windows -- such that we record where windows are and have been on a monitor (we have this already in UPDATE_FVWM_SCREEN already.

But really the problem here is going to know which monitor(s) are left, and therefore what is best to do. For example, if I have three monitors (one of which is my laptop screen), and I undock that, how do I know what to do?

ThomasAdam avatar Jul 19 '21 22:07 ThomasAdam

I haven't forgotten about it, but I haven't found/made time to actively look at it for the past few weeks. I still haven't figured out yet what actually causes the windows to switch desktop.

I agree that in general it's not clear what the desired behaviour is (maybe other window managers have a standard thing they do here?). For my personal use case what I want is "don't move anything, just wait and assume the monitor will come back"; but obviously that won't be right for all situations... I think that a good start would be to fix the outright bug we have at the moment, though, so we stop moving windows to different desktops.

pm215 avatar Jul 20 '21 09:07 pm215

Small progress: I've tracked down why the windows change desktop. In monitor_update_emwh() after updating the monitor list we loop through each window calling UPDATE_FVWM_SCREEN() on it. That macro calls FindScreenOfXY() for the window, which obviously returns a different monitor than the window was on (because that monitor is no longer in the monitor list). So UPDATE_FVWM_SCREEN() changes the window's fw->m to the new monitor (makes sense I guess). But it also changes the window's fw->Desk to the current desk of the new monitor's virtual screen. That might be right for some UPDATE_FVWM_SCREEN() uses but clearly isn't what we want here.

Also, the "change the desktop of this window" part isn't consistently telling the rest of fvwm about that change -- if you have a fvwm config that binds WindowList to a mouse button in the root window, you will see that even after UPDATE_FVWM_SCREEN has changed what it thinks the desktop for the windows should be, it hasn't told WindowList about this change. The Pager doesn't see the desktop change either, I think.

Side note: UPDATE_FVWM_SCREEN saves the old monitor for the window in fw->m_prev, but this is the only place that touches that field: nothing ever reads it afterwards...

pm215 avatar Jul 31 '21 11:07 pm215

I think this bug was introduced in commit f3ef5c32be836c, which added code to UPDATE_FVWM_SCREEN to make it change the desk when the window changes. If I make this change which reverts the changes to that macro made in that commit then fvwm3 stops incorrectly moving windows to the current desk when the monitor is unplugged and replugged:

diff --git a/fvwm/screen.h b/fvwm/screen.h
index 743eacb8..31a3b6d2 100644
--- a/fvwm/screen.h
+++ b/fvwm/screen.h
@@ -484,15 +484,8 @@ typedef struct ScreenInfo
                                                                           \
                get_unshaded_geometry((fw), &g);                           \
                mnew = FindScreenOfXY((fw)->g.frame.x, (fw)->g.frame.y);   \
-               /* Avoid unnecessary updates. */                           \
-               if (mnew == (fw)->m)                                       \
-                       break;                                             \
                (fw)->m_prev = (fw)->m;                                    \
                (fw)->m = mnew;                                            \
-               (fw)->Desk = mnew->virtual_scr.CurrentDesk;                \
-               EWMH_SetCurrentDesktop((fw)->m);                           \
-               desk_add_fw((fw));                                         \
-               BroadcastConfig(M_CONFIGURE_WINDOW, (fw));                 \
        } while(0)
 
 /* A macro to to simplify he "ewmh desktop code" */

But I assume that that will break whatever it is that commit f3ef5c32be836c was trying to fix. I don't really understand that commit. In particular it creates and sets a new 'initial_placement_done' style flag, but no code ever tests that flag. The commit also makes UPDATE_FVWM_SCREEN look up the style of the window, but then it does nothing with the result. The commit message says that changing desks "can't blindly happen in all cases" but the actual code does change the desk in all cases.

Thomas: could you explain what the situations are when we do want UPDATE_FVWM_SCREEN to make the window switch desk when it moves to a different monitor? The "monitor unplug/plug" case is one when we definitely do not want that; commit f3ef5c32be836c says that "initial mapping of a StartsOnDesk styled window" is another case when we definitely do not want that. But when do we want to do it? I don't understand enough of the different uses of this macro to be able to make a guess...

pm215 avatar Aug 07 '21 09:08 pm215

Hey @pm215

Thanks for continuing to look at this. As you've guessed, removing the contents of that macro isn't the correct thing to do. Don't worry about the style checks in that macro for now -- that's a future addition I've yet to make.

UPDATE_FVWM_SCREEN is called a lot to track the monitor/desk a window is on. I suspect you're hitting the case in monitor_update_ewmh -- if you try the following, does that help?

diff --git a/fvwm/events.c b/fvwm/events.c
index 51af3bfef..531360751 100644
--- a/fvwm/events.c
+++ b/fvwm/events.c
@@ -1804,10 +1804,11 @@ void monitor_update_ewmh(void)
        }
 
        BroadcastMonitorList(NULL);
-
+#if 0
        for (t = Scr.FvwmRoot.next; t; t = t->next) {
                UPDATE_FVWM_SCREEN(t);
        }
+#endif
 }
 
 void

ThomasAdam avatar Aug 07 '21 10:08 ThomasAdam

It is the call from monitor_update_ewmh, yes. Just removing the UPDATE_FVWM_SCREEN call there doesn't seem like it would be the right thing to do, though -- it will leave the windows pointing at a nonexistent monitor. It seems to me that we do want to change the monitor, but leave the desk alone. (We'll change the monitor back again when it gets replugged and the x,y coords of the window once again are in the part of the desktop covered by that monitor.)

pm215 avatar Aug 07 '21 10:08 pm215

Hey @pm215

Yup -- can you add the necessary smarts to monitor_update_ewmh to make this more robust?

One of the things we need to be careful of here is timing. For example, imagine this scenario: let's say I have a laptop plugged into three external monitors. If I unplug the laptop from that, RandR and fvwm3 will go nuts as the monitors are registered as active/inactive, and I suspect we'll end up with a lot of incorrect assumptions made when trying to ascertain how many active monitors are left.

Even after that, what then? I mean, we need to:

  • Figure out how many monitors are active/inactive (already done)
  • Remap windows from inactive monitors to... a.n.other active monitor (if there's just one, that's easy -- if there's more than one, then what?)
  • Consider the reverse case when the laptop is plugged back in -- should those previous windows get put back? I think so...

ThomasAdam avatar Aug 07 '21 10:08 ThomasAdam

I'm happy to write some code, which I guess would be something like "make UPDATE_FVWM_SCREEN take a parameter to say whether it should change the desktop of the window or not", and have the callsite in monitor_update_ewmh() say "don't change the desktop". But I don't know which callsites to UPDATE_FVWM_SCREEN would want to say "change the desktop" -- so far I've identified 2 cases where the answer is "do not change the desktop", but none where the answer is "do change the desktop".

I'm not sure what your timing issue is -- you'll see a bunch of "monitor goes away" events as the laptop is unplugged, and that should be fine: we'll just deal with the events as they arrive.

My take on what the behaviour here should be is: we have a virtual desktop, and when a monitor goes away we don't move any windows anywhere on the virtual desktop -- it's just that now part of the virtual desktop is no longer visible to the user, but instead they can page over to it or whatever as they would for any "virtual desktop bigger than physical" config. When the monitor comes back, now more of the virtual desktop is visible than before, and again we don't move any windows around. Separately, we might want to provide users with some helpful window-rearrangement tools for tasks like "move all the windows so they are on the visible part of the virtual desktop".

By the way, do you have a test process for fvwm3? I feel like it would be easy for me to make a change that fixes the bug I'm interested in but breaks a usecase or option that I don't use personally...

pm215 avatar Aug 07 '21 11:08 pm215

Hey @pm215

I suppose the timing issue I'm referring to is allowing enough time for RandR to have decided which monitors are in which state, and fvwm3 doing whatever it needs to do. Perhaps in reality it isn't a problem -- I was just surmising. This issue isn't something I personally run into, so my thinking is going to be theoretical or contrived, depending on how much of the problem I can reproduce. So far, I've been doing that by yanking the HDMI cable out the back of my monitor to see what happens... ;)

Hopefully @hrnick can confirm this is good enough for him once we've cobbled something together.

In terms of the callers of UPDATE_FVWM_SCREEN, I would say the two places you've identified which shouldn't update the desk a window is on is probably correct in its entirety, and hence the other callers do want the desk updated. This is typically done when:

  • A window is mapped
  • A window is recaptured (perhaps over a restart, for instance)
  • A window is moved (manual, automatically, placed by a style, etc.)

I think that at this point we might have outgrown the usefulness of using a macro for this -- please feel free to create an inline function instead if you feel that's going to make things easier.

In terms of what should happen with windows on RandR changes, yes we could create some stock functions for this -- I already added RandRFunc which a user can define to do things when any RandR event occurs. That's something for later though...

I've already alluded to the testing a little bit, but for a change like this, it's something I would use on a daily basis and that covers a lot of the edge cases. At a minimum:

  • Do windows update correctly with StartsOnScreen/StartsOnDesk/StartsOnPage
  • Do windows retain their correct desk/monitor when moving windows between pages/monitors
  • Do windows retain their correct desk/monitor when paging
  • Do windows retain their correct desk/monitor when restarting fvwm

ThomasAdam avatar Aug 07 '21 12:08 ThomasAdam

Agreed on the macro vs function thing :-)

Should 'recapture' really switch desks? When fvwm3 restarts, we definitely don't want to change the desks of all the windows -- we want them to stay exactly where they were under the old instance.

Window movement also should not switch desks, I think. I've just discovered that we incorrectly switch desks in at least one window-move situation: while currently viewing desk 1, use the FvwmPager window to move a window around on desk 2 using mouse-2-click-n-drag. It should stay on desk 2 but currently we move it to the current desk when you release the mouse pointer. Similarly, trying to drag a window away from the current desk to a different desk using the pager doesn't always work. In both cases we also hit the bug I noticed earlier that the "update the desk of the window" doesn't seem to be doing it completely, in that it changes it as far as fvwm is concerned (the representation of the window in the FvwmPager goes to the current desk, in the WindowList it appears in the section for the current desk) but it doesn't actually change the desk of the window from X's point of view -- the window does not get mapped on the current desk until you do a "switch to another desk and switch back".

Window mapping: certainly if we haven't put the window on any desk at all yet (i.e. initial placement) we should default to current-desk. But if the window already has a desk associated with it it's less clear -- if you say "unminimize window A, which happens to be on a different desk", maybe it ought to just stay on that other desk? We remember its old x/y/w/h, after all.

pm215 avatar Aug 07 '21 13:08 pm215

Recapturing is just setting the desk to where the window is -- which is what the macro does anyway.

Window movement most definitely does want to switch desks -- consider the case of DesktopConfiguration per-monitor where the desk is completely different to the desk the window was on before it was moved.

I can't reproduce the FvwmPager problem you've mentioned, but if that's a bug, it's separate to this and will need a new issue creating for it.

ThomasAdam avatar Aug 07 '21 13:08 ThomasAdam

The macro doesn't do "set the desk to where the window is". It figures out the monitor corresponding to the window's x,y position, and then it sets the desk to the current desk that monitor is viewing, which could be an entirely different desk to the one the window is (was) on. (Restart of fvwm does work for me, though. I might try to trace through why it doesn't move all the windows to the current desk, just to try to improve my understanding of the codebase.)

I'm not currently using "DesktopConfiguration per-monitor"; I use the default of "global". I agree that in the per-monitor case you do want to switch desks if you're doing something like dragging a window from one monitor to another. But if you're programmatically moving a window on a non-current desk then it shouldn't change desk as a side-effect. Maybe the rule should be "if the window was on the current-desktop of the monitor it used to be on then set it to the current-desktop of the monitor it's just moved to, otherwise leave the desktop alone" ? That's just an off-the-top-of-my-head guess, though.

pm215 avatar Aug 07 '21 13:08 pm215

I just gave fvwm3-1.0.6a a shot and I do no longer experience this problem when switching between my Gentoo desktop and whatever other connected computer and then back.

hrnick avatar Apr 01 '23 12:04 hrnick

Thanks for the update, @hrnick.

I'm not entirely sure what's changed to alleviate this problem though.

ThomasAdam avatar Apr 01 '23 13:04 ThomasAdam

I'm happy to help!

I looked through the change log to see if this had been addressed but decided to give it a whirl even though I didn't find anything and now I'm a happy fvwm3 user! So for me this ticket can be closed unless you want to keep it open and dig deeper into it.

hrnick avatar Apr 14 '23 11:04 hrnick

I still have problems with windows moving to the first desktop when disconnecting monitors. Like what pm215 describes in https://github.com/fvwmorg/fvwm3/issues/409#issuecomment-864406930

It was a bit finicky, but I reproduced it like this, with a real external monitor, using latest commit 79e445d6fb580c33d0ce739db06615b1cd56c24b and the default config:

  • Switch to desktop 2 by pressing ctrl-2
  • Open a window on the external monitor
  • Switch back to desktop 1 (ctrl-1)
  • Disconnect and reconnect the external monitor
  • Switch to desktop 2, now the window is gone
  • Switch to desktop 1, now the window has moved to desktop 1 (but still present on monitor 2 as it should)

Attaching the log if it might contain anything usable: https://pastebin.com/qT4beqbm

entropic77 avatar Apr 14 '23 17:04 entropic77

Hi,

I think some of this is now fixed -- certainly #978 might have helped this situation. I've done some testing on this and I can't any longer reproduce this, so I'm happy to close this.

Should this not be true, please reopen additional issues.

This will all be in the upcoming 1.1.0 release.

ThomasAdam avatar Mar 29 '24 18:03 ThomasAdam