Merge lp:~compiz-team/compiz/compiz.fix_1159324.1 into lp:compiz/0.9.10
- compiz.fix_1159324.1
- Merge into 0.9.10
Status: | Merged |
---|---|
Approved by: | Sam Spilsbury |
Approved revision: | 3638 |
Merged at revision: | 3652 |
Proposed branch: | lp:~compiz-team/compiz/compiz.fix_1159324.1 |
Merge into: | lp:compiz/0.9.10 |
Diff against target: |
763 lines (+570/-102) 6 files modified
plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h (+28/-0) plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp (+151/-0) plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp (+333/-0) plugins/place/src/place.cpp (+38/-102) src/window/extents/include/core/windowextents.h (+4/-0) src/window/extents/src/windowextents.cpp (+16/-0) |
To merge this branch: | bzr merge lp:~compiz-team/compiz/compiz.fix_1159324.1 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Sam Spilsbury | Approve | ||
MC Return | Pending | ||
Daniel van Vugt | Pending | ||
Review via email: mp+157987@code.launchpad.net |
This proposal supersedes a proposal from 2013-04-02.
Commit message
Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.
(LP: #1159324)
Description of the change
Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.
(LP: #1159324)
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
In 'plugins/
8 +
9 +
Other than those 2 minor issues this seems perfect and very useful !
Are you sure it fixes bug #1159324 ? It seems that this is already fixed.
But +1 for the restructuring anyway.
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal | # |
On Sun, Mar 31, 2013 at 8:00 PM, MC Return <email address hidden> wrote:
> Review: Approve
>
> In 'plugins/
>
> 8 +
> 9 +
>
> Other than those 2 minor issues this seems perfect and very useful !
>
> Are you sure it fixes bug #1159324 ? It seems that this is already fixed.
There was a corner case where it could still happen. Seemed to be
mostly on multimonitor cases. The window would be placed normally
(ok), and then request a new position, and then be constrained as if
it had a border, but that was wrong, because we were never meant to do
that.
>
> But +1 for the restructuring anyway.
> --
> https:/
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~compiz-team/compiz/compiz.fix_1159324.1 into lp:compiz.
--
Sam Spilsbury
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
>
> There was a corner case where it could still happen. Seemed to be
> mostly on multimonitor cases. The window would be placed normally
> (ok), and then request a new position, and then be constrained as if
> it had a border, but that was wrong, because we were never meant to do
> that.
>
Hmm, that reminds me of a bug I still experience (just with Emerald):
Open any media player-> maximize it -> make it go fullscreen -> restore it
Result:
The window still has no decoration (because it is Unity-maximized),
which is correct, but the window is placed, like it would be decorated,
shifted to the right and down, but with correct window size (making lower
and right part of the window go offscreen...
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
Another minor typo in a comment:
+/* Just here to keep ABI compatability */
should be
+/* Just here to keep ABI compatibility */
2x
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal | # |
Please re-target this for lp:compiz/0.9.10
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:3638
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 | # |
This is btw still broken in Raring.
It is not about me, as I got this fix running here locally, but if this won't land into 0.9.10, it probably will never land in Raring either...
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
MC Return (mc-return) wrote : Posted in a previous version of this proposal | # |
/tmp/buildd/
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:/
The bug is not reproducible, so it is likely a hardware or OS problem.
make[3]: *** [compizconfig/
make[3]: Leaving directory `/tmp/buildd/
make[2]: *** [compizconfig/
make[2]: *** Waiting for unfinished jobs....
Seems like a Jenkins problem...
Reapproving (probably without effect)
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
Sam Spilsbury (smspillaz) : | # |
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:3638
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Sam Spilsbury (smspillaz) wrote : | # |
So I think the arm builder is running out of memory? This is a problem ...
PS Jenkins bot (ps-jenkins) : | # |
Preview Diff
1 | === modified file 'plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h' |
2 | --- plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h 2013-03-24 05:51:41 +0000 |
3 | +++ plugins/place/src/constrain-to-workarea/include/constrain-to-workarea.h 2013-04-10 04:16:22 +0000 |
4 | @@ -47,6 +47,34 @@ |
5 | const CompWindowExtents &border, |
6 | const CompRect &workArea, |
7 | bool staticGravity); |
8 | + |
9 | + |
10 | +CompPoint getViewportRelativeCoordinates (const compiz::window::Geometry &geom, |
11 | + const CompSize &screen); |
12 | + |
13 | +CompWindowExtents getWindowEdgePositions (const CompPoint &position, |
14 | + const compiz::window::Geometry &geom, |
15 | + const CompWindowExtents &border, |
16 | + unsigned int gravity); |
17 | + |
18 | +void clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions, |
19 | + const CompRect &workArea); |
20 | +void clampVerticalEdgePositionsToWorkArea (CompWindowExtents &edgePositions, |
21 | + const CompRect &workArea); |
22 | + |
23 | +void subtractBordersFromEdgePositions (CompWindowExtents &edgePositions, |
24 | + const CompWindowExtents &border, |
25 | + unsigned int legacyBorder, |
26 | + unsigned int gravity); |
27 | + |
28 | +bool onlySizeChanged (unsigned int mask); |
29 | +bool applyWidthChange (const CompWindowExtents &edgePositions, |
30 | + XWindowChanges &xwc, |
31 | + unsigned int &mask); |
32 | + |
33 | +bool applyHeightChange (const CompWindowExtents &edgePositions, |
34 | + XWindowChanges &xwc, |
35 | + unsigned int &mask); |
36 | } |
37 | } |
38 | |
39 | |
40 | === modified file 'plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp' |
41 | --- plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp 2013-03-24 05:51:41 +0000 |
42 | +++ plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp 2013-04-10 04:16:22 +0000 |
43 | @@ -191,3 +191,154 @@ |
44 | |
45 | return pos; |
46 | } |
47 | + |
48 | +CompPoint cp::getViewportRelativeCoordinates (const cw::Geometry &geom, |
49 | + const CompSize &screen) |
50 | +{ |
51 | + |
52 | + /* left, right, top, bottom target coordinates, clamed to viewport |
53 | + * sizes as we don't need to validate movements to other viewports; |
54 | + * we are only interested in inner-viewport movements */ |
55 | + |
56 | + int x = geom.x () % screen.width (); |
57 | + if ((geom.x2 ()) < 0) |
58 | + x += screen.width (); |
59 | + |
60 | + int y = geom.y () % screen.height (); |
61 | + if ((geom.y2 ()) < 0) |
62 | + y += screen.height (); |
63 | + |
64 | + return CompPoint (x, y); |
65 | +} |
66 | + |
67 | +CompWindowExtents cp::getWindowEdgePositions (const CompPoint &position, |
68 | + const cw::Geometry &geom, |
69 | + const CompWindowExtents &border, |
70 | + unsigned int gravity) |
71 | +{ |
72 | + CompWindowExtents edgePositions; |
73 | + CompWindowExtents effectiveBorder (border); |
74 | + |
75 | + if (gravity & StaticGravity) |
76 | + effectiveBorder = CompWindowExtents (0, 0, 0, 0); |
77 | + |
78 | + edgePositions.left = position.x () - effectiveBorder.left; |
79 | + edgePositions.right = edgePositions.left + |
80 | + geom.widthIncBorders () + (effectiveBorder.left + |
81 | + effectiveBorder.right); |
82 | + edgePositions.top = position.y () - effectiveBorder.top; |
83 | + edgePositions.bottom = edgePositions.top + |
84 | + geom.heightIncBorders () + (effectiveBorder.top + |
85 | + effectiveBorder.bottom); |
86 | + |
87 | + return edgePositions; |
88 | +} |
89 | + |
90 | +void cp::clampHorizontalEdgePositionsToWorkArea (CompWindowExtents &edgePositions, |
91 | + const CompRect &workArea) |
92 | +{ |
93 | + if ((edgePositions.right - edgePositions.left) > workArea.width ()) |
94 | + { |
95 | + edgePositions.left = workArea.left (); |
96 | + edgePositions.right = workArea.right (); |
97 | + } |
98 | + else |
99 | + { |
100 | + if (edgePositions.left < workArea.left ()) |
101 | + { |
102 | + edgePositions.right += workArea.left () - edgePositions.left; |
103 | + edgePositions.left = workArea.left (); |
104 | + } |
105 | + |
106 | + if (edgePositions.right > workArea.right ()) |
107 | + { |
108 | + edgePositions.left -= edgePositions.right - workArea.right (); |
109 | + edgePositions.right = workArea.right (); |
110 | + } |
111 | + } |
112 | + |
113 | +} |
114 | + |
115 | +void cp::clampVerticalEdgePositionsToWorkArea (CompWindowExtents &edgePositions, |
116 | + const CompRect &workArea) |
117 | +{ |
118 | + if ((edgePositions.bottom - edgePositions.top) > workArea.height ()) |
119 | + { |
120 | + edgePositions.top = workArea.top (); |
121 | + edgePositions.bottom = workArea.bottom (); |
122 | + } |
123 | + else |
124 | + { |
125 | + if (edgePositions.top < workArea.top ()) |
126 | + { |
127 | + edgePositions.bottom += workArea.top () - edgePositions.top; |
128 | + edgePositions.top = workArea.top (); |
129 | + } |
130 | + |
131 | + if (edgePositions.bottom > workArea.bottom ()) |
132 | + { |
133 | + edgePositions.top -= edgePositions.bottom - workArea.bottom (); |
134 | + edgePositions.bottom = workArea.bottom (); |
135 | + } |
136 | + } |
137 | +} |
138 | + |
139 | +void cp::subtractBordersFromEdgePositions (CompWindowExtents &edgePositions, |
140 | + const CompWindowExtents &border, |
141 | + unsigned int legacyBorder, |
142 | + unsigned int gravity) |
143 | +{ |
144 | + const unsigned int doubleBorder = 2 * legacyBorder; |
145 | + CompWindowExtents effectiveBorder = border; |
146 | + |
147 | + if (gravity & StaticGravity) |
148 | + effectiveBorder = CompWindowExtents (0, 0, 0, 0); |
149 | + |
150 | + edgePositions.left += effectiveBorder.left; |
151 | + edgePositions.right -= effectiveBorder.right + doubleBorder; |
152 | + edgePositions.top += effectiveBorder.top; |
153 | + edgePositions.bottom -= effectiveBorder.bottom + doubleBorder; |
154 | +} |
155 | + |
156 | +bool cp::onlySizeChanged (unsigned int mask) |
157 | +{ |
158 | + return (!(mask & (CWX | CWY)) && (mask & (CWWidth | CWHeight))); |
159 | +} |
160 | + |
161 | +bool cp::applyWidthChange (const CompWindowExtents &edgePositions, |
162 | + XWindowChanges &xwc, |
163 | + unsigned int &mask) |
164 | +{ |
165 | + bool alreadySet = mask & CWWidth; |
166 | + |
167 | + if (alreadySet) |
168 | + alreadySet = edgePositions.right - edgePositions.left == xwc.width; |
169 | + |
170 | + if (!alreadySet) |
171 | + { |
172 | + xwc.width = edgePositions.right - edgePositions.left; |
173 | + mask |= CWWidth; |
174 | + return true; |
175 | + } |
176 | + |
177 | + return false; |
178 | +} |
179 | + |
180 | +bool cp::applyHeightChange (const CompWindowExtents &edgePositions, |
181 | + XWindowChanges &xwc, |
182 | + unsigned int &mask) |
183 | +{ |
184 | + bool alreadySet = mask & CWHeight; |
185 | + |
186 | + if (alreadySet) |
187 | + alreadySet = edgePositions.bottom - edgePositions.top == xwc.height; |
188 | + |
189 | + if (!alreadySet) |
190 | + { |
191 | + xwc.height = edgePositions.bottom - edgePositions.top; |
192 | + mask |= CWHeight; |
193 | + return true; |
194 | + } |
195 | + |
196 | + return false; |
197 | +} |
198 | |
199 | === modified file 'plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp' |
200 | --- plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp 2013-03-24 05:51:41 +0000 |
201 | +++ plugins/place/src/constrain-to-workarea/tests/constrain-to-workarea/src/test-place-constrain-to-workarea.cpp 2013-04-10 04:16:22 +0000 |
202 | @@ -278,3 +278,336 @@ |
203 | PlaceConstrainPositionToWorkArea, |
204 | Combine (ValuesIn (PossibleGeometries), |
205 | ValuesIn (PossibleExtents))); |
206 | +class PlaceGetEdgePositions : |
207 | + public CompPlaceTest |
208 | +{ |
209 | + public: |
210 | + |
211 | + PlaceGetEdgePositions () : |
212 | + geom (100, 200, 300, 400, 1), |
213 | + border (1, 2, 3, 4), |
214 | + pos (geom.pos ()) |
215 | + { |
216 | + } |
217 | + |
218 | + protected: |
219 | + |
220 | + cw::Geometry geom; |
221 | + cwe::Extents border; |
222 | + CompPoint pos; |
223 | +}; |
224 | + |
225 | +TEST_F (PlaceGetEdgePositions, GetEdgePositionsNWGravity) |
226 | +{ |
227 | + int left = geom.x () - border.left; |
228 | + int right = left + (geom.widthIncBorders ()) + |
229 | + (border.left + border.right); |
230 | + int top = geom.y () - border.top; |
231 | + int bottom = top + (geom.heightIncBorders ()) + |
232 | + (border.top + border.bottom); |
233 | + |
234 | + const cwe::Extents ExpectedExtents (left, right, top, bottom); |
235 | + cwe::Extents actualExtents (cp::getWindowEdgePositions (pos, |
236 | + geom, |
237 | + border, |
238 | + NorthWestGravity)); |
239 | + |
240 | + EXPECT_EQ (ExpectedExtents, actualExtents); |
241 | +} |
242 | + |
243 | +TEST_F (PlaceGetEdgePositions, GetEdgePositionsStaticGravity) |
244 | +{ |
245 | + /* Don't count borders in validation */ |
246 | + int left = geom.x (); |
247 | + int right = left + (geom.widthIncBorders ()); |
248 | + int top = geom.y (); |
249 | + int bottom = top + (geom.heightIncBorders ()); |
250 | + |
251 | + const cwe::Extents ExpectedExtents (left, right, top, bottom); |
252 | + cwe::Extents actualExtents (cp::getWindowEdgePositions (pos, |
253 | + geom, |
254 | + border, |
255 | + StaticGravity)); |
256 | + |
257 | + EXPECT_EQ (ExpectedExtents, actualExtents); |
258 | +} |
259 | + |
260 | +namespace |
261 | +{ |
262 | +const CompSize SCREEN_SIZE (1000, 1000); |
263 | +const unsigned int WINDOW_SIZE = 250; |
264 | +} |
265 | + |
266 | +TEST (PlaceGetViewportRelativeCoordinates, WithinScreenWidth) |
267 | +{ |
268 | + cw::Geometry geom (SCREEN_SIZE.width () / 2, |
269 | + SCREEN_SIZE.height () / 2, |
270 | + WINDOW_SIZE, WINDOW_SIZE, |
271 | + 0); |
272 | + |
273 | + CompPoint position (cp::getViewportRelativeCoordinates (geom, |
274 | + SCREEN_SIZE)); |
275 | + EXPECT_EQ (geom.pos ().x (), position.x ()); |
276 | +} |
277 | + |
278 | +TEST (PlaceGetViewportRelativeCoordinates, WithinScreenHeight) |
279 | +{ |
280 | + cw::Geometry geom (SCREEN_SIZE.width () / 2, |
281 | + SCREEN_SIZE.height () / 2, |
282 | + WINDOW_SIZE, WINDOW_SIZE, |
283 | + 0); |
284 | + |
285 | + CompPoint position (cp::getViewportRelativeCoordinates (geom, |
286 | + SCREEN_SIZE)); |
287 | + EXPECT_EQ (geom.pos ().y (), position.y ()); |
288 | +} |
289 | + |
290 | +TEST (PlaceGetViewportRelativeCoordinates, OutsideOuterScreenWidth) |
291 | +{ |
292 | + cw::Geometry geom (SCREEN_SIZE.width () + 1, |
293 | + SCREEN_SIZE.height () / 2, |
294 | + WINDOW_SIZE, WINDOW_SIZE, |
295 | + 0); |
296 | + |
297 | + CompPoint position (cp::getViewportRelativeCoordinates (geom, |
298 | + SCREEN_SIZE)); |
299 | + EXPECT_EQ (1, position.x ()); |
300 | +} |
301 | + |
302 | +TEST (PlaceGetViewportRelativeCoordinates, OutsideInnerScreenWidth) |
303 | +{ |
304 | + cw::Geometry geom (-(WINDOW_SIZE + 1), |
305 | + SCREEN_SIZE.height () / 2, |
306 | + WINDOW_SIZE, WINDOW_SIZE, |
307 | + 0); |
308 | + |
309 | + CompPoint position (cp::getViewportRelativeCoordinates (geom, |
310 | + SCREEN_SIZE)); |
311 | + EXPECT_EQ (SCREEN_SIZE.width () - (WINDOW_SIZE + 1), |
312 | + position.x ()); |
313 | +} |
314 | + |
315 | +TEST (PlaceGetViewportRelativeCoordinates, OutsideOuterScreenHeight) |
316 | +{ |
317 | + cw::Geometry geom (SCREEN_SIZE.width () / 2, |
318 | + SCREEN_SIZE.height () + 1, |
319 | + WINDOW_SIZE, WINDOW_SIZE, |
320 | + 0); |
321 | + |
322 | + CompPoint position (cp::getViewportRelativeCoordinates (geom, |
323 | + SCREEN_SIZE)); |
324 | + EXPECT_EQ (1, position.y ()); |
325 | +} |
326 | + |
327 | +TEST (PlaceGetViewportRelativeCoordinates, OutsideInnerScreenHeight) |
328 | +{ |
329 | + cw::Geometry geom (SCREEN_SIZE.width () / 2, |
330 | + -(WINDOW_SIZE + 1), |
331 | + WINDOW_SIZE, WINDOW_SIZE, |
332 | + 0); |
333 | + |
334 | + CompPoint position (cp::getViewportRelativeCoordinates (geom, |
335 | + SCREEN_SIZE)); |
336 | + EXPECT_EQ (SCREEN_SIZE.height () - (WINDOW_SIZE + 1), |
337 | + position.y ()); |
338 | +} |
339 | + |
340 | +namespace |
341 | +{ |
342 | +const CompRect WORK_AREA (25, 25, 1000, 1000); |
343 | + |
344 | +const cwe::Extents EdgePositions[] = |
345 | +{ |
346 | + /* Exact match */ |
347 | + cwe::Extents (WORK_AREA.left (), WORK_AREA.right (), |
348 | + WORK_AREA.top (), WORK_AREA.bottom ()), |
349 | + /* Just within */ |
350 | + cwe::Extents (WORK_AREA.left () + 1, WORK_AREA.right () - 1, |
351 | + WORK_AREA.top () + 1, WORK_AREA.bottom () - 1), |
352 | + /* Just outside right */ |
353 | + cwe::Extents (WORK_AREA.left () + 1, WORK_AREA.right () + 1, |
354 | + WORK_AREA.top (), WORK_AREA.bottom ()), |
355 | + /* Just outside left */ |
356 | + cwe::Extents (WORK_AREA.left () - 1, WORK_AREA.right () - 1, |
357 | + WORK_AREA.top (), WORK_AREA.bottom ()), |
358 | + /* Just outside top */ |
359 | + cwe::Extents (WORK_AREA.left (), WORK_AREA.right (), |
360 | + WORK_AREA.top () - 1, WORK_AREA.bottom () - 1), |
361 | + /* Just outside bottom */ |
362 | + cwe::Extents (WORK_AREA.left (), WORK_AREA.right (), |
363 | + WORK_AREA.top () + 1, WORK_AREA.bottom () + 1), |
364 | +}; |
365 | +} |
366 | + |
367 | +class PlaceClampEdgePositions : |
368 | + public CompPlaceTest, |
369 | + public WithParamInterface <CompWindowExtents> |
370 | +{ |
371 | +}; |
372 | + |
373 | +TEST_P (PlaceClampEdgePositions, WithinWorkAreaWidth) |
374 | +{ |
375 | + CompWindowExtents edgePositions (GetParam ()); |
376 | + cp::clampHorizontalEdgePositionsToWorkArea (edgePositions, |
377 | + WORK_AREA); |
378 | + |
379 | + const int edgePositionsWidth = edgePositions.right - edgePositions.left; |
380 | + |
381 | + ASSERT_GT (edgePositionsWidth, 0); |
382 | + EXPECT_LE (edgePositionsWidth, WORK_AREA.width ()); |
383 | +} |
384 | + |
385 | +TEST_P (PlaceClampEdgePositions, OutsideLeft) |
386 | +{ |
387 | + CompWindowExtents edgePositions (GetParam ()); |
388 | + cp::clampHorizontalEdgePositionsToWorkArea (edgePositions, |
389 | + WORK_AREA); |
390 | + |
391 | + ASSERT_GE (edgePositions.left, WORK_AREA.left ()); |
392 | + ASSERT_GE (edgePositions.right, WORK_AREA.left ()); |
393 | +} |
394 | + |
395 | +TEST_P (PlaceClampEdgePositions, OutsideRight) |
396 | +{ |
397 | + CompWindowExtents edgePositions (GetParam ()); |
398 | + cp::clampHorizontalEdgePositionsToWorkArea (edgePositions, |
399 | + WORK_AREA); |
400 | + |
401 | + ASSERT_LE (edgePositions.left, WORK_AREA.right ()); |
402 | + ASSERT_LE (edgePositions.right, WORK_AREA.right ()); |
403 | +} |
404 | + |
405 | +TEST_P (PlaceClampEdgePositions, WithinWorkAreaHeight) |
406 | +{ |
407 | + CompWindowExtents edgePositions (GetParam ()); |
408 | + cp::clampVerticalEdgePositionsToWorkArea (edgePositions, |
409 | + WORK_AREA); |
410 | + |
411 | + const int edgePositionsHeight = edgePositions.bottom - edgePositions.top; |
412 | + |
413 | + ASSERT_GT (edgePositionsHeight, 0); |
414 | + EXPECT_LE (edgePositionsHeight, WORK_AREA.height ()); |
415 | +} |
416 | + |
417 | +TEST_P (PlaceClampEdgePositions, OutsideTop) |
418 | +{ |
419 | + CompWindowExtents edgePositions (GetParam ()); |
420 | + cp::clampVerticalEdgePositionsToWorkArea (edgePositions, |
421 | + WORK_AREA); |
422 | + |
423 | + ASSERT_GE (edgePositions.top, WORK_AREA.top ()); |
424 | + ASSERT_GE (edgePositions.bottom, WORK_AREA.top ()); |
425 | +} |
426 | + |
427 | +TEST_P (PlaceClampEdgePositions, OutsideBottom) |
428 | +{ |
429 | + CompWindowExtents edgePositions (GetParam ()); |
430 | + cp::clampVerticalEdgePositionsToWorkArea (edgePositions, |
431 | + WORK_AREA); |
432 | + |
433 | + ASSERT_LE (edgePositions.top, WORK_AREA.bottom ()); |
434 | + ASSERT_LE (edgePositions.bottom, WORK_AREA.bottom ()); |
435 | +} |
436 | + |
437 | +INSTANTIATE_TEST_CASE_P (WAEdgePositions, PlaceClampEdgePositions, |
438 | + ValuesIn (EdgePositions)); |
439 | + |
440 | +TEST (PlaceSubtractBordersFromEdgePositions, NormalGravity) |
441 | +{ |
442 | + const CompWindowExtents borders (1, 2, 3, 4); |
443 | + const CompWindowExtents edgePositions (100, 200, 100, 200); |
444 | + const unsigned int legacyBorder = 1; |
445 | + |
446 | + CompWindowExtents expectedEdgePositions (edgePositions.left + (borders.left), |
447 | + edgePositions.right - (borders.right + 2 * legacyBorder), |
448 | + edgePositions.top + (borders.top), |
449 | + edgePositions.bottom - (borders.bottom + 2 * legacyBorder)); |
450 | + |
451 | + CompWindowExtents modifiedEdgePositions (edgePositions); |
452 | + |
453 | + cp::subtractBordersFromEdgePositions (modifiedEdgePositions, |
454 | + borders, |
455 | + legacyBorder, |
456 | + NorthWestGravity); |
457 | + |
458 | + EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions); |
459 | +} |
460 | + |
461 | +TEST (PlaceSubtractBordersFromEdgePositions, StaticGravityDefinition) |
462 | +{ |
463 | + const CompWindowExtents borders (1, 2, 3, 4); |
464 | + const CompWindowExtents edgePositions (100, 200, 100, 200); |
465 | + const unsigned int legacyBorder = 1; |
466 | + |
467 | + CompWindowExtents expectedEdgePositions (edgePositions.left, |
468 | + edgePositions.right - (2 * legacyBorder), |
469 | + edgePositions.top, |
470 | + edgePositions.bottom - (2 * legacyBorder)); |
471 | + |
472 | + CompWindowExtents modifiedEdgePositions (edgePositions); |
473 | + |
474 | + cp::subtractBordersFromEdgePositions (modifiedEdgePositions, |
475 | + borders, |
476 | + legacyBorder, |
477 | + StaticGravity); |
478 | + |
479 | + EXPECT_EQ (expectedEdgePositions, modifiedEdgePositions); |
480 | +} |
481 | + |
482 | +TEST (PlaceSizeAndPositionChanged, PositionChangedReturnsFalse) |
483 | +{ |
484 | + EXPECT_FALSE (cp::onlySizeChanged (CWX | CWY)); |
485 | +} |
486 | + |
487 | +TEST (PlaceSizeAndPositionChanged, SizeChangedReturnsTrue) |
488 | +{ |
489 | + EXPECT_TRUE (cp::onlySizeChanged (CWWidth | CWHeight)); |
490 | +} |
491 | + |
492 | +TEST (PlaceSizeAndPositionChanged, XAndWidthChangedReturnsFalse) |
493 | +{ |
494 | + EXPECT_FALSE (cp::onlySizeChanged (CWX | CWWidth)); |
495 | +} |
496 | + |
497 | +TEST (PlaceSizeAndPositionChanged, YAndHeightChangedReturnsFalse) |
498 | +{ |
499 | + EXPECT_FALSE (cp::onlySizeChanged (CWY | CWHeight)); |
500 | +} |
501 | + |
502 | +TEST (PlaceApplyWidthChange, ReturnFalseIfNoChange) |
503 | +{ |
504 | + CompWindowExtents edgePositions (100, 200, 100, 200); |
505 | + XWindowChanges xwc; |
506 | + xwc.width = 100; |
507 | + unsigned int mask = CWWidth; |
508 | + EXPECT_FALSE (cp::applyWidthChange(edgePositions, xwc, mask)); |
509 | +} |
510 | + |
511 | +TEST (PlaceApplyWidthChange, ApplyWidthAndSetFlag) |
512 | +{ |
513 | + CompWindowExtents edgePositions (100, 200, 100, 200); |
514 | + XWindowChanges xwc; |
515 | + unsigned int mask = 0; |
516 | + ASSERT_TRUE (cp::applyWidthChange(edgePositions, xwc, mask)); |
517 | + EXPECT_EQ (edgePositions.right - edgePositions.left, xwc.width); |
518 | + EXPECT_EQ (CWWidth, mask); |
519 | +} |
520 | + |
521 | +TEST (PlaceApplyHeightChange, ReturnFalseIfNoChange) |
522 | +{ |
523 | + CompWindowExtents edgePositions (100, 200, 100, 200); |
524 | + XWindowChanges xwc; |
525 | + xwc.height = 100; |
526 | + unsigned int mask = CWHeight; |
527 | + EXPECT_FALSE (cp::applyHeightChange(edgePositions, xwc, mask)); |
528 | +} |
529 | + |
530 | +TEST (PlaceApplyWidthChange, ApplyHeightAndSetFlag) |
531 | +{ |
532 | + CompWindowExtents edgePositions (100, 200, 100, 200); |
533 | + XWindowChanges xwc; |
534 | + unsigned int mask = 0; |
535 | + ASSERT_TRUE (cp::applyHeightChange(edgePositions, xwc, mask)); |
536 | + EXPECT_EQ (edgePositions.bottom - edgePositions.top, xwc.height); |
537 | + EXPECT_EQ (CWHeight, mask); |
538 | +} |
539 | |
540 | === modified file 'plugins/place/src/place.cpp' |
541 | --- plugins/place/src/place.cpp 2013-04-07 08:46:23 +0000 |
542 | +++ plugins/place/src/place.cpp 2013-04-10 04:16:22 +0000 |
543 | @@ -350,46 +350,23 @@ |
544 | CompRect |
545 | PlaceWindow::doValidateResizeRequest (unsigned int &mask, |
546 | XWindowChanges *xwc, |
547 | - bool sizeOnly, |
548 | + bool onlyValidateSize, |
549 | bool clampToViewport) |
550 | { |
551 | - CompRect workArea; |
552 | - int x, y, left, right, bottom, top; |
553 | - CompWindow::Geometry geom; |
554 | - int output; |
555 | - |
556 | - geom.set (xwc->x, xwc->y, xwc->width, xwc->height, |
557 | - window->serverGeometry ().border ()); |
558 | + CompWindow::Geometry geom (xwc->x, xwc->y, xwc->width, xwc->height, |
559 | + window->serverGeometry ().border ()); |
560 | + CompPoint pos (geom.pos ()); |
561 | |
562 | if (clampToViewport) |
563 | - { |
564 | - /* left, right, top, bottom target coordinates, clamed to viewport |
565 | - * sizes as we don't need to validate movements to other viewports; |
566 | - * we are only interested in inner-viewport movements */ |
567 | - |
568 | - x = geom.x () % screen->width (); |
569 | - if ((geom.x2 ()) < 0) |
570 | - x += screen->width (); |
571 | - |
572 | - y = geom.y () % screen->height (); |
573 | - if ((geom.y2 ()) < 0) |
574 | - y += screen->height (); |
575 | - } |
576 | - else |
577 | - { |
578 | - x = geom.x (); |
579 | - y = geom.y (); |
580 | - } |
581 | - |
582 | - left = x - window->border ().left; |
583 | - right = left + geom.widthIncBorders () + (window->border ().left + |
584 | - window->border ().right); |
585 | - top = y - window->border ().top; |
586 | - bottom = top + geom.heightIncBorders () + (window->border ().top + |
587 | - window->border ().bottom); |
588 | - |
589 | - output = screen->outputDeviceForGeometry (geom); |
590 | - workArea = screen->getWorkareaForOutput (output); |
591 | + pos = cp::getViewportRelativeCoordinates(geom, *screen); |
592 | + |
593 | + CompWindowExtents edgePositions = cp::getWindowEdgePositions (pos, |
594 | + geom, |
595 | + window->border (), |
596 | + window->sizeHints ().win_gravity); |
597 | + |
598 | + int output = screen->outputDeviceForGeometry (geom); |
599 | + CompRect workArea = screen->getWorkareaForOutput (output); |
600 | |
601 | if (clampToViewport && |
602 | xwc->width >= workArea.width () && |
603 | @@ -403,82 +380,41 @@ |
604 | } |
605 | } |
606 | |
607 | - if ((right - left) > workArea.width ()) |
608 | - { |
609 | - left = workArea.left (); |
610 | - right = workArea.right (); |
611 | - } |
612 | - else |
613 | - { |
614 | - if (left < workArea.left ()) |
615 | - { |
616 | - right += workArea.left () - left; |
617 | - left = workArea.left (); |
618 | - } |
619 | - |
620 | - if (right > workArea.right ()) |
621 | - { |
622 | - left -= right - workArea.right (); |
623 | - right = workArea.right (); |
624 | - } |
625 | - } |
626 | - |
627 | - if ((bottom - top) > workArea.height ()) |
628 | - { |
629 | - top = workArea.top (); |
630 | - bottom = workArea.bottom (); |
631 | - } |
632 | - else |
633 | - { |
634 | - if (top < workArea.top ()) |
635 | - { |
636 | - bottom += workArea.top () - top; |
637 | - top = workArea.top (); |
638 | - } |
639 | - |
640 | - if (bottom > workArea.bottom ()) |
641 | - { |
642 | - top -= bottom - workArea.bottom (); |
643 | - bottom = workArea.bottom (); |
644 | - } |
645 | - } |
646 | + cp::clampHorizontalEdgePositionsToWorkArea (edgePositions, workArea); |
647 | + cp::clampVerticalEdgePositionsToWorkArea (edgePositions, workArea); |
648 | |
649 | /* bring left/right/top/bottom to actual window coordinates */ |
650 | - left += window->border ().left; |
651 | - right -= window->border ().right + 2 * window->serverGeometry ().border (); |
652 | - top += window->border ().top; |
653 | - bottom -= window->border ().bottom + 2 * window->serverGeometry ().border (); |
654 | + cp::subtractBordersFromEdgePositions (edgePositions, |
655 | + window->border (), |
656 | + geom.border (), |
657 | + window->sizeHints ().win_gravity); |
658 | |
659 | /* always validate position if the application changed only its size, |
660 | * as it might become partially offscreen because of that */ |
661 | - if (!(mask) & (CWX | CWY) && (mask & (CWWidth | CWHeight))) |
662 | - sizeOnly = false; |
663 | - |
664 | - if ((right - left) != xwc->width) |
665 | - { |
666 | - xwc->width = right - left; |
667 | - mask |= CWWidth; |
668 | - sizeOnly = false; |
669 | - } |
670 | - |
671 | - if ((bottom - top) != xwc->height) |
672 | - { |
673 | - xwc->height = bottom - top; |
674 | - mask |= CWHeight; |
675 | - sizeOnly = false; |
676 | - } |
677 | - |
678 | - if (!sizeOnly) |
679 | - { |
680 | - if (left != x) |
681 | + if (cp::onlySizeChanged (mask)) |
682 | + onlyValidateSize = false; |
683 | + |
684 | + if (cp::applyWidthChange(edgePositions, |
685 | + *xwc, |
686 | + mask)) |
687 | + onlyValidateSize = false; |
688 | + |
689 | + if (cp::applyHeightChange(edgePositions, |
690 | + *xwc, |
691 | + mask)) |
692 | + onlyValidateSize = false; |
693 | + |
694 | + if (!onlyValidateSize) |
695 | + { |
696 | + if (edgePositions.left != pos.x ()) |
697 | { |
698 | - xwc->x += left - x; |
699 | + xwc->x += edgePositions.left - pos.x (); |
700 | mask |= CWX; |
701 | } |
702 | |
703 | - if (top != y) |
704 | + if (edgePositions.top != pos.y ()) |
705 | { |
706 | - xwc->y += top - y; |
707 | + xwc->y += edgePositions.top - pos.y (); |
708 | mask |= CWY; |
709 | } |
710 | } |
711 | |
712 | === modified file 'src/window/extents/include/core/windowextents.h' |
713 | --- src/window/extents/include/core/windowextents.h 2012-02-25 05:14:18 +0000 |
714 | +++ src/window/extents/include/core/windowextents.h 2013-04-10 04:16:22 +0000 |
715 | @@ -53,6 +53,10 @@ |
716 | int top; |
717 | int bottom; |
718 | |
719 | + bool operator== (const Extents &other) const; |
720 | + bool operator!= (const Extents &other) const; |
721 | + |
722 | + /* Only here for ABI compatability */ |
723 | bool operator== (const Extents &other); |
724 | bool operator!= (const Extents &other); |
725 | }; |
726 | |
727 | === modified file 'src/window/extents/src/windowextents.cpp' |
728 | --- src/window/extents/src/windowextents.cpp 2012-08-09 09:06:54 +0000 |
729 | +++ src/window/extents/src/windowextents.cpp 2013-04-10 04:16:22 +0000 |
730 | @@ -75,17 +75,33 @@ |
731 | { |
732 | } |
733 | |
734 | +/* Just here to keep ABI compatability */ |
735 | bool |
736 | compiz::window::extents::Extents::operator== (const Extents &other) |
737 | { |
738 | + const Extents &self = const_cast <const Extents &> (*this); |
739 | + return self == other; |
740 | +} |
741 | + |
742 | +bool |
743 | +compiz::window::extents::Extents::operator== (const Extents &other) const |
744 | +{ |
745 | return this->left == other.left && |
746 | this->right == other.right && |
747 | this->top == other.top && |
748 | this->bottom == other.bottom; |
749 | } |
750 | |
751 | +/* Just here to keep ABI compatability */ |
752 | bool |
753 | compiz::window::extents::Extents::operator!= (const Extents &other) |
754 | { |
755 | + const Extents &self = const_cast <const Extents &> (*this); |
756 | + return self != other; |
757 | +} |
758 | + |
759 | +bool |
760 | +compiz::window::extents::Extents::operator!= (const Extents &other) const |
761 | +{ |
762 | return !(*this == other); |
763 | } |
A typo in the comment here:
52 + /* left, right, top, bottom target coordinates, clamed to viewport