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

Proposed by Sam Spilsbury
Status: Superseded
Proposed branch: lp:~compiz-team/compiz/compiz.fix_1165343
Merge into: lp:compiz/0.9.10
Diff against target: 642 lines (+178/-158)
9 files modified
plugins/decor/src/decor.cpp (+78/-38)
plugins/decor/src/decor.h (+6/-0)
plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h (+3/-6)
plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp (+25/-49)
plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp (+4/-57)
plugins/place/src/place.cpp (+3/-8)
src/window/extents/src/windowextents.cpp (+10/-0)
tests/manual/README.txt (+9/-0)
tests/manual/plugins/decor.txt (+40/-0)
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_1165343
Reviewer Review Type Date Requested Status
MC Return Approve
PS Jenkins bot (community) continuous-integration Approve
Compiz Maintainers Pending
Review via email: mp+164762@code.launchpad.net

This proposal supersedes a proposal from 2013-04-18.

This proposal has been superseded by a proposal from 2013-05-26.

Commit message

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)

Description of the change

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)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

The commit message looks a bit strange...

Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

The commit message here still looks a bit strange ;)

Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

LGTM now. +1

review: Approve
Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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.
>

Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

> With our without this branch?

Without it.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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.
>

Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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

Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

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.

Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

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?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
MC Return (mc-return) wrote :

> 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
Revision history for this message
MC Return (mc-return) wrote :

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

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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-20 16:13:29 +0000
@@ -1165,6 +1165,8 @@
11651165
1166 xwc.x += w->serverGeometry ().x ();1166 xwc.x += w->serverGeometry ().x ();
1167 xwc.y += w->serverGeometry ().y ();1167 xwc.y += w->serverGeometry ().y ();
1168 xwc.width += w->serverGeometry ().width ();
1169 xwc.height += w->serverGeometry ().height ();
11681170
1169 w->configureXWindow (mask, &xwc);1171 w->configureXWindow (mask, &xwc);
1170 screen->handleCompizEvent ("decor", "window_decorated", o);1172 screen->handleCompizEvent ("decor", "window_decorated", o);
@@ -1509,38 +1511,68 @@
15091511
1510void1512void
1511DecorWindow::moveDecoratedWindowBy (const CompPoint &movement,1513DecorWindow::moveDecoratedWindowBy (const CompPoint &movement,
1514 const CompSize &sizeDelta,
1512 bool instant)1515 bool instant)
1513{1516{
1514 /* Need to actually move the window */1517 /* movement and sizeDelta are the shift of the client window
1515 if (window->placed () && !window->overrideRedirect () &&1518 * as a result of decoration from a theoretical neutral position,
1516 (movement.x () || movement.y ()))1519 * lastShift and lastSizeDelta are the last recorded shift
1520 * and size-change. The true difference between two decorations
1521 * is movement - lastShift, sizeDelta - sizeDelta */
1522
1523 int dx = movement.x () - lastShift.x ();
1524 int dy = movement.y () - lastShift.y ();
1525 int dwidth = sizeDelta.width () - lastSizeDelta.height ();
1526 int dheight = sizeDelta.height () - lastSizeDelta.height ();
1527
1528 /* We don't apply these rules to override-redirect windows
1529 * and we need to check that both the position and of the window
1530 * would change as a result of decoration in order to move it
1531 * (this is usually the case because as a window is decorated
1532 * it will necessarily get bigger or smaller in order to fit
1533 * inside its decoration) */
1534 if (!window->overrideRedirect () &&
1535 ((dx || dy) && (dwidth || dheight)))
1517 {1536 {
1518 XWindowChanges xwc;1537 XWindowChanges xwc;
1519 unsigned int mask = CWX | CWY;1538 unsigned int mask = CWX | CWY | CWWidth | CWHeight;
15201539
1521 memset (&xwc, 0, sizeof (XWindowChanges));1540 memset (&xwc, 0, sizeof (XWindowChanges));
15221541
1523 /* Grab the geometry last sent to server at configureXWindow1542 /* Grab the geometry last sent to server at configureXWindow
1524 * time and not here since serverGeometry may be updated by1543 * time and not here since serverGeometry may be updated by
1525 * the time that we do call configureXWindow */1544 * the time that we do call configureXWindow */
1526 xwc.x = movement.x ();1545 xwc.x = dx;
1527 xwc.y = movement.y ();1546 xwc.y = dy;
1547 xwc.width = dwidth;
1548 xwc.height = dheight;
15281549
1529 /* Except if it's fullscreen, maximized or such */1550 /* Except if it's fullscreen, maximized or such */
1530 if (window->state () & CompWindowStateFullscreenMask)1551 if (window->state () & CompWindowStateFullscreenMask)
1531 mask &= ~(CWX | CWY);1552 mask &= ~(CWX | CWY | CWWidth | CWHeight);
15321553
1533 if (window->state () & CompWindowStateMaximizedHorzMask)1554 if (window->state () & CompWindowStateMaximizedHorzMask)
1534 mask &= ~CWX;1555 mask &= ~(CWX | CWWidth);
15351556
1536 if (window->state () & CompWindowStateMaximizedVertMask)1557 if (window->state () & CompWindowStateMaximizedVertMask)
1537 mask &= ~CWY;1558 mask &= ~(CWY | CWHeight);
15381559
1539 if (window->saveMask () & CWX)1560 if (window->saveMask () & CWX)
1540 window->saveWc ().x += movement.x ();1561 window->saveWc ().x += xwc.x;
15411562
1542 if (window->saveMask () & CWY)1563 if (window->saveMask () & CWY)
1543 window->saveWc ().y += movement.y ();1564 window->saveWc ().y += xwc.y;
1565
1566 if (window->saveMask () & CWWidth)
1567 window->saveWc ().width += xwc.width;
1568
1569 if (window->saveMask () & CWHeight)
1570 window->saveWc ().height += xwc.height;
1571
1572 /* If the window has not been placed, do not move it this time
1573 * but record what we would have moved it by */
1574 if (!window->placed ())
1575 mask = 0;
15441576
1545 if (mask)1577 if (mask)
1546 {1578 {
@@ -1558,6 +1590,15 @@
1558 else1590 else
1559 moveUpdate.start (boost::bind (decorOffsetMove, window, xwc, mask), 0);1591 moveUpdate.start (boost::bind (decorOffsetMove, window, xwc, mask), 0);
1560 }1592 }
1593
1594 /* Even if the window has not yet been placed, we still
1595 * need to store what we would have moved it by in order
1596 * to put it in the right position. The place plugin will
1597 * set a position that makes the most sense, but we need
1598 * to know how much to move back by should the window
1599 * become undecorated again */
1600 lastShift = movement;
1601 lastSizeDelta = sizeDelta;
1561 }1602 }
1562}1603}
15631604
@@ -1568,9 +1609,9 @@
1568 bool shadowOnly,1609 bool shadowOnly,
1569 bool isSwitcher)1610 bool isSwitcher)
1570{1611{
1571 const bool visible = (w->frame () ||1612 const bool frameOrUnmapReference = (w->frame () ||
1572 w->hasUnmapReference ());1613 w->hasUnmapReference ());
1573 const bool realDecoration = visible && !shadowOnly;1614 const bool realDecoration = frameOrUnmapReference && !shadowOnly;
1574 const bool forceDecoration = isSwitcher;1615 const bool forceDecoration = isSwitcher;
15751616
1576 return realDecoration || forceDecoration;1617 return realDecoration || forceDecoration;
@@ -1616,7 +1657,8 @@
1616DecorWindow::update (bool allowDecoration)1657DecorWindow::update (bool allowDecoration)
1617{1658{
1618 Decoration::Ptr old, decoration;1659 Decoration::Ptr old, decoration;
1619 CompPoint oldShift, movement;1660 CompPoint movement;
1661 CompSize sizeDelta;
16201662
1621 if (wd)1663 if (wd)
1622 old = wd->decor;1664 old = wd->decor;
@@ -1650,13 +1692,9 @@
1650 if (decoration == old)1692 if (decoration == old)
1651 return false;1693 return false;
16521694
1653 /* Determine how much we moved the window for the old1695 /* Destroy the old WindowDecoration */
1654 * decoration and save that, also destroy the old
1655 * WindowDecoration */
1656 if (old)1696 if (old)
1657 {1697 {
1658 oldShift = cwe::shift (window->border (), window->sizeHints ().win_gravity);
1659
1660 WindowDecoration::destroy (wd);1698 WindowDecoration::destroy (wd);
1661 wd = NULL;1699 wd = NULL;
1662 }1700 }
@@ -1678,7 +1716,6 @@
1678 else if (!window->hasUnmapReference ())1716 else if (!window->hasUnmapReference ())
1679 window->setWindowFrameExtents (&decoration->border,1717 window->setWindowFrameExtents (&decoration->border,
1680 &decoration->input);1718 &decoration->input);
1681
1682 /* This window actually needs its decoration contents updated1719 /* This window actually needs its decoration contents updated
1683 * as it was actually visible */1720 * as it was actually visible */
1684 if (decorate ||1721 if (decorate ||
@@ -1695,7 +1732,11 @@
1695 }1732 }
16961733
1697 movement = cwe::shift (window->border (), window->sizeHints ().win_gravity);1734 movement = cwe::shift (window->border (), window->sizeHints ().win_gravity);
1698 movement -= oldShift;1735
1736 sizeDelta = CompSize (-(window->border ().left +
1737 window->border ().right),
1738 -(window->border ().top +
1739 window->border ().bottom));
16991740
1700 window->updateWindowOutputExtents ();1741 window->updateWindowOutputExtents ();
17011742
@@ -1724,8 +1765,6 @@
1724 memset (&emptyExtents, 0, sizeof (CompWindowExtents));1765 memset (&emptyExtents, 0, sizeof (CompWindowExtents));
17251766
1726 window->setWindowFrameExtents (&emptyExtents, &emptyExtents);1767 window->setWindowFrameExtents (&emptyExtents, &emptyExtents);
1727
1728 movement -= oldShift;
1729 }1768 }
17301769
1731 /* We need to damage the current output extents1770 /* We need to damage the current output extents
@@ -1739,6 +1778,7 @@
1739 }1778 }
17401779
1741 moveDecoratedWindowBy (movement,1780 moveDecoratedWindowBy (movement,
1781 sizeDelta,
1742 !allowDecoration);1782 !allowDecoration);
17431783
1744 return true;1784 return true;
@@ -2280,6 +2320,20 @@
2280}2320}
22812321
2282/*2322/*
2323 * DecorWindow::place
2324 *
2325 * Update any windows just before placement
2326 * so that placement algorithms will have the
2327 * border size at place-time
2328 */
2329bool
2330DecorWindow::place (CompPoint &pos)
2331{
2332 update (true);
2333 return window->place (pos);
2334}
2335
2336/*
2283 * DecorWindow::windowNotify2337 * DecorWindow::windowNotify
2284 *2338 *
2285 * Window event notification handler. On various2339 * Window event notification handler. On various
@@ -2957,9 +3011,6 @@
2957{3011{
2958 if (wd && wd->decor)3012 if (wd && wd->decor)
2959 {3013 {
2960 CompPoint oldShift = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
2961
2962
2963 if ((window->state () & MAXIMIZE_STATE))3014 if ((window->state () & MAXIMIZE_STATE))
2964 window->setWindowFrameExtents (&wd->decor->maxBorder,3015 window->setWindowFrameExtents (&wd->decor->maxBorder,
2965 &wd->decor->maxInput);3016 &wd->decor->maxInput);
@@ -2967,18 +3018,7 @@
2967 window->setWindowFrameExtents (&wd->decor->border,3018 window->setWindowFrameExtents (&wd->decor->border,
2968 &wd->decor->input);3019 &wd->decor->input);
29693020
2970 /* Since we immediately update the frame extents, we must3021 /* The shift will occurr in decorOffsetMove */
2971 * also update the stored saved window geometry in order
2972 * to prevent the window from shifting back too far once
2973 * unmaximized */
2974
2975 CompPoint movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity) - oldShift;
2976
2977 if (window->saveMask () & CWX)
2978 window->saveWc ().x += movement.x ();
2979
2980 if (window->saveMask () & CWY)
2981 window->saveWc ().y += movement.y ();
29823022
2983 updateFrame ();3023 updateFrame ();
2984 }3024 }
29853025
=== 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-20 16:13:29 +0000
@@ -293,6 +293,8 @@
293293
294 bool damageRect (bool, const CompRect &);294 bool damageRect (bool, const CompRect &);
295295
296 bool place (CompPoint &pos);
297
296 bool glDraw (const GLMatrix &, const GLWindowPaintAttrib &,298 bool glDraw (const GLMatrix &, const GLWindowPaintAttrib &,
297 const CompRegion &, unsigned int);299 const CompRegion &, unsigned int);
298 void glDecorate (const GLMatrix &, const GLWindowPaintAttrib &,300 void glDecorate (const GLMatrix &, const GLWindowPaintAttrib &,
@@ -380,12 +382,16 @@
380382
381 X11DecorPixmapRequestor mRequestor;383 X11DecorPixmapRequestor mRequestor;
382384
385 CompPoint lastShift;
386 CompSize lastSizeDelta;
387
383 private:388 private:
384389
385 bool bareDecorationOnly ();390 bool bareDecorationOnly ();
386 Decoration::Ptr findRealDecoration ();391 Decoration::Ptr findRealDecoration ();
387 Decoration::Ptr findBareDecoration ();392 Decoration::Ptr findBareDecoration ();
388 void moveDecoratedWindowBy (const CompPoint &movement,393 void moveDecoratedWindowBy (const CompPoint &movement,
394 const CompSize &sizeDelta,
389 bool instant);395 bool instant);
390};396};
391397
392398
=== 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-20 16:13:29 +0000
@@ -45,8 +45,7 @@
45CompPoint & constrainPositionToWorkArea (CompPoint &pos,45CompPoint & constrainPositionToWorkArea (CompPoint &pos,
46 const compiz::window::Geometry &serverGeometry,46 const compiz::window::Geometry &serverGeometry,
47 const CompWindowExtents &border,47 const CompWindowExtents &border,
48 const CompRect &workArea,48 const CompRect &workArea);
49 bool staticGravity);
5049
5150
52CompPoint getViewportRelativeCoordinates (const compiz::window::Geometry &geom,51CompPoint getViewportRelativeCoordinates (const compiz::window::Geometry &geom,
@@ -54,8 +53,7 @@
5453
55CompWindowExtents getWindowEdgePositions (const CompPoint &position,54CompWindowExtents getWindowEdgePositions (const CompPoint &position,
56 const compiz::window::Geometry &geom,55 const compiz::window::Geometry &geom,
57 const CompWindowExtents &border,56 const CompWindowExtents &border);
58 unsigned int gravity);
5957
60void clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,58void clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
61 const CompRect &workArea);59 const CompRect &workArea);
@@ -64,8 +62,7 @@
6462
65void subtractBordersFromEdgePositions (CompWindowExtents &edgePositions,63void subtractBordersFromEdgePositions (CompWindowExtents &edgePositions,
66 const CompWindowExtents &border,64 const CompWindowExtents &border,
67 unsigned int legacyBorder,65 unsigned int legacyBorder);
68 unsigned int gravity);
6966
70bool onlySizeChanged (unsigned int mask);67bool onlySizeChanged (unsigned int mask);
71bool applyWidthChange (const CompWindowExtents &edgePositions,68bool applyWidthChange (const CompWindowExtents &edgePositions,
7269
=== 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-20 16:13:29 +0000
@@ -140,35 +140,21 @@
140140
141CompPoint &141CompPoint &
142cp::constrainPositionToWorkArea (CompPoint &pos,142cp::constrainPositionToWorkArea (CompPoint &pos,
143 const cw::Geometry &serverGeometry,143 const cw::Geometry &serverGeometry,
144 const CompWindowExtents &border,144 const CompWindowExtents &border,
145 const CompRect &workArea,145 const CompRect &workArea)
146 bool staticGravity)
147{146{
148 CompWindowExtents extents;147 CompWindowExtents extents;
149 int delta;148 int delta;
150149
151 CompWindowExtents effectiveBorders = border;150 extents.left = pos.x () - border.left;
152151 extents.top = pos.y () - border.top;
153 /* Ignore borders in the StaticGravity case for placement
154 * because the window intended to be placed as if it didn't
155 * have them */
156 if (staticGravity)
157 {
158 effectiveBorders.left = 0;
159 effectiveBorders.right = 0;
160 effectiveBorders.top = 0;
161 effectiveBorders.bottom = 0;
162 }
163
164 extents.left = pos.x () - effectiveBorders.left;
165 extents.top = pos.y () - effectiveBorders.top;
166 extents.right = extents.left + serverGeometry.widthIncBorders () +152 extents.right = extents.left + serverGeometry.widthIncBorders () +
167 (effectiveBorders.left +153 (border.left +
168 effectiveBorders.right);154 border.right);
169 extents.bottom = extents.top + serverGeometry.heightIncBorders () +155 extents.bottom = extents.top + serverGeometry.heightIncBorders () +
170 (effectiveBorders.top +156 (border.top +
171 effectiveBorders.bottom);157 border.bottom);
172158
173 delta = workArea.right () - extents.right;159 delta = workArea.right () - extents.right;
174 if (delta < 0)160 if (delta < 0)
@@ -186,8 +172,8 @@
186 if (delta > 0)172 if (delta > 0)
187 extents.top += delta;173 extents.top += delta;
188174
189 pos.setX (extents.left + effectiveBorders.left);175 pos.setX (extents.left + border.left);
190 pos.setY (extents.top + effectiveBorders.top);176 pos.setY (extents.top + border.top);
191177
192 return pos;178 return pos;
193}179}
@@ -213,23 +199,18 @@
213199
214CompWindowExtents cp::getWindowEdgePositions (const CompPoint &position,200CompWindowExtents cp::getWindowEdgePositions (const CompPoint &position,
215 const cw::Geometry &geom,201 const cw::Geometry &geom,
216 const CompWindowExtents &border,202 const CompWindowExtents &border)
217 unsigned int gravity)
218{203{
219 CompWindowExtents edgePositions;204 CompWindowExtents edgePositions;
220 CompWindowExtents effectiveBorder (border);205
221206 edgePositions.left = position.x () - border.left;
222 if (gravity & StaticGravity)
223 effectiveBorder = CompWindowExtents (0, 0, 0, 0);
224
225 edgePositions.left = position.x () - effectiveBorder.left;
226 edgePositions.right = edgePositions.left +207 edgePositions.right = edgePositions.left +
227 geom.widthIncBorders () + (effectiveBorder.left +208 geom.widthIncBorders () + (border.left +
228 effectiveBorder.right);209 border.right);
229 edgePositions.top = position.y () - effectiveBorder.top;210 edgePositions.top = position.y () - border.top;
230 edgePositions.bottom = edgePositions.top +211 edgePositions.bottom = edgePositions.top +
231 geom.heightIncBorders () + (effectiveBorder.top +212 geom.heightIncBorders () + (border.top +
232 effectiveBorder.bottom);213 border.bottom);
233214
234 return edgePositions;215 return edgePositions;
235}216}
@@ -285,19 +266,14 @@
285266
286void cp::subtractBordersFromEdgePositions (CompWindowExtents &edgePositions,267void cp::subtractBordersFromEdgePositions (CompWindowExtents &edgePositions,
287 const CompWindowExtents &border,268 const CompWindowExtents &border,
288 unsigned int legacyBorder,269 unsigned int legacyBorder)
289 unsigned int gravity)
290{270{
291 const unsigned int doubleBorder = 2 * legacyBorder;271 const unsigned int doubleBorder = 2 * legacyBorder;
292 CompWindowExtents effectiveBorder = border;272
293273 edgePositions.left += border.left;
294 if (gravity & StaticGravity)274 edgePositions.right -= border.right + doubleBorder;
295 effectiveBorder = CompWindowExtents (0, 0, 0, 0);275 edgePositions.top += border.top;
296276 edgePositions.bottom -= border.bottom + doubleBorder;
297 edgePositions.left += effectiveBorder.left;
298 edgePositions.right -= effectiveBorder.right + doubleBorder;
299 edgePositions.top += effectiveBorder.top;
300 edgePositions.bottom -= effectiveBorder.bottom + doubleBorder;
301}277}
302278
303bool cp::onlySizeChanged (unsigned int mask)279bool cp::onlySizeChanged (unsigned int mask)
304280
=== 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-20 16:13:29 +0000
@@ -226,7 +226,7 @@
226 extents = WindowExtents (GetParam ());226 extents = WindowExtents (GetParam ());
227227
228 CompPoint pos = InitialPosition (GetParam ());228 CompPoint pos = InitialPosition (GetParam ());
229 pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, false);229 pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea);
230230
231 const CompPoint expectedAfterExtentsAdjustment = ExpectedPosition +231 const CompPoint expectedAfterExtentsAdjustment = ExpectedPosition +
232 CompPoint (extents.left,232 CompPoint (extents.left,
@@ -235,19 +235,6 @@
235 EXPECT_EQ (expectedAfterExtentsAdjustment, pos);235 EXPECT_EQ (expectedAfterExtentsAdjustment, pos);
236}236}
237237
238TEST_P (PlaceConstrainPositionToWorkArea, PositionConstrainedStaticGravity)
239{
240 g = WindowGeometry (GetParam ());
241 extents = WindowExtents (GetParam ());
242
243 CompPoint pos = InitialPosition (GetParam ());
244 pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, true);
245
246 /* Do not adjust residual position for extents with windows
247 * that have a static gravity */
248 EXPECT_EQ (ExpectedPosition, pos);
249}
250
251namespace238namespace
252{239{
253cwe::Extents PossibleExtents[] =240cwe::Extents PossibleExtents[] =
@@ -297,7 +284,7 @@
297 CompPoint pos;284 CompPoint pos;
298};285};
299286
300TEST_F (PlaceGetEdgePositions, GetEdgePositionsNWGravity)287TEST_F (PlaceGetEdgePositions, GetEdgePositions)
301{288{
302 int left = geom.x () - border.left;289 int left = geom.x () - border.left;
303 int right = left + (geom.widthIncBorders ()) +290 int right = left + (geom.widthIncBorders ()) +
@@ -309,25 +296,7 @@
309 const cwe::Extents ExpectedExtents (left, right, top, bottom);296 const cwe::Extents ExpectedExtents (left, right, top, bottom);
310 cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,297 cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,
311 geom,298 geom,
312 border,299 border));
313 NorthWestGravity));
314
315 EXPECT_EQ (ExpectedExtents, actualExtents);
316}
317
318TEST_F (PlaceGetEdgePositions, GetEdgePositionsStaticGravity)
319{
320 /* Don't count borders in validation */
321 int left = geom.x ();
322 int right = left + (geom.widthIncBorders ());
323 int top = geom.y ();
324 int bottom = top + (geom.heightIncBorders ());
325
326 const cwe::Extents ExpectedExtents (left, right, top, bottom);
327 cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,
328 geom,
329 border,
330 StaticGravity));
331300
332 EXPECT_EQ (ExpectedExtents, actualExtents);301 EXPECT_EQ (ExpectedExtents, actualExtents);
333}302}
@@ -527,29 +496,7 @@
527496
528 cp::subtractBordersFromEdgePositions (modifiedEdgePositions,497 cp::subtractBordersFromEdgePositions (modifiedEdgePositions,
529 borders,498 borders,
530 legacyBorder,499 legacyBorder);
531 NorthWestGravity);
532
533 EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
534}
535
536TEST (PlaceSubtractBordersFromEdgePositions, StaticGravityDefinition)
537{
538 const CompWindowExtents borders (1, 2, 3, 4);
539 const CompWindowExtents edgePositions (100, 200, 100, 200);
540 const unsigned int legacyBorder = 1;
541
542 CompWindowExtents expectedEdgePositions (edgePositions.left,
543 edgePositions.right - (2 * legacyBorder),
544 edgePositions.top,
545 edgePositions.bottom - (2 * legacyBorder));
546
547 CompWindowExtents modifiedEdgePositions (edgePositions);
548
549 cp::subtractBordersFromEdgePositions (modifiedEdgePositions,
550 borders,
551 legacyBorder,
552 StaticGravity);
553500
554 EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);501 EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
555}502}
556503
=== 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-20 16:13:29 +0000
@@ -362,8 +362,7 @@
362362
363 CompWindowExtents edgePositions = cp::getWindowEdgePositions (pos,363 CompWindowExtents edgePositions = cp::getWindowEdgePositions (pos,
364 geom,364 geom,
365 window->border (),365 window->border ());
366 window->sizeHints ().win_gravity);
367366
368 int output = screen->outputDeviceForGeometry (geom);367 int output = screen->outputDeviceForGeometry (geom);
369 CompRect workArea = screen->getWorkareaForOutput (output);368 CompRect workArea = screen->getWorkareaForOutput (output);
@@ -386,8 +385,7 @@
386 /* bring left/right/top/bottom to actual window coordinates */385 /* bring left/right/top/bottom to actual window coordinates */
387 cp::subtractBordersFromEdgePositions (edgePositions,386 cp::subtractBordersFromEdgePositions (edgePositions,
388 window->border (),387 window->border (),
389 geom.border (),388 geom.border ());
390 window->sizeHints ().win_gravity);
391389
392 /* always validate position if the application changed only its size,390 /* always validate position if the application changed only its size,
393 * as it might become partially offscreen because of that */391 * as it might become partially offscreen because of that */
@@ -1143,13 +1141,10 @@
1143PlaceWindow::constrainToWorkarea (const CompRect &workArea,1141PlaceWindow::constrainToWorkarea (const CompRect &workArea,
1144 CompPoint &pos)1142 CompPoint &pos)
1145{1143{
1146 bool staticGravity = window->sizeHints ().win_gravity & StaticGravity;
1147
1148 pos = cp::constrainPositionToWorkArea (pos,1144 pos = cp::constrainPositionToWorkArea (pos,
1149 window->serverGeometry (),1145 window->serverGeometry (),
1150 window->border (),1146 window->border (),
1151 workArea,1147 workArea);
1152 staticGravity);
11531148
1154}1149}
11551150
11561151
=== 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-20 16:13:29 +0000
@@ -35,6 +35,11 @@
35 CompPoint rv = CompPoint ();35 CompPoint rv = CompPoint ();
3636
37 switch (gravity) {37 switch (gravity) {
38 /* We treat StaticGravity like NorthWestGravity here
39 * as when decorating / undecorating the window we
40 * really do need to move it in order to handle
41 * any cases where it goes offscreen */
42 case StaticGravity:
38 case NorthGravity:43 case NorthGravity:
39 case NorthWestGravity:44 case NorthWestGravity:
40 case NorthEastGravity:45 case NorthEastGravity:
@@ -50,6 +55,11 @@
50 }55 }
5156
52 switch (gravity) {57 switch (gravity) {
58 /* We treat StaticGravity like NorthWestGravity here
59 * as when decorating / undecorating the window we
60 * really do need to move it in order to handle
61 * any cases where it goes offscreen */
62 case StaticGravity:
53 case WestGravity:63 case WestGravity:
54 case NorthWestGravity:64 case NorthWestGravity:
55 case SouthWestGravity:65 case SouthWestGravity:
5666
=== added file 'tests/manual/README.txt'
--- tests/manual/README.txt 1970-01-01 00:00:00 +0000
+++ tests/manual/README.txt 2013-05-20 16:13:29 +0000
@@ -0,0 +1,9 @@
1Compiz Manual Tests
2===================
3
4Avoid writing manual tests if you can. Acceptance tests that can be run
5on an automatic basis are always preferred.
6
7If getting some part of the code would be too difficult or invasive, then
8write a manual test in here so that we can remind ourselves to deploy test
9frameworks for the code in quesiton.
010
=== 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-20 16:13:29 +0000
@@ -0,0 +1,40 @@
1COMPIZ DECOR PLUGIN
2===================
3Sam Spilsbury <smspillaz@gmail.com>
4
5Static Gravity Handling - no decorations
6----------------------------------------
7Setup:
8# Install guake
9
10Actions:
11# Start and launch guake
12
13Expected Result:
14 Guake should sit flush with the panels and work area
15
16Static Gravity Handling - decorations
17-------------------------------------
18Setup:
19# Install friends-app
20
21Actions:
22# Start and launch friends-app
23
24Expected Result:
25 The QML window should have its decorations visible and
26 be contained in the top left hand corner of the work area
27
28_NET_REQUEST_FRAME_EXTENTS handling
29-----------------------------------
30Setup:
31# Install any sun-awt application - examples:
32 1. netbeans
33 2. ecplise
34
35Actions:
36# Run the application
37
38Expected Result:
39 The application should not have its contents offset
40 by its decoration size

Subscribers

People subscribed via source and target branches

to all changes: