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

[Sam Spilsbury]

Change the behaviour of undecorating windows.

Previously when a window was undecorated, we would shift it back to an appropriate position according to its gravity member. That behaviour was problematic because in the StaticGravity case the window has to just stay in the same place. But then if you had a window with StaticGravity which then did get a decoration and later removed it, it would be placed as though it was decorated and appear to be in the wrong place.

The correct behaviour is to place all windows as though they have decorations, and then when decorations are removed, to move the window back to the corner as indicated in its gravity and then expand its size to cover the obscured regions no longer hidden because the decorations went away.

(LP: #1165343)

Change the behaviour of undecorating windows.

Previously when a window was undecorated, we would shift it back to an appropriate position according to its gravity member. That behaviour was problematic because in the StaticGravity case the window has to just stay in the same place. But then if you had a window with StaticGravity which then did get a decoration and later removed it, it would be placed as though it was decorated and appear to be in the wrong place.

The correct behaviour is to place all windows as though they have decorations, and then when decorations are removed, to move the window back to the corner as indicated in its gravity and then expand its size to cover the obscured regions no longer hidden because the decorations went away.

The spec is unfortunately not so clear what to do in this case. But KWin does it this way
and it seems to be the sanest way of doing it.

(LP: #1165343)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3660
http://jenkins.qa.ubuntu.com/job/compiz-ci/137/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/175
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/526

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

review: Approve (continuous-integration)

[MC Return (mc-return)]

The commit message looks a bit strange...

[MC Return (mc-return)]

606 +# Install any sun-awt application

You could give some concrete example here.

596 +Actions:
597 +# Start and launch friends-app
598 +
599 +Expected Result:
600 + The QML window should have its decorations visible and
601 + be contained in the top right hand corner of the work area

It starts placed in the top-left corner here...

Note: I was not able to fully test this branch yet, just tested the changes
      to the place and decor plugins, but not core, so I have to "Abstain"
      for now.

review: Abstain

[Sam Spilsbury (smspillaz)]

On Mon, Apr 22, 2013 at 6:04 PM, MC Return <email address hidden> wrote:
> Review: Abstain
>
> 606 +# Install any sun-awt application
>
> You could give some concrete example here.
>
> 596 +Actions:
> 597 +# Start and launch friends-app
> 598 +
> 599 +Expected Result:
> 600 + The QML window should have its decorations visible and
> 601 + be contained in the top right hand corner of the work area
>
> It starts placed in the top-left corner here...

Ah, that's a typo. Thanks.

>
> Note: I was not able to fully test this branch yet, just tested the changes
> to the place and decor plugins, but not core, so I have to "Abstain"
> for now.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1165343/+merge/159540
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1165343 into lp:compiz.

--
Sam Spilsbury

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3661
http://jenkins.qa.ubuntu.com/job/compiz-ci/142/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/180
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-pbuilder/./build=pbuilder,distribution=raring,flavor=amd64/531

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

review: Approve (continuous-integration)

[MC Return (mc-return)]

The commit message here still looks a bit strange ;)

[MC Return (mc-return)]

LGTM now. +1

review: Approve

[MC Return (mc-return)]

I am still hitting bug #1159324 in current trunk... :(

[Sam Spilsbury (smspillaz)]

With our without this branch?
On 20/05/2013 1:39 AM, "MC Return" <email address hidden> wrote:

> I am still hitting bug #1159324 in current trunk... :(
> --
>
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1165343/+merge/159540
> Your team Compiz Maintainers is requested to review the proposed merge of
> lp:~compiz-team/compiz/compiz.fix_1165343 into lp:compiz.
>

[MC Return (mc-return)]

> With our without this branch?

Without it.

[Sam Spilsbury (smspillaz)]

Okay. Please comment on the bug reports then to raise that issue. I'm still
waiting on someone from canonical to review this branch and posting
comments here can get confusing.
On 20/05/2013 9:27 AM, "MC Return" <email address hidden> wrote:

> > With our without this branch?
>
> Without it.
> --
>
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1165343/+merge/159540
> Your team Compiz Maintainers is requested to review the proposed merge of
> lp:~compiz-team/compiz/compiz.fix_1165343 into lp:compiz.
>

[MC Return (mc-return)]

Guake is still broken, even with this MP :(

The strange thing is -> frist it broke, then you fixed it, then it broke again and now it is already broken for quite a while...

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

I'll have a quick look into it now I guess.

On Mon, May 20, 2013 at 6:15 PM, MC Return <email address hidden> wrote:
> Review: Needs Fixing
>
> Guake is still broken, even with this MP :(
>
> The strange thing is -> frist it broke, then you fixed it, then it broke again and now it is already broken for quite a while...
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1165343/+merge/159540
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1165343 into lp:compiz.

--
Sam Spilsbury

[MC Return (mc-return)]

> I'll have a quick look into it now I guess.
>
Thx. +1

[Sam Spilsbury (smspillaz)]

Should work as expected now.

This reminds me that the decor plugin really needs to be a priority in terms of getting an acceptance test framework up and running - the amount of regressions we get here is *stupidly* high because the behaviour is so tightly coupled with the rest of the geometry-changing plugins.

[MC Return (mc-return)]

> Should work as expected now.
>
Hmm, did you run the manual tests ?

[Sam Spilsbury (smspillaz)]

Yes, I did "run" the manual tests. They all verify just fine. friends-app and guake both sit flush with the panels upon placement.

Unless I missed something?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3663
http://jenkins.qa.ubuntu.com/job/compiz-ci/181/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/222
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/36

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

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3663
http://jenkins.qa.ubuntu.com/job/compiz-ci/182/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/223
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/39
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/39
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/37

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

review: Approve (continuous-integration)

[MC Return (mc-return)]

> Yes, I did "run" the manual tests. They all verify just fine. friends-app and
> guake both sit flush with the panels upon placement.
>
> Unless I missed something?

+1. Thanks.

(I missed the change in src/window/extents/src/windowextents.cpp ;))

review: Approve

[MC Return (mc-return)]

Note: I've linked your branch to the Guake bug report.

[MC Return (mc-return)]

How often do I need to approve this ?

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3726
http://jenkins.qa.ubuntu.com/job/compiz-ci/184/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-gles-ci/./build=pbuilder,distribution=raring,flavor=amd64/225
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-amd64-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-armhf-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-raring-i386-ci/39

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

review: Approve (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

Sorry its taken so long to review this... but my gnome-terminals lose their decor when running this branch :(

review: Needs Fixing

[MC Return (mc-return)]

gnome-terminals seem to have their own will sometimes :(

[Sam Spilsbury (smspillaz)]

Brandon could you post some steps to reproduce that?

[Brandon Schaefer (brandontschaefer)]

Built this branch, then built unity trunk and on a compiz --replace ccp my decor was just missing from terminals. Possibly had to do with going to maximized gnome terminal to a normal window? Ill try to get some more detail tomorrow!

[Sam Spilsbury (smspillaz)]

Were your terminals aleady maximized when you replaced the running
compiz instance? In such a case they would already be undecorated and
they wouldn't be redecorated once they destroyed and re-created
themselves because they preserve their decoration state.

In any case, gnome-terminal is particularly badly behaved whenever the
compositing state changes. As mentioned it destroys and re-creates its
own window whilst the window manager is being restarted. This gives
rise to a number of window management related race conditions. As
such, I generally ignore all the misbehavior that happens with
gnome-terminal when the running compiz instance is replaced. This is
probably just an example of that.

On Tue, May 28, 2013 at 11:30 PM, Brandon Schaefer
<email address hidden> wrote:
> Built this branch, then built unity trunk and on a compiz --replace ccp my decor was just missing from terminals. Possibly had to do with going to maximized gnome terminal to a normal window? Ill try to get some more detail tomorrow!
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1165343.1/+merge/165768
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

[Sam Spilsbury (smspillaz)]

As an aside, I believe this misbehavior had a lot to do with
transparency being completely removed in 3.8. (gnome-terminal needs to
change its colormap whenever the compositing state changes - ARGB is
required for compositing and RGB is required for fake transparency.)

On Tue, May 28, 2013 at 11:34 PM, Sam Spilsbury <email address hidden> wrote:
> Were your terminals aleady maximized when you replaced the running
> compiz instance? In such a case they would already be undecorated and
> they wouldn't be redecorated once they destroyed and re-created
> themselves because they preserve their decoration state.
>
> In any case, gnome-terminal is particularly badly behaved whenever the
> compositing state changes. As mentioned it destroys and re-creates its
> own window whilst the window manager is being restarted. This gives
> rise to a number of window management related race conditions. As
> such, I generally ignore all the misbehavior that happens with
> gnome-terminal when the running compiz instance is replaced. This is
> probably just an example of that.
>
> On Tue, May 28, 2013 at 11:30 PM, Brandon Schaefer
> <email address hidden> wrote:
>> Built this branch, then built unity trunk and on a compiz --replace ccp my decor was just missing from terminals. Possibly had to do with going to maximized gnome terminal to a normal window? Ill try to get some more detail tomorrow!
>> --
>> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1165343.1/+merge/165768
>> Your team Compiz Maintainers is subscribed to branch lp:compiz.
>
>
>
> --
> Sam Spilsbury

--
Sam Spilsbury

[Brandon Schaefer (brandontschaefer)]

Hmm interesting, as I do keep my gnome-terminals in transparent mode. Soo let me give it some more testing to see. The big reason I marked this as needs fixing because I reverted your changes and the decor came back.

[Brandon Schaefer (brandontschaefer)]

Sweet, things seem to be working fine now? (Must have messed up the installed and possibly broke the decor plugin?)

Either way, code looks good and the unit tests/manual tests are working for me :)

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

[MC Return (mc-return)]

This seems to be wrong:

37 + int dwidth = sizeDelta.width () - lastSizeDelta.height ();

and seems to be responsible for bug #1186723 -> Regression: Now Guake, Terra and other drop-down terminals are too large and extend to the right side into the next screen

I'll propose a fix...

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2013-05-26 06:43:26 +0000
@@ -0,0 +1,1 @@
+.bzr-repo

=== modified file 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp	2013-05-09 13:43:07 +0000
+++ plugins/decor/src/decor.cpp	2013-05-26 06:43:26 +0000
@@ -1165,6 +1165,8 @@
 
     xwc.x += w->serverGeometry ().x ();
     xwc.y += w->serverGeometry ().y ();
+    xwc.width += w->serverGeometry ().width ();
+    xwc.height += w->serverGeometry ().height ();
 
     w->configureXWindow (mask, &xwc);
     screen->handleCompizEvent ("decor", "window_decorated", o);
@@ -1509,38 +1511,68 @@
 
 void
 DecorWindow::moveDecoratedWindowBy (const CompPoint &movement,
+				    const CompSize  &sizeDelta,
 				    bool            instant)
 {
-    /* Need to actually move the window */
-    if (window->placed () && !window->overrideRedirect () &&
-	(movement.x () || movement.y ()))
+    /* movement and sizeDelta are the shift of the client window
+     * as a result of decoration from a theoretical neutral position,
+     * lastShift and lastSizeDelta are the last recorded shift 
+     * and size-change. The true difference between two decorations
+     * is movement - lastShift, sizeDelta - sizeDelta */
+
+    int dx = movement.x () - lastShift.x ();
+    int dy = movement.y () - lastShift.y ();
+    int dwidth = sizeDelta.width () - lastSizeDelta.height ();
+    int dheight = sizeDelta.height () - lastSizeDelta.height ();
+
+    /* We don't apply these rules to override-redirect windows
+     * and we need to check that both the position and of the window
+     * would change as a result of decoration in order to move it
+     * (this is usually the case because as a window is decorated
+     *  it will necessarily get bigger or smaller in order to fit
+     *  inside its decoration) */
+    if (!window->overrideRedirect () &&
+	((dx || dy) && (dwidth || dheight)))
     {
 	XWindowChanges xwc;
-	unsigned int   mask = CWX | CWY;
+	unsigned int   mask = CWX | CWY | CWWidth | CWHeight;
 
 	memset (&xwc, 0, sizeof (XWindowChanges));
 
 	/* Grab the geometry last sent to server at configureXWindow
 	 * time and not here since serverGeometry may be updated by
 	 * the time that we do call configureXWindow */
-	xwc.x = movement.x ();
-	xwc.y = movement.y ();
+	xwc.x = dx;
+	xwc.y = dy;
+	xwc.width = dwidth;
+	xwc.height = dheight;
 
 	/* Except if it's fullscreen, maximized or such */
 	if (window->state () & CompWindowStateFullscreenMask)
-	    mask &= ~(CWX | CWY);
+	    mask &= ~(CWX | CWY | CWWidth | CWHeight);
 
 	if (window->state () & CompWindowStateMaximizedHorzMask)
-	    mask &= ~CWX;
+	    mask &= ~(CWX | CWWidth);
 
 	if (window->state () & CompWindowStateMaximizedVertMask)
-	    mask &= ~CWY;
+	    mask &= ~(CWY | CWHeight);
 
 	if (window->saveMask () & CWX)
-	    window->saveWc ().x += movement.x ();
+	    window->saveWc ().x += xwc.x;
 
 	if (window->saveMask () & CWY)
-	    window->saveWc ().y += movement.y ();
+	    window->saveWc ().y += xwc.y;
+
+	if (window->saveMask () & CWWidth)
+	    window->saveWc ().width += xwc.width;
+
+	if (window->saveMask () & CWHeight)
+	    window->saveWc ().height += xwc.height;
+
+	/* If the window has not been placed, do not move it this time
+	 * but record what we would have moved it by */
+	if (!window->placed ())
+	    mask = 0;
 
 	if (mask)
 	{
@@ -1558,6 +1590,15 @@
 	    else
 		moveUpdate.start (boost::bind (decorOffsetMove, window, xwc, mask), 0);
 	}
+
+	/* Even if the window has not yet been placed, we still
+	 * need to store what we would have moved it by in order
+	 * to put it in the right position. The place plugin will
+	 * set a position that makes the most sense, but we need
+	 * to know how much to move back by should the window
+	 * become undecorated again */
+	lastShift = movement;
+	lastSizeDelta = sizeDelta;
     }
 }
 
@@ -1568,9 +1609,9 @@
                       bool       shadowOnly,
                       bool       isSwitcher)
 {
-    const bool visible = (w->frame () ||
+    const bool frameOrUnmapReference = (w->frame () ||
 		          w->hasUnmapReference ());
-    const bool realDecoration = visible && !shadowOnly;
+    const bool realDecoration = frameOrUnmapReference && !shadowOnly;
     const bool forceDecoration = isSwitcher;
 
     return realDecoration || forceDecoration;
@@ -1616,7 +1657,8 @@
 DecorWindow::update (bool allowDecoration)
 {
     Decoration::Ptr  old, decoration;
-    CompPoint        oldShift, movement;
+    CompPoint        movement;
+    CompSize         sizeDelta;
 
     if (wd)
 	old = wd->decor;
@@ -1650,13 +1692,9 @@
     if (decoration == old)
 	return false;
 
-    /* Determine how much we moved the window for the old
-     * decoration and save that, also destroy the old
-     * WindowDecoration */
+    /* Destroy the old WindowDecoration */
     if (old)
     {
-	oldShift = cwe::shift (window->border (), window->sizeHints ().win_gravity);
-
 	WindowDecoration::destroy (wd);
 	wd = NULL;
     }
@@ -1678,7 +1716,6 @@
 	else if (!window->hasUnmapReference ())
 	    window->setWindowFrameExtents (&decoration->border,
 					   &decoration->input);
-
 	/* This window actually needs its decoration contents updated
 	 * as it was actually visible */
 	if (decorate ||
@@ -1695,7 +1732,11 @@
 	    }
 
 	    movement = cwe::shift (window->border (), window->sizeHints ().win_gravity);
-	    movement -= oldShift;
+
+	    sizeDelta = CompSize (-(window->border ().left +
+				    window->border ().right),
+				  -(window->border ().top +
+				    window->border ().bottom));
 
 	    window->updateWindowOutputExtents ();
 
@@ -1724,8 +1765,6 @@
 	memset (&emptyExtents, 0, sizeof (CompWindowExtents));
 
 	window->setWindowFrameExtents (&emptyExtents, &emptyExtents);
-
-	movement -= oldShift;
     }
 
     /* We need to damage the current output extents
@@ -1739,6 +1778,7 @@
     }
 
     moveDecoratedWindowBy (movement,
+			   sizeDelta,
 			   !allowDecoration);
 
     return true;
@@ -2280,6 +2320,20 @@
 }
 
 /*
+ * DecorWindow::place
+ *
+ * Update any windows just before placement
+ * so that placement algorithms will have the
+ * border size at place-time
+ */
+bool
+DecorWindow::place (CompPoint &pos)
+{
+    update (true);
+    return window->place (pos);
+}
+
+/*
  * DecorWindow::windowNotify
  *
  * Window event notification handler. On various
@@ -2957,9 +3011,6 @@
 {
     if (wd && wd->decor)
     {
-	CompPoint oldShift = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
-	
-
 	if ((window->state () & MAXIMIZE_STATE))
 	    window->setWindowFrameExtents (&wd->decor->maxBorder,
 					   &wd->decor->maxInput);
@@ -2967,18 +3018,7 @@
 	    window->setWindowFrameExtents (&wd->decor->border,
 					   &wd->decor->input);
 
-	/* Since we immediately update the frame extents, we must
-	 * also update the stored saved window geometry in order
-	 * to prevent the window from shifting back too far once
-	 * unmaximized */
-
-	CompPoint movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity) - oldShift;
-
-	if (window->saveMask () & CWX)
-	    window->saveWc ().x += movement.x ();
-
-	if (window->saveMask () & CWY)
-	    window->saveWc ().y += movement.y ();
+	/* The shift will occurr in decorOffsetMove */
 
 	updateFrame ();
     }

=== modified file 'plugins/decor/src/decor.h'
--- plugins/decor/src/decor.h	2013-03-22 15:52:13 +0000
+++ plugins/decor/src/decor.h	2013-05-26 06:43:26 +0000
@@ -293,6 +293,8 @@
 
 	bool damageRect (bool, const CompRect &);
 
+	bool place (CompPoint &pos);
+
 	bool glDraw (const GLMatrix &, const GLWindowPaintAttrib &,
 		     const CompRegion &, unsigned int);
 	void glDecorate (const GLMatrix &, const GLWindowPaintAttrib &,
@@ -380,12 +382,16 @@
 
 	X11DecorPixmapRequestor   mRequestor;
 
+	CompPoint               lastShift;
+	CompSize                lastSizeDelta;
+
     private:
 
 	bool            bareDecorationOnly ();
 	Decoration::Ptr findRealDecoration ();
 	Decoration::Ptr findBareDecoration ();
 	void            moveDecoratedWindowBy (const CompPoint &movement,
+					       const CompSize &sizeDelta,
 					       bool instant);
 };
 

=== 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-30 04:11:22 +0000
+++ plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h	2013-05-26 06:43:26 +0000
@@ -45,8 +45,7 @@
 CompPoint & constrainPositionToWorkArea (CompPoint                      &pos,
 				         const compiz::window::Geometry &serverGeometry,
                                          const CompWindowExtents        &border,
-                                         const CompRect                 &workArea,
-                                         bool                           staticGravity);
+					 const CompRect                 &workArea);
 
 
 CompPoint getViewportRelativeCoordinates (const compiz::window::Geometry &geom,
@@ -54,8 +53,7 @@
 
 CompWindowExtents getWindowEdgePositions (const CompPoint                &position,
 					  const compiz::window::Geometry &geom,
-					  const CompWindowExtents        &border,
-					  unsigned int                   gravity);
+					  const CompWindowExtents        &border);
 
 void clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
 					    const CompRect    &workArea);
@@ -64,8 +62,7 @@
 
 void subtractBordersFromEdgePositions (CompWindowExtents       &edgePositions,
 				       const CompWindowExtents &border,
-				       unsigned int            legacyBorder,
-				       unsigned int            gravity);
+				       unsigned int            legacyBorder);
 
 bool onlySizeChanged (unsigned int mask);
 bool applyWidthChange (const CompWindowExtents &edgePositions,

=== 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-30 04:11:22 +0000
+++ plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp	2013-05-26 06:43:26 +0000
@@ -140,35 +140,21 @@
 
 CompPoint &
 cp::constrainPositionToWorkArea (CompPoint               &pos,
-			         const cw::Geometry      &serverGeometry,
-			         const CompWindowExtents &border,
-			         const CompRect          &workArea,
-			         bool                    staticGravity)
+				 const cw::Geometry      &serverGeometry,
+				 const CompWindowExtents &border,
+				 const CompRect          &workArea)
 {
     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.left   = pos.x () - border.left;
+    extents.top    = pos.y () - border.top;
     extents.right  = extents.left + serverGeometry.widthIncBorders () +
-		     (effectiveBorders.left +
-		      effectiveBorders.right);
+		     (border.left +
+		      border.right);
     extents.bottom = extents.top + serverGeometry.heightIncBorders () +
-		     (effectiveBorders.top +
-		      effectiveBorders.bottom);
+		     (border.top +
+		      border.bottom);
 
     delta = workArea.right () - extents.right;
     if (delta < 0)
@@ -186,8 +172,8 @@
     if (delta > 0)
 	extents.top += delta;
 
-    pos.setX (extents.left + effectiveBorders.left);
-    pos.setY (extents.top  + effectiveBorders.top);
+    pos.setX (extents.left + border.left);
+    pos.setY (extents.top  + border.top);
 
     return pos;
 }
@@ -213,23 +199,18 @@
 
 CompWindowExtents cp::getWindowEdgePositions (const CompPoint         &position,
 					      const cw::Geometry      &geom,
-					      const CompWindowExtents &border,
-					      unsigned int            gravity)
+					      const CompWindowExtents &border)
 {
     CompWindowExtents edgePositions;
-    CompWindowExtents effectiveBorder (border);
-
-    if (gravity & StaticGravity)
-	effectiveBorder = CompWindowExtents (0, 0, 0, 0);
-
-    edgePositions.left   = position.x () - effectiveBorder.left;
+
+    edgePositions.left   = position.x () - border.left;
     edgePositions.right  = edgePositions.left +
-			   geom.widthIncBorders () +  (effectiveBorder.left +
-						       effectiveBorder.right);
-    edgePositions.top    = position.y () - effectiveBorder.top;
+			   geom.widthIncBorders () +  (border.left +
+						       border.right);
+    edgePositions.top    = position.y () - border.top;
     edgePositions.bottom = edgePositions.top +
-			   geom.heightIncBorders () + (effectiveBorder.top +
-						       effectiveBorder.bottom);
+			   geom.heightIncBorders () + (border.top +
+						       border.bottom);
 
     return edgePositions;
 }
@@ -285,19 +266,14 @@
 
 void cp::subtractBordersFromEdgePositions (CompWindowExtents       &edgePositions,
 					   const CompWindowExtents &border,
-					   unsigned int            legacyBorder,
-					   unsigned int            gravity)
+					   unsigned int            legacyBorder)
 {
     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;
+
+    edgePositions.left   += border.left;
+    edgePositions.right  -= border.right + doubleBorder;
+    edgePositions.top    += border.top;
+    edgePositions.bottom -= border.bottom + doubleBorder;
 }
 
 bool cp::onlySizeChanged (unsigned int mask)

=== 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-30 04:11:22 +0000
+++ plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp	2013-05-26 06:43:26 +0000
@@ -226,7 +226,7 @@
     extents = WindowExtents (GetParam ());
 
     CompPoint pos = InitialPosition (GetParam ());
-    pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, false);
+    pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea);
 
     const CompPoint expectedAfterExtentsAdjustment = ExpectedPosition +
 						     CompPoint (extents.left,
@@ -235,19 +235,6 @@
     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[] =
@@ -297,7 +284,7 @@
 	CompPoint    pos;
 };
 
-TEST_F (PlaceGetEdgePositions, GetEdgePositionsNWGravity)
+TEST_F (PlaceGetEdgePositions, GetEdgePositions)
 {
     int left = geom.x () - border.left;
     int right = left + (geom.widthIncBorders ()) +
@@ -309,25 +296,7 @@
     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));
+							    border));
 
     EXPECT_EQ (ExpectedExtents, actualExtents);
 }
@@ -527,29 +496,7 @@
 
     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);
+					  legacyBorder);
 
     EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
 }

=== modified file 'plugins/place/src/place.cpp'
--- plugins/place/src/place.cpp	2013-05-09 13:43:07 +0000
+++ plugins/place/src/place.cpp	2013-05-26 06:43:26 +0000
@@ -362,8 +362,7 @@
 
     CompWindowExtents edgePositions = cp::getWindowEdgePositions (pos,
 								  geom,
-								  window->border (),
-								  window->sizeHints ().win_gravity);
+								  window->border ());
 
     int      output   = screen->outputDeviceForGeometry (geom);
     CompRect workArea = screen->getWorkareaForOutput (output);
@@ -386,8 +385,7 @@
     /* bring left/right/top/bottom to actual window coordinates */
     cp::subtractBordersFromEdgePositions (edgePositions,
 					  window->border (),
-					  geom.border (),
-					  window->sizeHints ().win_gravity);
+					  geom.border ());
 
     /* always validate position if the application changed only its size,
      * as it might become partially offscreen because of that */
@@ -1143,13 +1141,10 @@
 PlaceWindow::constrainToWorkarea (const CompRect &workArea,
 				  CompPoint      &pos)
 {
-    bool staticGravity = window->sizeHints ().win_gravity & StaticGravity;
-
     pos = cp::constrainPositionToWorkArea (pos,
                                            window->serverGeometry (),
                                            window->border (),
-                                           workArea,
-                                           staticGravity);
+                                           workArea);
 
 }
 

=== modified file 'src/window/extents/src/windowextents.cpp'
--- src/window/extents/src/windowextents.cpp	2013-03-30 04:11:22 +0000
+++ src/window/extents/src/windowextents.cpp	2013-05-26 06:43:26 +0000
@@ -35,6 +35,11 @@
     CompPoint rv = CompPoint ();
 
     switch (gravity) {
+	/* We treat StaticGravity like NorthWestGravity here
+	 * as when decorating / undecorating the window we
+	 * really do need to move it in order to handle
+	 * any cases where it goes offscreen */
+	case StaticGravity:
 	case NorthGravity:
 	case NorthWestGravity:
 	case NorthEastGravity:
@@ -50,6 +55,11 @@
     }
 
     switch (gravity) {
+	/* We treat StaticGravity like NorthWestGravity here
+	 * as when decorating / undecorating the window we
+	 * really do need to move it in order to handle
+	 * any cases where it goes offscreen */
+	case StaticGravity:
 	case WestGravity:
 	case NorthWestGravity:
 	case SouthWestGravity:

=== added file 'tests/manual/README.txt'
--- tests/manual/README.txt	1970-01-01 00:00:00 +0000
+++ tests/manual/README.txt	2013-05-26 06:43:26 +0000
@@ -0,0 +1,9 @@
+Compiz Manual Tests
+===================
+
+Avoid writing manual tests if you can. Acceptance tests that can be run
+on an automatic basis are always preferred.
+
+If getting some part of the code would be too difficult or invasive, then
+write a manual test in here so that we can remind ourselves to deploy test
+frameworks for the code in quesiton.

=== added file 'tests/manual/plugins/decor.txt'
--- tests/manual/plugins/decor.txt	1970-01-01 00:00:00 +0000
+++ tests/manual/plugins/decor.txt	2013-05-26 06:43:26 +0000
@@ -0,0 +1,40 @@
+COMPIZ DECOR PLUGIN
+===================
+Sam Spilsbury <[e-mail]>
+
+Static Gravity Handling - no decorations
+----------------------------------------
+Setup:
+# Install guake
+
+Actions:
+# Start and launch guake
+
+Expected Result:
+  Guake should sit flush with the panels and work area
+
+Static Gravity Handling - decorations
+-------------------------------------
+Setup:
+# Install friends-app
+
+Actions:
+# Start and launch friends-app
+
+Expected Result:
+  The QML window should have its decorations visible and
+  be contained in the top left hand corner of the work area
+
+_NET_REQUEST_FRAME_EXTENTS handling
+-----------------------------------
+Setup:
+# Install any sun-awt application - examples:
+  1. netbeans
+  2. ecplise
+
+Actions:
+# Run the application
+
+Expected Result:
+  The application should not have its contents offset
+  by its decoration size