drawable: Add pixmap member

Previously, ownership of the pixmaps that we are using for double buffering was
a little weird. The pixmap belonged to the drawin/titlebar, but the
corresponding cairo surface was owned by the drawable. Clean this up by moving
the pixmap to the drawable.

This cleans up lots of ugly code and also fixes a crash: When a drawable was
garbage collected before its drawin, drawin_wipe() would crash accessing the
drawable. This was needed to make it forget about the cairo surface we gave to
it for the pixmap that is being destroyed.

By moving the pixmap to the drawable, this whole issues goes away.

Signed-off-by: Uli Schlachter <psychon@znc.in>

objects/client.c

@@ -810,30 +810,7 @@ client_resize_do(client_t *c, area_t geometry, bool force_notice, bool honor_hin
         area.y += geometry.y;
         if (hide_titlebars)
             area.width = area.height = 0;
-
+        drawable_set_geometry(drawable, -1, area);
-        if (old_geometry.width != geometry.width || old_geometry.height != geometry.height ||
-                drawable->geometry.width == 0 || drawable->geometry.height == 0) {
-            /* Get rid of the old state */
-            drawable_unset_surface(drawable);
-            if (c->titlebar[bar].pixmap != XCB_NONE)
-                xcb_free_pixmap(globalconf.connection, c->titlebar[bar].pixmap);
-            c->titlebar[bar].pixmap = XCB_NONE;
-
-            /* And get us some new state */
-            if (c->titlebar[bar].size != 0 && !hide_titlebars)
-            {
-                c->titlebar[bar].pixmap = xcb_generate_id(globalconf.connection);
-                xcb_create_pixmap(globalconf.connection, globalconf.default_depth, c->titlebar[bar].pixmap,
-                                  globalconf.screen->root, area.width, area.height);
-                cairo_surface_t *surface = cairo_xcb_surface_create(globalconf.connection,
-                                                                    c->titlebar[bar].pixmap, globalconf.visual,
-                                                                    area.width, area.height);
-                drawable_set_surface(drawable, -1, surface, area);
-                cairo_surface_destroy(surface);
-            } else
-                drawable_set_geometry(drawable, -1, area);
-        } else
-            drawable_set_geometry(drawable, -1, area);
 
         /* Pop the client and the drawable */
         lua_pop(globalconf.L, 2);
@@ -1187,18 +1164,11 @@ client_unmanage(client_t *c, bool window_valid)
         if (c->titlebar[bar].drawable == NULL)
             continue;
 
+        /* Forget about the drawable */
         luaA_object_push(globalconf.L, c);
-        luaA_object_push_item(globalconf.L, -1, c->titlebar[bar].drawable);
+        luaA_object_unref_item(globalconf.L, -1, c->titlebar[bar].drawable);
-
-        /* Make the drawable unusable */
-        drawable_unset_surface(c->titlebar[bar].drawable);
-        if (c->titlebar[bar].pixmap != XCB_NONE)
-            xcb_free_pixmap(globalconf.connection, c->titlebar[bar].pixmap);
-
-        /* And forget about it */
-        luaA_object_unref_item(globalconf.L, -2, c->titlebar[bar].drawable);
         c->titlebar[bar].drawable = NULL;
-        lua_pop(globalconf.L, 2);
+        lua_pop(globalconf.L, 1);
     }
 
     /* Clear our event mask so that we don't receive any events from now on,
@@ -1554,7 +1524,7 @@ client_get_drawable(client_t *c, int x, int y)
 static void
 client_refresh_titlebar_partial(client_t *c, client_titlebar_t bar, int16_t x, int16_t y, uint16_t width, uint16_t height)
 {
-    if(c->titlebar[bar].drawable == NULL || c->titlebar[bar].drawable->surface == NULL)
+    if(c->titlebar[bar].drawable == NULL || c->titlebar[bar].drawable->pixmap == XCB_NONE)
         return;
     area_t area = titlebar_get_area(c, bar);
     /* Is the titlebar part of the area that should get redrawn? */
@@ -1564,7 +1534,7 @@ client_refresh_titlebar_partial(client_t *c, client_titlebar_t bar, int16_t x, i
         return;
     /* Redraw the affected parts */
     cairo_surface_flush(c->titlebar[bar].drawable->surface);
-    xcb_copy_area(globalconf.connection, c->titlebar[bar].pixmap, c->frame_window,
+    xcb_copy_area(globalconf.connection, c->titlebar[bar].drawable->pixmap, c->frame_window,
             globalconf.gc, x - area.x, y - area.y, x, y, width, height);
 }
 

objects/client.h

@@ -121,8 +121,6 @@ struct client_t
     struct {
         /** The size of this bar. */
         uint16_t size;
-        /** The pixmap for double buffering. */
-        xcb_pixmap_t pixmap;
         /** The drawable for this bar. */
         drawable_t *drawable;
     } titlebar[CLIENT_TITLEBAR_COUNT];

objects/drawable.c

@@ -35,40 +35,25 @@ drawable_allocator(lua_State *L, drawable_refresh_callback *callback, void *data
     d->refresh_callback = callback;
     d->refresh_data = data;
     d->surface = NULL;
+    d->pixmap = XCB_NONE;
     return d;
 }
 
+static void
+drawable_unset_surface(drawable_t *d)
+{
+    cairo_surface_finish(d->surface);
+    cairo_surface_destroy(d->surface);
+    if (d->pixmap)
+        xcb_free_pixmap(globalconf.connection, d->pixmap);
+    d->surface = NULL;
+    d->pixmap = XCB_NONE;
+}
+
 static void
 drawable_wipe(drawable_t *d)
-{
-    if (!d->surface)
-        return;
-    cairo_surface_finish(d->surface);
-    cairo_surface_destroy(d->surface);
-}
-
-void
-drawable_unset_surface(drawable_t *d)
-{
-    if (!d->surface)
-        return;
-    cairo_surface_finish(d->surface);
-    cairo_surface_destroy(d->surface);
-    d->surface = NULL;
-}
-
-void
-drawable_set_surface(drawable_t *d, int didx, cairo_surface_t *surface, area_t geom)
 {
     drawable_unset_surface(d);
-    d->surface = cairo_surface_reference(surface);
-    /* Make sure the surface doesn't contain garbage by filling it with black */
-    cairo_t *cr = cairo_create(surface);
-    cairo_set_operator(cr, CAIRO_OPERATOR_CLEAR);
-    cairo_paint(cr);
-    cairo_destroy(cr);
-    drawable_set_geometry(d, didx, geom);
-    luaA_object_emit_signal(globalconf.L, didx, "property::surface", 0);
 }
 
 void
@@ -77,6 +62,20 @@ drawable_set_geometry(drawable_t *d, int didx, area_t geom)
     area_t old = d->geometry;
     d->geometry = geom;
 
+    bool size_changed = (old.width != geom.width) || (old.height != geom.height);
+    if (size_changed)
+        drawable_unset_surface(d);
+    if (size_changed && geom.width > 0 && geom.height > 0)
+    {
+        d->pixmap = xcb_generate_id(globalconf.connection);
+        xcb_create_pixmap(globalconf.connection, globalconf.default_depth, d->pixmap,
+                          globalconf.screen->root, geom.width, geom.height);
+        d->surface = cairo_xcb_surface_create(globalconf.connection,
+                                              d->pixmap, globalconf.visual,
+                                              geom.width, geom.height);
+        luaA_object_emit_signal(globalconf.L, didx, "property::surface", 0);
+    }
+
     if (old.x != geom.x)
         luaA_object_emit_signal(globalconf.L, didx, "property::x", 0);
     if (old.y != geom.y)

objects/drawable.h

@@ -32,6 +32,8 @@ typedef void drawable_refresh_callback(void *);
 struct drawable_t
 {
     LUA_OBJECT_HEADER
+    /** The pixmap we are drawing to. */
+    xcb_pixmap_t pixmap;
     /** Surface for drawing. */
     cairo_surface_t *surface;
     /** The geometry of the drawable (in root window coordinates). */
@@ -43,8 +45,6 @@ struct drawable_t
 };
 
 drawable_t *drawable_allocator(lua_State *, drawable_refresh_callback *, void *);
-void drawable_unset_surface(drawable_t *);
-void drawable_set_surface(drawable_t *, int, cairo_surface_t *, area_t);
 void drawable_set_geometry(drawable_t *, int, area_t);
 void drawable_class_setup(lua_State *);
 

objects/drawin.c

@@ -62,24 +62,14 @@ drawin_wipe(drawin_t *w)
     /* The drawin must already be unmapped, else it
      * couldn't be garbage collected -> no unmap needed */
     p_delete(&w->cursor);
-    drawable_unset_surface(w->drawable);
     if(w->window)
     {
-        /* Activate BMA */
-        client_ignore_enterleave_events();
         /* Make sure we don't accidentally kill the systray window */
         drawin_systray_kickout(w);
         xcb_destroy_window(globalconf.connection, w->window);
-        /* Deactivate BMA */
-        client_restore_enterleave_events();
         w->window = XCB_NONE;
     }
-    if(w->pixmap)
+    /* No unref needed because we are being garbage collected */
-    {
-        xcb_free_pixmap(globalconf.connection, w->pixmap);
-        w->pixmap = XCB_NONE;
-    }
-    luaA_object_unref_item(globalconf.L, -1, w->drawable);
     w->drawable = NULL;
 }
 
@@ -92,34 +82,13 @@ drawin_unref_simplified(drawin_t **item)
 static void
 drawin_update_drawing(drawin_t *w, int widx)
 {
-    /* Clean up old stuff */
-    luaA_object_push_item(globalconf.L, widx, w->drawable);
-    drawable_unset_surface(w->drawable);
-    if(w->pixmap)
-    {
-        xcb_free_pixmap(globalconf.connection, w->pixmap);
-        w->pixmap = XCB_NONE;
-    }
-
     /* If this drawin isn't visible, we don't need an up-to-date cairo surface
      * for it. (drawin_map() will later make sure we are called again) */
     if(!w->visible)
-    {
-        lua_pop(globalconf.L, 1);
         return;
-    }
 
-    /* Create a pixmap */
+    luaA_object_push_item(globalconf.L, widx, w->drawable);
-    xcb_screen_t *s = globalconf.screen;
+    drawable_set_geometry(w->drawable, -1, w->geometry);
-    w->pixmap = xcb_generate_id(globalconf.connection);
-    xcb_create_pixmap(globalconf.connection, globalconf.default_depth, w->pixmap, s->root,
-                      w->geometry.width, w->geometry.height);
-    /* and create a surface for that pixmap */
-    cairo_surface_t *surface = cairo_xcb_surface_create(globalconf.connection,
-                                                        w->pixmap, globalconf.visual,
-                                                        w->geometry.width, w->geometry.height);
-    drawable_set_surface(w->drawable, -1, surface, w->geometry);
-    cairo_surface_destroy(surface);
     lua_pop(globalconf.L, 1);
 }
 
@@ -208,12 +177,12 @@ drawin_refresh_pixmap_partial(drawin_t *drawin,
                               int16_t x, int16_t y,
                               uint16_t w, uint16_t h)
 {
-    if (!drawin->visible)
+    if (!drawin->drawable || !drawin->drawable->pixmap)
         return;
 
     /* Make cairo do all pending drawing */
     cairo_surface_flush(drawin->drawable->surface);
-    xcb_copy_area(globalconf.connection, drawin->pixmap,
+    xcb_copy_area(globalconf.connection, drawin->drawable->pixmap,
                   drawin->window, globalconf.gc, x, y, x, y,
                   w, h);
 }

objects/drawin.h

@@ -38,8 +38,6 @@ struct drawin_t
     bool visible;
     /** Cursor */
     char *cursor;
-    /** The pixmap for double buffering. */
-    xcb_pixmap_t pixmap;
     /** The drawable for this drawin. */
     drawable_t *drawable;
     /** The window geometry. */