Compiz

Merge lp:~compiz-team/compiz/compiz.fix_1140505 into lp:compiz/0.9.9

  1. compiz.fix_1140505
  2. Merge into 0.9.9
Proposed by Sam Spilsbury
Status: Superseded
Proposed branch: lp:~compiz-team/compiz/compiz.fix_1140505
Merge into: lp:compiz/0.9.9
Diff against target: 453 lines (+267/-48)
4 files modified
plugins/decor/src/decor.cpp (+30/-21)
src/privatewindow.h (+2/-2)
src/window.cpp (+39/-14)
tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp (+196/-11)
To merge this branch: bzr merge lp:~compiz-team/compiz/compiz.fix_1140505
Reviewer Review Type Date Requested Status
MC Return Pending
Timo Jyrinki Pending
Andrea Azzarone Pending
PS Jenkins bot continuous-integration Pending
Review via email: mp+152618@code.launchpad.net

This proposal supersedes a proposal from 2013-03-06.

This proposal has been superseded by a proposal from 2013-03-11.

Description of the change

Use the actual change in window size to determine if we need to change
  the frame window size, don't just use the difference in frame extents
  to determine that.

  (LP: #1140505)

To post a comment you must log in.
Revision history for this message
MC Return (mc-return) wrote : Posted in a previous version of this proposal

LGTM.

review: Approve
Revision history for this message
Andrea Azzarone (azzar1) wrote : Posted in a previous version of this proposal

Code looks good. Works here. Thanks! :)

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/compiz-trunk-autolanding/1/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-trunk-raring-amd64-autolanding/1//console
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-trunk-raring-i386-autolanding/5//console

review: Needs Fixing (continuous-integration)
Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote : Posted in a previous version of this proposal

The test seems to fail at 'GetImmediateParent' according to jenkins logs.

review: Needs Fixing
Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

I'll get on to this later tonight.

On Mon, Mar 11, 2013 at 12:58 PM, Timo Jyrinki
<email address hidden> wrote:
> Review: Needs Fixing
>
> The test seems to fail at 'GetImmediateParent' according to jenkins logs.
> --
> https://code.launchpad.net/~compiz-team/compiz/compiz.fix_1140505/+merge/151951
> Your team Compiz Maintainers is subscribed to branch lp:compiz.

--
Sam Spilsbury

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/decor/src/decor.cpp'
2--- plugins/decor/src/decor.cpp 2013-02-26 01:08:15 +0000
3+++ plugins/decor/src/decor.cpp 2013-03-11 06:43:21 +0000
4@@ -1585,35 +1585,44 @@
5 */
6 if (decoration)
7 {
8- wd = WindowDecoration::create (decoration);
9- if (!wd)
10- return false;
11-
12 /* Set extents based on maximize/unmaximize state
13 * FIXME: With the new type system, this should be
14 * removed */
15 if ((window->state () & MAXIMIZE_STATE))
16- window->setWindowFrameExtents (&wd->decor->maxBorder,
17- &wd->decor->maxInput);
18+ window->setWindowFrameExtents (&decoration->maxBorder,
19+ &decoration->maxInput);
20 else if (!window->hasUnmapReference ())
21- window->setWindowFrameExtents (&wd->decor->border,
22- &wd->decor->input);
23-
24- movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
25- movement -= oldShift;
26-
27- /* Update the input and output frame */
28+ window->setWindowFrameExtents (&decoration->border,
29+ &decoration->input);
30+
31+ /* We actually need to decorate this window */
32 if (decorate)
33+ {
34+ wd = WindowDecoration::create (decoration);
35+ if (!wd)
36+ {
37+ /* Error condition, reset frame extents */
38+ CompWindowExtents emptyExtents;
39+ memset (&emptyExtents, 0, sizeof (CompWindowExtents));
40+ window->setWindowFrameExtents (&emptyExtents, &emptyExtents);
41+ return false;
42+ }
43+
44+ movement = compiz::window::extents::shift (window->border (), window->sizeHints ().win_gravity);
45+ movement -= oldShift;
46+
47+ /* Update the input and output frame */
48 updateFrame ();
49- window->updateWindowOutputExtents ();
50+ window->updateWindowOutputExtents ();
51
52- updateReg = true;
53- updateMatrix = true;
54- mOutputRegion = CompRegion (window->outputRect ());
55- updateGroupShadows ();
56- if (dScreen->cmActive)
57- cWindow->damageOutputExtents ();
58- updateDecorationScale ();
59+ updateReg = true;
60+ updateMatrix = true;
61+ mOutputRegion = CompRegion (window->outputRect ());
62+ updateGroupShadows ();
63+ if (dScreen->cmActive)
64+ cWindow->damageOutputExtents ();
65+ updateDecorationScale ();
66+ }
67 }
68 else
69 {
70
71=== modified file 'src/privatewindow.h'
72--- src/privatewindow.h 2013-02-10 05:01:50 +0000
73+++ src/privatewindow.h 2013-03-11 06:43:21 +0000
74@@ -155,7 +155,7 @@
75
76 void recalcNormalHints ();
77
78- void updateFrameWindow ();
79+ bool updateFrameWindow ();
80
81 void setWindowMatrix ();
82
83@@ -302,7 +302,7 @@
84 int gravity,
85 int direction);
86
87- void updateSize ();
88+ bool updateSize ();
89
90 bool getUserTime (Time& time);
91 void setUserTime (Time time);
92
93=== modified file 'src/window.cpp'
94--- src/window.cpp 2013-02-27 03:24:45 +0000
95+++ src/window.cpp 2013-03-11 06:43:21 +0000
96@@ -807,12 +807,11 @@
97 priv->type = type;
98 }
99
100-
101-void
102+bool
103 PrivateWindow::updateFrameWindow ()
104 {
105 if (!serverFrame)
106- return;
107+ return false;
108
109 XWindowChanges xwc = XWINDOWCHANGES_INIT;
110 unsigned int valueMask = CWX | CWY | CWWidth | CWHeight;
111@@ -826,6 +825,8 @@
112 window->configureXWindow (valueMask, &xwc);
113 window->windowNotify (CompWindowNotifyFrameUpdate);
114 window->recalcActions ();
115+
116+ return true;
117 }
118
119
120@@ -3330,11 +3331,19 @@
121 if (lastServerInput.top != serverInput.top)
122 valueMask |= CWY;
123
124- if (lastServerInput.right - lastServerInput.left !=
125- serverInput.right - serverInput.left)
126+ /* Calculate frame extents and protect against underflow */
127+ const unsigned int lastWrapperWidth = std::max (0, serverFrameGeometry.width () -
128+ (lastServerInput.right + lastServerInput.left));
129+ const unsigned int lastWrapperHeight = std::max (0, serverFrameGeometry.height () -
130+ (lastServerInput.bottom + lastServerInput.top));
131+ const unsigned int wrapperWidth = std::max (0, serverFrameGeometry.width () -
132+ (serverInput.right + serverInput.left));
133+ const unsigned int wrapperHeight = std::max (0, serverFrameGeometry.height () -
134+ (serverInput.bottom + serverInput.top));
135+
136+ if (lastWrapperWidth != wrapperWidth)
137 valueMask |= CWWidth;
138- if (lastServerInput.bottom - lastServerInput.top !=
139- serverInput.bottom - serverInput.top)
140+ if (lastWrapperHeight != wrapperHeight)
141 valueMask |= CWHeight;
142
143 if (valueMask)
144@@ -4109,11 +4118,11 @@
145 priv->placed = true;
146 }
147
148-void
149+bool
150 PrivateWindow::updateSize ()
151 {
152 if (window->overrideRedirect () || !managed)
153- return;
154+ return false;
155
156 XWindowChanges xwc = XWINDOWCHANGES_INIT;
157
158@@ -4124,7 +4133,10 @@
159 window->sendSyncRequest ();
160
161 window->configureXWindow (mask, &xwc);
162+ return true;
163 }
164+
165+ return false;
166 }
167
168 int
169@@ -6780,8 +6792,10 @@
170
171 recalcActions ();
172
173- priv->updateSize ();
174- priv->updateFrameWindow ();
175+ bool sizeUpdated = false;
176+
177+ sizeUpdated |= priv->updateSize ();
178+ sizeUpdated |= priv->updateFrameWindow ();
179
180 /* Always send a moveNotify
181 * whenever the frame extents update
182@@ -6789,7 +6803,8 @@
183 moveNotify (0, 0, true);
184
185 /* Once we have updated everything, re-set lastServerInput */
186- priv->lastServerInput = priv->serverInput;
187+ if (sizeUpdated)
188+ priv->lastServerInput = priv->serverInput;
189 }
190
191 /* Use b for _NET_WM_FRAME_EXTENTS here because
192@@ -6909,6 +6924,9 @@
193 * but that's all */
194 XSelectInput (dpy, screen->root (), SubstructureNotifyMask);
195
196+ /* Gravity here is assumed to be SouthEast, clients can update
197+ * that if need be */
198+
199 /* Awaiting a new frame to be given to us */
200 frame = None;
201 serverFrame = XCreateWindow (dpy, screen->root (), 0, 0,
202@@ -6918,10 +6936,17 @@
203 /* Do not get any events from here on */
204 XSelectInput (dpy, screen->root (), NoEventMask);
205
206- wrapper = XCreateWindow (dpy, serverFrame, 0, 0,
207- wa.width, wa.height, 0, wa.depth,
208+ /* If we have some frame extents, we should apply them here and
209+ * set lastFrameExtents */
210+ wrapper = XCreateWindow (dpy, serverFrame,
211+ serverInput.left, serverInput.top,
212+ wa.width - (serverInput.left + serverInput.right),
213+ wa.height - (serverInput.top + serverInput.bottom),
214+ 0, wa.depth,
215 InputOutput, visual, mask, &attr);
216
217+ lastServerInput = serverInput;
218+
219 xwc.stack_mode = Above;
220
221 /* Look for the client in the current server side stacking
222
223=== modified file 'tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp'
224--- tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp 2013-02-26 00:52:32 +0000
225+++ tests/system/xorg-gtest/tests/compiz_xorg_gtest_configure_window.cpp 2013-03-11 06:43:21 +0000
226@@ -55,27 +55,41 @@
227 return ct::AdvanceToNextEventOnSuccess (d, r);
228 }
229
230+Window GetImmediateParent (Display *display,
231+ Window w,
232+ Window &rootReturn)
233+{
234+ Window parentReturn = w;
235+ Window *childrenReturn;
236+ unsigned int nChildrenReturn;
237+
238+ XQueryTree (display,
239+ w,
240+ &rootReturn,
241+ &parentReturn,
242+ &childrenReturn,
243+ &nChildrenReturn);
244+ XFree (childrenReturn);
245+
246+ return parentReturn;
247+}
248+
249+
250 Window GetTopParent (Display *display,
251 Window w)
252 {
253- Window rootReturn;
254+ Window rootReturn = 0;
255 Window parentReturn = w;
256- Window *childrenReturn;
257- unsigned int nChildrenReturn;
258-
259 Window lastParent = 0;
260
261 do
262 {
263 lastParent = parentReturn;
264
265- XQueryTree (display,
266- lastParent,
267- &rootReturn,
268- &parentReturn,
269- &childrenReturn,
270- &nChildrenReturn);
271- XFree (childrenReturn);
272+ parentReturn = GetImmediateParent (display,
273+ lastParent,
274+ rootReturn);
275+
276 } while (parentReturn != rootReturn);
277
278 return lastParent;
279@@ -565,3 +579,174 @@
280 currentWidth + (left + right),
281 currentHeight + (top + bottom)));
282 }
283+
284+/* Check that changing the frame extents by one on each side
285+ * adjusts the wrapper window appropriately */
286+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsAdjWrapperWindow)
287+{
288+ ::Display *dpy = Display ();
289+
290+ ReparentedWindow w = CreateWindow (dpy);
291+
292+ int currentX, currentY;
293+ unsigned int currentWidth, currentHeight;
294+ ASSERT_TRUE (QueryGeometry (dpy,
295+ w.frame,
296+ currentX,
297+ currentY,
298+ currentWidth,
299+ currentHeight));
300+
301+ /* Set frame extents and get a response */
302+ int left = 1;
303+ int right = 1;
304+ int top = 1;
305+ int bottom = 1;
306+
307+ SendSetFrameExtentsRequest (w.client, left, right, top, bottom);
308+ ASSERT_TRUE (VerifySetFrameExtentsResponse (w.client, left, right, top, bottom));
309+
310+ /* Wrapper geometry is extents.xy, size.wh */
311+ Window root;
312+ Window wrapper = GetImmediateParent (dpy, w.client, root);
313+ ASSERT_TRUE (VerifyWindowSize (wrapper,
314+ left,
315+ top,
316+ currentWidth,
317+ currentHeight));
318+}
319+
320+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsUnmapped)
321+{
322+ ::Display *dpy = Display ();
323+
324+ Window client = ct::CreateNormalWindow (dpy);
325+ WaitForWindowCreation (client);
326+
327+ /* Set frame extents and get a response */
328+ int left = 1;
329+ int right = 1;
330+ int top = 1;
331+ int bottom = 1;
332+
333+ int currentX, currentY;
334+ unsigned int currentWidth, currentHeight;
335+ ASSERT_TRUE (QueryGeometry (dpy,
336+ client,
337+ currentX,
338+ currentY,
339+ currentWidth,
340+ currentHeight));
341+
342+ /* We should get a response with our frame extents but it shouldn't actually
343+ * do anything to the client as it is unmapped */
344+ SendSetFrameExtentsRequest (client, left, right, top, bottom);
345+ ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
346+
347+ ASSERT_TRUE (VerifyWindowSize (client,
348+ currentX,
349+ currentY,
350+ currentWidth,
351+ currentHeight));
352+}
353+
354+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsCorrectMapBehaviour)
355+{
356+ ::Display *dpy = Display ();
357+
358+ Window client = ct::CreateNormalWindow (dpy);
359+ WaitForWindowCreation (client);
360+
361+ /* Set frame extents and get a response */
362+ int left = 1;
363+ int right = 1;
364+ int top = 1;
365+ int bottom = 1;
366+
367+ int currentX, currentY;
368+ unsigned int currentWidth, currentHeight;
369+ ASSERT_TRUE (QueryGeometry (dpy,
370+ client,
371+ currentX,
372+ currentY,
373+ currentWidth,
374+ currentHeight));
375+
376+ SendSetFrameExtentsRequest (client, left, right, top, bottom);
377+ ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
378+
379+ /* Map the window */
380+ XMapRaised (dpy, client);
381+ WaitForReparentAndMap (dpy, client);
382+
383+ /* Check the geometry of the frame */
384+ Window frame = GetTopParent (dpy, client);
385+ ASSERT_TRUE (VerifyWindowSize (frame,
386+ currentX,
387+ currentY,
388+ currentWidth + (left + right),
389+ currentHeight + (top + bottom)));
390+
391+ /* Wrapper geometry is extents.xy, size.wh */
392+ Window root;
393+ Window wrapper = GetImmediateParent (dpy, client, root);
394+ ASSERT_TRUE (VerifyWindowSize (wrapper,
395+ left,
396+ top,
397+ currentWidth,
398+ currentHeight));
399+}
400+
401+TEST_F (CompizXorgSystemConfigureWindowTest, SetFrameExtentsConsistentBehaviourAfterMap)
402+{
403+ ::Display *dpy = Display ();
404+
405+ Window client = ct::CreateNormalWindow (dpy);
406+ WaitForWindowCreation (client);
407+
408+ /* Set frame extents and get a response */
409+ int left = 1;
410+ int right = 1;
411+ int top = 1;
412+ int bottom = 1;
413+
414+ int currentX, currentY;
415+ unsigned int currentWidth, currentHeight;
416+ ASSERT_TRUE (QueryGeometry (dpy,
417+ client,
418+ currentX,
419+ currentY,
420+ currentWidth,
421+ currentHeight));
422+
423+ SendSetFrameExtentsRequest (client, left, right, top, bottom);
424+ ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
425+
426+ /* Map the window */
427+ XMapRaised (dpy, client);
428+ WaitForReparentAndMap (dpy, client);
429+
430+ /* Send it another frame extents request */
431+ right = right + 1;
432+ bottom = bottom + 1;
433+
434+ SendSetFrameExtentsRequest (client, left, right, top, bottom);
435+ ASSERT_TRUE (VerifySetFrameExtentsResponse (client, left, right, top, bottom));
436+
437+ /* Check the geometry of the frame */
438+ Window frame = GetTopParent (dpy, client);
439+ ASSERT_TRUE (VerifyWindowSize (frame,
440+ currentX,
441+ currentY,
442+ currentWidth + (left + right),
443+ currentHeight + (top + bottom)));
444+
445+ /* Wrapper geometry is extents.xy, size.wh */
446+ Window root;
447+ Window wrapper = GetImmediateParent (dpy, client, root);
448+ ASSERT_TRUE (VerifyWindowSize (wrapper,
449+ left,
450+ top,
451+ currentWidth,
452+ currentHeight));
453+}

Subscribers

People subscribed via source and target branches