Merge pull request #1062 from emersion/wlr-buffer-comeback

Add back wlr_buffer

include/wlr/types/wlr_buffer.h

@@ -0,0 +1,63 @@
+#ifndef WLR_TYPES_WLR_BUFFER_H
+#define WLR_TYPES_WLR_BUFFER_H
+
+#include <pixman.h>
+#include <wayland-server.h>
+
+/**
+ * A client buffer.
+ */
+struct wlr_buffer {
+	/**
+	 * The buffer resource, if any. Will be NULL if the client destroys it.
+	 */
+	struct wl_resource *resource;
+	/**
+	 * The buffer's texture, if any. A buffer will not have a texture if the
+	 * client destroys the buffer before it has been released.
+	 */
+	struct wlr_texture *texture;
+	bool released;
+	size_t n_refs;
+
+	struct wl_listener resource_destroy;
+};
+
+struct wlr_renderer;
+
+/**
+ * Check if a resource is a wl_buffer resource.
+ */
+bool wlr_resource_is_buffer(struct wl_resource *resource);
+/**
+ * Get the size of a wl_buffer resource.
+ */
+bool wlr_buffer_get_resource_size(struct wl_resource *resource,
+	struct wlr_renderer *renderer, int *width, int *height);
+
+/**
+ * Upload a buffer to the GPU and reference it.
+ */
+struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer,
+	struct wl_resource *resource);
+/**
+ * Reference the buffer.
+ */
+struct wlr_buffer *wlr_buffer_ref(struct wlr_buffer *buffer);
+/**
+ * Unreference the buffer. After this call, `buffer` may not be accessed
+ * anymore.
+ */
+void wlr_buffer_unref(struct wlr_buffer *buffer);
+/**
+ * Try to update the buffer's content. On success, returns the updated buffer
+ * and destroys the provided `buffer`. On error, `buffer` is intact and NULL is
+ * returned.
+ *
+ * Fails if there's more than one reference to the buffer or if the texture
+ * isn't mutable.
+ */
+struct wlr_buffer *wlr_buffer_apply_damage(struct wlr_buffer *buffer,
+	struct wl_resource *resource, pixman_region32_t *damage);
+
+#endif

include/wlr/types/wlr_surface.h

@@ -69,7 +69,13 @@ struct wlr_subsurface {
 struct wlr_surface {
 	struct wl_resource *resource;
 	struct wlr_renderer *renderer;
-	struct wlr_texture *texture;
+	/**
+	 * The surface's buffer, if any. A surface has an attached buffer when it
+	 * commits with a non-null buffer in its pending state. A surface will not
+	 * have a buffer if it has never committed one, has committed a null buffer,
+	 * or something went wrong with uploading the buffer.
+	 */
+	struct wlr_buffer *buffer;
 	struct wlr_surface_state *current, *pending;
 	const char *role; // the lifetime-bound role or null
 
@@ -124,6 +130,13 @@ int wlr_surface_set_role(struct wlr_surface *surface, const char *role,
  */
 bool wlr_surface_has_buffer(struct wlr_surface *surface);
 
+/**
+ * Get the texture of the buffer currently attached to this surface. Returns
+ * NULL if no buffer is currently attached or if something went wrong with
+ * uploading the buffer.
+ */
+struct wlr_texture *wlr_surface_get_texture(struct wlr_surface *surface);
+
 /**
  * Create a new subsurface resource with the provided new ID. If `resource_list`
  * is non-NULL, adds the subsurface's resource to the list.

rootston/output.c

@@ -189,7 +189,8 @@ static void render_surface(struct wlr_surface *surface, int sx, int sy,
 	struct roots_output *output = data->output;
 	float rotation = data->layout.rotation;
 
-	if (!wlr_surface_has_buffer(surface)) {
+	struct wlr_texture *texture = wlr_surface_get_texture(surface);
+	if (texture == NULL) {
 		return;
 	}
 
@@ -230,8 +231,7 @@ static void render_surface(struct wlr_surface *surface, int sx, int sy,
 	pixman_box32_t *rects = pixman_region32_rectangles(&damage, &nrects);
 	for (int i = 0; i < nrects; ++i) {
 		scissor_output(output, &rects[i]);
-		wlr_render_texture_with_matrix(renderer, surface->texture, matrix,
-			data->alpha);
+		wlr_render_texture_with_matrix(renderer, texture, matrix, data->alpha);
 	}
 
 damage_finish:

types/meson.build

@@ -20,6 +20,7 @@ lib_wlr_types = static_library(
 		'xdg_shell_v6/wlr_xdg_surface_v6.c',
 		'xdg_shell_v6/wlr_xdg_toplevel_v6.c',
 		'wlr_box.c',
+		'wlr_buffer.c',
 		'wlr_compositor.c',
 		'wlr_cursor.c',
 		'wlr_gamma_control.c',

types/wlr_buffer.c

@@ -0,0 +1,193 @@
+#include <assert.h>
+#include <stdlib.h>
+#include <wlr/types/wlr_buffer.h>
+#include <wlr/types/wlr_linux_dmabuf.h>
+#include <wlr/render/wlr_renderer.h>
+#include <wlr/util/log.h>
+
+bool wlr_resource_is_buffer(struct wl_resource *resource) {
+	return strcmp(wl_resource_get_class(resource), wl_buffer_interface.name) == 0;
+}
+
+bool wlr_buffer_get_resource_size(struct wl_resource *resource,
+		struct wlr_renderer *renderer, int *width, int *height) {
+	assert(wlr_resource_is_buffer(resource));
+
+	struct wl_shm_buffer *shm_buf = wl_shm_buffer_get(resource);
+	if (shm_buf != NULL) {
+		*width = wl_shm_buffer_get_width(shm_buf);
+		*height = wl_shm_buffer_get_height(shm_buf);
+	} else if (wlr_renderer_resource_is_wl_drm_buffer(renderer,
+			resource)) {
+		wlr_renderer_wl_drm_buffer_get_size(renderer, resource,
+			width, height);
+	} else if (wlr_dmabuf_resource_is_buffer(resource)) {
+		struct wlr_dmabuf_buffer *dmabuf =
+			wlr_dmabuf_buffer_from_buffer_resource(resource);
+		*width = dmabuf->attributes.width;
+		*height = dmabuf->attributes.height;
+	} else {
+		*width = *height = 0;
+		return false;
+	}
+
+	return true;
+}
+
+
+static void buffer_resource_handle_destroy(struct wl_listener *listener,
+		void *data) {
+	struct wlr_buffer *buffer =
+		wl_container_of(listener, buffer, resource_destroy);
+	wl_list_remove(&buffer->resource_destroy.link);
+	wl_list_init(&buffer->resource_destroy.link);
+	buffer->resource = NULL;
+
+	// At this point, if the wl_buffer comes from linux-dmabuf or wl_drm, we
+	// still haven't released it (ie. we'll read it in the future) but the
+	// client destroyed it. Reading the texture itself should be fine because
+	// we still hold a reference to the DMA-BUF via the texture. However the
+	// client could decide to re-use the same DMA-BUF for something else, in
+	// which case we'll read garbage. We decide to accept this risk.
+}
+
+struct wlr_buffer *wlr_buffer_create(struct wlr_renderer *renderer,
+		struct wl_resource *resource) {
+	assert(wlr_resource_is_buffer(resource));
+
+	struct wlr_texture *texture = NULL;
+	bool released = false;
+
+	struct wl_shm_buffer *shm_buf = wl_shm_buffer_get(resource);
+	if (shm_buf != NULL) {
+		enum wl_shm_format fmt = wl_shm_buffer_get_format(shm_buf);
+		int32_t stride = wl_shm_buffer_get_stride(shm_buf);
+		int32_t width = wl_shm_buffer_get_width(shm_buf);
+		int32_t height = wl_shm_buffer_get_height(shm_buf);
+
+		wl_shm_buffer_begin_access(shm_buf);
+		void *data = wl_shm_buffer_get_data(shm_buf);
+		texture = wlr_texture_from_pixels(renderer, fmt, stride,
+			width, height, data);
+		wl_shm_buffer_end_access(shm_buf);
+
+		// We have uploaded the data, we don't need to access the wl_buffer
+		// anymore
+		wl_buffer_send_release(resource);
+		released = true;
+	} else if (wlr_renderer_resource_is_wl_drm_buffer(renderer, resource)) {
+		texture = wlr_texture_from_wl_drm(renderer, resource);
+	} else if (wlr_dmabuf_resource_is_buffer(resource)) {
+		struct wlr_dmabuf_buffer *dmabuf =
+			wlr_dmabuf_buffer_from_buffer_resource(resource);
+		texture = wlr_texture_from_dmabuf(renderer, &dmabuf->attributes);
+
+		// We have imported the DMA-BUF, but we need to prevent the client from
+		// re-using the same DMA-BUF for the next frames, so we don't release
+		// the buffer yet.
+	} else {
+		wlr_log(L_ERROR, "Cannot upload texture: unknown buffer type");
+		return NULL;
+	}
+
+	if (texture == NULL) {
+		wlr_log(L_ERROR, "Failed to upload texture");
+		return NULL;
+	}
+
+	struct wlr_buffer *buffer = calloc(1, sizeof(struct wlr_buffer));
+	if (buffer == NULL) {
+		wlr_texture_destroy(texture);
+		return NULL;
+	}
+	buffer->resource = resource;
+	buffer->texture = texture;
+	buffer->released = released;
+	buffer->n_refs = 1;
+
+	wl_resource_add_destroy_listener(resource, &buffer->resource_destroy);
+	buffer->resource_destroy.notify = buffer_resource_handle_destroy;
+
+	return buffer;
+}
+
+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) {
+		return;
+	}
+
+	if (!buffer->released && buffer->resource != NULL) {
+		wl_buffer_send_release(buffer->resource);
+	}
+
+	wl_list_remove(&buffer->resource_destroy.link);
+	wlr_texture_destroy(buffer->texture);
+	free(buffer);
+}
+
+struct wlr_buffer *wlr_buffer_apply_damage(struct wlr_buffer *buffer,
+		struct wl_resource *resource, pixman_region32_t *damage) {
+	assert(wlr_resource_is_buffer(resource));
+
+	if (buffer->n_refs > 1) {
+		// Someone else still has a reference to the buffer
+		return NULL;
+	}
+
+	struct wl_shm_buffer *shm_buf = wl_shm_buffer_get(resource);
+	if (shm_buf == NULL) {
+		// Uploading only damaged regions only works for wl_shm buffers
+		return NULL;
+	}
+
+	enum wl_shm_format fmt = wl_shm_buffer_get_format(shm_buf);
+	int32_t stride = wl_shm_buffer_get_stride(shm_buf);
+	int32_t width = wl_shm_buffer_get_width(shm_buf);
+	int32_t height = wl_shm_buffer_get_height(shm_buf);
+
+	int32_t texture_width, texture_height;
+	wlr_texture_get_size(buffer->texture, &texture_width, &texture_height);
+	if (width != texture_width || height != texture_height) {
+		return NULL;
+	}
+
+	wl_shm_buffer_begin_access(shm_buf);
+	void *data = wl_shm_buffer_get_data(shm_buf);
+
+	int n;
+	pixman_box32_t *rects = pixman_region32_rectangles(damage, &n);
+	for (int i = 0; i < n; ++i) {
+		pixman_box32_t *r = &rects[i];
+		if (!wlr_texture_write_pixels(buffer->texture, fmt, stride,
+				r->x2 - r->x1, r->y2 - r->y1, r->x1, r->y1,
+				r->x1, r->y1, data)) {
+			wl_shm_buffer_end_access(shm_buf);
+			return NULL;
+		}
+	}
+
+	wl_shm_buffer_end_access(shm_buf);
+
+	// We have uploaded the data, we don't need to access the wl_buffer
+	// anymore
+	wl_buffer_send_release(resource);
+
+	wl_list_remove(&buffer->resource_destroy.link);
+	wl_resource_add_destroy_listener(resource, &buffer->resource_destroy);
+	buffer->resource_destroy.notify = buffer_resource_handle_destroy;
+
+	buffer->resource = resource;
+	buffer->released = true;
+	return buffer;
+}

types/wlr_output.c

@@ -367,7 +367,8 @@ static void output_fullscreen_surface_render(struct wlr_output *output,
 	struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend);
 	assert(renderer);
 
-	if (!wlr_surface_has_buffer(surface)) {
+	struct wlr_texture *texture = wlr_surface_get_texture(surface);
+	if (texture == NULL) {
 		wlr_renderer_clear(renderer, (float[]){0, 0, 0, 1});
 		return;
 	}
@@ -386,8 +387,7 @@ static void output_fullscreen_surface_render(struct wlr_output *output,
 	for (int i = 0; i < nrects; ++i) {
 		output_scissor(output, &rects[i]);
 		wlr_renderer_clear(renderer, (float[]){0, 0, 0, 1});
-		wlr_render_texture_with_matrix(surface->renderer, surface->texture,
-			matrix, 1.0f);
+		wlr_render_texture_with_matrix(surface->renderer, texture, matrix, 1.0f);
 	}
 	wlr_renderer_scissor(renderer, NULL);
 
@@ -418,7 +418,7 @@ static void output_cursor_render(struct wlr_output_cursor *cursor,
 
 	struct wlr_texture *texture = cursor->texture;
 	if (cursor->surface != NULL) {
-		texture = cursor->surface->texture;
+		texture = wlr_surface_get_texture(cursor->surface);
 	}
 	if (texture == NULL) {
 		return;
@@ -700,7 +700,7 @@ static bool output_cursor_attempt_hardware(struct wlr_output_cursor *cursor) {
 	enum wl_output_transform transform = WL_OUTPUT_TRANSFORM_NORMAL;
 	struct wlr_texture *texture = cursor->texture;
 	if (cursor->surface != NULL) {
-		texture = cursor->surface->texture;
+		texture = wlr_surface_get_texture(cursor->surface);
 		scale = cursor->surface->current->scale;
 		transform = cursor->surface->current->transform;
 	}

types/wlr_surface.c

@@ -1,10 +1,9 @@
 #include <assert.h>
 #include <stdlib.h>
 #include <wayland-server.h>
-#include <wlr/render/egl.h>
 #include <wlr/render/interface.h>
+#include <wlr/types/wlr_buffer.h>
 #include <wlr/types/wlr_compositor.h>
-#include <wlr/types/wlr_linux_dmabuf.h>
 #include <wlr/types/wlr_matrix.h>
 #include <wlr/types/wlr_region.h>
 #include <wlr/types/wlr_surface.h>
@@ -46,13 +45,6 @@ static void surface_handle_buffer_destroy(struct wl_listener *listener,
 	surface_state_reset_buffer(state);
 }
 
-static void surface_state_release_buffer(struct wlr_surface_state *state) {
-	if (state->buffer) {
-		wl_buffer_send_release(state->buffer);
-		surface_state_reset_buffer(state);
-	}
-}
-
 static void surface_state_set_buffer(struct wlr_surface_state *state,
 		struct wl_resource *buffer) {
 	surface_state_reset_buffer(state);
@@ -175,24 +167,8 @@ static bool surface_update_size(struct wlr_surface *surface,
 	int scale = state->scale;
 	enum wl_output_transform transform = state->transform;
 
-	struct wl_shm_buffer *buf = wl_shm_buffer_get(state->buffer);
-	if (buf != NULL) {
-		state->buffer_width = wl_shm_buffer_get_width(buf);
-		state->buffer_height = wl_shm_buffer_get_height(buf);
-	} else if (wlr_renderer_resource_is_wl_drm_buffer(surface->renderer,
-			state->buffer)) {
-		wlr_renderer_wl_drm_buffer_get_size(surface->renderer, state->buffer,
+	wlr_buffer_get_resource_size(state->buffer, surface->renderer,
 		&state->buffer_width, &state->buffer_height);
-	} else if (wlr_dmabuf_resource_is_buffer(state->buffer)) {
-		struct wlr_dmabuf_buffer *dmabuf =
-			wlr_dmabuf_buffer_from_buffer_resource(state->buffer);
-		state->buffer_width = dmabuf->attributes.width;
-		state->buffer_height = dmabuf->attributes.height;
-	} else {
-		wlr_log(L_ERROR, "Unknown buffer handle attached");
-		state->buffer_width = 0;
-		state->buffer_height = 0;
-	}
 
 	int width = state->buffer_width / scale;
 	int height = state->buffer_height / scale;
@@ -243,7 +219,6 @@ static void surface_move_state(struct wlr_surface *surface,
 		update_size = true;
 	}
 	if ((next->invalid & WLR_SURFACE_INVALID_BUFFER)) {
-		surface_state_release_buffer(state);
 		surface_state_set_buffer(state, next->buffer);
 		surface_state_reset_buffer(next);
 		state->sx = next->sx;
@@ -351,91 +326,57 @@ static void surface_damage_subsurfaces(struct wlr_subsurface *subsurface) {
 }
 
 static void surface_apply_damage(struct wlr_surface *surface,
-		bool invalid_buffer, bool reupload_buffer) {
+		bool invalid_buffer) {
 	struct wl_resource *resource = surface->current->buffer;
 	if (resource == NULL) {
+		// NULL commit
+		wlr_buffer_unref(surface->buffer);
+		surface->buffer = NULL;
 		return;
 	}
 
-	struct wl_shm_buffer *buf = wl_shm_buffer_get(resource);
-	if (buf != NULL) {
-		wl_shm_buffer_begin_access(buf);
+	if (surface->buffer != NULL && !surface->buffer->released &&
+			!invalid_buffer) {
+		// The buffer is still the same, no need to re-upload
+		return;
+	}
 
-		enum wl_shm_format fmt = wl_shm_buffer_get_format(buf);
-		int32_t stride = wl_shm_buffer_get_stride(buf);
-		int32_t width = wl_shm_buffer_get_width(buf);
-		int32_t height = wl_shm_buffer_get_height(buf);
-		void *data = wl_shm_buffer_get_data(buf);
-
-		if (surface->texture == NULL || reupload_buffer) {
-			wlr_texture_destroy(surface->texture);
-			surface->texture = wlr_texture_from_pixels(surface->renderer, fmt,
-				stride, width, height, data);
-		} else {
+	if (surface->buffer != NULL && surface->buffer->released) {
 		pixman_region32_t damage;
 		pixman_region32_init(&damage);
 		pixman_region32_copy(&damage, &surface->current->buffer_damage);
 		pixman_region32_intersect_rect(&damage, &damage, 0, 0,
-				surface->current->buffer_width,
-				surface->current->buffer_height);
+			surface->current->buffer_width, surface->current->buffer_height);
 
-			int n;
-			pixman_box32_t *rects = pixman_region32_rectangles(&damage, &n);
-			for (int i = 0; i < n; ++i) {
-				pixman_box32_t *r = &rects[i];
-				if (!wlr_texture_write_pixels(surface->texture, fmt, stride,
-						r->x2 - r->x1, r->y2 - r->y1, r->x1, r->y1,
-						r->x1, r->y1, data)) {
-					break;
-				}
-			}
+		struct wlr_buffer *updated_buffer =
+			wlr_buffer_apply_damage(surface->buffer, resource, &damage);
 
 		pixman_region32_fini(&damage);
+
+		if (updated_buffer != NULL) {
+			surface->buffer = updated_buffer;
+			return;
+		}
 	}
 
-		wl_shm_buffer_end_access(buf);
+	wlr_buffer_unref(surface->buffer);
+	surface->buffer = NULL;
 
-		// We've uploaded the wl_shm_buffer data to the GPU, we won't access the
-		// wl_buffer anymore
-		surface_state_release_buffer(surface->current);
-	} else if (invalid_buffer || reupload_buffer) {
-		wlr_texture_destroy(surface->texture);
-
-		if (wlr_renderer_resource_is_wl_drm_buffer(surface->renderer, resource)) {
-			surface->texture =
-				wlr_texture_from_wl_drm(surface->renderer, resource);
-		} else if (wlr_dmabuf_resource_is_buffer(resource)) {
-			struct wlr_dmabuf_buffer *dmabuf =
-				wlr_dmabuf_buffer_from_buffer_resource(resource);
-			surface->texture =
-				wlr_texture_from_dmabuf(surface->renderer, &dmabuf->attributes);
-		} else {
-			surface->texture = NULL;
-			wlr_log(L_ERROR, "Unknown buffer handle attached");
+	struct wlr_buffer *buffer = wlr_buffer_create(surface->renderer, resource);
+	if (buffer == NULL) {
+		wlr_log(L_ERROR, "Failed to upload buffer");
+		return;
 	}
 
-		// Don't release the wl_buffer yet: since the texture is shared with the
-		// client, we'll access the wl_buffer when rendering
-	}
+	surface->buffer = buffer;
 }
 
 static void surface_commit_pending(struct wlr_surface *surface) {
-	int32_t oldw = surface->current->buffer_width;
-	int32_t oldh = surface->current->buffer_height;
-
 	bool invalid_buffer = surface->pending->invalid & WLR_SURFACE_INVALID_BUFFER;
-	bool null_buffer_commit = invalid_buffer && surface->pending->buffer == NULL;
 
 	surface_move_state(surface, surface->pending, surface->current);
 
-	if (null_buffer_commit) {
-		wlr_texture_destroy(surface->texture);
-		surface->texture = NULL;
-	}
-
-	bool reupload_buffer = oldw != surface->current->buffer_width ||
-		oldh != surface->current->buffer_height;
-	surface_apply_damage(surface, invalid_buffer, reupload_buffer);
+	surface_apply_damage(surface, invalid_buffer);
 
 	// commit subsurface order
 	struct wlr_subsurface *subsurface;
@@ -657,10 +598,9 @@ static void surface_handle_resource_destroy(struct wl_resource *resource) {
 	wl_list_remove(wl_resource_get_link(surface->resource));
 
 	wl_list_remove(&surface->renderer_destroy.link);
-	wlr_texture_destroy(surface->texture);
 	surface_state_destroy(surface->pending);
 	surface_state_destroy(surface->current);
-
+	wlr_buffer_unref(surface->buffer);
 	free(surface);
 }
 
@@ -717,8 +657,15 @@ struct wlr_surface *wlr_surface_create(struct wl_client *client,
 	return surface;
 }
 
+struct wlr_texture *wlr_surface_get_texture(struct wlr_surface *surface) {
+	if (surface->buffer == NULL) {
+		return NULL;
+	}
+	return surface->buffer->texture;
+}
+
 bool wlr_surface_has_buffer(struct wlr_surface *surface) {
-	return surface->texture != NULL;
+	return wlr_surface_get_texture(surface) != NULL;
 }
 
 int wlr_surface_set_role(struct wlr_surface *surface, const char *role,