Merge lp:~aacid/compiz/do_not_change_viewport_on_resize into lp:compiz/0.9.9

[Sam Spilsbury]

Do not move windows between viewports/workspaces when changing its size (e.g. maximizing)

Do not move windows between viewports/workspaces when changing its size (e.g. maximizing)

To be honest i'm a bit "scared" since this touches a function used by lots of other functions, but if we never want a window to change to a different viewports/workspaces i guess it makes sense.

[jenkins (martin-mrazik+qa)]

PASSED: Continuous integration, rev:3362
http://jenkins.qa.ubuntu.com/job/compiz-ci/64/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/64/console

review: Approve (continuous-integration)

[Sam Spilsbury (smspillaz)]

Hmm, I think the intention of this code is to prevent windows that maximize themselves from going on to other viewports. I think it might make sense to add a check here to check if the window is on the currently active viewport, and if so, to always maximize it on that viewport.

Do you think you might be able to get this code under test?

[Daniel van Vugt (vanvugt)]

Actually, I'm not sure about the validity of the bug itself. The bug seems to be that windows get maximized on the workspace which contains their centre-point. That's the way it was designed and the way it works. But that's not what's always intuitive to the user and hence the bug.

If that's the functionality we're aiming to change here then it looks like the wrong code is being modified.

[Daniel van Vugt (vanvugt)]

I think you should look at this instead:

screen.cpp: compiz::private_screen::viewports::viewportForGeometry

review: Needs Fixing

[Albert Astals Cid (aacid)]

Daniel, right, the code is doign what you say (maximize on the workspace which contains their centre-point) which both I and Sam (that set the bug to Confirmed) think does not make sense.

I disagree the wrong code is being modified, what
   CompScreenImpl::viewportForGeometry (const CompWindow::Geometry& gm, CompPoint& viewport)
does is not return in viewport the viewport that gm should have but the "difference" in viewports from the current one, i.e. it can return {-1, -1} if it thinks the window shall be one viewport left and one top from its current one.

Of course I could make that function return always {0,0} (i.e. don't move me) but that might potentially break other plugins that use it like the group and the wall plugins

[Daniel van Vugt (vanvugt)]

OK, I have no opinion until I can review this in detail later.

review: Abstain

[Albert Astals Cid (aacid)]

FWIW: I'm going on holiday tomorrow, will be back on 1st October

[Daniel van Vugt (vanvugt)]

The branches have changed. Please resubmit targeting lp:compiz

review: Needs Resubmitting

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3362
http://jenkins.qa.ubuntu.com/job/compiz-ci/139/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/139/console

review: Needs Fixing (continuous-integration)

[lp:~aacid/compiz/do_not_change_viewport_on_resize]

3363. By Albert Astals Cid on 2012-10-10

Merge lp:compiz

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3363
http://jenkins.qa.ubuntu.com/job/compiz-ci/169/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/169/console

review: Needs Fixing (continuous-integration)

Unmerged revisions

=== modified file 'src/window.cpp'
--- src/window.cpp	2012-09-18 15:51:59 +0000
+++ src/window.cpp	2012-10-10 11:48:23 +0000
@@ -3495,15 +3495,9 @@
 {
     CompRect  workArea;
     int	      mask = 0;
-    int	      x, y;
     CompOutput *output;
     CompPoint viewport;
 
-    screen->viewportForGeometry (old, viewport);
-
-    x = (viewport.x () - screen->vp ().x ()) * screen->width ();
-    y = (viewport.y () - screen->vp ().y ()) * screen->height ();
-
     /* Try to select and output device that the window is on first
      * and make sure if we are fullscreening or maximizing that the
      * window is actually able to fit on this output ... otherwise
@@ -3585,15 +3579,15 @@
 
 	if (fullscreenMonitorsSet)
 	{
-	    xwc->x      = x + fullscreenMonitorRect.x ();
-	    xwc->y      = y + fullscreenMonitorRect.y ();
+	    xwc->x      = fullscreenMonitorRect.x ();
+	    xwc->y      = fullscreenMonitorRect.y ();
 	    xwc->width  = fullscreenMonitorRect.width ();
 	    xwc->height = fullscreenMonitorRect.height ();
 	}
 	else
 	{
-	    xwc->x      = x + output->x ();
-	    xwc->y      = y + output->y ();
+	    xwc->x      = output->x ();
+	    xwc->y      = output->y ();
 	    xwc->width  = output->width ();
 	    xwc->height = output->height ();
 	}
@@ -3695,7 +3689,7 @@
 		 * by the gravity value (so that the corner that the gravity specifies
 		 * is 'anchored' to that edge of the workarea) */
 
-		xwc->y = y + workArea.y () + border.top;
+		xwc->y = workArea.y () + border.top;
 		mask |= CWY;
 
 		switch (priv->sizeHints.win_gravity)
@@ -3706,7 +3700,7 @@
 			/* Shift the window so that the bottom meets the top of the bottom */
 			height = xwc->height + old.border () * 2;
 
-			max = y + workArea.bottom ();
+			max = workArea.bottom ();
 			if (xwc->y + xwc->height + border.bottom > max)
 			{
 			    xwc->y = max - height - border.bottom;
@@ -3732,7 +3726,7 @@
 
 	    if (state & CompWindowStateMaximizedHorzMask)
 	    {
-		xwc->x = x + workArea.x () + border.left;
+		xwc->x = workArea.x () + border.left;
 		mask |= CWX;
 
 		switch (priv->sizeHints.win_gravity)
@@ -3742,7 +3736,7 @@
 		    case EastGravity:
 			width = xwc->width + old.border () * 2;
 
-			max = x + workArea.right ();
+			max = workArea.right ();
 
 			if (old.x () + (int) old.width () + border.right > max)
 			{
@@ -3751,7 +3745,7 @@
 			}
 			else if (old.x () + width + border.right > max)
 			{
-			    xwc->x = x + workArea.x () +
+			    xwc->x = workArea.x () +
 				     (workArea.width () - border.left - width -
 				      border.right) / 2 + border.left;
 			    mask |= CWX;