From 22cdd9d923fc2a2a35c31b7b0ee462b08d7ea0e1 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 16 Mar 2014 17:08:36 +0100 Subject: [PATCH] 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 --- objects/client.c | 42 ++++++------------------------------ objects/client.h | 2 -- objects/drawable.c | 53 +++++++++++++++++++++++----------------------- objects/drawable.h | 4 ++-- objects/drawin.c | 41 +++++------------------------------ objects/drawin.h | 2 -- 6 files changed, 39 insertions(+), 105 deletions(-) diff --git a/objects/client.c b/objects/client.c index fced0875..9a452235 100644 --- a/objects/client.c +++ b/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; - - 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); + 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); - - /* 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); + luaA_object_unref_item(globalconf.L, -1, 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); } diff --git a/objects/client.h b/objects/client.h index 8d7c09b3..ceec91e9 100644 --- a/objects/client.h +++ b/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]; diff --git a/objects/drawable.c b/objects/drawable.c index 84a032e7..88220554 100644 --- a/objects/drawable.c +++ b/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) diff --git a/objects/drawable.h b/objects/drawable.h index 732fe7e7..78dd97c3 100644 --- a/objects/drawable.h +++ b/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 *); diff --git a/objects/drawin.c b/objects/drawin.c index 290c3bee..7e82fc0c 100644 --- a/objects/drawin.c +++ b/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) - { - xcb_free_pixmap(globalconf.connection, w->pixmap); - w->pixmap = XCB_NONE; - } - luaA_object_unref_item(globalconf.L, -1, w->drawable); + /* No unref needed because we are being garbage collected */ 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 */ - xcb_screen_t *s = globalconf.screen; - 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); + luaA_object_push_item(globalconf.L, widx, w->drawable); + drawable_set_geometry(w->drawable, -1, w->geometry); 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); } diff --git a/objects/drawin.h b/objects/drawin.h index 412bc8fd..c2a7608a 100644 --- a/objects/drawin.h +++ b/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. */