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

6 files changed, 94 insertions, 33 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.
+ */
+void wlr_buffer_drop(struct wlr_buffer *buffer);
+/**
+ * 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.
  */
-struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer);
+struct wlr_buffer *wlr_buffer_lock(struct wlr_buffer *buffer);
 /**
- * Unreference the buffer. After this call, `buffer` may not be accessed
- * anymore.
+ * Unlock the buffer. This function should be called by consumers once they are
+ * done with the buffer.
  */
-void wlr_buffer_unref(struct wlr_buffer *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,29 +10,48 @@ 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;
+static void buffer_consider_destroy(struct wlr_buffer *buffer) {
+	if (!buffer->dropped || buffer->n_locks > 0) {
+		return;
+	}
+
+	wlr_signal_emit_safe(&buffer->events.destroy, NULL);
+
+	buffer->impl->destroy(buffer);
 }
 
-void wlr_buffer_unref(struct wlr_buffer *buffer) {
+void wlr_buffer_drop(struct wlr_buffer *buffer) {
 	if (buffer == NULL) {
 		return;
 	}
 
-	assert(buffer->n_refs > 0);
-	buffer->n_refs--;
-	if (buffer->n_refs > 0) {
+	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;
 	}
 
-	wlr_signal_emit_safe(&buffer->events.destroy, NULL);
+	assert(buffer->n_locks > 0);
+	buffer->n_locks--;
 
-	buffer->impl->destroy(buffer);
+	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,
@@ -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);
 }