Merge lp:~compiz-team/compiz/compiz.fix_1165343 into lp:compiz/0.9.10
- compiz.fix_1165343
- Merge into 0.9.10
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 |
Related bugs: |
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)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
The commit message looks a bit strange...
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.
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:/
> 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) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:3661
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
The commit message here still looks a bit strange ;)
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
LGTM now. +1
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
I am still hitting bug #1159324 in current trunk... :(
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:/
> 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) wrote : Posted in a previous version of this proposal | # |
> With our without this branch?
Without it.
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:/
> 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) 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...
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:/
> 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) wrote : Posted in a previous version of this proposal | # |
> I'll have a quick look into it now I guess.
>
Thx. +1
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.
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 ?
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?
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:3663
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:3663
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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/
MC Return (mc-return) wrote : | # |
Note: I've linked your branch to the Guake bug report.
Unmerged revisions
Preview Diff
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 |
PASSED: Continuous integration, rev:3660 jenkins. qa.ubuntu. com/job/ compiz- ci/137/ jenkins. qa.ubuntu. com/job/ compiz- gles-ci/ ./build= pbuilder, distribution= raring, flavor= amd64/175 jenkins. qa.ubuntu. com/job/ compiz- pbuilder/ ./build= pbuilder, distribution= raring, flavor= amd64/526
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins: 8080/job/ compiz- ci/137/ rebuild
http://