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

@@ -32,21 +32,43 @@
 struct lab_data_buffer {
 	struct wlr_buffer base;
 
-	cairo_t *cairo;
+	cairo_surface_t *surface; /* optional */
-	void *data;
+	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;
+	 * The logical size of the surface in layout pixels.
-	uint32_t unscaled_height;
+	 * 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,
+ * Create a buffer which holds (and takes ownership of) an existing
-	float scale, bool free_on_destroy);
+ * 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,
+ * Create a buffer which holds a new CAIRO_FORMAT_ARGB32 image surface.
-	uint32_t height, uint32_t stride, bool free_on_destroy);
+ * 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 */

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,
+buffer_adopt_cairo_surface(cairo_surface_t *surface)
-	bool free_on_destroy)
 {
-	struct lab_data_buffer *buffer = znew(*buffer);
+	assert(cairo_surface_get_type(surface) == CAIRO_SURFACE_TYPE_IMAGE);
-	buffer->unscaled_width = width;
+	assert(cairo_image_surface_get_format(surface) == CAIRO_FORMAT_ARGB32);
-	buffer->unscaled_height = height;
-	width *= scale;
-	height *= scale;
 
-	/* 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);
+	 * Adopt the image surface into a buffer, set the correct
-	buffer->format = DRM_FORMAT_ARGB8888;
+	 * logical size, and create a cairo context for drawing
-	buffer->stride = cairo_image_surface_get_stride(surf);
+	 */
-	buffer->free_on_destroy = free_on_destroy;
+	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,
+buffer_create_from_data(void *pixel_data, uint32_t width, uint32_t height,
-	uint32_t stride, bool free_on_destroy)
+		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->logical_width = width;
-	buffer->unscaled_height = height;
+	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;
 }

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;

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 w = buffer->logical_width;
-	int h = buffer->unscaled_height;
+	int h = buffer->logical_height;
 	cairo_surface_t *surface =
 		cairo_image_surface_create(CAIRO_FORMAT_ARGB32, w, h);
 	if (!surface) {

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->width = buffer ? buffer->logical_width : 0;
-	self->height = buffer ? buffer->unscaled_height : 0;
+	self->height = buffer ? buffer->logical_height : 0;
 	return buffer;
 }
 

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->width = buffer ? buffer->logical_width : 0;
-	self->height = buffer ? buffer->unscaled_height : 0;
+	self->height = buffer ? buffer->logical_height : 0;
 
 	/* Create or reuse cache entry */
 	if (wl_list_length(&self->cache) < LAB_SCALED_BUFFER_MAX_CACHE) {

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);
+	if (cairo_image_surface_get_format(image) == CAIRO_FORMAT_ARGB32) {
-	double h = cairo_image_surface_get_height(image);
+		/* we can wrap ARGB32 image surfaces directly */
-	*buffer = buffer_create_cairo((int)w, (int)h, 1.0, true);
+		*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);
 }

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);
+	*buffer = buffer_create_cairo(size, size, 1.0);
-	cairo_t *cr = cairo_create(image);
+	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);
+	g_object_unref(svg);
-	double h = cairo_image_surface_get_height(image);
+	return;
-	*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);
 
 error:
-	cairo_destroy(cr);
+	wlr_buffer_drop(&(*buffer)->base);
-	cairo_surface_destroy(image);
+	*buffer = NULL;
 	g_object_unref(svg);
 }

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,
+	*buffer = buffer_create_from_data(pixmap.data, pixmap.width, pixmap.height,
-		pixmap.width * 4, /* free_on_destroy */ true);
+		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,
+		*buffer = buffer_create_from_data(pixmap.data, pixmap.width,
-			pixmap.height, pixmap.width * 4, true);
+			pixmap.height, pixmap.width * 4);
 	}
 }

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);

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;

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_active->logical_height
-			: theme->shadow_corner_top_inactive->unscaled_height;
+			: theme->shadow_corner_top_inactive->logical_height;
 
 		wl_list_for_each(part, &subtree->parts, link) {
 			set_shadow_part_geometry(part, width, height,

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) {

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);