Merge lp:~compiz-team/compiz/compiz.fix_1159324.1 into lp:compiz/0.9.10

[Sam Spilsbury]

Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.

(LP: #1159324)

Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.

(LP: #1159324)

[MC Return (mc-return)]

A typo in the comment here:

52 + /* left, right, top, bottom target coordinates, clamed to viewport

[MC Return (mc-return)]

In 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h' one newline here is redundant:

8 +
9 +

Other than those 2 minor issues this seems perfect and very useful !

Are you sure it fixes bug #1159324 ? It seems that this is already fixed.

But +1 for the restructuring anyway.

review: Approve

[Sam Spilsbury (smspillaz)]

On Sun, Mar 31, 2013 at 8:00 PM, MC Return <email address hidden> wrote:
> Review: Approve
>
> In 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h' one newline here is redundant:
>
> 8 +
> 9 +
>
> Other than those 2 minor issues this seems perfect and very useful !
>
> Are you sure it fixes bug #1159324 ? It seems that this is already fixed.

There was a corner case where it could still happen. Seemed to be
mostly on multimonitor cases. The window would be placed normally
(ok), and then request a new position, and then be constrained as if
it had a border, but that was wrong, because we were never meant to do
that.

>
> But +1 for the restructuring anyway.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1159324.1/+merge/156245
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1159324.1 into lp:compiz.

--
Sam Spilsbury

[MC Return (mc-return)]

>
> There was a corner case where it could still happen. Seemed to be
> mostly on multimonitor cases. The window would be placed normally
> (ok), and then request a new position, and then be constrained as if
> it had a border, but that was wrong, because we were never meant to do
> that.
>
Hmm, that reminds me of a bug I still experience (just with Emerald):

Open any media player-> maximize it -> make it go fullscreen -> restore it
Result:
The window still has no decoration (because it is Unity-maximized),
which is correct, but the window is placed, like it would be decorated,
shifted to the right and down, but with correct window size (making lower
and right part of the window go offscreen...

[MC Return (mc-return)]

Another minor typo in a comment:

+/* Just here to keep ABI compatability */

should be

+/* Just here to keep ABI compatibility */

2x

[Daniel van Vugt (vanvugt)]

Please re-target this for lp:compiz/0.9.10

review: Needs Resubmitting

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3638
http://jenkins.qa.ubuntu.com/job/compiz-ci/117/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/154
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/506

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/117/rebuild

review: Approve (continuous-integration)

[MC Return (mc-return)]

This is btw still broken in Raring.
It is not about me, as I got this fix running here locally, but if this won't land into 0.9.10, it probably will never land in Raring either...

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/compiz-mbs-autolanding/62/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-arm-mbs-autolanding/./build=panda-pbuilder,distribution=raring,flavor=armhf/60/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-x86-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=amd64/62
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-x86-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=i386/62

review: Needs Fixing (continuous-integration)

[MC Return (mc-return)]

/tmp/buildd/compiz-0.9.9~daily13.02.28bzr3640pkg0raring60/compizconfig/gsettings/tests/test_gsettings_tests.cpp:2270:1: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-4.7/README.Bugs> for instructions.
The bug is not reproducible, so it is likely a hardware or OS problem.
make[3]: *** [compizconfig/gsettings/tests/CMakeFiles/compizconfig_test_gsettings.dir/test_gsettings_tests.cpp.o] Error 1
make[3]: Leaving directory `/tmp/buildd/compiz-0.9.9~daily13.02.28bzr3640pkg0raring60/obj-arm-linux-gnueabihf'
make[2]: *** [compizconfig/gsettings/tests/CMakeFiles/compizconfig_test_gsettings.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....

Seems like a Jenkins problem...

Reapproving (probably without effect)

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/compiz-mbs-autolanding/63/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-arm-mbs-autolanding/./build=panda-pbuilder,distribution=raring,flavor=armhf/61/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-x86-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=amd64/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-x86-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=i386/63

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/compiz-mbs-autolanding/64/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-arm-mbs-autolanding/./build=panda-pbuilder,distribution=raring,flavor=armhf/62/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-x86-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=amd64/64
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-x86-mbs-autolanding/./build=pbuilder,distribution=raring,flavor=i386/64

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3638
http://jenkins.qa.ubuntu.com/job/compiz-ci/125/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/162
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/514

Click here to trigger a rebuild:
http://s-jenkins:8080/job/compiz-ci/125/rebuild

review: Approve (continuous-integration)

[Sam Spilsbury (smspillaz)]

So I think the arm builder is running out of memory? This is a problem ...

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h'
--- plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h	2013-03-24 05:51:41 +0000
+++ plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h	2013-04-10 04:16:22 +0000
@@ -47,6 +47,34 @@
                                          const CompWindowExtents        &border,
                                          const CompRect                 &workArea,
                                          bool                           staticGravity);
+
+
+CompPoint getViewportRelativeCoordinates (const compiz::window::Geometry &geom,
+					  const CompSize                 &screen);
+
+CompWindowExtents getWindowEdgePositions (const CompPoint                &position,
+					  const compiz::window::Geometry &geom,
+					  const CompWindowExtents        &border,
+					  unsigned int                   gravity);
+
+void clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
+					    const CompRect    &workArea);
+void clampVerticalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
+					   const CompRect    &workArea);
+
+void subtractBordersFromEdgePositions (CompWindowExtents       &edgePositions,
+				       const CompWindowExtents &border,
+				       unsigned int            legacyBorder,
+				       unsigned int            gravity);
+
+bool onlySizeChanged (unsigned int mask);
+bool applyWidthChange (const CompWindowExtents &edgePositions,
+		       XWindowChanges          &xwc,
+		       unsigned int            &mask);
+
+bool applyHeightChange (const CompWindowExtents &edgePositions,
+			XWindowChanges          &xwc,
+			unsigned int            &mask);
 }
 }
 
 
=== modified file 'plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp'
--- plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp	2013-03-24 05:51:41 +0000
+++ plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp	2013-04-10 04:16:22 +0000
@@ -191,3 +191,154 @@
 
     return pos;
 }
+
+CompPoint cp::getViewportRelativeCoordinates (const cw::Geometry &geom,
+					      const CompSize     &screen)
+{
+
+    /* left, right, top, bottom target coordinates, clamed to viewport
+     * sizes as we don't need to validate movements to other viewports;
+     * we are only interested in inner-viewport movements */
+
+    int x = geom.x () % screen.width ();
+    if ((geom.x2 ()) < 0)
+	x += screen.width ();
+
+    int y = geom.y () % screen.height ();
+    if ((geom.y2 ()) < 0)
+	y += screen.height ();
+
+    return CompPoint (x, y);
+}
+
+CompWindowExtents cp::getWindowEdgePositions (const CompPoint         &position,
+					      const cw::Geometry      &geom,
+					      const CompWindowExtents &border,
+					      unsigned int            gravity)
+{
+    CompWindowExtents edgePositions;
+    CompWindowExtents effectiveBorder (border);
+
+    if (gravity & StaticGravity)
+	effectiveBorder = CompWindowExtents (0, 0, 0, 0);
+
+    edgePositions.left   = position.x () - effectiveBorder.left;
+    edgePositions.right  = edgePositions.left +
+			   geom.widthIncBorders () +  (effectiveBorder.left +
+						       effectiveBorder.right);
+    edgePositions.top    = position.y () - effectiveBorder.top;
+    edgePositions.bottom = edgePositions.top +
+			   geom.heightIncBorders () + (effectiveBorder.top +
+						       effectiveBorder.bottom);
+
+    return edgePositions;
+}
+
+void cp::clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
+						 const CompRect    &workArea)
+{
+    if ((edgePositions.right - edgePositions.left) > workArea.width ())
+    {
+	edgePositions.left  = workArea.left ();
+	edgePositions.right = workArea.right ();
+    }
+    else
+    {
+	if (edgePositions.left < workArea.left ())
+	{
+	    edgePositions.right += workArea.left () - edgePositions.left;
+	    edgePositions.left  = workArea.left ();
+	}
+
+	if (edgePositions.right > workArea.right ())
+	{
+	    edgePositions.left -= edgePositions.right - workArea.right ();
+	    edgePositions.right = workArea.right ();
+	}
+    }
+
+}
+
+void cp::clampVerticalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
+					       const CompRect    &workArea)
+{
+    if ((edgePositions.bottom - edgePositions.top) > workArea.height ())
+    {
+	edgePositions.top    = workArea.top ();
+	edgePositions.bottom = workArea.bottom ();
+    }
+    else
+    {
+	if (edgePositions.top < workArea.top ())
+	{
+	    edgePositions.bottom += workArea.top () - edgePositions.top;
+	    edgePositions.top    = workArea.top ();
+	}
+
+	if (edgePositions.bottom > workArea.bottom ())
+	{
+	    edgePositions.top   -= edgePositions.bottom - workArea.bottom ();
+	    edgePositions.bottom = workArea.bottom ();
+	}
+    }
+}
+
+void cp::subtractBordersFromEdgePositions (CompWindowExtents       &edgePositions,
+					   const CompWindowExtents &border,
+					   unsigned int            legacyBorder,
+					   unsigned int            gravity)
+{
+    const unsigned int doubleBorder = 2 * legacyBorder;
+    CompWindowExtents  effectiveBorder = border;
+
+    if (gravity & StaticGravity)
+	effectiveBorder = CompWindowExtents (0, 0, 0, 0);
+
+    edgePositions.left   += effectiveBorder.left;
+    edgePositions.right  -= effectiveBorder.right + doubleBorder;
+    edgePositions.top    += effectiveBorder.top;
+    edgePositions.bottom -= effectiveBorder.bottom + doubleBorder;
+}
+
+bool cp::onlySizeChanged (unsigned int mask)
+{
+    return (!(mask & (CWX | CWY)) && (mask & (CWWidth | CWHeight)));
+}
+
+bool cp::applyWidthChange (const CompWindowExtents &edgePositions,
+			   XWindowChanges          &xwc,
+			   unsigned int            &mask)
+{
+    bool alreadySet = mask & CWWidth;
+
+    if (alreadySet)
+	alreadySet = edgePositions.right - edgePositions.left == xwc.width;
+
+    if (!alreadySet)
+    {
+	xwc.width = edgePositions.right - edgePositions.left;
+	mask       |= CWWidth;
+	return true;
+    }
+
+    return false;
+}
+
+bool cp::applyHeightChange (const CompWindowExtents &edgePositions,
+			    XWindowChanges          &xwc,
+			    unsigned int            &mask)
+{
+    bool alreadySet = mask & CWHeight;
+
+    if (alreadySet)
+	alreadySet = edgePositions.bottom - edgePositions.top == xwc.height;
+
+    if (!alreadySet)
+    {
+	xwc.height = edgePositions.bottom - edgePositions.top;
+	mask        |= CWHeight;
+	return true;
+    }
+
+    return false;
+}
 
=== modified file 'plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp'
--- plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp	2013-03-24 05:51:41 +0000
+++ plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp	2013-04-10 04:16:22 +0000
@@ -278,3 +278,336 @@
                          PlaceConstrainPositionToWorkArea,
                          Combine (ValuesIn (PossibleGeometries),
 				  ValuesIn (PossibleExtents)));
+class PlaceGetEdgePositions :
+    public CompPlaceTest
+{
+    public:
+
+	PlaceGetEdgePositions () :
+	    geom (100, 200, 300, 400, 1),
+	    border (1, 2, 3, 4),
+	    pos (geom.pos ())
+	{
+	}
+
+    protected:
+
+	cw::Geometry geom;
+	cwe::Extents border;
+	CompPoint    pos;
+};
+
+TEST_F (PlaceGetEdgePositions, GetEdgePositionsNWGravity)
+{
+    int left = geom.x () - border.left;
+    int right = left + (geom.widthIncBorders ()) +
+		    (border.left + border.right);
+    int top = geom.y () - border.top;
+    int bottom = top + (geom.heightIncBorders ()) +
+		    (border.top + border.bottom);
+
+    const cwe::Extents ExpectedExtents (left, right, top, bottom);
+    cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,
+							    geom,
+							    border,
+							    NorthWestGravity));
+
+    EXPECT_EQ (ExpectedExtents, actualExtents);
+}
+
+TEST_F (PlaceGetEdgePositions, GetEdgePositionsStaticGravity)
+{
+    /* Don't count borders in validation */
+    int left = geom.x ();
+    int right = left + (geom.widthIncBorders ());
+    int top = geom.y ();
+    int bottom = top + (geom.heightIncBorders ());
+
+    const cwe::Extents ExpectedExtents (left, right, top, bottom);
+    cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,
+							    geom,
+							    border,
+							    StaticGravity));
+
+    EXPECT_EQ (ExpectedExtents, actualExtents);
+}
+
+namespace
+{
+const CompSize     SCREEN_SIZE (1000, 1000);
+const unsigned int WINDOW_SIZE = 250;
+}
+
+TEST (PlaceGetViewportRelativeCoordinates, WithinScreenWidth)
+{
+    cw::Geometry geom (SCREEN_SIZE.width () / 2,
+		       SCREEN_SIZE.height () / 2,
+		       WINDOW_SIZE, WINDOW_SIZE,
+		       0);
+
+    CompPoint    position (cp::getViewportRelativeCoordinates (geom,
+							       SCREEN_SIZE));
+    EXPECT_EQ (geom.pos ().x (), position.x ());
+}
+
+TEST (PlaceGetViewportRelativeCoordinates, WithinScreenHeight)
+{
+    cw::Geometry geom (SCREEN_SIZE.width () / 2,
+		       SCREEN_SIZE.height () / 2,
+		       WINDOW_SIZE, WINDOW_SIZE,
+		       0);
+
+    CompPoint    position (cp::getViewportRelativeCoordinates (geom,
+							       SCREEN_SIZE));
+    EXPECT_EQ (geom.pos ().y (), position.y ());
+}
+
+TEST (PlaceGetViewportRelativeCoordinates, OutsideOuterScreenWidth)
+{
+    cw::Geometry geom (SCREEN_SIZE.width () + 1,
+		       SCREEN_SIZE.height () / 2,
+		       WINDOW_SIZE, WINDOW_SIZE,
+		       0);
+
+    CompPoint    position (cp::getViewportRelativeCoordinates (geom,
+							       SCREEN_SIZE));
+    EXPECT_EQ (1, position.x ());
+}
+
+TEST (PlaceGetViewportRelativeCoordinates, OutsideInnerScreenWidth)
+{
+    cw::Geometry geom (-(WINDOW_SIZE + 1),
+		       SCREEN_SIZE.height () / 2,
+		       WINDOW_SIZE, WINDOW_SIZE,
+		       0);
+
+    CompPoint    position (cp::getViewportRelativeCoordinates (geom,
+							       SCREEN_SIZE));
+    EXPECT_EQ (SCREEN_SIZE.width () - (WINDOW_SIZE + 1),
+	       position.x ());
+}
+
+TEST (PlaceGetViewportRelativeCoordinates, OutsideOuterScreenHeight)
+{
+    cw::Geometry geom (SCREEN_SIZE.width () / 2,
+		       SCREEN_SIZE.height () + 1,
+		       WINDOW_SIZE, WINDOW_SIZE,
+		       0);
+
+    CompPoint    position (cp::getViewportRelativeCoordinates (geom,
+							       SCREEN_SIZE));
+    EXPECT_EQ (1, position.y ());
+}
+
+TEST (PlaceGetViewportRelativeCoordinates, OutsideInnerScreenHeight)
+{
+    cw::Geometry geom (SCREEN_SIZE.width () / 2,
+		       -(WINDOW_SIZE + 1),
+		       WINDOW_SIZE, WINDOW_SIZE,
+		       0);
+
+    CompPoint    position (cp::getViewportRelativeCoordinates (geom,
+							       SCREEN_SIZE));
+    EXPECT_EQ (SCREEN_SIZE.height () - (WINDOW_SIZE + 1),
+	       position.y ());
+}
+
+namespace
+{
+const CompRect WORK_AREA (25, 25, 1000, 1000);
+
+const cwe::Extents EdgePositions[] =
+{
+    /* Exact match */
+    cwe::Extents (WORK_AREA.left (), WORK_AREA.right (),
+		  WORK_AREA.top (), WORK_AREA.bottom ()),
+    /* Just within */
+    cwe::Extents (WORK_AREA.left () + 1, WORK_AREA.right () - 1,
+		  WORK_AREA.top () + 1, WORK_AREA.bottom () - 1),
+    /* Just outside right */
+    cwe::Extents (WORK_AREA.left () + 1, WORK_AREA.right () + 1,
+		  WORK_AREA.top (), WORK_AREA.bottom ()),
+    /* Just outside left */
+    cwe::Extents (WORK_AREA.left () - 1, WORK_AREA.right () - 1,
+		  WORK_AREA.top (), WORK_AREA.bottom ()),
+    /* Just outside top */
+    cwe::Extents (WORK_AREA.left (), WORK_AREA.right (),
+		  WORK_AREA.top () - 1, WORK_AREA.bottom () - 1),
+    /* Just outside bottom */
+    cwe::Extents (WORK_AREA.left (), WORK_AREA.right (),
+		  WORK_AREA.top () + 1, WORK_AREA.bottom () + 1),
+};
+}
+
+class PlaceClampEdgePositions :
+    public CompPlaceTest,
+    public WithParamInterface <CompWindowExtents>
+{
+};
+
+TEST_P (PlaceClampEdgePositions, WithinWorkAreaWidth)
+{
+    CompWindowExtents edgePositions (GetParam ());
+    cp::clampHorizontalEdgePositionsToWorkArea (edgePositions,
+						WORK_AREA);
+
+    const int edgePositionsWidth = edgePositions.right - edgePositions.left;
+
+    ASSERT_GT (edgePositionsWidth, 0);
+    EXPECT_LE (edgePositionsWidth, WORK_AREA.width ());
+}
+
+TEST_P (PlaceClampEdgePositions, OutsideLeft)
+{
+    CompWindowExtents edgePositions (GetParam ());
+    cp::clampHorizontalEdgePositionsToWorkArea (edgePositions,
+						WORK_AREA);
+
+    ASSERT_GE (edgePositions.left, WORK_AREA.left ());
+    ASSERT_GE (edgePositions.right, WORK_AREA.left ());
+}
+
+TEST_P (PlaceClampEdgePositions, OutsideRight)
+{
+    CompWindowExtents edgePositions (GetParam ());
+    cp::clampHorizontalEdgePositionsToWorkArea (edgePositions,
+						WORK_AREA);
+
+    ASSERT_LE (edgePositions.left, WORK_AREA.right ());
+    ASSERT_LE (edgePositions.right, WORK_AREA.right ());
+}
+
+TEST_P (PlaceClampEdgePositions, WithinWorkAreaHeight)
+{
+    CompWindowExtents edgePositions (GetParam ());
+    cp::clampVerticalEdgePositionsToWorkArea (edgePositions,
+					      WORK_AREA);
+
+    const int edgePositionsHeight = edgePositions.bottom - edgePositions.top;
+
+    ASSERT_GT (edgePositionsHeight, 0);
+    EXPECT_LE (edgePositionsHeight, WORK_AREA.height ());
+}
+
+TEST_P (PlaceClampEdgePositions, OutsideTop)
+{
+    CompWindowExtents edgePositions (GetParam ());
+    cp::clampVerticalEdgePositionsToWorkArea (edgePositions,
+					      WORK_AREA);
+
+    ASSERT_GE (edgePositions.top, WORK_AREA.top ());
+    ASSERT_GE (edgePositions.bottom, WORK_AREA.top ());
+}
+
+TEST_P (PlaceClampEdgePositions, OutsideBottom)
+{
+    CompWindowExtents edgePositions (GetParam ());
+    cp::clampVerticalEdgePositionsToWorkArea (edgePositions,
+					      WORK_AREA);
+
+    ASSERT_LE (edgePositions.top, WORK_AREA.bottom ());
+    ASSERT_LE (edgePositions.bottom, WORK_AREA.bottom ());
+}
+
+INSTANTIATE_TEST_CASE_P (WAEdgePositions, PlaceClampEdgePositions,
+			 ValuesIn (EdgePositions));
+
+TEST (PlaceSubtractBordersFromEdgePositions, NormalGravity)
+{
+    const CompWindowExtents borders (1, 2, 3, 4);
+    const CompWindowExtents edgePositions (100, 200, 100, 200);
+    const unsigned int      legacyBorder = 1;
+
+    CompWindowExtents       expectedEdgePositions (edgePositions.left + (borders.left),
+						   edgePositions.right - (borders.right + 2 * legacyBorder),
+						   edgePositions.top + (borders.top),
+						   edgePositions.bottom - (borders.bottom + 2 * legacyBorder));
+
+    CompWindowExtents       modifiedEdgePositions (edgePositions);
+
+    cp::subtractBordersFromEdgePositions (modifiedEdgePositions,
+					  borders,
+					  legacyBorder,
+					  NorthWestGravity);
+
+    EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
+}
+
+TEST (PlaceSubtractBordersFromEdgePositions, StaticGravityDefinition)
+{
+    const CompWindowExtents borders (1, 2, 3, 4);
+    const CompWindowExtents edgePositions (100, 200, 100, 200);
+    const unsigned int      legacyBorder = 1;
+
+    CompWindowExtents       expectedEdgePositions (edgePositions.left,
+						   edgePositions.right - (2 * legacyBorder),
+						   edgePositions.top,
+						   edgePositions.bottom - (2 * legacyBorder));
+
+    CompWindowExtents       modifiedEdgePositions (edgePositions);
+
+    cp::subtractBordersFromEdgePositions (modifiedEdgePositions,
+					  borders,
+					  legacyBorder,
+					  StaticGravity);
+
+    EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
+}
+
+TEST (PlaceSizeAndPositionChanged, PositionChangedReturnsFalse)
+{
+    EXPECT_FALSE (cp::onlySizeChanged (CWX | CWY));
+}
+
+TEST (PlaceSizeAndPositionChanged, SizeChangedReturnsTrue)
+{
+    EXPECT_TRUE (cp::onlySizeChanged (CWWidth | CWHeight));
+}
+
+TEST (PlaceSizeAndPositionChanged, XAndWidthChangedReturnsFalse)
+{
+    EXPECT_FALSE (cp::onlySizeChanged (CWX | CWWidth));
+}
+
+TEST (PlaceSizeAndPositionChanged, YAndHeightChangedReturnsFalse)
+{
+    EXPECT_FALSE (cp::onlySizeChanged (CWY | CWHeight));
+}
+
+TEST (PlaceApplyWidthChange, ReturnFalseIfNoChange)
+{
+    CompWindowExtents edgePositions (100, 200, 100, 200);
+    XWindowChanges xwc;
+    xwc.width = 100;
+    unsigned int mask = CWWidth;
+    EXPECT_FALSE (cp::applyWidthChange(edgePositions, xwc, mask));
+}
+
+TEST (PlaceApplyWidthChange, ApplyWidthAndSetFlag)
+{
+    CompWindowExtents edgePositions (100, 200, 100, 200);
+    XWindowChanges xwc;
+    unsigned int mask = 0;
+    ASSERT_TRUE (cp::applyWidthChange(edgePositions, xwc, mask));
+    EXPECT_EQ (edgePositions.right - edgePositions.left, xwc.width);
+    EXPECT_EQ (CWWidth, mask);
+}
+
+TEST (PlaceApplyHeightChange, ReturnFalseIfNoChange)
+{
+    CompWindowExtents edgePositions (100, 200, 100, 200);
+    XWindowChanges xwc;
+    xwc.height = 100;
+    unsigned int mask = CWHeight;
+    EXPECT_FALSE (cp::applyHeightChange(edgePositions, xwc, mask));
+}
+
+TEST (PlaceApplyWidthChange, ApplyHeightAndSetFlag)
+{
+    CompWindowExtents edgePositions (100, 200, 100, 200);
+    XWindowChanges xwc;
+    unsigned int mask = 0;
+    ASSERT_TRUE (cp::applyHeightChange(edgePositions, xwc, mask));
+    EXPECT_EQ (edgePositions.bottom - edgePositions.top, xwc.height);
+    EXPECT_EQ (CWHeight, mask);
+}
 
=== modified file 'plugins/place/src/place.cpp'
--- plugins/place/src/place.cpp	2013-04-07 08:46:23 +0000
+++ plugins/place/src/place.cpp	2013-04-10 04:16:22 +0000
@@ -350,46 +350,23 @@
 CompRect
 PlaceWindow::doValidateResizeRequest (unsigned int &mask,
 				      XWindowChanges *xwc,
-				      bool	   sizeOnly,
+				      bool	   onlyValidateSize,
 				      bool	   clampToViewport)
 {
-    CompRect workArea;
-    int	     x, y, left, right, bottom, top;
-    CompWindow::Geometry geom;
-    int      output;
-
-    geom.set (xwc->x, xwc->y, xwc->width, xwc->height,
-	      window->serverGeometry ().border ());
+    CompWindow::Geometry geom (xwc->x, xwc->y, xwc->width, xwc->height,
+			       window->serverGeometry ().border ());
+    CompPoint pos (geom.pos ());
 
     if (clampToViewport)
-    {
-	/* left, right, top, bottom target coordinates, clamed to viewport
-	 * sizes as we don't need to validate movements to other viewports;
-	 * we are only interested in inner-viewport movements */
-
-	x = geom.x () % screen->width ();
-	if ((geom.x2 ()) < 0)
-	    x += screen->width ();
-
-	y = geom.y () % screen->height ();
-	if ((geom.y2 ()) < 0)
-	    y += screen->height ();
-    }
-    else
-    {
-	x = geom.x ();
-	y = geom.y ();
-    }
-
-    left   = x - window->border ().left;
-    right  = left + geom.widthIncBorders () +  (window->border ().left +
-						window->border ().right);
-    top    = y - window->border ().top;
-    bottom = top + geom.heightIncBorders () + (window->border ().top +
-					       window->border ().bottom);
-
-    output   = screen->outputDeviceForGeometry (geom);
-    workArea = screen->getWorkareaForOutput (output);
+	pos = cp::getViewportRelativeCoordinates(geom, *screen);
+
+    CompWindowExtents edgePositions = cp::getWindowEdgePositions (pos,
+								  geom,
+								  window->border (),
+								  window->sizeHints ().win_gravity);
+
+    int      output   = screen->outputDeviceForGeometry (geom);
+    CompRect workArea = screen->getWorkareaForOutput (output);
 
     if (clampToViewport &&
     	xwc->width >= workArea.width () &&
@@ -403,82 +380,41 @@
 	}
     }
 
-    if ((right - left) > workArea.width ())
-    {
-	left  = workArea.left ();
-	right = workArea.right ();
-    }
-    else
-    {
-	if (left < workArea.left ())
-	{
-	    right += workArea.left () - left;
-	    left  = workArea.left ();
-	}
-
-	if (right > workArea.right ())
-	{
-	    left -= right - workArea.right ();
-	    right = workArea.right ();
-	}
-    }
-
-    if ((bottom - top) > workArea.height ())
-    {
-	top    = workArea.top ();
-	bottom = workArea.bottom ();
-    }
-    else
-    {
-	if (top < workArea.top ())
-	{
-	    bottom += workArea.top () - top;
-	    top    = workArea.top ();
-	}
-
-	if (bottom > workArea.bottom ())
-	{
-	    top   -= bottom - workArea.bottom ();
-	    bottom = workArea.bottom ();
-	}
-    }
+    cp::clampHorizontalEdgePositionsToWorkArea (edgePositions, workArea);
+    cp::clampVerticalEdgePositionsToWorkArea (edgePositions, workArea);
 
     /* bring left/right/top/bottom to actual window coordinates */
-    left   += window->border ().left;
-    right  -= window->border ().right + 2 * window->serverGeometry ().border ();
-    top    += window->border ().top;
-    bottom -= window->border ().bottom + 2 * window->serverGeometry ().border ();
+    cp::subtractBordersFromEdgePositions (edgePositions,
+					  window->border (),
+					  geom.border (),
+					  window->sizeHints ().win_gravity);
 
     /* always validate position if the application changed only its size,
      * as it might become partially offscreen because of that */
-    if (!(mask) & (CWX | CWY) && (mask & (CWWidth | CWHeight)))
-	sizeOnly = false;
-
-    if ((right - left) != xwc->width)
-    {
-	xwc->width = right - left;
-	mask       |= CWWidth;
-	sizeOnly   = false;
-    }
-
-    if ((bottom - top) != xwc->height)
-    {
-	xwc->height = bottom - top;
-	mask        |= CWHeight;
-	sizeOnly    = false;
-    }
-
-    if (!sizeOnly)
-    {
-	if (left != x)
+    if (cp::onlySizeChanged (mask))
+	onlyValidateSize = false;
+
+    if (cp::applyWidthChange(edgePositions,
+			     *xwc,
+			     mask))
+	onlyValidateSize = false;
+
+    if (cp::applyHeightChange(edgePositions,
+			      *xwc,
+			      mask))
+	onlyValidateSize = false;
+
+    if (!onlyValidateSize)
+    {
+	if (edgePositions.left != pos.x ())
 	{
-	    xwc->x += left - x;
+	    xwc->x += edgePositions.left - pos.x ();
 	    mask   |= CWX;
 	}
 
-	if (top != y)
+	if (edgePositions.top != pos.y ())
 	{
-	    xwc->y += top - y;
+	    xwc->y += edgePositions.top - pos.y ();
 	    mask   |= CWY;
 	}
     }
 
=== modified file 'src/window/extents/include/core/windowextents.h'
--- src/window/extents/include/core/windowextents.h	2012-02-25 05:14:18 +0000
+++ src/window/extents/include/core/windowextents.h	2013-04-10 04:16:22 +0000
@@ -53,6 +53,10 @@
 	int top;
 	int bottom;
 
+	bool operator== (const Extents &other) const;
+	bool operator!= (const Extents &other) const;
+
+	/* Only here for ABI compatability */
 	bool operator== (const Extents &other);
 	bool operator!= (const Extents &other);
 };
 
=== modified file 'src/window/extents/src/windowextents.cpp'
--- src/window/extents/src/windowextents.cpp	2012-08-09 09:06:54 +0000
+++ src/window/extents/src/windowextents.cpp	2013-04-10 04:16:22 +0000
@@ -75,17 +75,33 @@
 {
 }
 
+/* Just here to keep ABI compatability */
 bool
 compiz::window::extents::Extents::operator== (const Extents &other)
 {
+    const Extents &self = const_cast <const Extents &> (*this);
+    return self == other;
+}
+
+bool
+compiz::window::extents::Extents::operator== (const Extents &other) const
+{
     return this->left == other.left &&
 	   this->right == other.right &&
 	   this->top == other.top &&
 	   this->bottom == other.bottom;
 }
 
+/* Just here to keep ABI compatability */
 bool
 compiz::window::extents::Extents::operator!= (const Extents &other)
 {
+    const Extents &self = const_cast <const Extents &> (*this);
+    return self != other;
+}
+
+bool
+compiz::window::extents::Extents::operator!= (const Extents &other) const
+{
     return !(*this == other);
 }