Re: Merging spif-2 branch

[Soeren Sandmann <sandmann redhat com>]

Havoc Pennington wrote:

> Some fairly random thoughts looking through here.
And here is a reply with some other fairly random thoughts.

> I did not look at
> compositor.c much; I'm going to consider that file maintained by you I
> think and only comment on core WM changes.
>
> "Ws *ws" doesn't read very well (what's a Ws?)
It's a windowsystem, but shorter to type. I can change the name of it 
though -
any suggestions?

> This code seems to raise a red flag about future bugs:
>
> +      meta_display_ungrab (display);
> +      
> +      meta_compositor_manage_screen (screen->display->compositor,
> +				     screen);
> +
> +      meta_display_grab (display);

> Essentially anytime we have a display grab, we can't call a meta_compositor_*, right,
>
> or it will deadlock on the other display connection... probably inevitable, but it
>
> might be interesting to put in assertions that will quickly track any such bug down,
>
> I don't know though.
Actually, the compositor calls XTestGrabControl (dpy, ignore), so it 
will ignore grabs.
This is necessary not only because of being in-process with metacity, 
but also simply
because we can't turn off screen updating just because someone grabbed 
the server ...
On the other hand this does mean that the compositor has to be careful 
what it does
to avoid confusing other clients.

The reason the server needs to be ungrabbed in this particular case is 
that for
some reason the libGL.so I am using opens a _third_ connection to the X 
server,
and is blocking in XOpenDisplay(). I don't know what this third 
connection is
used for, but I don't think it would present any further problems, 
because if it did,
it would also cause problems for normal GL applications that happened to 
grab the
server.

> Or maybe we should explicitly adopt a model where the compositor is
> async with respect to the main WM, i.e. all core->compositor messages go
> in a queue... ? does it end up basically working that way anyway due to
> the two X connections?
Right, my long term plan is to actually have the CM be a separate 
process that
just manages a scene graph. Metacity and other interested desktop 
applications
would then modify this scene graph through some sort of IPC.

Putting the CM in its own thread is something I have thought about doing 
anyway.
That would allow us to have the compositing manager only ignore grabs 
when it was
drawing and not other times.

> Where you removed the damage_window in begin/end grab op... what
> replaces that? (Does wireframe even work with a compositor? What is the
> interaction there?)
Nothing has replaced it for now. The model that damage_window() implies 
is like
"metacity maintains the state of the window, and the CM reads it and 
paints the
desktop accordingly." But in a model where the CM maintains a scene 
graph in a
separate process, it will have to be told explicitly what the desktop 
looks like, so
as a first step I want to add an 'effects' indirection layer that has 
API like

   effects_begin_move_window()
   effects_end_move_window()
   effects_minimize_window()
   effects_maximize_window()

etc. This layer will then maintain animation states and modify the scene 
graph
accordingly. Hopefully this API will survive the compositing manager moving
out of process.

> I think the error.c change could use a comment about when that happens,
> just "/* error traps could be 0 if the error is from another display */"
I'll put in that comment.

> This change doesn't look related to the rest of the patch?
> -          meta_workspace_focus_default_window
Yeah, for an RH internal demo I did a cheesy genie type effect; to make 
that work
I had to make the compositing manager do both the final window_hide(), 
and the
focus_default() since these needed to happen after the minimize effect 
was complete.

It needs to be figured out how to make sure semantic/behaviorial things 
like this
don't get mixed up with transitional effects like genie type stuff. A 
possibility might
be to have the effects layer call back when the event should take 
"semantic effect".


Soren