From 6595db64099fa95ec1b43dd2f137e0de58da9e41 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Fri, 28 Feb 2020 13:25:05 +0100 Subject: [PATCH] buffer: add a release event Consumers call wlr_buffer_lock. Once all consumers are done with the buffer, only the producer should have a reference to the buffer. In this case, we can release the buffer (and let the producer re-use it). --- backend/drm/drm.c | 10 ++--- backend/wayland/output.c | 4 +- include/wlr/types/wlr_buffer.h | 41 ++++++++++++++++---- types/wlr_buffer.c | 68 ++++++++++++++++++++++++++-------- types/wlr_output.c | 4 +- types/wlr_surface.c | 8 ++-- 6 files changed, 98 insertions(+), 37 deletions(-) diff --git a/backend/drm/drm.c b/backend/drm/drm.c index b62de849..d845a888 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -404,8 +404,8 @@ static bool drm_connector_commit_buffer(struct wlr_output *output) { conn->pageflip_pending = true; if (output->pending.buffer_type == WLR_OUTPUT_STATE_BUFFER_SCANOUT) { - wlr_buffer_unref(conn->pending_buffer); - conn->pending_buffer = wlr_buffer_ref(output->pending.buffer); + wlr_buffer_unlock(conn->pending_buffer); + conn->pending_buffer = wlr_buffer_lock(output->pending.buffer); } wlr_output_update_enabled(output, true); @@ -1539,7 +1539,7 @@ static void page_flip_handler(int fd, unsigned seq, // Release the old buffer as it's not displayed anymore. The pending // buffer becomes the current buffer. - wlr_buffer_unref(conn->current_buffer); + wlr_buffer_unlock(conn->current_buffer); conn->current_buffer = conn->pending_buffer; conn->pending_buffer = NULL; @@ -1660,8 +1660,8 @@ static void drm_connector_cleanup(struct wlr_drm_connector *conn) { conn->output.needs_frame = false; conn->output.frame_pending = false; - wlr_buffer_unref(conn->pending_buffer); - wlr_buffer_unref(conn->current_buffer); + wlr_buffer_unlock(conn->pending_buffer); + wlr_buffer_unlock(conn->current_buffer); conn->pending_buffer = conn->current_buffer = NULL; /* Fallthrough */ diff --git a/backend/wayland/output.c b/backend/wayland/output.c index 2c80c77f..1f00209b 100644 --- a/backend/wayland/output.c +++ b/backend/wayland/output.c @@ -114,7 +114,7 @@ static void destroy_wl_buffer(struct wlr_wl_buffer *buffer) { return; } wl_buffer_destroy(buffer->wl_buffer); - wlr_buffer_unref(buffer->buffer); + wlr_buffer_unlock(buffer->buffer); free(buffer); } @@ -173,7 +173,7 @@ static struct wlr_wl_buffer *create_wl_buffer(struct wlr_wl_backend *wl, return NULL; } buffer->wl_buffer = wl_buffer; - buffer->buffer = wlr_buffer_ref(wlr_buffer); + buffer->buffer = wlr_buffer_lock(wlr_buffer); wl_buffer_add_listener(wl_buffer, &buffer_listener, buffer); diff --git a/include/wlr/types/wlr_buffer.h b/include/wlr/types/wlr_buffer.h index d6a98f4b..fb308e61 100644 --- a/include/wlr/types/wlr_buffer.h +++ b/include/wlr/types/wlr_buffer.h @@ -21,27 +21,49 @@ struct wlr_buffer_impl { struct wlr_dmabuf_attributes *attribs); }; +/** + * A buffer containing pixel data. + * + * A buffer has a single producer (the party who created the buffer) and + * multiple consumers (parties reading the buffer). When all consumers are done + * with the buffer, it gets released and can be re-used by the producer. When + * the producer and all consumers are done with the buffer, it gets destroyed. + */ struct wlr_buffer { const struct wlr_buffer_impl *impl; - size_t n_refs; + bool dropped; + size_t n_locks; struct { struct wl_signal destroy; + struct wl_signal release; } events; }; +/** + * Initialize a buffer. This function should be called by producers. The + * initialized buffer is referenced: once the producer is done with the buffer + * they should call wlr_buffer_drop. + */ void wlr_buffer_init(struct wlr_buffer *buffer, const struct wlr_buffer_impl *impl); /** - * Reference the buffer. + * Unreference the buffer. This function should be called by producers when + * they are done with the buffer. */ -struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer); +void wlr_buffer_drop(struct wlr_buffer *buffer); /** - * Unreference the buffer. After this call, `buffer` may not be accessed - * anymore. + * Lock the buffer. This function should be called by consumers to make + * sure the buffer can be safely read from. Once the consumer is done with the + * buffer, they should call wlr_buffer_unlock. */ -void wlr_buffer_unref(struct wlr_buffer *buffer); +struct wlr_buffer *wlr_buffer_lock(struct wlr_buffer *buffer); +/** + * Unlock the buffer. This function should be called by consumers once they are + * done with the buffer. + */ +void wlr_buffer_unlock(struct wlr_buffer *buffer); /** * Reads the DMA-BUF attributes of the buffer. If this buffer isn't a DMA-BUF, * returns false. @@ -70,6 +92,7 @@ struct wlr_client_buffer { struct wlr_texture *texture; struct wl_listener resource_destroy; + struct wl_listener release; }; struct wlr_renderer; @@ -84,9 +107,11 @@ bool wlr_resource_is_buffer(struct wl_resource *resource); bool wlr_resource_get_buffer_size(struct wl_resource *resource, struct wlr_renderer *renderer, int *width, int *height); /** - * Upload a buffer to the GPU and reference it. + * Import a client buffer and lock it. + * + * Once the caller is done with the buffer, they must call wlr_buffer_unlock. */ -struct wlr_client_buffer *wlr_client_buffer_create( +struct wlr_client_buffer *wlr_client_buffer_import( struct wlr_renderer *renderer, struct wl_resource *resource); /** * Try to update the buffer's content. On success, returns the updated buffer diff --git a/types/wlr_buffer.c b/types/wlr_buffer.c index 3d6ae719..1b0f53d8 100644 --- a/types/wlr_buffer.c +++ b/types/wlr_buffer.c @@ -10,23 +10,12 @@ void wlr_buffer_init(struct wlr_buffer *buffer, const struct wlr_buffer_impl *impl) { assert(impl->destroy); buffer->impl = impl; - buffer->n_refs = 1; wl_signal_init(&buffer->events.destroy); + wl_signal_init(&buffer->events.release); } -struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer) { - buffer->n_refs++; - return buffer; -} - -void wlr_buffer_unref(struct wlr_buffer *buffer) { - if (buffer == NULL) { - return; - } - - assert(buffer->n_refs > 0); - buffer->n_refs--; - if (buffer->n_refs > 0) { +static void buffer_consider_destroy(struct wlr_buffer *buffer) { + if (!buffer->dropped || buffer->n_locks > 0) { return; } @@ -35,6 +24,36 @@ void wlr_buffer_unref(struct wlr_buffer *buffer) { buffer->impl->destroy(buffer); } +void wlr_buffer_drop(struct wlr_buffer *buffer) { + if (buffer == NULL) { + return; + } + + assert(!buffer->dropped); + buffer->dropped = true; + buffer_consider_destroy(buffer); +} + +struct wlr_buffer *wlr_buffer_lock(struct wlr_buffer *buffer) { + buffer->n_locks++; + return buffer; +} + +void wlr_buffer_unlock(struct wlr_buffer *buffer) { + if (buffer == NULL) { + return; + } + + assert(buffer->n_locks > 0); + buffer->n_locks--; + + if (buffer->n_locks == 0) { + wl_signal_emit(&buffer->events.release, NULL); + } + + buffer_consider_destroy(buffer); +} + bool wlr_buffer_get_dmabuf(struct wlr_buffer *buffer, struct wlr_dmabuf_attributes *attribs) { if (!buffer->impl->get_dmabuf) { @@ -134,7 +153,17 @@ static void client_buffer_resource_handle_destroy(struct wl_listener *listener, // which case we'll read garbage. We decide to accept this risk. } -struct wlr_client_buffer *wlr_client_buffer_create( +static void client_buffer_handle_release(struct wl_listener *listener, + void *data) { + struct wlr_client_buffer *buffer = + wl_container_of(listener, buffer, release); + if (!buffer->resource_released && buffer->resource != NULL) { + wl_buffer_send_release(buffer->resource); + buffer->resource_released = true; + } +} + +struct wlr_client_buffer *wlr_client_buffer_import( struct wlr_renderer *renderer, struct wl_resource *resource) { assert(wlr_resource_is_buffer(resource)); @@ -198,6 +227,13 @@ struct wlr_client_buffer *wlr_client_buffer_create( wl_resource_add_destroy_listener(resource, &buffer->resource_destroy); buffer->resource_destroy.notify = client_buffer_resource_handle_destroy; + buffer->release.notify = client_buffer_handle_release; + wl_signal_add(&buffer->base.events.release, &buffer->release); + + // Ensure the buffer will be released before being destroyed + wlr_buffer_lock(&buffer->base); + wlr_buffer_drop(&buffer->base); + return buffer; } @@ -206,7 +242,7 @@ struct wlr_client_buffer *wlr_client_buffer_apply_damage( pixman_region32_t *damage) { assert(wlr_resource_is_buffer(resource)); - if (buffer->base.n_refs > 1) { + if (buffer->base.n_locks > 1) { // Someone else still has a reference to the buffer return NULL; } diff --git a/types/wlr_output.c b/types/wlr_output.c index 4855fa4b..1692a2a0 100644 --- a/types/wlr_output.c +++ b/types/wlr_output.c @@ -429,7 +429,7 @@ static void output_state_clear_buffer(struct wlr_output_state *state) { return; } - wlr_buffer_unref(state->buffer); + wlr_buffer_unlock(state->buffer); state->buffer = NULL; state->committed &= ~WLR_OUTPUT_STATE_BUFFER; @@ -601,7 +601,7 @@ bool wlr_output_attach_buffer(struct wlr_output *output, output_state_clear_buffer(&output->pending); output->pending.committed |= WLR_OUTPUT_STATE_BUFFER; output->pending.buffer_type = WLR_OUTPUT_STATE_BUFFER_SCANOUT; - output->pending.buffer = wlr_buffer_ref(buffer); + output->pending.buffer = wlr_buffer_lock(buffer); return true; } diff --git a/types/wlr_surface.c b/types/wlr_surface.c index 0b2e8c09..5cf119a8 100644 --- a/types/wlr_surface.c +++ b/types/wlr_surface.c @@ -283,7 +283,7 @@ static void surface_apply_damage(struct wlr_surface *surface) { if (resource == NULL) { // NULL commit if (surface->buffer != NULL) { - wlr_buffer_unref(&surface->buffer->base); + wlr_buffer_unlock(&surface->buffer->base); } surface->buffer = NULL; return; @@ -300,14 +300,14 @@ static void surface_apply_damage(struct wlr_surface *surface) { } struct wlr_client_buffer *buffer = - wlr_client_buffer_create(surface->renderer, resource); + wlr_client_buffer_import(surface->renderer, resource); if (buffer == NULL) { wlr_log(WLR_ERROR, "Failed to upload buffer"); return; } if (surface->buffer != NULL) { - wlr_buffer_unref(&surface->buffer->base); + wlr_buffer_unlock(&surface->buffer->base); } surface->buffer = buffer; } @@ -580,7 +580,7 @@ static void surface_handle_resource_destroy(struct wl_resource *resource) { pixman_region32_fini(&surface->opaque_region); pixman_region32_fini(&surface->input_region); if (surface->buffer != NULL) { - wlr_buffer_unref(&surface->buffer->base); + wlr_buffer_unlock(&surface->buffer->base); } free(surface); }