From a42908204f86d7afcbcf0ebe9e7acb39576982bf Mon Sep 17 00:00:00 2001 From: Daniel Stone Date: Sat, 15 Jan 2022 02:12:32 +0000 Subject: [PATCH] weston_buffer: Separate buffer release from lifetime In the original conception, a weston_buffer_reference indicated that the underlying contents of the wl_buffer would or could be accessed, so wl_buffer.release must not be sent until the last reference was released, as the compositor may still use it. This meant that renderers or backends which copied the buffer content - such as the GL renderer with SHM buffers - could only send a buffer release event to the client by 'losing' the buffer reference altogether. The main side effect is that `weston-debug scene-graph` could not show any information at all about SHM buffers when using the GL renderer, but it also meant that renderers and backends grew increasingly exotic structures to cache information about the buffer. Now that we have an additional buffer-reference mode (still referring to the weston_buffer/wl_buffer, but not going to access its content), we can allow the weston_buffer_reference and weston_buffer to live as long as the buffer itself, even if we do send a release event. This will enable a bunch of backend and renderer deduplication, as well as finally making scene-graph more useful. Signed-off-by: Daniel Stone --- include/libweston/libweston.h | 1 + libweston/compositor.c | 54 ++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/include/libweston/libweston.h b/include/libweston/libweston.h index 2f60dc8e..bfa592d0 100644 --- a/include/libweston/libweston.h +++ b/include/libweston/libweston.h @@ -1191,6 +1191,7 @@ struct weston_buffer { int32_t width, height; uint32_t busy_count; + uint32_t passive_count; enum { ORIGIN_TOP_LEFT, /* buffer content starts at (0,0) */ ORIGIN_BOTTOM_LEFT, /* buffer content starts at (0, height) */ diff --git a/libweston/compositor.c b/libweston/compositor.c index 62eff5f1..179fe5be 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -2389,7 +2389,7 @@ weston_buffer_destroy_handler(struct wl_listener *listener, void *data) buffer->resource = NULL; buffer->shm_buffer = NULL; - if (buffer->busy_count > 0) + if (buffer->busy_count + buffer->passive_count > 0) return; weston_signal_emit_mutable(&buffer->destroy_signal, buffer); @@ -2476,28 +2476,56 @@ weston_buffer_reference(struct weston_buffer_reference *ref, struct weston_buffer *buffer, enum weston_buffer_reference_type type) { + struct weston_buffer_reference old_ref = *ref; + assert(buffer != NULL || type == BUFFER_WILL_NOT_BE_ACCESSED); - if (buffer == ref->buffer) + if (buffer == ref->buffer && type == ref->type) return; - if (ref->buffer && --ref->buffer->busy_count == 0) { - if (ref->buffer->resource) { - assert(wl_resource_get_client(ref->buffer->resource)); - wl_buffer_send_release(ref->buffer->resource); - } else { - weston_signal_emit_mutable(&ref->buffer->destroy_signal, - ref->buffer); - free(ref->buffer); - } + /* First ref the incoming buffer, so we keep positive refcount */ + if (buffer) { + if (type == BUFFER_MAY_BE_ACCESSED) + buffer->busy_count++; + else + buffer->passive_count++; } ref->buffer = buffer; + ref->type = type; - if (!ref->buffer) + /* Now drop refs to the old buffer, if any */ + if (!old_ref.buffer) return; - ref->buffer->busy_count++; + ref = NULL; /* will no longer be accessed */ + + if (old_ref.type == BUFFER_MAY_BE_ACCESSED) { + assert(old_ref.buffer->busy_count > 0); + old_ref.buffer->busy_count--; + + /* If the wl_buffer lives, then hold on to the weston_buffer, + * but send a release event to the client */ + if (old_ref.buffer->busy_count == 0 && + old_ref.buffer->resource) { + assert(wl_resource_get_client(old_ref.buffer->resource)); + wl_buffer_send_release(old_ref.buffer->resource); + } + } else if (old_ref.type == BUFFER_WILL_NOT_BE_ACCESSED) { + assert(old_ref.buffer->passive_count > 0); + old_ref.buffer->passive_count--; + } else { + assert(!"unknown buffer ref type"); + } + + /* If the wl_buffer has gone and this was the last ref, destroy the + * weston_buffer, since we'll never need it again */ + if (old_ref.buffer->busy_count + old_ref.buffer->passive_count == 0 && + !old_ref.buffer->resource) { + weston_signal_emit_mutable(&old_ref.buffer->destroy_signal, + old_ref.buffer); + free(old_ref.buffer); + } } static void