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

diff --git a/include/buffer.h b/include/buffer.h
index a0ab30967d2be3e906e4af8b2eb4520773c47e50..67d494cea9b3a81282385ca0488a3637bd995423 100644 (file)
--- 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 0921acdd093c7a27879bc234cc3bf5fcf91235f0..56d9b046d273ddeacf240898f779ac2ad0f02cba 100644 (file)
--- 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 e917dec9803a393d100261fee42024effc739a93..ff984ce4a5563957eaf938c4896a55dd4e33d83e 100644 (file)
--- 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 0ecdd697204a47b657da97f1e4a65f8c6c8b2423..106e9a362fc7194c8f0d16b1545904f9e4701a3b 100644 (file)
--- 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 c93823780dba5e18723804c3f354e714be182c1b..844aeaef95c11f4384e9e60f381babd29f5156b3 100644 (file)
--- 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 7c018ace9ad0f37a3b07baef84807fde9223a965..e6fd49da1ffd36441082e55fecb5058f68888303 100644 (file)
--- 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 07e6a8a758657024eb7022b9270460ea6f419582..03357f36701d8c327ffeace88bfa9678d7b68180 100644 (file)
--- 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 12c91102cfd49895841f0fca4366afdc0ff1fc7a..498c1bd0932fc2cdd67ed07ac48aeed10dc0cb64 100644 (file)
--- 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 98d8f618f8a4ccdee170b93e1d6b7c48e716995a..af9c90ac6ba33a86a22a32bc9a58cc90a0687a1e 100644 (file)
--- 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 45ca0f48be6ae3a98d744c4bde000d65b522e541..d3a98a6ce6a6e6361521ef56aa6d12dc0894228a 100644 (file)
--- 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 7e2aedab848e845214c07ea552dde6debd2de5fb..1c3b85a7e78803d9bcd97d7153c58124fd4097fb 100644 (file)
--- 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 3e6227aaa06aa64a024b7a0dabc9f4757c6814fe..dfd18bd2f8cddea1ac1eb3e4bfffdd64e31379cb 100644 (file)
--- 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 87015ea014ea0087f2e00350ba6d8e3c5a8e6381..886ffa15fe577af994fa4172bc0f97eb60f4d2df 100644 (file)
--- 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 41336ce19b5736c512c4e96d6c5502026c7c8a02..96e19635551ef557a42d510c1db9f75bd1b5441d 100644 (file)
--- 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);