After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 356715 - On moving horizontally maximized windows between xinerama monitors.
On moving horizontally maximized windows between xinerama monitors.
Status: RESOLVED OBSOLETE
Product: metacity
Classification: Other
Component: general
trunk
Other All
: Normal normal
: ---
Assigned To: Carlo Wood
Metacity maintainers list
Depends on:
Blocks:
 
 
Reported: 2006-09-19 12:06 UTC by Carlo Wood
Modified: 2020-11-06 20:05 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
Version 1 (20060919) of "shake loose" patch. (27.10 KB, patch)
2006-09-19 12:24 UTC, Carlo Wood
none Details | Review
Disregard this. (43.91 KB, patch)
2006-09-26 21:05 UTC, Carlo Wood
none Details | Review
Version 3 (20060926) of "xinerama" patch. (38.42 KB, patch)
2006-09-26 22:20 UTC, Carlo Wood
none Details | Review
Version 4 (20061001) of "xinerama" patch. (41.34 KB, patch)
2006-10-01 21:10 UTC, Carlo Wood
none Details | Review
Version 5 (20070315) of "xinerama" patch. (40.64 KB, patch)
2007-03-15 03:08 UTC, Carlo Wood
none Details | Review
Version 6 (20070507) of "xinerama" patch. (40.37 KB, patch)
2007-05-07 00:07 UTC, Carlo Wood
none Details | Review
Version 7 (20080129) of "xinerama" patch. (40.63 KB, patch)
2008-01-28 23:39 UTC, Carlo Wood
none Details | Review
Old same shit, again. (40.72 KB, patch)
2008-08-19 01:03 UTC, Carlo Wood
accepted-commit_now Details | Review
Version 10 (20080822) of "xinerama" patch. (41.04 KB, patch)
2008-08-22 00:00 UTC, Carlo Wood
none Details | Review

Description Carlo Wood 2006-09-19 12:06:30 UTC
Summary: The current 'shaken_loose' code allows to move fully maximized
windows from one monitor to the next. This is not possible for only-horizontally
maximized windows. A patch was written that treats horizontal and vertical
direction (nearly) symmetrically in regard to 'shake loose'. Fully maximized
windows are not treated as special, though their behaviour does not really
change -- however, as a side effect of this patch a few other issues for
fully maximized windows have been fixed (pointer stays inside the window,
window stays on monitor where the pointer is, etc).

Background: I recently bought a new PC, after such severe problems during
upgrading fedora core 4 to 5, that I wasn't able to recover from it anymore(!).
Needless to say I switched distributions this time (I've never upgraded
redhat/fedora and NOT ran into serious problems). At the same time I decided
switch desktops from KDE to gnome, because my choice has been GTK+ over QT
for a few years already - and recently I've started to write GUI applications
myself. Being a perfectionist, I'm willing to do whatever it takes to get my
"desktop" perfect. After long consideration, I decided that metacity (the
default window manager of gnome/debian) was the best choice for me, although
I'd have to patch it here and there (I think this is take me less time than
-say- configuring fvwm to fit my needs). I got the source code and read
all of the documentation as well as several online publications that it
refered to -- and yes, it became clear to me that metacity is NOT interested
in new features. I don't believe this is a new feature however; it is a bug fix.
I started hacking the code a few days until I figured out where the problem
was. I mailed Havoc Pennington to poll him for interest in my ideas and he
replied with "Sounds reasonable", but directed me to the metacity list or
bugzilla as he wasn't the most active maintainer anymore. I posted a mail
to the developlist first - but Elijah requested me to post it on bugzilla
instead, it being the main means of communication.

Description of the bug:

This addresses the case where a user have a xinerama setup, and uses
partially maximized windows. Although the patch treats vertical and
horizontal the same; for the sake of clarity, lets talk about the most
common case of one monitor being LeftOf (or RightOf) another monitor,
rather than one above the other. In that case we observe that means are
provided to easily move a fully maximized window from one monitor to another
(by ALT-clicking it, or by clicking on the title bar, or by pressing ALT-F7
and using the arrows) by first moving it down until it un-maximizes - move
it over to the other monitor and then place it against the top of the
monitor, at which point it maximizes again. This is not possible for
horizontally maximized windows. The expected behaviour would be that one
grabs a horizontally maximized window and attempts to move it far enough
to the right or left, certainly towards another monitor, that it would also
unmaximize and allow to be dragged over, where it would maximize horizontally
again once it touched the left or right edge of the monitor there.

This patch fixes that particular bug:
* Allowing to shake loose partically maximized windows for the
  sake of moving them to another xinerama monitor and having them
  re-maximize there.

It does not attempt to address many other bugs that are in metacity
in relation to moving windows, (un)maximizing, moving while snapping etc.
I believe however that those bugs are unrelated, this patch does not
introduce NEW bugs *), and - although I might be interested in fixing more
bugs - this patch should be committed to CVS before even looking into those
other problems.

*) There are a few things that still need work however. I'll need help with
that. First of all, I could only _test_ this with my own setup (two monitors
next to eachother). I was not able to test wireframe mode, because I have
simply not been able to figure out how to turn that on :p. I have not yet
tested the case where one monitor has a different resolution. And finally,
the patch contains a function that tries to guess on what monitor a window
would be maximized if the appropriate maximize function is called; it does
a perfect job for me - but I think it should be fixed to use an existing call
that is also used by that very maximize function (I couldn't find it).

I think the code (patch) is self explanatory, as I commented it well.

I've studied all previous bug reports related to 'shake loose', 'xinerama',
and 'maximized window' and think that the only REALLY related bug is
http://bugzilla.gnome.org/show_bug.cgi?id=304927
However, that bug report starts off taking about resizing maximized
windows - I did not touch that code. This patch is about moving them only.
http://bugzilla.gnome.org/show_bug.cgi?id=323820
seems more related, but is marked as duplicate of #304927

I've added everyone of #304927 to the CC field in order to allow them to
follow this discussion.

Bugzilla doesn't allow me to attach anything yet, I'll add the patch later.

Carlo Wood
Comment 1 Carlo Wood 2006-09-19 12:24:12 UTC
Created attachment 73031 [details] [review]
Version 1 (20060919) of "shake loose" patch.

I noted one more case where the window jumps from one monitor
to the next - while the mouse stays on the current monitor.
I will look into that and fix it later. This patch can already be
used to review the general idea, however.

Also, the changes in src/keybindings.c still need to be fixed to support
partially maximized windows as well.
Comment 2 Thomas Thurman 2006-09-26 01:24:16 UTC
It all seems reasonable to me, but I haven't actually compiled it yet. Some thoughts on the patch so far:

check_threshold:

-- I'm not sure I like the fudge factor of 100. It seems a bit arbitrary.

-- I would prefer to return a named constant of -1 rather than the particular value.

do_shake_loose:

-- What does it mean if this fn returns true/false?

/* At this point only one of relevant_x or relevant_y is set. */
-- they could both be clear (prefer "are not both set")

-  /* Don't bother doing anything if no move has been specified.  (This
-   * happens often, even in keyboard moving, due to the warping of the
-   * pointer.
+  /* Don't bother doing anything if no move has been specified.
+   * (This happens often, even in keyboard moving, due to the
+   * warping of the pointer.

-- Why did this get reformatted?

Spelling nitpicks:

+       *    proportially equivalent to where it was initially at the
"proportionally"
+       * Point 1 garantees that also then the grab position remains
"guarantees"
+       * sight. Therefore, unmaximizing is prohibitted if moving the
"prohibited"

I will get this compiled and tested, and then I will tell you what I think about that.
Comment 3 Carlo Wood 2006-09-26 02:18:24 UTC
(In reply to comment #2)
> -- I'm not sure I like the fudge factor of 100. It seems a bit arbitrary.

Agreed, this is why I said "And finally,
the patch contains a function that tries to guess on what monitor a window
would be maximized if the appropriate maximize function is called; it does
a perfect job for me - but I think it should be fixed to use an existing call
that is also used by that very maximize function"

In the meantime I have made substantial additional changes, mostly additional bug fixes for metacity. I have already fixed this point and the '100' is not any longer present. The last thing I'm now working on is placement at maximization, which turns out to be heavily influenced by the value of user_rect. I gave saved_rect a more defined meaning than what it was, which was a bit fuzzy: it now means "the position the window was at, before it was maximized", most importantly - it is no longer changed while moving around a window that was shaken loose (which is only a temporary state until it is maximized again). The meaning of user_rect seems to be "the target position when maximizing", but from it's name that is clearly not what was intended. This bothers me a lot.

> -- I would prefer to return a named constant of -1 rather than the particular
> value.

Will do.

> do_shake_loose:
> 
> -- What does it mean if this fn returns true/false?

false means that nothing was done, and a normal move needs to be handled.
true means that the shake loose code has been in effect, and move_update
can return because everything has been handled. This is not really
different from the old code, except that I did put all the code related
to 'shake loose' in a separate function.

> /* At this point only one of relevant_x or relevant_y is set. */
> -- they could both be clear (prefer "are not both set")

I'll change that into "at most one of ..."

> -  /* Don't bother doing anything if no move has been specified.  (This
> -   * happens often, even in keyboard moving, due to the warping of the
> -   * pointer.
> +  /* Don't bother doing anything if no move has been specified.
> +   * (This happens often, even in keyboard moving, due to the
> +   * warping of the pointer.
> 
> -- Why did this get reformatted?

It was changed, and then changed back I guess. I didn't write the patch
by sitting down and hitting the right keys ;). I've added a lot of debugging
code - did many tests (and made many fixes). I have even done a lot of
debugging sessions in gdb, which turns out to be possible when you own a
second PC with a third monitor ;)

I tried to correct weird looking things in the final diff (especially
things that got reformatted without actually changing something), and I
guess I missed this one.

> 
> Spelling nitpicks:
> 
> +       *    proportially equivalent to where it was initially at the
> "proportionally"
> +       * Point 1 garantees that also then the grab position remains
> "guarantees"
> +       * sight. Therefore, unmaximizing is prohibitted if moving the
> "prohibited"

Thanks

> I will get this compiled and tested, and then I will tell you what I think
> about that.

Thanks, but realize that a new patch is coming up. This doesn't mean that I will continue to post new patches. The reason for these two steps is this: My first patch was written for version 2.14.x - or whatever the version is that comes with debian 'testing'. It worked fine for me, and really wanted to get some feedback from the maintainers at that point. Of course, I first got the CVS tree now and applied my patch to this. The diff didn't apply cleanly and I had to make some changes before it compiled. I have posted it here after that, even though I never really tested it with the CVS sources. New problems have surfaced, though all of them related to inconsistencies in metacity, not my original patch (I take pride in writing bugfree code, so that is important to me ;). My next patch will be written FOR the current CVS, and I will have dealt with every loose end that I've mentioned in my first post, and any point that you brought up or will bring up before I post it.

Thanks for your reply,
Carlo Wood

Comment 4 Carlo Wood 2006-09-26 12:36:56 UTC
I'm having problems with the maximization code: I can't decide what is most logical -- or rather, perhaps there isn't just one way that is most logical.
Surely different people will have different opinions/needs considering this, so it seems that some extra configuration possibilities are needed.

However, for a start I'd like to try this without extra configuration options. I'd like feedback from the maintainers on the following (and please, don't wait a week with that: I work on this every day - I need the feedback ASAP).
I repeat,

Maintainers, PLEASE READ THIS -- AND REPLY AS SOON AS POSSIBLE

Because this is rather complex, let me approach the issue by starting with a few "axioms". Note that I did bind ALT-F10 to 'maximize fully', ALT-F11 to 'toggle maximize vertically' and ALT-F12 to 'toggle maximize horizontally'. Below, the notation "ALT-F11/12" means: Pressing ALT-F11 OR pressing ALT-F12: there is a lot of symmetry between horizontal and vertical behaviour, so we can compress the sentences by collapsing remarks about horizontal and vertical into one.

The first axiom has a very large impact on the overall behaviour and it is not very locigal, but you have to start somewhere :(

1) At any time, pressing ALT-F11/12 (toggle max.vert./hor.) twice on a row
   should have no effect on the lay-out on the screen.

The rationale behind this, is that I think that those bindings really should be a 'toggle', in the sense that if one makes a mistake and is not happy with the result of pressing ALT-F11/12, then pressing it again should act as an UNDO.
It could be highly annoying when the window ended up on -say- a different monitor after pressing the key a second time than where it was initially.

2) The vertical and horizontal (un)maximization algorithms should be orthogonal and symmetrical in behaviour.

The rationale here is that maximizing fully is literally handled in the code in a symmetric way with horizontally/vertically: full maximized means that it is maximized horizontally AND vertically. Therefore, when maximizing in both directions, in any order, the result has to be the same. Also, in a xinerama setup monitors can be next to eachother, or above eachother, or in a matrix.
There really isn't any reason to treat horizontally and vertically different.

3) Starting with any unmaximized window, pressing ALT-F10 should maximize the window on the same monitor that it is currently on.

The rationale behind this is is that I think that the algorithm for the decision of where to place a window should be stateless: not depending on the previous history of where the window was placed before. The reason for that is that the user might or might not be aware of that history (anymore). The normal behaviour  of a new window that is being maximized is obviously that it is maximized to the same monitor as it is on and therefore this is the expected behaviour for a window that has been unmaximized and on that monitor for a full week, too: the user will not remember where that window was before that week and will expect it to behave like any (newly opened) window when being maximized. An algorithm that would take time into account (how long have I been on this place?) is too complex imho.

Using the previous axioms, we now can draw some conclusions:

4) Starting with any unmaximized window, pressing ALT-F11 followed by ALT-F12
should have the exact same effect as first pressing ALT-F12 and then ALT-F11.
The end result should be the same as pressing ALT-F10 instead.

This is a result of axiom 2.

5) Starting with any unmaximized window, pressing ALT-F11/12 should keep the window on the same monitor.

This is a result of the symmetry demand as well. First of all, we want that if we press first ALT-F11 and then ALT-F12, then the result should be the same as pressing ALT-F10: maximized on the original monitor (axioms 3 combined with 4). Now assume that pressing ALT-F11 would move the window to a different monitor,
then pressing subsequentially ALT-F12 should move it back to the original monitor again. This makes no sense, as horizontally and vertically behaviour should be orthogonal (axiom 2): the course behaviour of maximizing horizontally (like switching monitors) may not be influenced by it being maximized vertically or not. Perhaps 5 is partially axiomatic... but I really feel that also pressing ALT-F11 or ALT-F12 should keep the window on the same monitor as it was, just like when maximizing fully, as a result of the symmetry demand.

The following is more like a corollary:

6) Assuming two monitors next to each other, the behaviour of ALT-F11 (toggle maximize vertically) should not differ for windows that are moved horizontally (albeit in vertically maximized state or not) a little (staying on the same monitor) or a lot (being moved to an adjacent monitor).

This is what defines 'Xinerama' imho: the two monitors form a single desktop.
You can move a window horizontally and it moves from one monitor to another.
You can even put it halfway. It would be counter intuitive when the algorithm of  (un)maximizing vertically would be influenced by any amount of horizontal movement.

Now, combining 2, 5 and 6 we have to draw the following conclusion:

7) (Un)maximizing vertically should never alter the width, or the horizontal position of a window, and likewise, (un)maximizing horizontally should never alter the height, or the vertical position of a window.

Firmly believing in this conclusion, we can now tackle the more fuzzy behaviour of the 'shake loose' algorithm.

8) Assuming two monitors next to each other, and having a vertically maximized window, moving the window horizontally from position A to B (without shaking it loose) should result in the exact same state as when we would shake the window loose and re-maximize it vertically at position B by placing it close to the top of the screen again.

The rationale here is that it is too easy to accidentally shake a window loose.
If the user doesn't let go of the mouse button (or whatever he's using to grab the window) and corrects his 'mistake' by re-maximizing the window - then window might have followed a different path to end up where it is - that really should not alter it's future behaviour in regard to anything (like, pressing escape or pressing ALT-F11).

Likewise,

9) Assuming two monitors next to each other, and having a horizontally maximized window, moving the window vertically from position A to B without shaking it loose, should result in the exact same state as when we would shake the window loose and re-maximize it horizontally at position B by placing it close to the border of the same monitor again - independent of the path we take to get there.
Even if we first move the window over to a different monitor, having it re-maximize there and, without letting go of the mouse button, shake it loose again and move the window back the original monitor.

Note that this state includes the position to where the window will unmaximize (saved_rect). It therefore seems most logical to move this position along to a different monitor every time we re-maximize on a different monitor (in order to satisfy the demand that unmaximizing is not allowed to switch monitors, we have to move saved_rect over to the new monitor), in a way that moving it back to the original monitor makes it end up in the same place: by keeping the relative position to the border of the monitor(s), in the direction of maximization, fixed.

This might cause a problem when the two monitors have a different size: theorectically a window could completely fall off a monitor that way -- I don't believe in resizing it however, so we'll just have to make a special case of that and check if a window is still visible after switching monitors, and if not - then break the rules and move it to the top-left corner (as is the current code doing at all times).

Ok, I hope all of the above appeared to you as boring and trivial - then we at least agree ;). If however you have concerns about the described behaviour let me know as soon as possible. Possibly to your surprise, in order to achieve the described behaviour, I will have to make some rather drastic changes to the current code - so, it is important we are all on one line here.

Thanks,
Carlo Wood
Comment 5 Elijah Newren 2006-09-26 18:10:28 UTC
(In reply to comment #4)

In regards to 1-7, all sound reasonable.  However, note that your wording for #3 about the "monitor that it is currently on" is ambiguous; windows can be on more than one monitor at a time.  I believe you meant something like "monitor where the largest percentage of pixels of that window are located"?

In regards to 8-9, _if_ you assume that the "shaken loose" stuff is desirable, then this sounds reasonable.  I still think the "shaken loose" stuff is unnecessary, over-complicated, buggy, and should just be nuked.  More on that below.

> This might cause a problem when the two monitors have a different size:
> theorectically a window could completely fall off a monitor that way

This has already been filed as bug 350353; should be relatively easy to fix.  Once we just add a few more STRUTs in such cases, the constraints code will automatically take care of cases like this.

> check if a window is still visible after switching monitors, and if
> not - then break the rules and move it to the top-left corner (as is the
> current code doing at all times).

That sounds like you're triggering bug 329152 or something like it.  I don't think the top-left corner is what we want; rather, the position closest to where we would have unmaximized to is what we want.

> Ok, I hope all of the above appeared to you as boring and trivial - then we at
> least agree ;). If however you have concerns about the described behaviour let
> me know as soon as possible. Possibly to your surprise, in order to achieve
> the described behaviour, I will have to make some rather drastic changes to
> the current code - so, it is important we are all on one line here.

drastic changes to the current code?  That comment and a brief glance at the current patch sound like we're headed towards code that will be rather difficult to maintain.  I agree that the current shaken_loose code is ridiculously buggy and needs to be changed (see bug 304927 comment 3), but why do we even want to keep the shaken loose behavior?

I know a lot of people complained when I suggested removing the shaken loose behavior before, but all the complaints were "I depend on being able to move maximized windows between xineramas; it's such a cool feature."  I'm not arguing with moving windows between xineramas being a cool or needed feature, just the conclusion they draw from that argument: I don't see why shaken_loose is necessary in order to move maximized windows between xineramas.

Granted, I haven't looked at this as closely as you but can't we fix all these problems by doing something like
  - Nuke all the shaken_loose code
  - Remove the "Don't allow movement in the maximized directions" section of
    code in update_move(), since the constraints correctly forces to the
    nearest monitor anyway.  (This may mean moving of maximized windows via
    the keyboard won't work, but I'm pretty sure it didn't work before
    anyway, so that's a separate bug)
  - change meta_window_unmaximize() to use a mixture of window->rect and
    window->saved_rect instead of always passing window->saved_rect to 
    meta_window_move_resize() (we should use saved_rect only in the
    directions the window is being unmaximized in; not doing so is causing
    many of the bugs you are seeing)
  - store a saved_xinerama as well.  When unmaximizing, interpret saved_rect
    relative to saved_xinerama (i.e. add current_xinerama offset -
    saved_xinerama offset to the saved_rect positions just before unmaximizing)

Anyway, that's my $0.02.  Thoughts?

Thanks for working on this.  :)
Comment 6 Elijah Newren 2006-09-26 18:18:55 UTC
I'm not sure whether to mark bug 355497 as a duplicate of this bug or not, since this bug seems to be about so many issues.  Anyway bug 355497 is specifically about Carlos' issue #7 from comment 4.
Comment 7 Elijah Newren 2006-09-26 18:18:59 UTC
*** Bug 355497 has been marked as a duplicate of this bug. ***
Comment 8 Carlo Wood 2006-09-26 21:02:00 UTC
Elijah, thanks for (finally) giving a reaction.
I just did finish the final patch however, removing your argument
that the shake loose code is buggy :p

This patch fixes everything - it's behaviour is logical, intuitive
and consistent. I set the shake loose threshold very high - so you
won't even be bothered by it if you don't like windows to be shaken
loose (it won't happen by accident like this). Re-attaching is set
a bit lower, so it's possible to place a window on a reasonable
place on the screen without that it maximizes. Of course, these
thresholds should really be configurable by the user (like the snap
threshold), but for now these values will do.

About maintainability. Surely, nuked code is easier to maintain: there
is no code to maintain anymore. But you'd throw away some perfectly
working code that I spend the last three weeks of my life on :/
Also, you shouldn't judge maintainability of code from it's patch.
Apply the patch and then get acquainted with the code. It is very well
commented. This shouldn't be hard to maintain at all. If anything, things
are a lot more logical now and the result is EASIER to maintain then
the buggy code we had before.
Comment 9 Carlo Wood 2006-09-26 21:05:48 UTC
Created attachment 73461 [details] [review]
Disregard this.

This patch fixes a lot more than just the shake loose code, although
most fixes are only really apparent in conjunction with xinerama and
shaking loose windows. Nevertheless, I think it should really be
called "xinerama" patch now.
Comment 10 Carlo Wood 2006-09-26 21:43:23 UTC
More about maintainability:

ALL shakeloose code, except for the places where
shaken_loose_horizontally or shaken_loose_vertically is tested
along side with maximized_horizontally or maximized_vertically
is basically in one place: do_shake_loose(). This function
uses a few other static functions that are defined directly
above it. Nuking this code merely mean that inside update_move
you would remove these lines:

  /* Handle (un)maximization thresholds */
  if (do_shake_loose(window, x, y, dx, dy))
    return;

Imho, leaving that in doesn't make things harder to maintain.
There are some clearly recognizable other places where
shake_loose_* is tested, like in process_cancel_grab().

All other changes, outside of do_shake_loose(), are bug fixes
that are unrelated to shake loose. You will need to keep it anyway.
These changes are mainly needed to better define the meaning of
'saved_rect'. Before, saved_rect had a rather fuzzy meaning that
gave rize to all kinds of unexpected and/or inconsistent behaviour
(though mainly in conjunction with moving maximized windows to other
monitors - not really related to the shake loose code. You would run
into the same inconsistencies if you 'solve' the moving of maximized
windows in a different way than using the shaken_loose_horizontally and
shaken_loose_vertically state booleans).

The meaning of saved_rect with this patch is now:
The x coordinate and the width are the last position of the
horizontally unmaximized window. The y coordinate and the height are
the last position of the vertically unmaximized window. Together they
form the rectangle that a fully unmaximized window would return to.
This means, for example, that a vertically maximized window that is being
moved horizontally, *does* update the x coordinate.

Hmm, I just realized something - please totally disregard my last
posted patch - I will upload a new patch tomorrow.



Comment 11 Carlo Wood 2006-09-26 22:20:33 UTC
Created attachment 73467 [details] [review]
Version 3 (20060926) of "xinerama" patch.

The final patch - I had forgotten two lines of code, to update
saved_rect.width and height as well :p. Things now also seem to
work under resizing.
Comment 12 Carlo Wood 2006-10-01 21:10:38 UTC
Created attachment 73775 [details] [review]
Version 4 (20061001) of "xinerama" patch.

The same as version 3, except that:
1) It doesn't introduce TAB's.
2) I'm now using a shake loose threshold of 256 pixels, instead of 40 times the 'drag' threshold. The 40 was needed on my machine to make it really hard to shake something loose - but it's too big use as multiplier for a basically unknown value. The rationale here is: a shakeloose should happen when a user tries to drag a window over to a different monitor, and not accidentially when he is adjusting an unidirectionally maximized window. A high threshold also makes it way more stable while snapping the window back to the edge of the monitor (when it sticks, it sticks).
Comment 13 Christof Krüger 2006-10-23 10:04:15 UTC
Hi Carlo,

I'm a metacity user who tried your patch because there were a lot of bugs with metacity and xinerama. First of all: Great work! It solved a bug I was about to file in bugzilla (that was acutally the reason I came here but then I stumbled upon this bug): The previous behavior when unmaximizing windows that were on both monitors before maximizing was strange. It moved the window so that it was only on one monitor afterwards. This is fixed with your patch. Thanks for that.

Now some comments:

I don't use only horizontally or vertically maximized windows often, so I can't give you feedback on that. But I can tell you my experiences with fully maximized windows.

1) I find the shaken-loose threshold too high. I use the shaken loose feature to move windows between monitors or to unmaximize windows (because I'm too lazy to find and hit the right keys on the keyboard or aiming at the right button on the top right corner of the window). I would prefer 50 pixels at maximum, or let's say 64 to have "round" numbers ;)

2) I'd like to be able to move windows between monitors without having to shake them loose vertically first. This is very buggy in the current release. Your patch removes the strange behavior but unfortunately, nothing happens when I move the mouse horizontally only. I've described the problem in Bug:358715. I had created a crude patch for it but I've not the time to look deeper into the code and try to hack it myself these days. I'd really appreciate if you could take a look at it.

3) I've found a bug:
* Maximize a window on the left monitor
* Grab the window bar with the mouse being quite to the right of the window.
* Move the mouse down until the window "shakes loose"
* Move the window horizontally until the cursor is on the right monitor but let the center of the window stay on the left monitor
* move the window up so that it will be in vertical "snap distance"
* now move the windows horizontally to the right.
The window will maximize on the left monitor instead of the right one.

4) I don't like the fact that the mouse cursor is being moved when maximizing. It confuses me for half a second because I have to check where it has moved to. I'd prefer if the grab point position had been updated upon maximizing, keeping the mouse position constant. In contrast, the current code is changing the cursor position and keeping the grab point constant.
----

Even with the issues above, I'd like to see your changes checked in ASAP because they fix some things I run into quite often. It's a big improvement to the current behavior. But then again, I'm just a simple user ;)

Bye,
 Christof
Comment 14 Carlo Wood 2006-12-05 15:49:35 UTC
Hi Christof, thanks for you kind words. Sorry that it took me this long before replying. I'm still waiting actually for Elijah to comment on
http://bugzilla.gnome.org/show_bug.cgi?id=358311
(and to finally commit that) before I'll look at this patch again.
At this moment it seems that Elijah is ignoring my patches, so I see no reason to continue on them.
Comment 15 Carlo Wood 2007-03-15 03:08:33 UTC
Created attachment 84625 [details] [review]
Version 5 (20070315) of "xinerama" patch.

This is "just" an update for the current state of SVN.
I'd really appreciate it if this patch (and the two others
that are pending, which influence the behaviour too much
not to be applied before judging the changes) could be committed
before I have to fix collisions again.
Comment 16 Carlo Wood 2007-05-07 00:07:33 UTC
Created attachment 87669 [details] [review]
Version 6 (20070507) of "xinerama" patch.

This is -again- "just" an update for the current state of SVN.
I'd really appreciate it if -this time- this patch could be committed
before I have to fix collisions again...
Comment 17 Carlo Wood 2007-07-31 14:46:13 UTC
When is this patch FINALLY going to be reviewed and committed?
I am getting more then tired about this now :/

I understand the maintainers don't have a multi-head setup,
but that doesn't mean you don't have to support those users
who do. This patch exists already - what is the problem with
applying it?

If there are any problems, I will work on that. The reason that
I've stopped working on metacity at all is just because of this
lack of feedback. It seems to make no use to write patches for
an application when they are being ignored for two years.
Comment 18 Havoc Pennington 2007-07-31 15:29:46 UTC
"doesn't mean you don't have to support those users who do"

Let's be clear - everyone involved here is a volunteer and doesn't "have" to support anyone or anything on any timetable.

I am copied on all the metacity bugs, and I can tell you that Elijah and Thomas make it through a good many of them. Making it through all of them would be impossible. They also work on non-metacity stuff.

The way maintenance works is that once you are fixing bugs more slowly than they come in, the backlog just grows forever, unfortunately, unless some major new infusion of the right kind of help appears.

Getting upset isn't going to make the bug load lower for a limited supply of volunteers.

I'm telling you the facts since the current metacity maintainers will probably be too polite.

Looking at this patch, it is _not_ easy to review. If I were reviewing it, I think it would take me *most of a day* to do so responsibly.

To be honest I think metacity is in a state where processing the bugs just moves the bugs around; i.e. each patch is about as likely to create new bugs as it is to fix things, unless the patch review is very, very careful and conservative. Certainly it appears that about as many regressions are appearing in bugzilla as fixes. Given this, trying to get the maintainers to apply more patches with less review is NOT helpful.

All this said I'm sure patches are appreciated. The main point is, it isn't like someone just has to click "please commit" - they have to spend a day looking at something like this.
Comment 19 Havoc Pennington 2007-07-31 15:31:51 UTC
Incidentally, the reason metacity did not originally have vertical and horizontal maximize was that I was worried about extra complexity and maintenance time ...
Comment 20 Carlo Wood 2008-01-28 23:39:58 UTC
Created attachment 103921 [details] [review]
Version 7 (20080129)  of "xinerama" patch.

This is an update that deals with the changes that were made in SVN since version 6. Again: in the hope that I don't need to KEEP doing this, it would be nice it was committed.
Comment 21 Carlo Wood 2008-08-17 16:30:36 UTC
Bump ... another 6 months have passed.
Comment 22 Carlo Wood 2008-08-19 01:03:23 UTC
Created attachment 116919 [details] [review]
Old same shit, again.

And again... an update to match the changes in SVN...
*cry*
Comment 23 Carlo Wood 2008-08-21 23:56:12 UTC
(In reply to comment #13)
Hi Carlo,

> I'm a metacity user who tried your patch because there were a lot of bugs with
> metacity and xinerama. First of all: Great work! It solved a bug I was about to
> file in bugzilla (that was acutally the reason I came here but then I stumbled
> upon this bug): The previous behavior when unmaximizing windows that were on
> both monitors before maximizing was strange. It moved the window so that it was
> only on one monitor afterwards. This is fixed with your patch. Thanks for that.
...
> 3) I've found a bug:
> * Maximize a window on the left monitor
> * Grab the window bar with the mouse being quite to the right of the window.
> * Move the mouse down until the window "shakes loose"
> * Move the window horizontally until the cursor is on the right monitor but let
> the center of the window stay on the left monitor
> * move the window up so that it will be in vertical "snap distance"
> * now move the windows horizontally to the right.
> The window will maximize on the left monitor instead of the right one.

Hi Christof, I finally had a look at this. It turns out to be a bug
in metacity (svn) and not in my patch. Nevertheless, I fixed it as part
of my patch now. I'll attach version 10 soon which behaves correctly.
It also fixes monitor jumping for this case (same bug):
* maximize a window on some monitor.
* Shake it loose and move it to the other monitor but don't maximize
  it again (just let let go of the mouse/title bar).
* Then maximize it again.
The window will jump back to the previous monitor and maximize there.
Comment 24 Carlo Wood 2008-08-22 00:00:18 UTC
Created attachment 117166 [details] [review]
Version 10 (20080822) of "xinerama" patch.

Fixes monitor jumping because at the end of grab op the user_rect wasn't saved again.
Comment 25 Rolf Leggewie 2008-12-17 08:18:08 UTC
I'll jump in here and say that I think I experience similar problems.  I haven't read all comments, the bug has grown rather long.  I understand Carlo's frustrations in not seeing his work being applied.  I do understand the devs' hesitation to commit something without review, though.

Carlo, would it be possible to carve out certain things from your 40KB patch so they can be reviewed and applied piecemeal?  Maybe that would be a way to get things moving.

The issue I experience (I think I'll open a separate bug about this) is with a multi-monitor set-up (one small monitor on left side A, one big monitor on right side B).  Maximizing a window on the small monitor, then grabbing the title bar on the right end and moving the window to the bigger screen will result in the cursor being outside the window frame (` denotes cursor position).

maximized window on left side before move:
+-A---------+-B---------------------+
|+--------`+|                       |
||         ||                       |
||         ||                       |
||         ||                       |
||         ||                       |
|+---------+|                       |
+-----------+                       |
            |                       |
            |                       |
            |                       |
            +-----------------------+


previously maximized window moved to right side:
+-A---------+-B---------------------+
|           |  +---------+    `     |
|           |  |         |          |
|           |  |         |          |
|           |  |         |          |
|           |  |         |          |
|           |  +---------+          |
+-----------+                       |
            |                       |
            |                       |
            |                       |
            +-----------------------+
Comment 26 Carlo Wood 2008-12-17 15:20:17 UTC
Did you try my patch? It should solve this (and other) problems.
Although I can understand if you'd rather open a new bug report,
since this one is just ignored.
Comment 27 Carlo Wood 2008-12-17 15:23:00 UTC
I am removing the "depends on", because this patch is NOT depending on 564822
and 564822 is NOT blocking this patch. If this patch is applied then 564822
is fixed too, so instead 564822 should be marked as a duplicate.
Comment 28 Rolf Leggewie 2008-12-17 16:45:19 UTC
No, I did not yet try your patch.  I am too busy right now for that.  I may eventually get around to trying, though.

My guess as to why your patch might be lingering is that you may not be breaking it down far enough into subtasks and -patches.  I would assume that the smaller and the more specific a patch, the more likely it is to be applied quickly.  40K patches have a rather small chance of "making it in" in any project that I know.

My understanding of this bug is that it has grown into a meta-bug of what is wrong when moving maximized windows in multi-monitor setups and thus is really unfixable.  That's the reason I blocked my report on this one, to do exactly what I wrote above, breaking things down into manageable pieces. If you prefer to have them unrelated, I am fine with that.
Comment 29 Rolf Leggewie 2008-12-17 16:48:57 UTC
My report is not a dupe to this one because it is much smaller in scope.
Comment 30 Thomas Thurman 2011-01-22 04:19:12 UTC
Review of attachment 117166 [details] [review]:

Many apologies for the delays in testing this.  I'm still looking at it.  Meanwhile, I have created a branch for it:

http://git.gnome.org/browse/metacity/commit/?h=bug356715&id=12cbf9c7306296a559a71b2ca9cc725465a96816

If you could, on IRC or here, walk me through testing this, that would be appreciated.
Comment 31 Carlo Wood 2011-01-23 13:47:57 UTC
That's easy enough. Make a cinerama setup (ie, two monitors next to eachother),
then maximize windows, either horizontally, vertically or both.
Grab windows by clicking their title bar and start moving them from one monitor to another (you have to move the mouse down a lot if they are maximized vertically (or completely), and left/right a lot when they are maximized horizontally (or completely) before they unmaximize. Don't let go of the mouse button, but move a window to the other monitor until it maximizes again on that monitor.

The test then is: it should be intuitive and logical and easy to move
(partically) maximized windows from one monitor to another with just your mouse.
Comment 32 Carlo Wood 2011-01-23 13:51:29 UTC
PS I've been using it very satisfactory for 5 years now.
PS2 Did you notice how I replied after just ONE DAY?
Comment 33 André Klapper 2020-11-06 20:05:14 UTC
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years.

If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/metacity/-/issues/

Thank you for reporting this issue and we are sorry it could not be fixed.