Merge lp:~compiz-team/compiz-core/compiz-core.fix_geometry_cycling into lp:compiz-core/0.9.5

Proposed by Sam Spilsbury
Status: Merged
Approved by: Mirco Müller
Approved revision: 2798
Merged at revision: 2812
Proposed branch: lp:~compiz-team/compiz-core/compiz-core.fix_geometry_cycling
Merge into: lp:compiz-core/0.9.5
Diff against target: 94 lines (+24/-1)
2 files modified
src/privatewindow.h (+3/-0)
src/window.cpp (+21/-1)
To merge this branch: bzr merge lp:~compiz-team/compiz-core/compiz-core.fix_geometry_cycling
Reviewer Review Type Date Requested Status
Mirco Müller (community) Approve
Review via email: mp+73127@code.launchpad.net

Description of the change

Don't allow outdated server position to move a window back if a plugin
has already requested at a point in time past when the server was told to move
the window to change the geometry without syncing the position.

This should solve the problem of plugin A doing w->move, w->syncPosition and plugin
B doing w->move (wait) w->syncPosition where we return to handleEvent in between
the move and the position sync and the server will send us a ConfigureNotify
event for the position that was last sent to the server even if a plugin
has more recently updated the current geometry, and this geometry recieved
from the server will overwrite the current plugin's working geometry that
it was going to sync anyways later and cause moveNotify to be called.

At the moment, we're just comparing timestamps from when the geometry was
last updated by a plugin and when geometry was last sent to the server. Note
that in reality, CompWindow::move should not be updating priv->geometry directly,
this should only really be allowed to be updated by the server. However, it
doesn't make sense for it to update priv->serverGeometry as that was the
geometry last sent to the server. In reality, a larger API break will be
needed in order to represent the four possible states of geometry at any given
time.

a) geometry by the compositor (current geometry, not pending to send to server)
b) geometry last sent to server
c) geometry last recieved by the server
d) current server side geometry (which must be retrieved via a round-trip
   to the server)

Note that I don't think that this change should go in the 0.9.5.x series, but we should really wait for 0.9.7

To post a comment you must log in.
Revision history for this message
Mirco Müller (macslow) wrote :

I don't have my system setup to compile compiz atm, but as far as I can tell all this look ok. BTW just get into the habit of initializing any declared variables/structs (lastGeometryUpdate, lastConfigureRequest).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/privatewindow.h'
2--- src/privatewindow.h 2011-08-23 06:16:21 +0000
3+++ src/privatewindow.h 2011-08-27 05:17:02 +0000
4@@ -238,6 +238,9 @@
5 XSizeHints sizeHints;
6 XWMHints *hints;
7
8+ struct timeval lastGeometryUpdate;
9+ struct timeval lastConfigureRequest;
10+
11 bool inputHint;
12 bool alpha;
13 int width;
14
15=== modified file 'src/window.cpp'
16--- src/window.cpp 2011-08-23 13:15:20 +0000
17+++ src/window.cpp 2011-08-27 05:17:02 +0000
18@@ -768,6 +768,8 @@
19 if (!frame)
20 return;
21
22+ gettimeofday (&lastConfigureRequest, NULL);
23+
24 if (input.left || input.right || input.top || input.bottom)
25 {
26 int x, y, width, height;
27@@ -1483,6 +1485,8 @@
28 dwidth = gm.width () - priv->geometry.width ();
29 dheight = gm.height () - priv->geometry.height ();
30
31+ gettimeofday (&priv->lastGeometryUpdate, NULL);
32+
33 priv->geometry.set (gm.x (), gm.y (),
34 gm.width (), gm.height (),
35 gm.border ());
36@@ -1505,7 +1509,13 @@
37 dx = gm.x () - priv->geometry.x ();
38 dy = gm.y () - priv->geometry.y ();
39
40- move (dx, dy);
41+ /* Don't move the window here if a plugin has already updated
42+ * the geometry of that window after the last time the server
43+ * was sent a configure request since the configureNotify we
44+ * receieve here will be out of date. Instead wait for the plugin
45+ * to call syncPosition again */
46+ if (TIMEVALDIFF (&priv->lastConfigureRequest, &priv->lastGeometryUpdate))
47+ move (dx, dy);
48 }
49
50 return true;
51@@ -1719,6 +1729,8 @@
52 priv->geometry.setX (priv->geometry.x () + dx);
53 priv->geometry.setY (priv->geometry.y () + dy);
54
55+ gettimeofday (&priv->lastGeometryUpdate, NULL);
56+
57 priv->region.translate (dx, dy);
58 priv->inputRegion.translate (dx, dy);
59 if (!priv->frameRegion.isEmpty ())
60@@ -1736,6 +1748,8 @@
61 priv->serverGeometry.setX (priv->geometry.x ());
62 priv->serverGeometry.setY (priv->geometry.y ());
63
64+ gettimeofday (&priv->lastConfigureRequest, NULL);
65+
66 XMoveWindow (screen->dpy (), ROOTPARENT (this),
67 priv->geometry.x () - priv->input.left,
68 priv->geometry.y () - priv->input.top);
69@@ -2477,6 +2491,8 @@
70 {
71 XWindowChanges wc = *xwc;
72
73+ gettimeofday (&lastConfigureRequest, NULL);
74+
75 wc.x -= input.left - serverGeometry.border ();
76 wc.y -= input.top - serverGeometry.border ();
77 wc.width += input.left + input.right + serverGeometry.border ();
78@@ -5315,6 +5331,8 @@
79
80 screen->insertWindow (this, aboveId);
81
82+ gettimeofday (&priv->lastGeometryUpdate, NULL);
83+
84 priv->attrib = wa;
85 priv->serverGeometry.set (priv->attrib.x, priv->attrib.y,
86 priv->attrib.width, priv->attrib.height,
87@@ -5883,6 +5901,8 @@
88 return false;
89 }
90
91+ gettimeofday (&lastConfigureRequest, NULL);
92+
93 if (attrib.override_redirect)
94 return false;
95

Subscribers

People subscribed via source and target branches