compositor: introduce weston_buffer_reference

The wl_buffer reference counting API has been inconsistent. You would
manually increment the refcount and register a destroy listener, as
opposed to calling weston_buffer_post_release(), which internally
decremented the refcount, and then removing a list item.

Replace both cases with a single function:
weston_buffer_reference(weston_buffer_reference *ref, wl_buffer *buffer)

Buffer is assigned to ref->buffer, while taking care of all the refcounting
and release posting. You take a reference by passing a non-NULL buffer, and
release a reference by passing NULL as buffer. The function uses an
internal wl_buffer destroy listener, so the pointer gets reset on
destruction automatically.

This is inspired by the pipe_resource_reference() of Mesa, and modified
by krh's suggestion to add struct weston_buffer_reference.

Additionally, when a surface gets destroyed, the associated wl_buffer
will send a release event. Often the buffer is already destroyed on
client side, so the event will be discarded by libwayland-client.

Compositor-drm.c is converted to use weston_buffer_reference.

Signed-off-by: Pekka Paalanen <ppaalanen@gmail.com>

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 27f2c7d..da36cd3 100644 (file)
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -118,8 +118,7 @@ struct drm_fb {
        struct drm_output *output;
        uint32_t fb_id;
        int is_client_buffer;
-       struct wl_buffer *buffer;
-       struct wl_listener buffer_destroy_listener;
+       struct weston_buffer_reference buffer_ref;
 };
 
 struct drm_output {
@@ -208,10 +207,7 @@ drm_fb_destroy_callback(struct gbm_bo *bo, void *data)
        if (fb->fb_id)
                drmModeRmFB(gbm_device_get_fd(gbm), fb->fb_id);
 
-       if (fb->buffer) {
-               weston_buffer_post_release(fb->buffer);
-               wl_list_remove(&fb->buffer_destroy_listener.link);
-       }
+       weston_buffer_reference(&fb->buffer_ref, NULL);
 
        free(data);
 }
@@ -231,7 +227,7 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_compositor *compositor)
 
        fb->bo = bo;
        fb->is_client_buffer = 0;
-       fb->buffer = NULL;
+       fb->buffer_ref.buffer = NULL;
 
        width = gbm_bo_get_width(bo);
        height = gbm_bo_get_height(bo);
@@ -283,26 +279,13 @@ err_free:
 }
 
 static void
-fb_handle_buffer_destroy(struct wl_listener *listener, void *data)
-{
-       struct drm_fb *fb = container_of(listener, struct drm_fb,
-                                        buffer_destroy_listener);
-
-       fb->buffer = NULL;
-}
-
-static void
 drm_fb_set_buffer(struct drm_fb *fb, struct wl_buffer *buffer)
 {
-       assert(fb->buffer == NULL);
+       assert(fb->buffer_ref.buffer == NULL);
 
        fb->is_client_buffer = 1;
-       fb->buffer = buffer;
-       fb->buffer->busy_count++;
-       fb->buffer_destroy_listener.notify = fb_handle_buffer_destroy;
 
-       wl_signal_add(&fb->buffer->resource.destroy_signal,
-                     &fb->buffer_destroy_listener);
+       weston_buffer_reference(&fb->buffer_ref, buffer);
 }
 
 static int
@@ -340,19 +323,20 @@ drm_output_prepare_scanout_surface(struct weston_output *_output,
        struct drm_output *output = (struct drm_output *) _output;
        struct drm_compositor *c =
                (struct drm_compositor *) output->base.compositor;
+       struct wl_buffer *buffer = es->buffer_ref.buffer;
        struct gbm_bo *bo;
 
        if (es->geometry.x != output->base.x ||
            es->geometry.y != output->base.y ||
-           es->buffer == NULL ||
-           es->buffer->width != output->base.current->width ||
-           es->buffer->height != output->base.current->height ||
+           buffer == NULL ||
+           buffer->width != output->base.current->width ||
+           buffer->height != output->base.current->height ||
            output->base.transform != es->buffer_transform ||
            es->transform.enabled)
                return NULL;
 
        bo = gbm_bo_import(c->gbm, GBM_BO_IMPORT_WL_BUFFER,
-                          es->buffer, GBM_BO_USE_SCANOUT);
+                          buffer, GBM_BO_USE_SCANOUT);
 
        /* Unable to use the buffer for scanout */
        if (!bo)
@@ -369,7 +353,7 @@ drm_output_prepare_scanout_surface(struct weston_output *_output,
                return NULL;
        }
 
-       drm_fb_set_buffer(output->next, es->buffer);
+       drm_fb_set_buffer(output->next, buffer);
 
        return &output->fb_plane;
 }
@@ -601,13 +585,13 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base,
        if (es->output_mask != (1u << output_base->id))
                return NULL;
 
-       if (es->buffer == NULL)
+       if (es->buffer_ref.buffer == NULL)
                return NULL;
 
        if (es->alpha != 1.0f)
                return NULL;
 
-       if (wl_buffer_is_shm(es->buffer))
+       if (wl_buffer_is_shm(es->buffer_ref.buffer))
                return NULL;
 
        if (!drm_surface_transform_supported(es))
@@ -628,7 +612,7 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base,
                return NULL;
 
        bo = gbm_bo_import(c->gbm, GBM_BO_IMPORT_WL_BUFFER,
-                          es->buffer, GBM_BO_USE_SCANOUT);
+                          es->buffer_ref.buffer, GBM_BO_USE_SCANOUT);
        if (!bo)
                return NULL;
 
@@ -645,7 +629,7 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base,
                return NULL;
        }
 
-       drm_fb_set_buffer(s->next, es->buffer);
+       drm_fb_set_buffer(s->next, es->buffer_ref.buffer);
 
        box = pixman_region32_extents(&es->transform.boundingbox);
        s->plane.x = box->x1;
@@ -715,7 +699,8 @@ drm_output_prepare_cursor_surface(struct weston_output *output_base,
                return NULL;
        if (c->cursors_are_broken)
                return NULL;
-       if (es->buffer == NULL || !wl_buffer_is_shm(es->buffer) ||
+       if (es->buffer_ref.buffer == NULL ||
+           !wl_buffer_is_shm(es->buffer_ref.buffer) ||
            es->geometry.width > 64 || es->geometry.height > 64)
                return NULL;
 
@@ -742,14 +727,15 @@ drm_output_set_cursor(struct drm_output *output)
                return;
        }
 
-       if (es->buffer && pixman_region32_not_empty(&output->cursor_plane.damage)) {
+       if (es->buffer_ref.buffer &&
+           pixman_region32_not_empty(&output->cursor_plane.damage)) {
                pixman_region32_fini(&output->cursor_plane.damage);
                pixman_region32_init(&output->cursor_plane.damage);
                output->current_cursor ^= 1;
                bo = output->cursor_bo[output->current_cursor];
                memset(buf, 0, sizeof buf);
-               stride = wl_shm_buffer_get_stride(es->buffer);
-               s = wl_shm_buffer_get_data(es->buffer);
+               stride = wl_shm_buffer_get_stride(es->buffer_ref.buffer);
+               s = wl_shm_buffer_get_data(es->buffer_ref.buffer);
                for (i = 0; i < es->geometry.height; i++)
                        memcpy(buf + i * 64, s + i * stride,
                               es->geometry.width * 4);

diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
index 4621b72..e77c5c6 100644 (file)
--- a/src/compositor-rpi.c
+++ b/src/compositor-rpi.c
@@ -746,7 +746,8 @@ rpi_assign_plane(struct weston_surface *surface, struct rpi_output *output)
        }
 
        /* only shm surfaces supported */
-       if (surface->buffer && !wl_buffer_is_shm(surface->buffer)) {
+       if (surface->buffer_ref.buffer &&
+           !wl_buffer_is_shm(surface->buffer_ref.buffer)) {
                DBG("surface %p rejected: not shm\n", surface);
                return NULL;
        }
@@ -760,7 +761,7 @@ rpi_assign_plane(struct weston_surface *surface, struct rpi_output *output)
                element->plane.y = floor(surface->geometry.y);
                DBG("surface %p reuse element %p\n", surface, element);
        } else {
-               if (!surface->buffer) {
+               if (!surface->buffer_ref.buffer) {
                        DBG("surface %p rejected: no buffer\n", surface);
                        return NULL;
                }
@@ -817,7 +818,8 @@ rpi_output_assign_planes(struct weston_output *base)
                         * damage, if the plane is new, so no need to force
                         * initial resource update.
                         */
-                       if (rpi_element_damage(element, surface->buffer,
+                       if (rpi_element_damage(element,
+                                              surface->buffer_ref.buffer,
                                               &surface->damage) < 0) {
                                rpi_element_schedule_destroy(element);
                                DBG("surface %p rejected: resource update failed\n",

diff --git a/src/compositor.c b/src/compositor.c
index d80d929..869fffa 100644 (file)
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -181,16 +181,6 @@ weston_client_launch(struct weston_compositor *compositor,
 }
 
 static void
-surface_handle_buffer_destroy(struct wl_listener *listener, void *data)
-{
-       struct weston_surface *es =
-               container_of(listener, struct weston_surface, 
-                            buffer_destroy_listener);
-
-       es->buffer = NULL;
-}
-
-static void
 surface_handle_pending_buffer_destroy(struct wl_listener *listener, void *data)
 {
        struct weston_surface *surface =
@@ -241,7 +231,6 @@ weston_surface_create(struct weston_compositor *compositor)
 
        pixman_region32_init(&surface->texture_damage);
 
-       surface->buffer = NULL;
        surface->buffer_transform = WL_OUTPUT_TRANSFORM_NORMAL;
        surface->pending.buffer_transform = surface->buffer_transform;
        surface->output = NULL;
@@ -254,9 +243,6 @@ weston_surface_create(struct weston_compositor *compositor)
        pixman_region32_init(&surface->transform.opaque);
        wl_list_init(&surface->frame_callback_list);
 
-       surface->buffer_destroy_listener.notify =
-               surface_handle_buffer_destroy;
-
        wl_list_init(&surface->geometry.transformation_list);
        wl_list_insert(&surface->geometry.transformation_list,
                       &surface->transform.position.link);
@@ -731,9 +717,9 @@ weston_surface_buffer_width(struct weston_surface *surface)
        case WL_OUTPUT_TRANSFORM_270:
        case WL_OUTPUT_TRANSFORM_FLIPPED_90:
        case WL_OUTPUT_TRANSFORM_FLIPPED_270:
-               return surface->buffer->height;
+               return surface->buffer_ref.buffer->height;
        default:
-               return surface->buffer->width;
+               return surface->buffer_ref.buffer->width;
        }
 }
 
@@ -745,9 +731,9 @@ weston_surface_buffer_height(struct weston_surface *surface)
        case WL_OUTPUT_TRANSFORM_270:
        case WL_OUTPUT_TRANSFORM_FLIPPED_90:
        case WL_OUTPUT_TRANSFORM_FLIPPED_270:
-               return surface->buffer->width;
+               return surface->buffer_ref.buffer->width;
        default:
-               return surface->buffer->height;
+               return surface->buffer_ref.buffer->height;
        }
 }
 
@@ -877,8 +863,7 @@ destroy_surface(struct wl_resource *resource)
        if (surface->pending.buffer)
                wl_list_remove(&surface->pending.buffer_destroy_listener.link);
 
-       if (surface->buffer)
-               wl_list_remove(&surface->buffer_destroy_listener.link);
+       weston_buffer_reference(&surface->buffer_ref, NULL);
 
        pixman_region32_fini(&surface->texture_damage);
        compositor->renderer->destroy_surface(surface);
@@ -907,26 +892,47 @@ weston_surface_destroy(struct weston_surface *surface)
 }
 
 static void
-weston_surface_attach(struct weston_surface *surface, struct wl_buffer *buffer)
+weston_buffer_reference_handle_destroy(struct wl_listener *listener,
+                                      void *data)
+{
+       struct weston_buffer_reference *ref =
+               container_of(listener, struct weston_buffer_reference,
+                            destroy_listener);
+
+       assert((struct wl_buffer *)data == ref->buffer);
+       ref->buffer = NULL;
+}
+
+WL_EXPORT void
+weston_buffer_reference(struct weston_buffer_reference *ref,
+                       struct wl_buffer *buffer)
 {
-       if (surface->buffer && buffer != surface->buffer) {
-               weston_buffer_post_release(surface->buffer);
-               wl_list_remove(&surface->buffer_destroy_listener.link);
+       if (ref->buffer && buffer != ref->buffer) {
+               weston_buffer_post_release(ref->buffer);
+               wl_list_remove(&ref->destroy_listener.link);
        }
 
-       if (buffer && buffer != surface->buffer) {
+       if (buffer && buffer != ref->buffer) {
                buffer->busy_count++;
                wl_signal_add(&buffer->resource.destroy_signal,
-                             &surface->buffer_destroy_listener);
+                             &ref->destroy_listener);
        }
 
+       ref->buffer = buffer;
+       ref->destroy_listener.notify = weston_buffer_reference_handle_destroy;
+}
+
+static void
+weston_surface_attach(struct weston_surface *surface, struct wl_buffer *buffer)
+{
+       weston_buffer_reference(&surface->buffer_ref, buffer);
+
        if (!buffer) {
                if (weston_surface_is_mapped(surface))
                        weston_surface_unmap(surface);
        }
 
        surface->compositor->renderer->attach(surface, buffer);
-       surface->buffer = buffer;
 }
 
 WL_EXPORT void
@@ -1006,7 +1012,8 @@ static void
 surface_accumulate_damage(struct weston_surface *surface,
                          pixman_region32_t *opaque)
 {
-       if (surface->buffer && wl_buffer_is_shm(surface->buffer))
+       if (surface->buffer_ref.buffer &&
+           wl_buffer_is_shm(surface->buffer_ref.buffer))
                surface->compositor->renderer->flush_damage(surface);
 
        if (surface->transform.enabled) {
@@ -1243,6 +1250,8 @@ surface_attach(struct wl_client *client,
        if (buffer_resource)
                buffer = buffer_resource->data;
 
+       /* Attach, attach, without commit in between does not send
+        * wl_buffer.release. */
        if (surface->pending.buffer)
                wl_list_remove(&surface->pending.buffer_destroy_listener.link);
 
@@ -1380,7 +1389,7 @@ surface_commit(struct wl_client *client, struct wl_resource *resource)
        if (surface->pending.buffer || surface->pending.remove_contents)
                weston_surface_attach(surface, surface->pending.buffer);
 
-       if (surface->buffer && surface->configure)
+       if (surface->buffer_ref.buffer && surface->configure)
                surface->configure(surface, surface->pending.sx,
                                   surface->pending.sy);
        surface->pending.sx = 0;
@@ -2126,7 +2135,8 @@ pointer_cursor_surface_configure(struct weston_surface *es,
        y = wl_fixed_to_int(seat->seat.pointer->y) - seat->hotspot_y;
 
        weston_surface_configure(seat->sprite, x, y,
-                                es->buffer->width, es->buffer->height);
+                                es->buffer_ref.buffer->width,
+                                es->buffer_ref.buffer->height);
 
        empty_region(&es->pending.input);
 
@@ -2192,7 +2202,7 @@ pointer_set_cursor(struct wl_client *client, struct wl_resource *resource,
        seat->hotspot_x = x;
        seat->hotspot_y = y;
 
-       if (surface->buffer)
+       if (surface->buffer_ref.buffer)
                pointer_cursor_surface_configure(surface, 0, 0);
 }
 
@@ -2567,7 +2577,8 @@ drag_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy)
 
        weston_surface_configure(es,
                                 es->geometry.x + sx, es->geometry.y + sy,
-                                es->buffer->width, es->buffer->height);
+                                es->buffer_ref.buffer->width,
+                                es->buffer_ref.buffer->height);
 }
 
 static int
@@ -2615,7 +2626,7 @@ device_map_drag_surface(struct weston_seat *seat)
        struct wl_list *list;
 
        if (weston_surface_is_mapped(seat->drag_surface) ||
-           !seat->drag_surface->buffer)
+           !seat->drag_surface->buffer_ref.buffer)
                return;
 
        if (seat->sprite && weston_surface_is_mapped(seat->sprite))

diff --git a/src/compositor.h b/src/compositor.h
index ff7c932..53f9cfb 100644 (file)
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -345,6 +345,11 @@ struct weston_compositor {
        struct weston_xkb_info xkb_info;
 };
 
+struct weston_buffer_reference {
+       struct wl_buffer *buffer;
+       struct wl_listener destroy_listener;
+};
+
 struct weston_region {
        struct wl_resource resource;
        pixman_region32_t region;
@@ -437,8 +442,7 @@ struct weston_surface {
 
        struct wl_list frame_callback_list;
 
-       struct wl_buffer *buffer;
-       struct wl_listener buffer_destroy_listener;
+       struct weston_buffer_reference buffer_ref;
        uint32_t buffer_transform;
 
        /* All the pending state, that wl_surface.commit will apply. */
@@ -686,6 +690,10 @@ weston_surface_unmap(struct weston_surface *surface);
 void
 weston_buffer_post_release(struct wl_buffer *buffer);
 
+void
+weston_buffer_reference(struct weston_buffer_reference *ref,
+                       struct wl_buffer *buffer);
+
 uint32_t
 weston_compositor_get_time(void);
 

diff --git a/src/gl-renderer.c b/src/gl-renderer.c
index d459c21..f581482 100644 (file)
--- a/src/gl-renderer.c
+++ b/src/gl-renderer.c
@@ -1038,6 +1038,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
 {
        struct gl_renderer *gr = get_renderer(surface->compositor);
        struct gl_surface_state *gs = get_surface_state(surface);
+       struct wl_buffer *buffer = surface->buffer_ref.buffer;
 
 #ifdef GL_UNPACK_ROW_LENGTH
        pixman_box32_t *rectangles;
@@ -1061,9 +1062,9 @@ gl_renderer_flush_damage(struct weston_surface *surface)
 
        if (!gr->has_unpack_subimage) {
                glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT,
-                            surface->pitch, surface->buffer->height, 0,
+                            surface->pitch, buffer->height, 0,
                             GL_BGRA_EXT, GL_UNSIGNED_BYTE,
-                            wl_shm_buffer_get_data(surface->buffer));
+                            wl_shm_buffer_get_data(buffer));
 
                goto done;
        }
@@ -1071,7 +1072,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
 #ifdef GL_UNPACK_ROW_LENGTH
        /* Mesa does not define GL_EXT_unpack_subimage */
        glPixelStorei(GL_UNPACK_ROW_LENGTH, surface->pitch);
-       data = wl_shm_buffer_get_data(surface->buffer);
+       data = wl_shm_buffer_get_data(buffer);
        rectangles = pixman_region32_rectangles(&surface->texture_damage, &n);
        for (i = 0; i < n; i++) {
                pixman_box32_t r;

diff --git a/src/shell.c b/src/shell.c
index 28fe6cb..232d413 100644 (file)
--- a/src/shell.c
+++ b/src/shell.c
@@ -1667,7 +1667,7 @@ shell_configure_fullscreen(struct shell_surface *shsurf)
 
        switch (shsurf->fullscreen.type) {
        case WL_SHELL_SURFACE_FULLSCREEN_METHOD_DEFAULT:
-               if (surface->buffer)
+               if (surface->buffer_ref.buffer)
                        center_on_output(surface, shsurf->fullscreen_output);
                break;
        case WL_SHELL_SURFACE_FULLSCREEN_METHOD_SCALE:

diff --git a/src/tablet-shell.c b/src/tablet-shell.c
index 3623736..9e88e27 100644 (file)
--- a/src/tablet-shell.c
+++ b/src/tablet-shell.c
@@ -132,8 +132,8 @@ tablet_shell_surface_configure(struct weston_surface *surface,
        if (weston_surface_is_mapped(surface))
                return;
 
-       width = surface->buffer->width;
-       height = surface->buffer->height;
+       width = surface->buffer_ref.buffer->width;
+       height = surface->buffer_ref.buffer->height;
 
        weston_surface_configure(surface, 0, 0, width, height);