Created attachment 278572 [details] [review]
window: Keep track of preferred output

Remember the last monitor a window was moved to by user action and
try to move it back on monitor changes; this should match user
expectations much better when a monitor is unplugged temporarily.

Just a note, I've just found out that the extension doesn't work correctly with dynamic workspaces:
https://github.com/aiguofer/return-to-monitor/issues/2

I can't read from the patch whether that was taken into account or not, so I just mention it, in case. It would be nice to also keep windows on correct workspaces, not just squash them to the first one, although on the correct monitor. (But of course, I don't want to get greedy, any improvement is welcome:)).

(In reply to comment #2)
> It would be nice to also keep windows on correct workspaces, not just squash
> them to the first one, although on the correct monitor.

By default we pretend that workspaces don't exist on non-primary monitors, so what the correct workspace should be when moving to the primary monitor is not always obvious (in particular when the app has originally been started on a secondary monitor). So while the patch does not share any code with the extension, it is likely affected by this (more general) issue as well ...

My naive idea was simply copying the workspaces from the disconnected monitor to the new primary monitor. After reconnecting the original monitor, moving the workspaces back (exactly as they are), with the exception of apps previously being on the secondary monitor. Not sure if it's easily achievable. It seems easy on paper :-)


                            Primary
                            ~~~~~~~
                   .-----------------------.
      Secondary    |        firefox        |
      ~~~~~~~~~    |                       |
  .---------------.|-----------------------|
  |               ||        nautilus       |
  |               ||                       |
  |     xchat     ||-----------------------|
  |               ||         gedit         |
  |               ||                       |
  '---------------''-----------------------'

                   |
                   | disconnect monitor / undock
                   |
       Primary     v
       ~~~~~~~
  .---------------.
  |firefox, xchat |
  |---------------|
  |    nautilus   |
  |---------------|
  |     gedit     |
  '---------------'

                   |
                   | make some changes in apps  
                   |
       Primary     v
       ~~~~~~~
  .---------------.
  |firefox, xchat |
  |---------------|
  |     gedit     |
  |---------------|
  |   rhythmbox   |
  '---------------'

                   |
                   | reconnect monitor / dock   
                   |
                   v        Primary
                            ~~~~~~~
                   .-----------------------.
      Secondary    |        firefox        |
      ~~~~~~~~~    |                       |
  .---------------.|-----------------------|
  |               ||         gedit         |
  |               ||                       |
  |     xchat     ||-----------------------|
  |               ||       rhythmbox       |
  |               ||                       |
  '---------------''-----------------------'

This is great! It was fun making the extension just to get a little better understanding of the extension system, but integrating this into the shelf itself would be the best solution :)

Just wanted to comment on a few use cases that should be considered (if possible):

- 'show only on primary display' set to OFF and 'static' workspaces
  - this is the easiest, and the set up I had in mind originally with the extension
- 'show only on primary display' set to ON and 'static' workspaces
  - I think this may need to account for which physical monitor is which MetaMonitor
- 'show only on primary display' set to OFF and 'dynamic' workspaces
- 'show only on primary display' set to ON and 'dynamic' workspaces

Review of attachment 278572 [details] [review]:

LG, fixes one of the most annoying multi-monitor bugs.

(In reply to comment #4)
> My naive idea was simply copying the workspaces from the disconnected monitor
> to the new primary monitor. After reconnecting the original monitor, moving the
> workspaces back (exactly as they are), with the exception of apps previously
> being on the secondary monitor. Not sure if it's easily achievable. It seems
> easy on paper :-)

It's not, given that workspaces are per-screen and not per-monitor (or in other words: there is only a single set of workspaces that spans all monitors). There are basically two issues at hand (both only tangentially related to the attached patch) when workspaces-only-on-primary is set:
 (1) when moving a window from a non-primary monitor to the primary,
     we also move it to the active workspace; that's because when
     actually moving the window (dragging or <super><shift>left/right),
     having it suddenly display would be weird (see comment[0]).
     I don't think this applies when moving the window due to changes
     in the screen configuration (e.g. its monitor got unplugged) though,
     I'll attach a patch to preserve the workspace in that case
 (2) when dynamic-workspaces is set, windows on secondary monitors may
     be moved to a different workspace when their workspace is removed;
     that's because windows that are shown on all workspaces are ignored
     when determining "empty" workspaces - I don't see a way around this,
     apart maybe from differentiating between visible-on-all-workspaces-because-sticky
    and visible-on-all-workspaces-because-on-non-primary cases. However
    I think keeping apparently empty workspaces alive for windows on
    secondary monitors would cause more confusion than the current behavior,
    and not sure storing a preferred workspace index and restoring it
    is worth the code (what should happen when a window is moved from the
    sixth workspace to the secondary monitor, but at the time it is returned
    there are only two workspaces?). It's also a bit of a fringe case IMHO - if
    I care deeply about the workspace a window on non-primary monitors is on,
    why would I "disable" workspaces on non-primary monitors?

[0] https://git.gnome.org/browse/mutter/tree/src/core/window.c#n3606

Created attachment 279092 [details] [review]
window: Add user_op parameter to update_monitor()

When workspaces-only-on-primary is set and a window is moved back to the
primary, we also move it to the active workspace to avoid the confusion
of a visible window suddenly disappearing when crossing the monitor border.
However when the window is not actually moved by the user, preserving the
workspace makes more sense - we already do this in some cases (e.g. when
moving between primary monitors), but miss others (unplugging the previous
monitor); just add an explicit user_op parameter as used elsewhere to cover
all exceptions.

I'd have to try it, but I think the patch above implements what bothered me most, thanks for that! Simply put, I'd love this to work: 4 workspaces on notebook monitor -> dock it -> 4 workspaces moved to external (now primary) monitor -> undock it -> 4 workspaces moved back to notebook (now primary and only) monitor. I that works, I'll be ecstatic :-) Currently both docking and undocking squashes all windows from all workspaces to a single workspace and I have to manually separate them back into workspaces every time. I have a lot of windows and I dock and undock several times a day.

(In reply to comment #9)
> I'd have to try it, but I think the patch above implements what bothered me
> most, thanks for that!

Probably not - the case you describe should already work without the patch (i.e. moving from primary to primary); it does appear to be the case here in a brief test ...

Attachment 278572 [details] pushed as 00c7a27 - window: Keep track of preferred output
Attachment 279092 [details] pushed as 967b6c3 - window: Add user_op parameter to update_monitor()