From 794318c1a07125bf2cb3bba53133f2a9c514bf01 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 6f96588b5..f721f2de4 100644 --- a/objects/client.c +++ b/objects/client.c @@ -811,30 +811,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); @@ -1188,18 +1165,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, @@ -1555,7 +1525,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? */ @@ -1565,7 +1535,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 712cbd3ed..5fd4fa04a 100644 --- a/objects/client.h +++ b/objects/client.h @@ -124,8 +124,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 84a032e7b..88220554f 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 732fe7e7b..78dd97c35 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 290c3beee..7e82fc0cd 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 412bc8fd1..c2a7608a6 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. */