[PATCH 3/4] xwayland: Draw decoration on window manager side

[Louis-Francis Ratté-Boulianne]

On Wednesday, July 31, 2013 10:41 EDT, Uli Schlachter <psychon at znc.in> wrote: 
 
> Hi,
> 
> On 30.07.2013 18:13, Louis-Francis Ratté-Boulianne wrote:
> > Draw everything in a cairo image surface before copying it to the XCB
> > surface. That removes the flickering visible on rpi whenever the
> > decoration were redrawn.
> > ---
> >  src/xwayland/window-manager.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/xwayland/window-manager.c b/src/xwayland/window-manager.c
> > index b3c9251..57a5d99 100644
> > --- a/src/xwayland/window-manager.c
> > +++ b/src/xwayland/window-manager.c
> > @@ -97,6 +97,7 @@ struct weston_wm_window {
> >  	xcb_window_t id;
> >  	xcb_window_t frame_id;
> >  	cairo_surface_t *cairo_surface;
> > +	cairo_surface_t *cairo_xcb_surface;
> >  	struct weston_surface *surface;
> >  	struct shell_surface *shsurf;
> >  	struct wl_listener surface_destroy_listener;
> > @@ -900,12 +901,14 @@ weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event)
> >  	xcb_map_window(wm->conn, map_request->window);
> >  	xcb_map_window(wm->conn, window->frame_id);
> >  
> > -	window->cairo_surface =
> > +	window->cairo_xcb_surface =
> >  		cairo_xcb_surface_create_with_xrender_format(wm->conn,
> >  							     wm->screen,
> >  							     window->frame_id,
> >  							     &wm->format_rgba,
> >  							     width, height);
> > +	window->cairo_surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32,
> > +							    width, height);
> [...]
> 
> Why do you use an image surface for this? I would suggest to use
> cairo_surface_create_similar() instead. This might then give you an XCB surface
> drawing to a temporary pixmap (but it could also be an image surface or
> something completely different).

I tried this way, but it still flickers between a black surface and the drawn decoration when the decorations are updated or the window is resized. It's still better than having each element showing up one after the other, but still really ugly.

Also, I was wondering..if the similar surface that has been created is a XCB one, is the rendering on the server side? The objective of my patch was to move the drawing on client side (wm) so we know when it's done and we can push it to the screen.

> If there is some reason for forcing image surfaces and if depending on cairo
> 1.12 is no problem, I would suggest to use cairo_surface_create_similar_image()
> at least. This would make it possible for cairo to use shared memory instead of
> sending all the pixels over the X11 socket.

That's seem to be the only solution that avoids any flickering while drawing the decoration on slow hardware (Raspberry Pi in my case). Unfortunately, there is a bug in Cairo < 1.12.6. So if you feel confident that we can update the dependency, that's the patch you should push. If not, the old patch still seems to be the only working solution for me.

Related cairo commit: "xcb: Clear the result of create_similar_image"
 
> And a third idea would be to use cairo_push_group(); /
> cairo_pop_group_to_source(); cairo_paint();. This would need to be done around
> the drawing code. I don't know this code and I am too lazy to look it up, but if
> this works and could be done easily, this would avoid the memory usage for
> keeping the double-buffering surface around always.

That solution would have been really great to avoid the memory usage, but a similar problem occurs (flickering). I guess it's because the decorations are still rendered on server side.

Thanks for the review. If you want to see the patches I came up with to test the other solutions, let me know.

--
Louis-Francis

> Cheers,
> Uli
> -- 
> - He made himself, me nothing, you nothing out of the dust
> - Er machte sich mir nichts, dir nichts aus dem Staub