Merge lp:~compiz-team/compiz/compiz.fix_1158161 into lp:compiz/0.9.9
- compiz.fix_1158161
- Merge into 0.9.9
Status: | Superseded |
---|---|
Proposed branch: | lp:~compiz-team/compiz/compiz.fix_1158161 |
Merge into: | lp:compiz/0.9.9 |
Diff against target: |
472 lines (+216/-150) 2 files modified
plugins/decor/src/decor.cpp (+201/-150) plugins/decor/src/decor.h (+15/-0) |
To merge this branch: | bzr merge lp:~compiz-team/compiz/compiz.fix_1158161 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Needs Fixing | |
Sam Spilsbury | Needs Fixing | ||
Łukasz Zemczak | Approve | ||
MC Return | Approve | ||
Review via email:
|
This proposal has been superseded by a proposal from 2013-03-22.
Commit message
Also decorate shadow-only windows. Strangely this fixes (LP: #1158161)
Description of the change
Also decorate shadow-only windows. Strangely this fixes (LP: #1158161).
I have no idea why it does. Scary.
(I checked the relevant java apps and they work fine).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
MC Return (mc-return) wrote : | # |
This is tested and works.
I can confirm it fixes bug #1158161.
It should be merged into lp:compiz/raring ASAP as well.
/offtopic on
Unfortunately it does not fix another important regression, namely bug #1141079, which is still valid.
It would be important to fix this regression as well, Sam could you take a look at that ?
/offtopic off
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
MC Return (mc-return) wrote : | # |
forget the offtopic above, I see you already noticed that one... +1
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Łukasz Zemczak (sil2100) wrote : | # |
All ok, let's get it merged! As for fix #1141079, I'll just finish the testing code and we can get that in as well. Just need some time, can't do 3 things at once! ;p
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Sam Spilsbury (smspillaz) wrote : | # |
@sil2100: just ping me when you've got the test for that.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Sam Spilsbury (smspillaz) wrote : | # |
Stopping the line over a small regression.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
MC Return (mc-return) wrote : | # |
Just FYI:
This also fixes application/
Looks much better with glow/shadows re-enabled :)
The visual difference with gtk-window-
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
Approved revid is not set in launchpad (maybe a permission problem?).
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
- 3635. By Sam Spilsbury
-
Cleanup - split up DecorWindow:
:update, reduce some of the fall-out condition
from allowing shadow-only windows to enter into the update function
Unmerged revisions
Preview Diff
1 | === modified file 'plugins/decor/src/decor.cpp' |
2 | --- plugins/decor/src/decor.cpp 2013-03-11 06:31:34 +0000 |
3 | +++ plugins/decor/src/decor.cpp 2013-03-22 15:54:41 +0000 |
4 | @@ -42,6 +42,8 @@ |
5 | |
6 | COMPIZ_PLUGIN_20090315 (decor, DecorPluginVTable) |
7 | |
8 | +namespace cwe = compiz::window::extents; |
9 | + |
10 | MatchedDecorClipGroup::MatchedDecorClipGroup (const CompMatch &match) : |
11 | mMatch (match) |
12 | { |
13 | @@ -1399,6 +1401,181 @@ |
14 | return *cit; |
15 | } |
16 | |
17 | +bool |
18 | +DecorWindow::bareDecorationOnly () |
19 | +{ |
20 | + bool shadowOnly = true; |
21 | + /* Only want to decorate windows which have a frame or are in the process |
22 | + * of waiting for an animation to be unmapped (in which case we can give |
23 | + * them a new pixmap type frame since we don't actually need an input |
24 | + * window to go along with that |
25 | + * |
26 | + * FIXME: That's not going to play nice with reparented decorations in core |
27 | + * since the window gets reparented right away before plugins are done |
28 | + * with it */ |
29 | + |
30 | + /* Unconditionally decorate switchers */ |
31 | + if (!isSwitcher) |
32 | + { |
33 | + switch (window->type ()) { |
34 | + case CompWindowTypeDialogMask: |
35 | + case CompWindowTypeModalDialogMask: |
36 | + case CompWindowTypeUtilMask: |
37 | + case CompWindowTypeMenuMask: |
38 | + case CompWindowTypeNormalMask: |
39 | + if (window->mwmDecor () & (MwmDecorAll | MwmDecorTitle)) |
40 | + shadowOnly = false; |
41 | + default: |
42 | + break; |
43 | + } |
44 | + |
45 | + if (window->overrideRedirect ()) |
46 | + shadowOnly = true; |
47 | + |
48 | + if (window->wmType () & (CompWindowTypeDockMask | CompWindowTypeDesktopMask)) |
49 | + shadowOnly = true; |
50 | + |
51 | + if (!shadowOnly) |
52 | + { |
53 | + if (!dScreen->optionGetDecorationMatch ().evaluate (window)) |
54 | + shadowOnly = true; |
55 | + } |
56 | + |
57 | + /* Never on unmapped windows */ |
58 | + if (!window->isViewable ()) |
59 | + shadowOnly = false; |
60 | + } |
61 | + else |
62 | + shadowOnly = false; |
63 | + |
64 | + return shadowOnly; |
65 | +} |
66 | + |
67 | +Decoration::Ptr |
68 | +DecorWindow::findRealDecoration () |
69 | +{ |
70 | + Decoration::Ptr decoration; |
71 | + |
72 | + /* Attempt to find a matching decoration */ |
73 | + try |
74 | + { |
75 | + decoration = decor.findMatchingDecoration (window, true); |
76 | + } |
77 | + catch (...) |
78 | + { |
79 | + /* Find an appropriate default decoration to use */ |
80 | + if (dScreen->dmSupports & WINDOW_DECORATION_TYPE_PIXMAP && |
81 | + dScreen->cmActive && |
82 | + !(dScreen->dmSupports & WINDOW_DECORATION_TYPE_WINDOW && |
83 | + pixmapFailed)) |
84 | + { |
85 | + try |
86 | + { |
87 | + decoration = dScreen->decor[DECOR_ACTIVE].findMatchingDecoration (window, false); |
88 | + } |
89 | + catch (...) |
90 | + { |
91 | + compLogMessage ("decor", CompLogLevelWarn, "No default decoration found, placement will not be correct"); |
92 | + decoration.reset (); |
93 | + } |
94 | + } |
95 | + else if (dScreen->dmSupports & WINDOW_DECORATION_TYPE_WINDOW) |
96 | + decoration = dScreen->windowDefault; |
97 | + } |
98 | + |
99 | + return decoration; |
100 | +} |
101 | + |
102 | +Decoration::Ptr |
103 | +DecorWindow::findBareDecoration () |
104 | +{ |
105 | + Decoration::Ptr decoration; |
106 | + /* This window isn't "decorated" but it still gets a shadow as long |
107 | + * as it isn't shaped weirdly, since the shadow is just a quad rect */ |
108 | + if (dScreen->optionGetShadowMatch ().evaluate (window)) |
109 | + { |
110 | + if (window->region ().numRects () == 1 && !window->alpha () && dScreen->decor[DECOR_BARE].mList.size ()) |
111 | + decoration = dScreen->decor[DECOR_BARE].mList.front (); |
112 | + |
113 | + if (decoration) |
114 | + { |
115 | + if (!checkSize (decoration)) |
116 | + decoration.reset (); |
117 | + } |
118 | + } |
119 | + |
120 | + return decoration; |
121 | +} |
122 | + |
123 | +void |
124 | +DecorWindow::moveDecoratedWindowBy (const CompPoint &movement, |
125 | + bool instant) |
126 | +{ |
127 | + /* Need to actually move the window */ |
128 | + if (window->placed () && !window->overrideRedirect () && |
129 | + (movement.x () || movement.y ())) |
130 | + { |
131 | + XWindowChanges xwc; |
132 | + unsigned int mask = CWX | CWY; |
133 | + |
134 | + memset (&xwc, 0, sizeof (XWindowChanges)); |
135 | + |
136 | + /* Grab the geometry last sent to server at configureXWindow |
137 | + * time and not here since serverGeometry may be updated by |
138 | + * the time that we do call configureXWindow */ |
139 | + xwc.x = movement.x (); |
140 | + xwc.y = movement.y (); |
141 | + |
142 | + /* Except if it's fullscreen, maximized or such */ |
143 | + if (window->state () & CompWindowStateFullscreenMask) |
144 | + mask &= ~(CWX | CWY); |
145 | + |
146 | + if (window->state () & CompWindowStateMaximizedHorzMask) |
147 | + mask &= ~CWX; |
148 | + |
149 | + if (window->state () & CompWindowStateMaximizedVertMask) |
150 | + mask &= ~CWY; |
151 | + |
152 | + if (window->saveMask () & CWX) |
153 | + window->saveWc ().x += movement.x (); |
154 | + |
155 | + if (window->saveMask () & CWY) |
156 | + window->saveWc ().y += movement.y (); |
157 | + |
158 | + if (mask) |
159 | + { |
160 | + /* instant is only true in the case of |
161 | + * the destructor calling the update function so since it |
162 | + * is not safe to put the function in a timer (since |
163 | + * it will get unref'd on the vtable destruction) we |
164 | + * need to do it immediately |
165 | + * |
166 | + * FIXME: CompTimer should really be PIMPL and allow |
167 | + * refcounting in case we need to keep it alive |
168 | + */ |
169 | + if (instant) |
170 | + decorOffsetMove (window, xwc, mask); |
171 | + else |
172 | + moveUpdate.start (boost::bind (decorOffsetMove, window, xwc, mask), 0); |
173 | + } |
174 | + } |
175 | +} |
176 | + |
177 | +namespace |
178 | +{ |
179 | +bool |
180 | +shouldDecorateWindow (CompWindow *w, |
181 | + bool shadowOnly, |
182 | + bool isSwitcher) |
183 | +{ |
184 | + const bool visible = (w->frame () || |
185 | + w->hasUnmapReference ()); |
186 | + const bool realDecoration = visible && !shadowOnly; |
187 | + const bool forceDecoration = isSwitcher; |
188 | + |
189 | + return realDecoration || forceDecoration; |
190 | +} |
191 | +} |
192 | /* |
193 | * DecorWindow::update |
194 | * This is the master function for managing decorations on windows |
195 | @@ -1439,8 +1616,6 @@ |
196 | DecorWindow::update (bool allowDecoration) |
197 | { |
198 | Decoration::Ptr old, decoration; |
199 | - bool decorate = false; |
200 | - bool shadowOnly = true; |
201 | CompPoint oldShift, movement; |
202 | |
203 | if (wd) |
204 | @@ -1448,99 +1623,19 @@ |
205 | else |
206 | old.reset (); |
207 | |
208 | - /* Only want to decorate windows which have a frame or are in the process |
209 | - * of waiting for an animation to be unmapped (in which case we can give |
210 | - * them a new pixmap type frame since we don't actually need an input |
211 | - * window to go along with that |
212 | - * |
213 | - * FIXME: That's not going to play nice with reparented decorations in core |
214 | - * since the window gets reparented right away before plugins are done |
215 | - * with it */ |
216 | - |
217 | - /* Unconditionally decorate switchers */ |
218 | - if (!isSwitcher) |
219 | - { |
220 | - switch (window->type ()) { |
221 | - case CompWindowTypeDialogMask: |
222 | - case CompWindowTypeModalDialogMask: |
223 | - case CompWindowTypeUtilMask: |
224 | - case CompWindowTypeMenuMask: |
225 | - case CompWindowTypeNormalMask: |
226 | - if (window->mwmDecor () & (MwmDecorAll | MwmDecorTitle)) |
227 | - shadowOnly = false; |
228 | - default: |
229 | - break; |
230 | - } |
231 | - |
232 | - if (window->overrideRedirect ()) |
233 | - shadowOnly = true; |
234 | - |
235 | - if (window->wmType () & (CompWindowTypeDockMask | CompWindowTypeDesktopMask)) |
236 | - shadowOnly = true; |
237 | - |
238 | - if (!shadowOnly) |
239 | - { |
240 | - if (!dScreen->optionGetDecorationMatch ().evaluate (window)) |
241 | - shadowOnly = true; |
242 | - } |
243 | - } |
244 | - else |
245 | - shadowOnly = false; |
246 | - |
247 | - decorate = ((window->frame () || |
248 | - window->hasUnmapReference ()) && !shadowOnly) || |
249 | - isSwitcher; |
250 | + bool shadowOnly = bareDecorationOnly (); |
251 | + bool decorate = shouldDecorateWindow (window, shadowOnly, isSwitcher); |
252 | |
253 | if (decorate || frameExtentsRequested) |
254 | { |
255 | - /* Attempt to find a matching decoration */ |
256 | - try |
257 | - { |
258 | - decoration = decor.findMatchingDecoration (window, true); |
259 | - } |
260 | - catch (...) |
261 | - { |
262 | - /* Find an appropriate default decoration to use */ |
263 | - if (dScreen->dmSupports & WINDOW_DECORATION_TYPE_PIXMAP && |
264 | - dScreen->cmActive && |
265 | - !(dScreen->dmSupports & WINDOW_DECORATION_TYPE_WINDOW && |
266 | - pixmapFailed)) |
267 | - { |
268 | - try |
269 | - { |
270 | - decoration = dScreen->decor[DECOR_ACTIVE].findMatchingDecoration (window, false); |
271 | - } |
272 | - catch (...) |
273 | - { |
274 | - compLogMessage ("decor", CompLogLevelWarn, "No default decoration found, placement will not be correct"); |
275 | - decoration.reset (); |
276 | - } |
277 | - } |
278 | - else if (dScreen->dmSupports & WINDOW_DECORATION_TYPE_WINDOW) |
279 | - decoration = dScreen->windowDefault; |
280 | - } |
281 | - |
282 | + decoration = findRealDecoration (); |
283 | /* Do not allow windows which are later undecorated |
284 | * to have a set _NET_FRAME_EXTENTS */ |
285 | if (decorate) |
286 | frameExtentsRequested = false; |
287 | } |
288 | else |
289 | - { |
290 | - /* This window isn't "decorated" but it still gets a shadow as long |
291 | - * as it isn't shaped weirdly, since the shadow is just a quad rect */ |
292 | - if (dScreen->optionGetShadowMatch ().evaluate (window)) |
293 | - { |
294 | - if (window->region ().numRects () == 1 && !window->alpha () && dScreen->decor[DECOR_BARE].mList.size ()) |
295 | - decoration = dScreen->decor[DECOR_BARE].mList.front (); |
296 | - |
297 | - if (decoration) |
298 | - { |
299 | - if (!checkSize (decoration)) |
300 | - decoration.reset (); |
301 | - } |
302 | - } |
303 | - } |
304 | + decoration = findBareDecoration (); |
305 | |
306 | /* Don't allow the windows to be decorated if |
307 | * we're tearing down or if a decorator isn't running |
308 | @@ -1555,25 +1650,14 @@ |
309 | if (decoration == old) |
310 | return false; |
311 | |
312 | - /* We need to damage the current output extents |
313 | - * and recompute the shadow region if a compositor |
314 | - * is running |
315 | - */ |
316 | - if (dScreen->cmActive) |
317 | - { |
318 | - cWindow->damageOutputExtents (); |
319 | - updateGroupShadows (); |
320 | - } |
321 | - |
322 | /* Determine how much we moved the window for the old |
323 | * decoration and save that, also destroy the old |
324 | * WindowDecoration */ |
325 | if (old) |
326 | { |
327 | - oldShift = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity); |
328 | + oldShift = cwe::shift (window->border (), window->sizeHints ().win_gravity); |
329 | |
330 | WindowDecoration::destroy (wd); |
331 | - |
332 | wd = NULL; |
333 | } |
334 | |
335 | @@ -1595,8 +1679,10 @@ |
336 | window->setWindowFrameExtents (&decoration->border, |
337 | &decoration->input); |
338 | |
339 | - /* We actually need to decorate this window */ |
340 | - if (decorate) |
341 | + /* This window actually needs its decoration contents updated |
342 | + * as it was actually visible */ |
343 | + if (decorate || |
344 | + shadowOnly) |
345 | { |
346 | wd = WindowDecoration::create (decoration); |
347 | if (!wd) |
348 | @@ -1608,20 +1694,21 @@ |
349 | return false; |
350 | } |
351 | |
352 | - movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity); |
353 | + movement = cwe::shift (window->border (), window->sizeHints ().win_gravity); |
354 | movement -= oldShift; |
355 | |
356 | - /* Update the input and output frame */ |
357 | - updateFrame (); |
358 | window->updateWindowOutputExtents (); |
359 | |
360 | updateReg = true; |
361 | updateMatrix = true; |
362 | mOutputRegion = CompRegion (window->outputRect ()); |
363 | - updateGroupShadows (); |
364 | if (dScreen->cmActive) |
365 | cWindow->damageOutputExtents (); |
366 | updateDecorationScale (); |
367 | + |
368 | + /* Update the input and output frame */ |
369 | + if (decorate) |
370 | + updateFrame (); |
371 | } |
372 | } |
373 | else |
374 | @@ -1641,55 +1728,19 @@ |
375 | movement -= oldShift; |
376 | } |
377 | |
378 | - /* Need to actually move the window */ |
379 | - if (window->placed () && !window->overrideRedirect () && |
380 | - (movement.x () || movement.y ())) |
381 | + /* We need to damage the current output extents |
382 | + * and recompute the shadow region if a compositor |
383 | + * is running |
384 | + */ |
385 | + if (dScreen->cmActive) |
386 | { |
387 | - XWindowChanges xwc; |
388 | - unsigned int mask = CWX | CWY; |
389 | - |
390 | - memset (&xwc, 0, sizeof (XWindowChanges)); |
391 | - |
392 | - /* Grab the geometry last sent to server at configureXWindow |
393 | - * time and not here since serverGeometry may be updated by |
394 | - * the time that we do call configureXWindow */ |
395 | - xwc.x = movement.x (); |
396 | - xwc.y = movement.y (); |
397 | - |
398 | - /* Except if it's fullscreen, maximized or such */ |
399 | - if (window->state () & CompWindowStateFullscreenMask) |
400 | - mask &= ~(CWX | CWY); |
401 | - |
402 | - if (window->state () & CompWindowStateMaximizedHorzMask) |
403 | - mask &= ~CWX; |
404 | - |
405 | - if (window->state () & CompWindowStateMaximizedVertMask) |
406 | - mask &= ~CWY; |
407 | - |
408 | - if (window->saveMask () & CWX) |
409 | - window->saveWc ().x += movement.x (); |
410 | - |
411 | - if (window->saveMask () & CWY) |
412 | - window->saveWc ().y += movement.y (); |
413 | - |
414 | - if (mask) |
415 | - { |
416 | - /* allowDecoration is only false in the case of |
417 | - * the destructor calling the update function so since it |
418 | - * is not safe to put the function in a timer (since |
419 | - * it will get unref'd on the vtable destruction) we |
420 | - * need to do it immediately |
421 | - * |
422 | - * FIXME: CompTimer should really be PIMPL and allow |
423 | - * refcounting in case we need to keep it alive |
424 | - */ |
425 | - if (!allowDecoration) |
426 | - decorOffsetMove (window, xwc, mask); |
427 | - else |
428 | - moveUpdate.start (boost::bind (decorOffsetMove, window, xwc, mask), 0); |
429 | - } |
430 | + cWindow->damageOutputExtents (); |
431 | + updateGroupShadows (); |
432 | } |
433 | |
434 | + moveDecoratedWindowBy (movement, |
435 | + !allowDecoration); |
436 | + |
437 | return true; |
438 | } |
439 | |
440 | |
441 | === modified file 'plugins/decor/src/decor.h' |
442 | --- plugins/decor/src/decor.h 2013-02-20 03:07:58 +0000 |
443 | +++ plugins/decor/src/decor.h 2013-03-22 15:54:41 +0000 |
444 | @@ -152,6 +152,13 @@ |
445 | |
446 | unsigned int updateState; |
447 | X11DecorPixmapReceiver mPixmapReceiver; |
448 | + |
449 | + private: |
450 | + |
451 | + bool bareDecorationOnly (); |
452 | + Decoration::Ptr findRealDecoration (); |
453 | + Decoration::Ptr findBareDecoration (); |
454 | + void moveDecoratedWindowBy (const CompPoint &movement); |
455 | }; |
456 | |
457 | class DecorationList : |
458 | @@ -372,6 +379,14 @@ |
459 | CompRegion mInputRegion; |
460 | |
461 | X11DecorPixmapRequestor mRequestor; |
462 | + |
463 | + private: |
464 | + |
465 | + bool bareDecorationOnly (); |
466 | + Decoration::Ptr findRealDecoration (); |
467 | + Decoration::Ptr findBareDecoration (); |
468 | + void moveDecoratedWindowBy (const CompPoint &movement, |
469 | + bool instant); |
470 | }; |
471 | |
472 | class DecorPluginVTable : |
Thanks, Sam :)