Merge lp:~compiz-team/compiz/compiz.fix_1037164 into lp:compiz/0.9.9

[Sam Spilsbury]

Fixed: Clicking on semi-maximized windows in a different workspace fails to
switch to the correct workspace. (LP: #1037164)

Handle negative values returned by viewportForGeometry correclty.

Previously if we got a negative value for viewportForGeometry, we'd try and
move to -n, -m, which would fail because checkDestination disallows those values.

checkDestination updated to accept negative values, because by default, we
actually pass negative values to it (since -dx, and -dy are negated before entry
into moveViewport). In any case, positive values are preferred, and the caller
was updated to provide positive values in case viewportForGeometry returns
negative values in order to avoid any unexpected behaviour.

In addition, if the window position needed to be changed, we'd be moving
the window by viewport-distance and not window distance due to a typo. That
was also fixed.

(LP: #1037164)

Not sure about the best way to autotest this. The only way I can think of would involve significant refactoring of calling code, which is no good.

[PS Jenkins bot (ps-jenkins)]

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

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/339//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Verified the bug seems to be fixed (using plain compiz and the switcher (Ctrl+)Alt+Tab).

However this change causes a regression: I can now wrap backwards from left-most to right-most workspaces using Ctrl+Alt+Left even though allow_wraparound is turned off.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Thanks, I'll have a look into it.

On Fri, Jan 4, 2013 at 3:44 PM, Daniel van Vugt
<email address hidden> wrote:
> Review: Needs Fixing
>
> Verified the bug seems to be fixed (using plain compiz and the switcher (Ctrl+)Alt+Tab).
>
> However this change causes a regression: I can now wrap backwards from left-most to right-most workspaces using Ctrl+Alt+Left even though allow_wraparound is turned off.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1037164/+merge/141779
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

[lp:~compiz-team/compiz/compiz.fix_1037164]

3536. By Sam Spilsbury on 2013-01-04

Revert the handling of negative co-ords.

While the old behaviour is still bad, changing it has unintended side effects

[Sam Spilsbury (smspillaz)]

Fixed

[PS Jenkins bot (ps-jenkins)]

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

Click here to trigger a rebuild:
http://jenkins.qa.ubuntu.com/job/compiz-ci/345//rebuild/?

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Verified bug fixed. No regression any more.

review: Approve

=== modified file 'plugins/wall/src/wall.cpp'
--- plugins/wall/src/wall.cpp	2012-12-04 16:08:27 +0000
+++ plugins/wall/src/wall.cpp	2013-01-04 08:46:22 +0000
@@ -608,6 +608,10 @@
 	dx       = viewport.x ();
 	dy       = viewport.y ();
 
+	/* Handle negative value */
+	dx = (unsigned int) dx % screen->vpSize ().width ();
+	dy = (unsigned int) dy % screen->vpSize ().height ();
+
 	dx -= screen->vp ().x ();
 	dy -= screen->vp ().y ();
 
@@ -637,8 +641,8 @@
 	    mask |= d.x () !=0 ? CWX : 0;
 	    mask |= d.y () !=0 ? CWY : 0;
 
-	    xwc.x = window->serverGeometry ().x () + dx;
-	    xwc.y = window->serverGeometry ().y () + dy;
+	    xwc.x = window->serverGeometry ().x () + d.x ();
+	    xwc.y = window->serverGeometry ().y () + d.y ();
 
 	    window->configureXWindow (mask, &xwc);
 	}