Merge lp:~charon030/compiz/fix-751605 into lp:compiz/0.9.8

[Charon]

Prevent windows from maximizing on the wrong monitor (LP: #751605)

  This fixes two causes I have found:
    1. outputDeviceForGeometry was wrapping to the wrong monitor (back to the
       top or left) if a window was mostly off the bottom/right of the screen.
    2. Even when outputDeviceForGeometry returned the correct output,
       PrivateWindow::addWindowSizeChanges was sometimes changing it to an
       incorrect output in the case where the old size of a window exceeds
       the dimensions of the smaller monitor you're trying to maximize it on.
  .

This is the same bugfix as it was integrated into mainline by Daniel van Vugt before in:
https://code.launchpad.net/~vanvugt/compiz/fix-751605

[Michael Terry (mterry)]

Seems like we should have a test for this case.

[Daniel van Vugt (vanvugt)]

The automated tests are in the upstream version (lp:compiz) and should not be backported here because they required some architectural change.

[Daniel van Vugt (vanvugt)]

Yep, it's identical to my upstream fix. Just minus the test cases which are not feasible to backport due to architectural changes.

review: Approve

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-09-18 01:44:23 +0000
+++ src/screen.cpp	2012-12-08 21:26:22 +0000
@@ -4243,28 +4243,11 @@
 
     if (strategy == CoreOptions::OverlappingOutputsSmartMode)
     {
-	int centerX, centerY;
-
-	/* for smart mode, calculate the overlap of the whole rectangle
-	   with the output device rectangle */
-	geomRect.setWidth (gm.width () + 2 * gm.border ());
-	geomRect.setHeight (gm.height () + 2 * gm.border ());
-
-	x = gm.x () % screen->width ();
-	centerX = (x + (geomRect.width () / 2));
-	if (centerX < 0)
-	    x += screen->width ();
-	else if (centerX > screen->width ())
-	    x -= screen->width ();
-	geomRect.setX (x);
-
-	y = gm.y () % screen->height ();
-	centerY = (y + (geomRect.height () / 2));
-	if (centerY < 0)
-	    y += screen->height ();
-	else if (centerY > screen->height ())
-	    y -= screen->height ();
-	geomRect.setY (y);
+	/* We're only going to use geomRect for overlapping area calculations,
+	   so the window rectangle is enough. We don't need to consider
+	   anything more like the border because it will never be significant
+	   to the result */
+	geomRect = gm;
     }
     else
     {

=== modified file 'src/window.cpp'
--- src/window.cpp	2012-09-18 15:51:59 +0000
+++ src/window.cpp	2012-12-08 21:26:22 +0000
@@ -3511,71 +3511,20 @@
      * but at least the user will be able to see all of the window */
     output   = &screen->outputDevs ().at (screen->outputDeviceForGeometry (old));
 
-    if (state & CompWindowStateFullscreenMask ||
-	state & CompWindowStateMaximizedHorzMask)
-    {
-	int width = (mask & CWWidth) ? xwc->width : old.width ();
-	int height = (mask & CWHeight) ? xwc->height : old.height ();
-
-	window->constrainNewWindowSize (width, height, &width, &height);
-
-	if (width > output->width ())
-	{
-	    int        distance = std::numeric_limits <int>::max ();
-	    CompOutput *selected = output;
-	    /* That's no good ... try and find the closest output device to this one
-	     * which has a large enough size */
-	    foreach (CompOutput &o, screen->outputDevs ())
-	    {
-		if (o.workArea ().width () > width)
-		{
-		    int tDistance = sqrt (pow (abs (o.x () - output->x ()), 2) +
-					  pow (abs (o.y () - output->y ()), 2));
-
-		    if (tDistance < distance)
-		    {
-			selected = &o;
-			tDistance = distance;
-		    }
-		}
-	    }
-
-	    output = selected;
-	}
-    }
-
-    if (state & CompWindowStateFullscreenMask ||
-	state & CompWindowStateMaximizedVertMask)
-    {
-	int width = (mask & CWWidth) ? xwc->width : old.width ();
-	int height = (mask & CWHeight) ? xwc->height : old.height ();
-
-	window->constrainNewWindowSize (width, height, &width, &height);
-
-	if (height > output->height ())
-	{
-	    int        distance = std::numeric_limits <int>::max ();
-	    CompOutput *selected = output;
-	    /* That's no good ... try and find the closest output device to this one
-	     * which has a large enough size */
-	    foreach (CompOutput &o, screen->outputDevs ())
-	    {
-		if (o.workArea ().height () > height)
-		{
-		    int tDistance = sqrt (pow (abs (o.x () - output->x ()), 2) +
-					  pow (abs (o.y () - output->y ()), 2));
-
-		    if (tDistance < distance)
-		    {
-			selected = &o;
-			tDistance = distance;
-		    }
-		}
-	    }
-
-	    output = selected;
-	}
-    }
+    /*
+     * output is now the correct output for the given geometry.
+     * There used to be a lot more logic here to handle the rare special
+     * case of maximizing a window whose hints say it is too large to fit
+     * the output and choose a different one. However that logic was a bad
+     * idea because:
+     *   (1) It's confusing to the user to auto-magically move a window
+     *       between monitors when they didn't ask for it. So don't.
+     *   (2) In the worst case where the window can't go small enough to fit
+     *       the output, they can simply move it with Alt+drag, Alt+F7 or
+     *       expo.
+     * Not moving the window at all is much less annoying than moving it when
+     * the user never asked to.
+     */
 
     workArea = output->workArea ();