Merge lp:~compiz-team/compiz/compiz.fix_1159324.1 into lp:compiz/0.9.9
- compiz.fix_1159324.1
- Merge into 0.9.9
Status: | Superseded |
---|---|
Proposed branch: | lp:~compiz-team/compiz/compiz.fix_1159324.1 |
Merge into: | lp:compiz/0.9.9 |
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 |
---|---|---|---|
Daniel van Vugt | Needs Resubmitting | ||
MC Return | Approve | ||
Review via email: mp+156245@code.launchpad.net |
This proposal has been superseded by 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 : | # |
MC Return (mc-return) wrote : | # |
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 : | # |
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 : | # |
>
> 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 : | # |
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 : | # |
Please re-target this for lp:compiz/0.9.10
Unmerged revisions
- 3638. By Sam Spilsbury
-
Also take into account the gravity requirements when validating reposition
requests. Get most of that code under test too.(LP: #1159324)
- 3637. By Sam Spilsbury
-
Merge lp:compiz
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-03-30 06:24: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-03-30 06:24: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-03-30 06:24: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-03-24 05:51:41 +0000 |
542 | +++ plugins/place/src/place.cpp 2013-03-30 06:24:22 +0000 |
543 | @@ -348,46 +348,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 | @@ -401,82 +378,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-03-30 06:24: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-03-30 06:24: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