Hi,
As I've been working through some bugs in the code, it's become
increasingly clear that there are fundamental problems with the way
some of the code in core works and the way the API is structured,
especially in terms of the way that we handle communication with the X
Server and the updating of the compiz side window attributes such as
positioning and stacking.
In particular, I think that the problem lies in the fact that there is
a lot of confusion in both core and the usage of the API as to what
position plugins and core need to use, and I think there are four
identifiable positions that core and plugins need to know about.
1) Current compositor side window position and stacking
2) Position and stacking information that was last sent to the server
3) Position and stacking information that was last received from the server
4) Current server side window position and stacking (round-trip)
A lot of our problems currently seem to stem from the fact that we
often respond to server side events by processing "differences", eg,
the server sends us what has changed and we respond by updating
internal information. However, this same internal information is also
directly accessible to the plugins, which means that there are race
conditions where a plugin can overwrite this information and we
process events based on the last update a plugin made to a structure
rather than the last update the server gave us. This means that we
process the server event and now we have a picture of what the server
is doing which is inaccurate since it wasn't based on what the server
last sent us.
The principal offenders here are CompWindow::move, CompWindow::resize,
CompScreen::insertWindow, CompScreen::unhookWindow and
CompWindow::restack.
Eg:
void
CompWindow::move (int dx,
int dy,
bool immediate)
{
if (dx || dy)
{
priv->geometry.setX (priv->geometry.x () + dx);
priv->geometry.setY (priv->geometry.y () + dy);
priv->region.translate (dx, dy);
priv->inputRegion.translate (dx, dy);
if (!priv->frameRegion.isEmpty ())
priv->frameRegion.translate (dx, dy);
priv->invisible = WINDOW_INVISIBLE (priv);
moveNotify (dx, dy, immediate);
}
}
CompWindow::move updates priv->geometry directly and immediately sends
a moveNotify event once it completes. However, this is incorrect
because priv->geometry is actually meant to be a reflection of the
geometry last sent to us by the server. As such, this problem can
happen:
void
PrivateWindow::configureFrame (XConfigureEvent *ce)
{
int x, y, width, height;
CompWindow *above;
if (!priv->frame)
return;
x = ce->x + priv->input.left;
y = ce->y + priv->input.top;
width = ce->width - priv->serverGeometry.border () * 2 -
priv->input.left - priv->input.right;
height = ce->height - priv->serverGeometry.border () * 2 -
priv->input.top - priv->input.bottom;
if (priv->syncWait)
{
priv->syncGeometry.set (x, y, width, height, ce->border_width);
}
else
{
if (ce->override_redirect)
{
priv->serverGeometry.set (x, y, width, height, ce->border_width);
}
window->resize (x, y, width, height, ce->border_width);
}
}
Which calls:
bool
CompWindow::resize (CompWindow::Geometry gm)
{
if (priv->geometry.width () != gm.width () ||
priv->geometry.height () != gm.height () ||
priv->geometry.border () != gm.border ())
{
int pw, ph;
int dx, dy, dwidth, dheight;
pw = gm.width () + gm.border () * 2;
ph = gm.height () + gm.border () * 2;
if (priv->shaded)
ph = 0;
dx = gm.x () - priv->geometry.x ();
dy = gm.y () - priv->geometry.y ();
dwidth = gm.width () - priv->geometry.width ();
dheight = gm.height () - priv->geometry.height ();
priv->geometry.set (gm.x (), gm.y (),
gm.width (), gm.height (),
gm.border ());
priv->width = pw;
priv->height = ph;
if (priv->mapNum)
priv->updateRegion ();
resizeNotify (dx, dy, dwidth, dheight);
priv->invisible = WINDOW_INVISIBLE (priv);
priv->updateFrameWindow ();
}
else if (priv->geometry.x () != gm.x () || priv->geometry.y () != gm.y ())
{
int dx, dy;
dx = gm.x () - priv->geometry.x ();
dy = gm.y () - priv->geometry.y ();
move (dx, dy);
}
return true;
}
Both a plugin accessible API and the server are able to update the
geometry directly! This becomes a problem when you think of this:
CompWindow::move, CompWindow::syncPosition, CompWindow::move,
CompScreen::handleEvent, PrivateWindow::configureFrame,
CompWindow::resize, CompWindow::move
In this chain of calls, the position has been sent to the server, and
the server has essentially clobbered the working position used by
other plugins (eg in the second call to CompWindow::move) since there
was a difference in geometry by the time we responded to the server's
request.
I think that as such, we need to seriously reinvent the way the API
works here to make it absolutely clear to users of the API that we are
a window manager communicating with an external server and there are
four different states which are accessible.
1) The server side state (round trip) [read only]
2) The state last sent to the server (so that plugins can base their
requests for changes to windows based on this) [writable only by
making requests]
3) The state last received from the server (state used to present
information) [read only]
4) Compositor side state
I think our current implementation of 4) definitely needs to go. The
server side state needs to reign supreme in order that the window
manager can keep in sync with the server. Changes to 4) should only be
transient and make through the use of, eg, getWindowPaintList and
glPaint in plugins to move windows around and restack them that way.
CompScreen::insertWindow and CompScreen::unhookWindow *must* be
removed from the public API, and I believe that CompWindow::move and
CompWindow::resize should only be wrappers around
CompWindow::configureXWindow (which really should be private).
No part of the plugin facing API should ever touch
PrivateWindow::geometry or PrivateScreen::windows directly, this
should only be modified by the server.
I think that in order to set and retrieve window position information
we should have a unified API which requires plugins to explicitly
specify which position information they actually need, eg
window->geometry (Server)->
window->geometry (LastSent)->
window->geometry (LastRecv)->
window->geometry (Current)->
LastSent geometry is modified by actually making requests to the
server (eg w->resize (), w->move ()). LastRecv is read only, Current
should likely be modified through the use of an external API to set
"offsets" on each of these properties which are stored separately and
the "absolute" value will be retrieved by combining the server side
value with the "offset" stored.
Let me know what you think. This will probably one of the first things
I work on in the 0.9.7x series. However, in the immediate term for the
0.9.5x series (such as to fix the stacking bug) I'd like to propose an
API break to have a serverWindows () and windows () as well as using
timestamps to ignore outdated events from the server in both ::move
and ::resize.
Regards,
Sam
--
Sam Spilsbury
_______________________________________________
dev mailing list
[email protected]
http://lists.compiz.org/mailman/listinfo/dev
