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

[Sam Spilsbury]

Don't offset placement by window extents for windows that have a static
gravity.

Those windows want to be placed as though they didn't have decorations.

Get cp::constrainPositionToWorkArea under test and clean up the other
place tests too, so that they only test one thing at a time.

(LP: #1159324)

Don't offset placement by window extents for windows that have a static
gravity.

Those windows want to be placed as though they didn't have decorations.

Get cp::constrainPositionToWorkArea under test and clean up the other
place tests too, so that they only test one thing at a time.

(LP: #1159324)

[MC Return (mc-return)]

Confirming this fixes bug #1159324. \o/
All regressions recently introduced are gone.
Tested Guake, Terra and Yakuake drop-down terminals.
All open with right sizes and in correct positions.

Thanks a lot, Sam. +1

P.S.:
Raring would need this fix also, because of:
http://bazaar.launchpad.net/~compiz-team/compiz/raring/revision/3639

It is a bit painful to suddenly have to deal with a trunk != raring situation...

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	2012-01-20 06:13:07 +0000
+++ plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h	2013-03-24 05:56:21 +0000
@@ -41,6 +41,12 @@
 			      const CompWindowExtents &border,
 			      unsigned int flags,
 			      const CompSize &screenSize);
+
+CompPoint & constrainPositionToWorkArea (CompPoint                      &pos,
+				         const compiz::window::Geometry &serverGeometry,
+                                         const CompWindowExtents        &border,
+                                         const CompRect                 &workArea,
+                                         bool                           staticGravity);
 }
 }
 

=== modified file 'plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp'
--- plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp	2012-04-04 04:47:00 +0000
+++ plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp	2013-03-24 05:56:21 +0000
@@ -31,12 +31,15 @@
 }
 }
 
+namespace cp = compiz::place;
+namespace cw = compiz::window;
+
 void
-compiz::place::clampGeometryToWorkArea (compiz::window::Geometry &g,
-					const CompRect           &workArea,
-					const CompWindowExtents  &border,
-					unsigned int             flags,
-					const CompSize           &screenSize)
+cp::clampGeometryToWorkArea (cw::Geometry            &g,
+			     const CompRect          &workArea,
+			     const CompWindowExtents &border,
+			     unsigned int            flags,
+			     const CompSize          &screenSize)
 {
     int	     x, y, left, right, bottom, top;
 
@@ -134,3 +137,57 @@
 	    g.setY (g.y () + top - y);
     }
 }
+
+CompPoint &
+cp::constrainPositionToWorkArea (CompPoint               &pos,
+			         const cw::Geometry      &serverGeometry,
+			         const CompWindowExtents &border,
+			         const CompRect          &workArea,
+			         bool                    staticGravity)
+{
+    CompWindowExtents extents;
+    int               delta;
+
+    CompWindowExtents effectiveBorders = border;
+
+    /* Ignore borders in the StaticGravity case for placement
+     * because the window intended to be placed as if it didn't
+     * have them */
+    if (staticGravity)
+    {
+	effectiveBorders.left = 0;
+	effectiveBorders.right = 0;
+	effectiveBorders.top = 0;
+	effectiveBorders.bottom = 0;
+    }
+
+    extents.left   = pos.x () - effectiveBorders.left;
+    extents.top    = pos.y () - effectiveBorders.top;
+    extents.right  = extents.left + serverGeometry.widthIncBorders () +
+		     (effectiveBorders.left +
+		      effectiveBorders.right);
+    extents.bottom = extents.top + serverGeometry.heightIncBorders () +
+		     (effectiveBorders.top +
+		      effectiveBorders.bottom);
+
+    delta = workArea.right () - extents.right;
+    if (delta < 0)
+	extents.left += delta;
+
+    delta = workArea.left () - extents.left;
+    if (delta > 0)
+	extents.left += delta;
+
+    delta = workArea.bottom () - extents.bottom;
+    if (delta < 0)
+	extents.top += delta;
+
+    delta = workArea.top () - extents.top;
+    if (delta > 0)
+	extents.top += delta;
+
+    pos.setX (extents.left + effectiveBorders.left);
+    pos.setY (extents.top  + effectiveBorders.top);
+
+    return pos;
+}

=== 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	2012-03-30 16:30:13 +0000
+++ plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp	2013-03-24 05:56:21 +0000
@@ -23,81 +23,258 @@
  * Authored by: Sam Spilsbury <[e-mail]>
  */
 
+#include <tr1/tuple>
 #include <test-constrain-to-workarea.h>
 #include <constrain-to-workarea.h>
 #include <iostream>
 #include <stdlib.h>
 #include <cstring>
 
-class CompPlaceTestConstrainToWorkarea :
+namespace cw = compiz::window;
+namespace cwe = cw::extents;
+namespace cp = compiz::place;
+
+using ::testing::WithParamInterface;
+using ::testing::ValuesIn;
+using ::testing::Combine;
+
+class PlaceClampGeometryToWorkArea :
     public CompPlaceTest
 {
+    public:
+
+	PlaceClampGeometryToWorkArea () :
+	    screensize (1000, 2000),
+	    workArea (50, 50, 900, 1900),
+            flags (0)
+        {
+	    memset (&extents, 0, sizeof (cwe::Extents));
+        }
+
+    protected:
+
+	CompSize screensize;
+	CompRect workArea;
+	cw::Geometry g;
+	cwe::Extents extents;
+	unsigned int flags;
 };
 
-TEST_F(CompPlaceTestConstrainToWorkarea, TestConstrainToWorkarea)
-{
-    CompSize screensize (1000, 2000);
-    CompRect workArea (50, 50, 900, 1900);
-    compiz::window::Geometry g (100, 100, 200, 200, 0);
-    compiz::window::extents::Extents extents;
-    unsigned int flags = 0;
-
-    memset (&extents, 0, sizeof (compiz::window::extents::Extents));
+namespace
+{
+    const cw::Geometry LimitOfAllowedGeometry (50, 50, 900, 1900, 0);
+}
+
+namespace compiz
+{
+namespace window
+{
+std::ostream & operator<<(std::ostream &os, const Geometry &g)
+{
+    return os << "Window Geometry: (" << g.x () <<
+                 ", " << g.y () <<
+                 ", " << g.width () <<
+                 ", " << g.height () <<
+                 ", " << g.border () << ")";
+}
+namespace extents
+{
+std::ostream & operator<<(std::ostream &os, const Extents &e)
+{
+    return os << "Window Extents: (left: " << e.left <<
+                 ", right: " << e.right <<
+                 ", top: " << e.top <<
+                 ", bottom: " << e.bottom << ")";
+}
+}
+}
+}
+
+std::ostream & operator<<(std::ostream &os, const CompPoint &p)
+{
+    return os << "Point: (" << p.x () << ", " << p.y () << ")";
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, NoConstrainmentRequired)
+{
+    g = cw::Geometry (100, 100, 200, 200, 0);
 
     /* Do nothing */
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (100, 100, 200, 200, 0));
-
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, cw::Geometry (100, 100, 200, 200, 0));
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, LargerThanWorkAreaConstrainsToWorkAreaSize)
+{
     /* Larger than workArea */
-    g = compiz::window::Geometry (50, 50, 950, 1950, 0);
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (50, 50, 900, 1900, 0));
-
+    g = cw::Geometry (50, 50, 950, 1950, 0);
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, LimitOfAllowedGeometry);
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, OutsideTopLeftConstrainment)
+{
     /* Outside top left */
-    g = compiz::window::Geometry (0, 0, 900, 1900, 0);
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (50, 50, 900, 1900, 0));
-
+    g = cw::Geometry (0, 0, 900, 1900, 0);
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, LimitOfAllowedGeometry);
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, OutsideTopRightConstrainment)
+{
     /* Outside top right */
-    g = compiz::window::Geometry (100, 0, 900, 1900, 0);
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (50, 50, 900, 1900, 0));
-
+    g = cw::Geometry (100, 0, 900, 1900, 0);
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, LimitOfAllowedGeometry);
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, OutsideBottomLeftConstrainment)
+{
     /* Outside bottom left */
-    g = compiz::window::Geometry (0, 100, 900, 1900, 0);
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (50, 50, 900, 1900, 0));
-
+    g = cw::Geometry (0, 100, 900, 1900, 0);
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, LimitOfAllowedGeometry);
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, OutsideBottomRightConstrainment)
+{
     /* Outside bottom right */
-    g = compiz::window::Geometry (100, 100, 900, 1900, 0);
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (50, 50, 900, 1900, 0));
-
+    g = cw::Geometry (100, 100, 900, 1900, 0);
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, LimitOfAllowedGeometry);
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, NoChangePositionIfSizeUnchanged)
+{
     /* For the size only case, we should not
      * change the position of the window if
      * the size does not change */
-    g = compiz::window::Geometry (0, 0, 900, 1900, 0);
-    flags = compiz::place::clampGeometrySizeOnly;
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (0, 0, 900, 1900, 0));
-
-    g = compiz::window::Geometry (0, 0, 1000, 2000, 0);
-    flags = compiz::place::clampGeometrySizeOnly;
-
-    compiz::place::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
-
-    EXPECT_EQ (g, compiz::window::Geometry (50, 50, 900, 1900, 0));
-}
+    g = cw::Geometry (0, 0, 900, 1900, 0);
+    flags = cp::clampGeometrySizeOnly;
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, cw::Geometry (0, 0, 900, 1900, 0));
+}
+
+TEST_F (PlaceClampGeometryToWorkArea, ChangePositionIfSizeChanged)
+{
+    g = cw::Geometry (0, 0, 1000, 2000, 0);
+    flags = cp::clampGeometrySizeOnly;
+
+    cp::clampGeometryToWorkArea (g, workArea, extents, flags, screensize);
+
+    EXPECT_EQ (g, LimitOfAllowedGeometry);
+}
+
+namespace
+{
+typedef std::tr1::tuple <cw::Geometry, cwe::Extents> ConstrainPositionToWorkAreaParam;
+
+const cw::Geometry & WindowGeometry (const ConstrainPositionToWorkAreaParam &p)
+{
+    return std::tr1::get <0> (p);
+}
+
+const cwe::Extents & WindowExtents (const ConstrainPositionToWorkAreaParam &p)
+{
+    return std::tr1::get <1> (p);
+}
+
+CompPoint InitialPosition (const ConstrainPositionToWorkAreaParam &p)
+{
+    /* Initial position is where the window is right now */
+    return (std::tr1::get <0> (p)).pos ();
+}
+
+const CompRect  WArea (50, 50, 900, 1900);
+const CompPoint ExpectedPosition (WArea.pos ());
+}
+
+class PlaceConstrainPositionToWorkArea :
+    public CompPlaceTest,
+    public WithParamInterface <ConstrainPositionToWorkAreaParam>
+{
+    public:
+
+	PlaceConstrainPositionToWorkArea ()
+        {
+	    memset (&extents, 0, sizeof (cwe::Extents));
+        }
+
+    protected:
+
+	CompRect workArea;
+	cw::Geometry g;
+	cwe::Extents extents;
+};
+
+TEST_P (PlaceConstrainPositionToWorkArea, PositionConstrainedWithExtents)
+{
+    g = WindowGeometry (GetParam ());
+    extents = WindowExtents (GetParam ());
+
+    CompPoint pos = InitialPosition (GetParam ());
+    pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, false);
+
+    const CompPoint expectedAfterExtentsAdjustment = ExpectedPosition +
+						     CompPoint (extents.left,
+								extents.top);
+
+    EXPECT_EQ (expectedAfterExtentsAdjustment, pos);
+}
+
+TEST_P (PlaceConstrainPositionToWorkArea, PositionConstrainedStaticGravity)
+{
+    g = WindowGeometry (GetParam ());
+    extents = WindowExtents (GetParam ());
+
+    CompPoint pos = InitialPosition (GetParam ());
+    pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, true);
+
+    /* Do not adjust residual position for extents with windows
+     * that have a static gravity */
+    EXPECT_EQ (ExpectedPosition, pos);
+}
+
+namespace
+{
+cwe::Extents PossibleExtents[] =
+{
+    cwe::Extents (0, 0, 0, 0),
+    cwe::Extents (1, 0, 0, 0),
+    cwe::Extents (0, 1, 0, 0),
+    cwe::Extents (0, 0, 1, 0),
+    cwe::Extents (0, 0, 0, 1)
+};
+
+cw::Geometry PossibleGeometries[] =
+{
+    cw::Geometry (WArea.x (), WArea.y (),
+		  WArea.width (), WArea.height (), 0),
+    cw::Geometry (WArea.x () - 1, WArea.y (),
+		  WArea.width (), WArea.height (), 0),
+    cw::Geometry (WArea.x (), WArea.y () - 1,
+		  WArea.width (), WArea.height (), 0),
+    cw::Geometry (WArea.x () + 1, WArea.y (),
+		  WArea.width (), WArea.height (), 0),
+    cw::Geometry (WArea.x (), WArea.y () + 1,
+		  WArea.width (), WArea.height (), 0)
+};
+}
+
+INSTANTIATE_TEST_CASE_P (PlacementData,
+                         PlaceConstrainPositionToWorkArea,
+                         Combine (ValuesIn (PossibleGeometries),
+				  ValuesIn (PossibleExtents)));

=== modified file 'plugins/place/src/place.cpp'
--- plugins/place/src/place.cpp	2012-12-19 18:05:01 +0000
+++ plugins/place/src/place.cpp	2013-03-24 05:56:21 +0000
@@ -24,6 +24,8 @@
 
 COMPIZ_PLUGIN_20090315 (place, PlacePluginVTable)
 
+namespace cp = compiz::place;
+
 #define XWINDOWCHANGES_INIT {0, 0, 0, 0, 0, None, 0}
 
 PlaceScreen::PlaceScreen (CompScreen *screen) :
@@ -1203,36 +1205,13 @@
 PlaceWindow::constrainToWorkarea (const CompRect &workArea,
 				  CompPoint      &pos)
 {
-    CompWindowExtents extents;
-    int               delta;
-
-    extents.left   = pos.x () - window->border ().left;
-    extents.top    = pos.y () - window->border ().top;
-    extents.right  = extents.left + window->serverGeometry ().widthIncBorders () +
-		     (window->border ().left +
-		      window->border ().right);
-    extents.bottom = extents.top + window->serverGeometry ().heightIncBorders () +
-		     (window->border ().top +
-		      window->border ().bottom);
-
-    delta = workArea.right () - extents.right;
-    if (delta < 0)
-	extents.left += delta;
-
-    delta = workArea.left () - extents.left;
-    if (delta > 0)
-	extents.left += delta;
-
-    delta = workArea.bottom () - extents.bottom;
-    if (delta < 0)
-	extents.top += delta;
-
-    delta = workArea.top () - extents.top;
-    if (delta > 0)
-	extents.top += delta;
-
-    pos.setX (extents.left + window->border ().left);
-    pos.setY (extents.top  + window->border ().top);
+    bool staticGravity = window->sizeHints ().win_gravity & StaticGravity;
+
+    pos = cp::constrainPositionToWorkArea (pos,
+                                           window->serverGeometry (),
+                                           window->border (),
+                                           workArea,
+                                           staticGravity);
 
 }