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
1=== modified file 'plugins/decor/src/decor.cpp'
2--- plugins/decor/src/decor.cpp 2013-05-09 13:43:07 +0000
3+++ plugins/decor/src/decor.cpp 2013-05-20 16:13:29 +0000
4@@ -1165,6 +1165,8 @@
5
6 xwc.x += w->serverGeometry ().x ();
7 xwc.y += w->serverGeometry ().y ();
8+ xwc.width += w->serverGeometry ().width ();
9+ xwc.height += w->serverGeometry ().height ();
10
11 w->configureXWindow (mask, &xwc);
12 screen->handleCompizEvent ("decor", "window_decorated", o);
13@@ -1509,38 +1511,68 @@
14
15 void
16 DecorWindow::moveDecoratedWindowBy (const CompPoint &movement,
17+ const CompSize &sizeDelta,
18 bool instant)
19 {
20- /* Need to actually move the window */
21- if (window->placed () && !window->overrideRedirect () &&
22- (movement.x () || movement.y ()))
23+ /* movement and sizeDelta are the shift of the client window
24+ * as a result of decoration from a theoretical neutral position,
25+ * lastShift and lastSizeDelta are the last recorded shift
26+ * and size-change. The true difference between two decorations
27+ * is movement - lastShift, sizeDelta - sizeDelta */
28+
29+ int dx = movement.x () - lastShift.x ();
30+ int dy = movement.y () - lastShift.y ();
31+ int dwidth = sizeDelta.width () - lastSizeDelta.height ();
32+ int dheight = sizeDelta.height () - lastSizeDelta.height ();
33+
34+ /* We don't apply these rules to override-redirect windows
35+ * and we need to check that both the position and of the window
36+ * would change as a result of decoration in order to move it
37+ * (this is usually the case because as a window is decorated
38+ * it will necessarily get bigger or smaller in order to fit
39+ * inside its decoration) */
40+ if (!window->overrideRedirect () &&
41+ ((dx || dy) && (dwidth || dheight)))
42 {
43 XWindowChanges xwc;
44- unsigned int mask = CWX | CWY;
45+ unsigned int mask = CWX | CWY | CWWidth | CWHeight;
46
47 memset (&xwc, 0, sizeof (XWindowChanges));
48
49 /* Grab the geometry last sent to server at configureXWindow
50 * time and not here since serverGeometry may be updated by
51 * the time that we do call configureXWindow */
52- xwc.x = movement.x ();
53- xwc.y = movement.y ();
54+ xwc.x = dx;
55+ xwc.y = dy;
56+ xwc.width = dwidth;
57+ xwc.height = dheight;
58
59 /* Except if it's fullscreen, maximized or such */
60 if (window->state () & CompWindowStateFullscreenMask)
61- mask &= ~(CWX | CWY);
62+ mask &= ~(CWX | CWY | CWWidth | CWHeight);
63
64 if (window->state () & CompWindowStateMaximizedHorzMask)
65- mask &= ~CWX;
66+ mask &= ~(CWX | CWWidth);
67
68 if (window->state () & CompWindowStateMaximizedVertMask)
69- mask &= ~CWY;
70+ mask &= ~(CWY | CWHeight);
71
72 if (window->saveMask () & CWX)
73- window->saveWc ().x += movement.x ();
74+ window->saveWc ().x += xwc.x;
75
76 if (window->saveMask () & CWY)
77- window->saveWc ().y += movement.y ();
78+ window->saveWc ().y += xwc.y;
79+
80+ if (window->saveMask () & CWWidth)
81+ window->saveWc ().width += xwc.width;
82+
83+ if (window->saveMask () & CWHeight)
84+ window->saveWc ().height += xwc.height;
85+
86+ /* If the window has not been placed, do not move it this time
87+ * but record what we would have moved it by */
88+ if (!window->placed ())
89+ mask = 0;
90
91 if (mask)
92 {
93@@ -1558,6 +1590,15 @@
94 else
95 moveUpdate.start (boost::bind (decorOffsetMove, window, xwc, mask), 0);
96 }
97+
98+ /* Even if the window has not yet been placed, we still
99+ * need to store what we would have moved it by in order
100+ * to put it in the right position. The place plugin will
101+ * set a position that makes the most sense, but we need
102+ * to know how much to move back by should the window
103+ * become undecorated again */
104+ lastShift = movement;
105+ lastSizeDelta = sizeDelta;
106 }
107 }
108
109@@ -1568,9 +1609,9 @@
110 bool shadowOnly,
111 bool isSwitcher)
112 {
113- const bool visible = (w->frame () ||
114+ const bool frameOrUnmapReference = (w->frame () ||
115 w->hasUnmapReference ());
116- const bool realDecoration = visible && !shadowOnly;
117+ const bool realDecoration = frameOrUnmapReference && !shadowOnly;
118 const bool forceDecoration = isSwitcher;
119
120 return realDecoration || forceDecoration;
121@@ -1616,7 +1657,8 @@
122 DecorWindow::update (bool allowDecoration)
123 {
124 Decoration::Ptr old, decoration;
125- CompPoint oldShift, movement;
126+ CompPoint movement;
127+ CompSize sizeDelta;
128
129 if (wd)
130 old = wd->decor;
131@@ -1650,13 +1692,9 @@
132 if (decoration == old)
133 return false;
134
135- /* Determine how much we moved the window for the old
136- * decoration and save that, also destroy the old
137- * WindowDecoration */
138+ /* Destroy the old WindowDecoration */
139 if (old)
140 {
141- oldShift = cwe::shift (window->border (), window->sizeHints ().win_gravity);
142-
143 WindowDecoration::destroy (wd);
144 wd = NULL;
145 }
146@@ -1678,7 +1716,6 @@
147 else if (!window->hasUnmapReference ())
148 window->setWindowFrameExtents (&decoration->border,
149 &decoration->input);
150-
151 /* This window actually needs its decoration contents updated
152 * as it was actually visible */
153 if (decorate ||
154@@ -1695,7 +1732,11 @@
155 }
156
157 movement = cwe::shift (window->border (), window->sizeHints ().win_gravity);
158- movement -= oldShift;
159+
160+ sizeDelta = CompSize (-(window->border ().left +
161+ window->border ().right),
162+ -(window->border ().top +
163+ window->border ().bottom));
164
165 window->updateWindowOutputExtents ();
166
167@@ -1724,8 +1765,6 @@
168 memset (&emptyExtents, 0, sizeof (CompWindowExtents));
169
170 window->setWindowFrameExtents (&emptyExtents, &emptyExtents);
171-
172- movement -= oldShift;
173 }
174
175 /* We need to damage the current output extents
176@@ -1739,6 +1778,7 @@
177 }
178
179 moveDecoratedWindowBy (movement,
180+ sizeDelta,
181 !allowDecoration);
182
183 return true;
184@@ -2280,6 +2320,20 @@
185 }
186
187 /*
188+ * DecorWindow::place
189+ *
190+ * Update any windows just before placement
191+ * so that placement algorithms will have the
192+ * border size at place-time
193+ */
194+bool
195+DecorWindow::place (CompPoint &pos)
196+{
197+ update (true);
198+ return window->place (pos);
199+}
200+
201+/*
202 * DecorWindow::windowNotify
203 *
204 * Window event notification handler. On various
205@@ -2957,9 +3011,6 @@
206 {
207 if (wd && wd->decor)
208 {
209- CompPoint oldShift = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
210-
211-
212 if ((window->state () & MAXIMIZE_STATE))
213 window->setWindowFrameExtents (&wd->decor->maxBorder,
214 &wd->decor->maxInput);
215@@ -2967,18 +3018,7 @@
216 window->setWindowFrameExtents (&wd->decor->border,
217 &wd->decor->input);
218
219- /* Since we immediately update the frame extents, we must
220- * also update the stored saved window geometry in order
221- * to prevent the window from shifting back too far once
222- * unmaximized */
223-
224- CompPoint movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity) - oldShift;
225-
226- if (window->saveMask () & CWX)
227- window->saveWc ().x += movement.x ();
228-
229- if (window->saveMask () & CWY)
230- window->saveWc ().y += movement.y ();
231+ /* The shift will occurr in decorOffsetMove */
232
233 updateFrame ();
234 }
235
236=== modified file 'plugins/decor/src/decor.h'
237--- plugins/decor/src/decor.h 2013-03-22 15:52:13 +0000
238+++ plugins/decor/src/decor.h 2013-05-20 16:13:29 +0000
239@@ -293,6 +293,8 @@
240
241 bool damageRect (bool, const CompRect &);
242
243+ bool place (CompPoint &pos);
244+
245 bool glDraw (const GLMatrix &, const GLWindowPaintAttrib &,
246 const CompRegion &, unsigned int);
247 void glDecorate (const GLMatrix &, const GLWindowPaintAttrib &,
248@@ -380,12 +382,16 @@
249
250 X11DecorPixmapRequestor mRequestor;
251
252+ CompPoint lastShift;
253+ CompSize lastSizeDelta;
254+
255 private:
256
257 bool bareDecorationOnly ();
258 Decoration::Ptr findRealDecoration ();
259 Decoration::Ptr findBareDecoration ();
260 void moveDecoratedWindowBy (const CompPoint &movement,
261+ const CompSize &sizeDelta,
262 bool instant);
263 };
264
265
266=== modified file 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h'
267--- plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h 2013-03-30 04:11:22 +0000
268+++ plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h 2013-05-20 16:13:29 +0000
269@@ -45,8 +45,7 @@
270 CompPoint & constrainPositionToWorkArea (CompPoint &pos,
271 const compiz::window::Geometry &serverGeometry,
272 const CompWindowExtents &border,
273- const CompRect &workArea,
274- bool staticGravity);
275+ const CompRect &workArea);
276
277
278 CompPoint getViewportRelativeCoordinates (const compiz::window::Geometry &geom,
279@@ -54,8 +53,7 @@
280
281 CompWindowExtents getWindowEdgePositions (const CompPoint &position,
282 const compiz::window::Geometry &geom,
283- const CompWindowExtents &border,
284- unsigned int gravity);
285+ const CompWindowExtents &border);
286
287 void clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions,
288 const CompRect &workArea);
289@@ -64,8 +62,7 @@
290
291 void subtractBordersFromEdgePositions (CompWindowExtents &edgePositions,
292 const CompWindowExtents &border,
293- unsigned int legacyBorder,
294- unsigned int gravity);
295+ unsigned int legacyBorder);
296
297 bool onlySizeChanged (unsigned int mask);
298 bool applyWidthChange (const CompWindowExtents &edgePositions,
299
300=== modified file 'plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp'
301--- plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp 2013-03-30 04:11:22 +0000
302+++ plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp 2013-05-20 16:13:29 +0000
303@@ -140,35 +140,21 @@
304
305 CompPoint &
306 cp::constrainPositionToWorkArea (CompPoint &pos,
307- const cw::Geometry &serverGeometry,
308- const CompWindowExtents &border,
309- const CompRect &workArea,
310- bool staticGravity)
311+ const cw::Geometry &serverGeometry,
312+ const CompWindowExtents &border,
313+ const CompRect &workArea)
314 {
315 CompWindowExtents extents;
316 int delta;
317
318- CompWindowExtents effectiveBorders = border;
319-
320- /* Ignore borders in the StaticGravity case for placement
321- * because the window intended to be placed as if it didn't
322- * have them */
323- if (staticGravity)
324- {
325- effectiveBorders.left = 0;
326- effectiveBorders.right = 0;
327- effectiveBorders.top = 0;
328- effectiveBorders.bottom = 0;
329- }
330-
331- extents.left = pos.x () - effectiveBorders.left;
332- extents.top = pos.y () - effectiveBorders.top;
333+ extents.left = pos.x () - border.left;
334+ extents.top = pos.y () - border.top;
335 extents.right = extents.left + serverGeometry.widthIncBorders () +
336- (effectiveBorders.left +
337- effectiveBorders.right);
338+ (border.left +
339+ border.right);
340 extents.bottom = extents.top + serverGeometry.heightIncBorders () +
341- (effectiveBorders.top +
342- effectiveBorders.bottom);
343+ (border.top +
344+ border.bottom);
345
346 delta = workArea.right () - extents.right;
347 if (delta < 0)
348@@ -186,8 +172,8 @@
349 if (delta > 0)
350 extents.top += delta;
351
352- pos.setX (extents.left + effectiveBorders.left);
353- pos.setY (extents.top + effectiveBorders.top);
354+ pos.setX (extents.left + border.left);
355+ pos.setY (extents.top + border.top);
356
357 return pos;
358 }
359@@ -213,23 +199,18 @@
360
361 CompWindowExtents cp::getWindowEdgePositions (const CompPoint &position,
362 const cw::Geometry &geom,
363- const CompWindowExtents &border,
364- unsigned int gravity)
365+ const CompWindowExtents &border)
366 {
367 CompWindowExtents edgePositions;
368- CompWindowExtents effectiveBorder (border);
369-
370- if (gravity & StaticGravity)
371- effectiveBorder = CompWindowExtents (0, 0, 0, 0);
372-
373- edgePositions.left = position.x () - effectiveBorder.left;
374+
375+ edgePositions.left = position.x () - border.left;
376 edgePositions.right = edgePositions.left +
377- geom.widthIncBorders () + (effectiveBorder.left +
378- effectiveBorder.right);
379- edgePositions.top = position.y () - effectiveBorder.top;
380+ geom.widthIncBorders () + (border.left +
381+ border.right);
382+ edgePositions.top = position.y () - border.top;
383 edgePositions.bottom = edgePositions.top +
384- geom.heightIncBorders () + (effectiveBorder.top +
385- effectiveBorder.bottom);
386+ geom.heightIncBorders () + (border.top +
387+ border.bottom);
388
389 return edgePositions;
390 }
391@@ -285,19 +266,14 @@
392
393 void cp::subtractBordersFromEdgePositions (CompWindowExtents &edgePositions,
394 const CompWindowExtents &border,
395- unsigned int legacyBorder,
396- unsigned int gravity)
397+ unsigned int legacyBorder)
398 {
399 const unsigned int doubleBorder = 2 * legacyBorder;
400- CompWindowExtents effectiveBorder = border;
401-
402- if (gravity & StaticGravity)
403- effectiveBorder = CompWindowExtents (0, 0, 0, 0);
404-
405- edgePositions.left += effectiveBorder.left;
406- edgePositions.right -= effectiveBorder.right + doubleBorder;
407- edgePositions.top += effectiveBorder.top;
408- edgePositions.bottom -= effectiveBorder.bottom + doubleBorder;
409+
410+ edgePositions.left += border.left;
411+ edgePositions.right -= border.right + doubleBorder;
412+ edgePositions.top += border.top;
413+ edgePositions.bottom -= border.bottom + doubleBorder;
414 }
415
416 bool cp::onlySizeChanged (unsigned int mask)
417
418=== modified file 'plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp'
419--- plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp 2013-03-30 04:11:22 +0000
420+++ plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp 2013-05-20 16:13:29 +0000
421@@ -226,7 +226,7 @@
422 extents = WindowExtents (GetParam ());
423
424 CompPoint pos = InitialPosition (GetParam ());
425- pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, false);
426+ pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea);
427
428 const CompPoint expectedAfterExtentsAdjustment = ExpectedPosition +
429 CompPoint (extents.left,
430@@ -235,19 +235,6 @@
431 EXPECT_EQ (expectedAfterExtentsAdjustment, pos);
432 }
433
434-TEST_P (PlaceConstrainPositionToWorkArea, PositionConstrainedStaticGravity)
435-{
436- g = WindowGeometry (GetParam ());
437- extents = WindowExtents (GetParam ());
438-
439- CompPoint pos = InitialPosition (GetParam ());
440- pos = cp::constrainPositionToWorkArea (pos, g, extents, WArea, true);
441-
442- /* Do not adjust residual position for extents with windows
443- * that have a static gravity */
444- EXPECT_EQ (ExpectedPosition, pos);
445-}
446-
447 namespace
448 {
449 cwe::Extents PossibleExtents[] =
450@@ -297,7 +284,7 @@
451 CompPoint pos;
452 };
453
454-TEST_F (PlaceGetEdgePositions, GetEdgePositionsNWGravity)
455+TEST_F (PlaceGetEdgePositions, GetEdgePositions)
456 {
457 int left = geom.x () - border.left;
458 int right = left + (geom.widthIncBorders ()) +
459@@ -309,25 +296,7 @@
460 const cwe::Extents ExpectedExtents (left, right, top, bottom);
461 cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,
462 geom,
463- border,
464- NorthWestGravity));
465-
466- EXPECT_EQ (ExpectedExtents, actualExtents);
467-}
468-
469-TEST_F (PlaceGetEdgePositions, GetEdgePositionsStaticGravity)
470-{
471- /* Don't count borders in validation */
472- int left = geom.x ();
473- int right = left + (geom.widthIncBorders ());
474- int top = geom.y ();
475- int bottom = top + (geom.heightIncBorders ());
476-
477- const cwe::Extents ExpectedExtents (left, right, top, bottom);
478- cwe::Extents actualExtents (cp::getWindowEdgePositions (pos,
479- geom,
480- border,
481- StaticGravity));
482+ border));
483
484 EXPECT_EQ (ExpectedExtents, actualExtents);
485 }
486@@ -527,29 +496,7 @@
487
488 cp::subtractBordersFromEdgePositions (modifiedEdgePositions,
489 borders,
490- legacyBorder,
491- NorthWestGravity);
492-
493- EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
494-}
495-
496-TEST (PlaceSubtractBordersFromEdgePositions, StaticGravityDefinition)
497-{
498- const CompWindowExtents borders (1, 2, 3, 4);
499- const CompWindowExtents edgePositions (100, 200, 100, 200);
500- const unsigned int legacyBorder = 1;
501-
502- CompWindowExtents expectedEdgePositions (edgePositions.left,
503- edgePositions.right - (2 * legacyBorder),
504- edgePositions.top,
505- edgePositions.bottom - (2 * legacyBorder));
506-
507- CompWindowExtents modifiedEdgePositions (edgePositions);
508-
509- cp::subtractBordersFromEdgePositions (modifiedEdgePositions,
510- borders,
511- legacyBorder,
512- StaticGravity);
513+ legacyBorder);
514
515 EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions);
516 }
517
518=== modified file 'plugins/place/src/place.cpp'
519--- plugins/place/src/place.cpp 2013-05-09 13:43:07 +0000
520+++ plugins/place/src/place.cpp 2013-05-20 16:13:29 +0000
521@@ -362,8 +362,7 @@
522
523 CompWindowExtents edgePositions = cp::getWindowEdgePositions (pos,
524 geom,
525- window->border (),
526- window->sizeHints ().win_gravity);
527+ window->border ());
528
529 int output = screen->outputDeviceForGeometry (geom);
530 CompRect workArea = screen->getWorkareaForOutput (output);
531@@ -386,8 +385,7 @@
532 /* bring left/right/top/bottom to actual window coordinates */
533 cp::subtractBordersFromEdgePositions (edgePositions,
534 window->border (),
535- geom.border (),
536- window->sizeHints ().win_gravity);
537+ geom.border ());
538
539 /* always validate position if the application changed only its size,
540 * as it might become partially offscreen because of that */
541@@ -1143,13 +1141,10 @@
542 PlaceWindow::constrainToWorkarea (const CompRect &workArea,
543 CompPoint &pos)
544 {
545- bool staticGravity = window->sizeHints ().win_gravity & StaticGravity;
546-
547 pos = cp::constrainPositionToWorkArea (pos,
548 window->serverGeometry (),
549 window->border (),
550- workArea,
551- staticGravity);
552+ workArea);
553
554 }
555
556
557=== modified file 'src/window/extents/src/windowextents.cpp'
558--- src/window/extents/src/windowextents.cpp 2013-03-30 04:11:22 +0000
559+++ src/window/extents/src/windowextents.cpp 2013-05-20 16:13:29 +0000
560@@ -35,6 +35,11 @@
561 CompPoint rv = CompPoint ();
562
563 switch (gravity) {
564+ /* We treat StaticGravity like NorthWestGravity here
565+ * as when decorating / undecorating the window we
566+ * really do need to move it in order to handle
567+ * any cases where it goes offscreen */
568+ case StaticGravity:
569 case NorthGravity:
570 case NorthWestGravity:
571 case NorthEastGravity:
572@@ -50,6 +55,11 @@
573 }
574
575 switch (gravity) {
576+ /* We treat StaticGravity like NorthWestGravity here
577+ * as when decorating / undecorating the window we
578+ * really do need to move it in order to handle
579+ * any cases where it goes offscreen */
580+ case StaticGravity:
581 case WestGravity:
582 case NorthWestGravity:
583 case SouthWestGravity:
584
585=== added file 'tests/manual/README.txt'
586--- tests/manual/README.txt 1970-01-01 00:00:00 +0000
587+++ tests/manual/README.txt 2013-05-20 16:13:29 +0000
588@@ -0,0 +1,9 @@
589+Compiz Manual Tests
590+===================
591+
592+Avoid writing manual tests if you can. Acceptance tests that can be run
593+on an automatic basis are always preferred.
594+
595+If getting some part of the code would be too difficult or invasive, then
596+write a manual test in here so that we can remind ourselves to deploy test
597+frameworks for the code in quesiton.
598
599=== added file 'tests/manual/plugins/decor.txt'
600--- tests/manual/plugins/decor.txt 1970-01-01 00:00:00 +0000
601+++ tests/manual/plugins/decor.txt 2013-05-20 16:13:29 +0000
602@@ -0,0 +1,40 @@
603+COMPIZ DECOR PLUGIN
604+===================
605+Sam Spilsbury <smspillaz@gmail.com>
606+
607+Static Gravity Handling - no decorations
608+----------------------------------------
609+Setup:
610+# Install guake
611+
612+Actions:
613+# Start and launch guake
614+
615+Expected Result:
616+ Guake should sit flush with the panels and work area
617+
618+Static Gravity Handling - decorations
619+-------------------------------------
620+Setup:
621+# Install friends-app
622+
623+Actions:
624+# Start and launch friends-app
625+
626+Expected Result:
627+ The QML window should have its decorations visible and
628+ be contained in the top left hand corner of the work area
629+
630+_NET_REQUEST_FRAME_EXTENTS handling
631+-----------------------------------
632+Setup:
633+# Install any sun-awt application - examples:
634+ 1. netbeans
635+ 2. ecplise
636+
637+Actions:
638+# Run the application
639+
640+Expected Result:
641+ The application should not have its contents offset
642+ by its decoration size

Subscribers

People subscribed via source and target branches

to all changes: