From d2b161bdf8122f0240075cfcc2c88b4fee8417d9 Mon Sep 17 00:00:00 2001 From: John Lindgren Date: Sun, 6 Oct 2024 01:42:54 -0400 Subject: [PATCH] buffer: reduce unnecessary painting to new cairo surfaces Add buffer_adopt_cairo_surface(), which allows wrapping an existing cairo image surface in a struct lab_data_buffer. This is useful when loading PNGs since most will be loaded as ARGB32 already. Fix a memory leak in the non-ARGB32 PNG case, where we do still need to paint to a new image surface -- we were leaking the original surface. Eliminate an unnecessary temporary image surface in SVG loading and just render the SVG to the image surface held by the lab_data_buffer. I also cleaned up and clarified the buffer API a bit: - Add a pointer to the held cairo_surface_t (so we can still access it if there is no cairo_t). - Remove the free_on_destroy bool (it was always true). - Rename unscaled_width/height to logical_width/height and add an explanatory comment. It was unclear what "unscaled" meant. - Rename buffer_create_wrap() to buffer_create_from_data(). This is laying groundwork for some more icon fixes I am working on (making sure icons are loaded and rendered at the correct scale). --- include/buffer.h | 44 +++++++++++++----- src/buffer.c | 76 ++++++++++++++++++-------------- src/common/font.c | 2 +- src/common/graphic-helpers.c | 4 +- src/common/scaled-font-buffer.c | 4 +- src/common/scaled-scene-buffer.c | 4 +- src/img/img-png.c | 18 ++++++-- src/img/img-svg.c | 17 +++---- src/img/img-xbm.c | 10 ++--- src/img/img-xpm.c | 2 +- src/osd.c | 2 +- src/ssd/ssd-shadow.c | 4 +- src/theme.c | 16 +++---- src/workspaces.c | 4 +- 14 files changed, 123 insertions(+), 84 deletions(-) diff --git a/include/buffer.h b/include/buffer.h index a0ab3096..67d494ce 100644 --- a/include/buffer.h +++ b/include/buffer.h @@ -32,21 +32,43 @@ struct lab_data_buffer { struct wlr_buffer base; - cairo_t *cairo; - void *data; + cairo_surface_t *surface; /* optional */ + cairo_t *cairo; /* optional */ + void *data; /* owned by surface if surface != NULL */ uint32_t format; size_t stride; - bool free_on_destroy; - uint32_t unscaled_width; - uint32_t unscaled_height; + /* + * The logical size of the surface in layout pixels. + * The raw pixel data may be larger or smaller. + */ + uint32_t logical_width; + uint32_t logical_height; }; -/* Create a buffer which creates a new cairo CAIRO_FORMAT_ARGB32 surface */ -struct lab_data_buffer *buffer_create_cairo(uint32_t width, uint32_t height, - float scale, bool free_on_destroy); +/* + * Create a buffer which holds (and takes ownership of) an existing + * CAIRO_FORMAT_ARGB32 image surface. + * + * The logical size is set to the surface size in pixels, ignoring + * device scale. No cairo context is created. + */ +struct lab_data_buffer *buffer_adopt_cairo_surface(cairo_surface_t *surface); -/* Create a buffer which wraps a given DRM_FORMAT_ARGB8888 pointer */ -struct lab_data_buffer *buffer_create_wrap(void *pixel_data, uint32_t width, - uint32_t height, uint32_t stride, bool free_on_destroy); +/* + * Create a buffer which holds a new CAIRO_FORMAT_ARGB32 image surface. + * Additionally create a cairo context for drawing to the surface. + */ +struct lab_data_buffer *buffer_create_cairo(uint32_t logical_width, + uint32_t logical_height, float scale); + +/* + * Create a buffer which holds (and takes ownership of) raw pixel data + * in pre-multiplied ARGB32 format. + * + * The logical size is set to the width and height of the pixel data. + * No cairo surface or context is created. + */ +struct lab_data_buffer *buffer_create_from_data(void *pixel_data, uint32_t width, + uint32_t height, uint32_t stride); #endif /* LABWC_BUFFER_H */ diff --git a/src/buffer.c b/src/buffer.c index 0921acdd..56d9b046 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -44,14 +44,12 @@ static void data_buffer_destroy(struct wlr_buffer *wlr_buffer) { struct lab_data_buffer *buffer = data_buffer_from_buffer(wlr_buffer); - if (!buffer->free_on_destroy) { - free(buffer); - return; - } if (buffer->cairo) { - cairo_surface_t *surf = cairo_get_target(buffer->cairo); cairo_destroy(buffer->cairo); - cairo_surface_destroy(surf); + } + if (buffer->surface) { + /* this also frees buffer->data */ + cairo_surface_destroy(buffer->surface); } else if (buffer->data) { free(buffer->data); buffer->data = NULL; @@ -85,19 +83,35 @@ static const struct wlr_buffer_impl data_buffer_impl = { }; struct lab_data_buffer * -buffer_create_cairo(uint32_t width, uint32_t height, float scale, - bool free_on_destroy) +buffer_adopt_cairo_surface(cairo_surface_t *surface) { - struct lab_data_buffer *buffer = znew(*buffer); - buffer->unscaled_width = width; - buffer->unscaled_height = height; - width *= scale; - height *= scale; + assert(cairo_surface_get_type(surface) == CAIRO_SURFACE_TYPE_IMAGE); + assert(cairo_image_surface_get_format(surface) == CAIRO_FORMAT_ARGB32); - /* Allocate the buffer with the scaled size */ + int width = cairo_image_surface_get_width(surface); + int height = cairo_image_surface_get_height(surface); + + struct lab_data_buffer *buffer = znew(*buffer); wlr_buffer_init(&buffer->base, &data_buffer_impl, width, height); - cairo_surface_t *surf = - cairo_image_surface_create(CAIRO_FORMAT_ARGB32, width, height); + + buffer->surface = surface; + buffer->data = cairo_image_surface_get_data(buffer->surface); + buffer->format = DRM_FORMAT_ARGB8888; + buffer->stride = cairo_image_surface_get_stride(buffer->surface); + buffer->logical_width = width; + buffer->logical_height = height; + + return buffer; +} + +struct lab_data_buffer * +buffer_create_cairo(uint32_t logical_width, uint32_t logical_height, float scale) +{ + /* Create an image surface with the scaled size */ + cairo_surface_t *surface = + cairo_image_surface_create(CAIRO_FORMAT_ARGB32, + lroundf(logical_width * scale), + lroundf(logical_height * scale)); /** * Tell cairo about the device scale so we can keep drawing in unscaled @@ -106,34 +120,30 @@ buffer_create_cairo(uint32_t width, uint32_t height, float scale, * * For a more complete explanation see PR #389 */ - cairo_surface_set_device_scale(surf, scale, scale); + cairo_surface_set_device_scale(surface, scale, scale); - buffer->cairo = cairo_create(surf); - buffer->data = cairo_image_surface_get_data(surf); - buffer->format = DRM_FORMAT_ARGB8888; - buffer->stride = cairo_image_surface_get_stride(surf); - buffer->free_on_destroy = free_on_destroy; + /* + * Adopt the image surface into a buffer, set the correct + * logical size, and create a cairo context for drawing + */ + struct lab_data_buffer *buffer = buffer_adopt_cairo_surface(surface); + buffer->logical_width = logical_width; + buffer->logical_height = logical_height; + buffer->cairo = cairo_create(surface); - if (!buffer->data) { - cairo_destroy(buffer->cairo); - cairo_surface_destroy(surf); - free(buffer); - buffer = NULL; - } return buffer; } struct lab_data_buffer * -buffer_create_wrap(void *pixel_data, uint32_t width, uint32_t height, - uint32_t stride, bool free_on_destroy) +buffer_create_from_data(void *pixel_data, uint32_t width, uint32_t height, + uint32_t stride) { struct lab_data_buffer *buffer = znew(*buffer); wlr_buffer_init(&buffer->base, &data_buffer_impl, width, height); - buffer->unscaled_width = width; - buffer->unscaled_height = height; + buffer->logical_width = width; + buffer->logical_height = height; buffer->data = pixel_data; buffer->format = DRM_FORMAT_ARGB8888; buffer->stride = stride; - buffer->free_on_destroy = free_on_destroy; return buffer; } diff --git a/src/common/font.c b/src/common/font.c index e917dec9..ff984ce4 100644 --- a/src/common/font.c +++ b/src/common/font.c @@ -109,7 +109,7 @@ font_buffer_create(struct lab_data_buffer **buffer, int max_width, } *buffer = buffer_create_cairo(text_extents.width + arrow_extents.width, - text_extents.height, scale, true); + text_extents.height, scale); if (!*buffer) { wlr_log(WLR_ERROR, "Failed to create font buffer"); return; diff --git a/src/common/graphic-helpers.c b/src/common/graphic-helpers.c index 0ecdd697..106e9a36 100644 --- a/src/common/graphic-helpers.c +++ b/src/common/graphic-helpers.c @@ -125,8 +125,8 @@ get_cairo_surface_from_lab_data_buffer(struct lab_data_buffer *buffer) } /* Handle DRM_FORMAT_ARGB8888 buffers */ - int w = buffer->unscaled_width; - int h = buffer->unscaled_height; + int w = buffer->logical_width; + int h = buffer->logical_height; cairo_surface_t *surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w, h); if (!surface) { diff --git a/src/common/scaled-font-buffer.c b/src/common/scaled-font-buffer.c index c9382378..844aeaef 100644 --- a/src/common/scaled-font-buffer.c +++ b/src/common/scaled-font-buffer.c @@ -25,8 +25,8 @@ _create_buffer(struct scaled_scene_buffer *scaled_buffer, double scale) wlr_log(WLR_ERROR, "font_buffer_create() failed"); } - self->width = buffer ? buffer->unscaled_width : 0; - self->height = buffer ? buffer->unscaled_height : 0; + self->width = buffer ? buffer->logical_width : 0; + self->height = buffer ? buffer->logical_height : 0; return buffer; } diff --git a/src/common/scaled-scene-buffer.c b/src/common/scaled-scene-buffer.c index 7c018ace..e6fd49da 100644 --- a/src/common/scaled-scene-buffer.c +++ b/src/common/scaled-scene-buffer.c @@ -72,8 +72,8 @@ _update_buffer(struct scaled_scene_buffer *self, double scale) /* Ensure the buffer doesn't get deleted behind our back */ wlr_buffer_lock(&buffer->base); } - self->width = buffer ? buffer->unscaled_width : 0; - self->height = buffer ? buffer->unscaled_height : 0; + self->width = buffer ? buffer->logical_width : 0; + self->height = buffer ? buffer->logical_height : 0; /* Create or reuse cache entry */ if (wl_list_length(&self->cache) < LAB_SCALED_BUFFER_MAX_CACHE) { diff --git a/src/img/img-png.c b/src/img/img-png.c index 07e6a8a7..03357f36 100644 --- a/src/img/img-png.c +++ b/src/img/img-png.c @@ -64,10 +64,20 @@ img_png_load(const char *filename, struct lab_data_buffer **buffer) } cairo_surface_flush(image); - double w = cairo_image_surface_get_width(image); - double h = cairo_image_surface_get_height(image); - *buffer = buffer_create_cairo((int)w, (int)h, 1.0, true); + if (cairo_image_surface_get_format(image) == CAIRO_FORMAT_ARGB32) { + /* we can wrap ARGB32 image surfaces directly */ + *buffer = buffer_adopt_cairo_surface(image); + return; + } + + /* convert to ARGB32 by painting to a new surface */ + uint32_t w = cairo_image_surface_get_width(image); + uint32_t h = cairo_image_surface_get_height(image); + *buffer = buffer_create_cairo(w, h, 1); cairo_t *cairo = (*buffer)->cairo; cairo_set_source_surface(cairo, image, 0, 0); - cairo_paint_with_alpha(cairo, 1.0); + cairo_paint(cairo); + cairo_surface_flush((*buffer)->surface); + /* destroy original surface */ + cairo_surface_destroy(image); } diff --git a/src/img/img-svg.c b/src/img/img-svg.c index 12c91102..498c1bd0 100644 --- a/src/img/img-svg.c +++ b/src/img/img-svg.c @@ -39,8 +39,9 @@ img_svg_load(const char *filename, struct lab_data_buffer **buffer, return; } - cairo_surface_t *image = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, size, size); - cairo_t *cr = cairo_create(image); + *buffer = buffer_create_cairo(size, size, 1.0); + cairo_surface_t *image = (*buffer)->surface; + cairo_t *cr = (*buffer)->cairo; rsvg_handle_render_document(svg, cr, &viewport, &err); if (err) { @@ -55,15 +56,11 @@ img_svg_load(const char *filename, struct lab_data_buffer **buffer, } cairo_surface_flush(image); - double w = cairo_image_surface_get_width(image); - double h = cairo_image_surface_get_height(image); - *buffer = buffer_create_cairo((int)w, (int)h, 1.0, /* free_on_destroy */ true); - cairo_t *cairo = (*buffer)->cairo; - cairo_set_source_surface(cairo, image, 0, 0); - cairo_paint_with_alpha(cairo, 1.0); + g_object_unref(svg); + return; error: - cairo_destroy(cr); - cairo_surface_destroy(image); + wlr_buffer_drop(&(*buffer)->base); + *buffer = NULL; g_object_unref(svg); } diff --git a/src/img/img-xbm.c b/src/img/img-xbm.c index 98d8f618..af9c90ac 100644 --- a/src/img/img-xbm.c +++ b/src/img/img-xbm.c @@ -266,8 +266,8 @@ img_xbm_from_bitmap(const char *bitmap, struct lab_data_buffer **buffer, } color = argb32(rgba); pixmap = parse_xbm_builtin(bitmap, 6); - *buffer = buffer_create_wrap(pixmap.data, pixmap.width, pixmap.height, - pixmap.width * 4, /* free_on_destroy */ true); + *buffer = buffer_create_from_data(pixmap.data, pixmap.width, pixmap.height, + pixmap.width * 4); } void @@ -298,9 +298,9 @@ img_xbm_load(const char *filename, struct lab_data_buffer **buffer, return; } - /* Create buffer with free_on_destroy being true */ + /* Create buffer */ if (pixmap.data) { - *buffer = buffer_create_wrap(pixmap.data, pixmap.width, - pixmap.height, pixmap.width * 4, true); + *buffer = buffer_create_from_data(pixmap.data, pixmap.width, + pixmap.height, pixmap.width * 4); } } diff --git a/src/img/img-xpm.c b/src/img/img-xpm.c index 45ca0f48..d3a98a6c 100644 --- a/src/img/img-xpm.c +++ b/src/img/img-xpm.c @@ -387,7 +387,7 @@ pixbuf_create_from_xpm(struct file_handle *handle) free(colors); free(name_buf); - return buffer_create_wrap(data, w, h, 4 * w, true); + return buffer_create_from_data(data, w, h, 4 * w); out: g_hash_table_destroy(color_hash); diff --git a/src/osd.c b/src/osd.c index 7e2aedab..1c3b85a7 100644 --- a/src/osd.c +++ b/src/osd.c @@ -351,7 +351,7 @@ display_osd(struct output *output, struct wl_array *views) if (output->osd_buffer) { wlr_buffer_drop(&output->osd_buffer->base); } - output->osd_buffer = buffer_create_cairo(w, h, scale, true); + output->osd_buffer = buffer_create_cairo(w, h, scale); if (!output->osd_buffer) { wlr_log(WLR_ERROR, "Failed to allocate cairo buffer for the window switcher"); return; diff --git a/src/ssd/ssd-shadow.c b/src/ssd/ssd-shadow.c index 3e6227aa..dfd18bd2 100644 --- a/src/ssd/ssd-shadow.c +++ b/src/ssd/ssd-shadow.c @@ -196,8 +196,8 @@ set_shadow_geometry(struct ssd *ssd) * is different). The buffers are square so width == height. */ int corner_size = active - ? theme->shadow_corner_top_active->unscaled_height - : theme->shadow_corner_top_inactive->unscaled_height; + ? theme->shadow_corner_top_active->logical_height + : theme->shadow_corner_top_inactive->logical_height; wl_list_for_each(part, &subtree->parts, link) { set_shadow_part_geometry(part, width, height, diff --git a/src/theme.c b/src/theme.c index 87015ea0..886ffa15 100644 --- a/src/theme.c +++ b/src/theme.c @@ -110,7 +110,7 @@ copy_icon_buffer(struct theme *theme, struct lab_data_buffer *icon_buffer) int buffer_width = (double)width * scale; int buffer_height = (double)height * scale; struct lab_data_buffer *buffer = buffer_create_cairo( - buffer_width, buffer_height, 1.0, true); + buffer_width, buffer_height, 1.0); cairo_t *cairo = buffer->cairo; cairo_set_source_surface(cairo, icon.surface, @@ -1087,7 +1087,7 @@ rounded_rect(struct rounded_corner_ctx *ctx) struct lab_data_buffer *buffer; /* TODO: scale */ - buffer = buffer_create_cairo(w, h, 1, /*free_on_destroy*/ true); + buffer = buffer_create_cairo(w, h, 1); cairo_t *cairo = buffer->cairo; cairo_surface_t *surf = cairo_get_target(cairo); @@ -1406,11 +1406,11 @@ create_shadows(struct theme *theme) */ if (visible_active_size > 0) { theme->shadow_edge_active = buffer_create_cairo( - visible_active_size, 1, 1.0, true); + visible_active_size, 1, 1.0); theme->shadow_corner_top_active = buffer_create_cairo( - total_active_size, total_active_size, 1.0, true); + total_active_size, total_active_size, 1.0); theme->shadow_corner_bottom_active = buffer_create_cairo( - total_active_size, total_active_size, 1.0, true); + total_active_size, total_active_size, 1.0); if (!theme->shadow_corner_top_active || !theme->shadow_corner_bottom_active || !theme->shadow_edge_active) { @@ -1420,11 +1420,11 @@ create_shadows(struct theme *theme) } if (visible_inactive_size > 0) { theme->shadow_edge_inactive = buffer_create_cairo( - visible_inactive_size, 1, 1.0, true); + visible_inactive_size, 1, 1.0); theme->shadow_corner_top_inactive = buffer_create_cairo( - total_inactive_size, total_inactive_size, 1.0, true); + total_inactive_size, total_inactive_size, 1.0); theme->shadow_corner_bottom_inactive = buffer_create_cairo( - total_inactive_size, total_inactive_size, 1.0, true); + total_inactive_size, total_inactive_size, 1.0); if (!theme->shadow_corner_top_inactive || !theme->shadow_corner_bottom_inactive || !theme->shadow_edge_inactive) { diff --git a/src/workspaces.c b/src/workspaces.c index 41336ce1..96e19635 100644 --- a/src/workspaces.c +++ b/src/workspaces.c @@ -84,7 +84,7 @@ _osd_update(struct server *server) continue; } struct lab_data_buffer *buffer = buffer_create_cairo(width, height, - output->wlr_output->scale, true); + output->wlr_output->scale); if (!buffer) { wlr_log(WLR_ERROR, "Failed to allocate buffer for workspace OSD"); continue; @@ -168,7 +168,7 @@ _osd_update(struct server *server) wlr_scene_node_set_position(&output->workspace_osd->node, lx, ly); wlr_scene_buffer_set_buffer(output->workspace_osd, &buffer->base); wlr_scene_buffer_set_dest_size(output->workspace_osd, - buffer->unscaled_width, buffer->unscaled_height); + buffer->logical_width, buffer->logical_height); /* And finally drop the buffer so it will get destroyed on OSD hide */ wlr_buffer_drop(&buffer->base);