screncopy: update protocol

examples/screencopy.c

@@ -44,11 +44,24 @@ static struct wl_output *output = NULL;
 static struct {
 	struct wl_buffer *wl_buffer;
 	void *data;
+	enum wl_shm_format format;
 	int width, height, stride;
 	bool y_invert;
 } buffer;
 bool buffer_copy_done = false;
 
+// wl_shm_format describes little-endian formats, ImageMagick uses big-endian
+// formats.
+static const struct {
+	enum wl_shm_format wl_format;
+	char *str_format;
+} formats[] = {
+	{WL_SHM_FORMAT_XRGB8888, "BGRA"},
+	{WL_SHM_FORMAT_ARGB8888, "BGRA"},
+	{WL_SHM_FORMAT_XBGR8888, "RGBA"},
+	{WL_SHM_FORMAT_ABGR8888, "RGBA"},
+};
+
 static int backingfile(off_t size) {
 	char template[] = "/tmp/wlroots-shared-XXXXXX";
 	int fd = mkstemp(template);
@@ -69,9 +82,8 @@ static int backingfile(off_t size) {
 	return fd;
 }
 
-static struct wl_buffer *create_shm_buffer(int width, int height,
+static struct wl_buffer *create_shm_buffer(enum wl_shm_format fmt,
-		int *stride_out, void **data_out) {
+		int width, int height, int stride, void **data_out) {
-	int stride = width * 4;
 	int size = stride * height;
 
 	int fd = backingfile(size);
@@ -90,21 +102,22 @@ static struct wl_buffer *create_shm_buffer(int width, int height,
 	struct wl_shm_pool *pool = wl_shm_create_pool(shm, fd, size);
 	close(fd);
 	struct wl_buffer *buffer = wl_shm_pool_create_buffer(pool, 0, width, height,
-		stride, WL_SHM_FORMAT_XRGB8888);
+		stride, fmt);
 	wl_shm_pool_destroy(pool);
 
 	*data_out = data;
-	*stride_out = stride;
 	return buffer;
 }
 
 static void frame_handle_buffer(void *data,
-		struct zwlr_screencopy_frame_v1 *frame, uint32_t width, uint32_t height,
+		struct zwlr_screencopy_frame_v1 *frame, uint32_t format,
-		uint32_t format, uint32_t stride) {
+		uint32_t width, uint32_t height, uint32_t stride) {
+	buffer.format = format;
 	buffer.width = width;
 	buffer.height = height;
+	buffer.stride = stride;
 	buffer.wl_buffer =
-		create_shm_buffer(width, height, &buffer.stride, &buffer.data);
+		create_shm_buffer(format, width, height, stride, &buffer.data);
 	if (buffer.wl_buffer == NULL) {
 		fprintf(stderr, "failed to create buffer\n");
 		exit(EXIT_FAILURE);
@@ -160,11 +173,26 @@ static const struct wl_registry_listener registry_listener = {
 	.global_remove = handle_global_remove,
 };
 
-static void write_image(char *filename, int width, int height, int stride,
+static void write_image(char *filename, enum wl_shm_format wl_fmt, int width,
-		bool y_invert, void *data) {
+		int height, int stride, bool y_invert, void *data) {
 	char size[10 + 1 + 10 + 2 + 1]; // int32_t are max 10 digits
 	sprintf(size, "%dx%d+0", width, height);
 
+	const char *fmt_str = NULL;
+	for (size_t i = 0; i < sizeof(formats) / sizeof(formats[0]); ++i) {
+		if (formats[i].wl_format == wl_fmt) {
+			fmt_str = formats[i].str_format;
+			break;
+		}
+	}
+	if (fmt_str == NULL) {
+		fprintf(stderr, "unsupported format %"PRIu32"\n", wl_fmt);
+		exit(EXIT_FAILURE);
+	}
+	char convert[strlen(fmt_str) + 3];
+	memcpy(convert, fmt_str, strlen(fmt_str) + 1);
+	strcat(convert, ":-");
+
 	int fd[2];
 	if (pipe(fd) != 0) {
 		fprintf(stderr, "cannot create pipe: %s\n", strerror(errno));
@@ -191,9 +219,7 @@ static void write_image(char *filename, int width, int height, int stride,
 		}
 		close(fd[0]);
 
-		// We requested WL_SHM_FORMAT_XRGB8888 in little endian, so that's BGRA
+		char *argv[11] = {"convert", "-depth", "8", "-size", size, convert,
-		// in big endian.
-		char *argv[11] = {"convert", "-depth", "8", "-size", size, "bgra:-",
 			"-alpha", "opaque", filename, NULL};
 		if (y_invert) {
 			argv[8] = "-flip";
@@ -241,8 +267,8 @@ int main(int argc, char *argv[]) {
 		// This space is intentionally left blank
 	}
 
-	write_image("wayland-screenshot.png", buffer.width, buffer.height,
+	write_image("wayland-screenshot.png", buffer.format, buffer.width,
-		buffer.stride, buffer.y_invert, buffer.data);
+		buffer.height, buffer.stride, buffer.y_invert, buffer.data);
 	wl_buffer_destroy(buffer.wl_buffer);
 
 	return EXIT_SUCCESS;

include/wlr/types/wlr_screencopy_v1.h

@@ -18,7 +18,9 @@ struct wlr_screencopy_frame_v1 {
 	struct wlr_screencopy_manager_v1 *manager;
 	struct wl_list link;
 
-	struct wlr_box buffer_box;
+	enum wl_shm_format format;
+	struct wlr_box box;
+	int stride;
 
 	struct wl_shm_buffer *buffer;
 

protocol/wlr-screencopy-unstable-v1.xml

@@ -43,11 +43,6 @@
       source.
     </description>
 
-    <enum name="error">
-      <entry name="invalid_output_region" value="0"
-        summary="tried to capture an invalid output region"/>
-    </enum>
-
     <request name="capture_output">
       <description summary="capture an output">
         Capture the next frame of an entire output.
@@ -63,8 +58,8 @@
         Capture the next frame of an output's region.
 
         The region is given in output logical coordinates, see
-        xdg_output.logical_size. Trying to capture a region spanning outside the
+        xdg_output.logical_size. The region will be clipped to the output's
-        output extents is a protocol error.
+        extents.
       </description>
       <arg name="frame" type="new_id" interface="zwlr_screencopy_frame_v1"/>
       <arg name="overlay_cursor" type="int"
@@ -90,7 +85,7 @@
 
       When created, a "buffer" event will be sent. The client will then be able
       to send a "copy" request. If the capture is successful, the compositor
-      will finally a "ready" event.
+      will send a "flags" followed by a "ready" event.
 
       If the capture failed, the "failed" event is sent. This can happen anytime
       before the "ready" event.
@@ -104,14 +99,13 @@
         Provides information about the frame's buffer. This event is sent once
         as soon as the frame is created.
 
-        The client should then create a buffer with the provided width and
+        The client should then create a buffer with the provided attributes, and
-        height, and send a copy request. It can optionally create a buffer with
+        send a "copy" request.
-        the preferred format and stride.
       </description>
+      <arg name="format" type="uint" summary="buffer format"/>
       <arg name="width" type="uint" summary="buffer width"/>
       <arg name="height" type="uint" summary="buffer height"/>
-      <arg name="format" type="uint" summary="preferred DRM_FORMAT"/>
+      <arg name="stride" type="uint" summary="buffer stride"/>
-      <arg name="stride" type="uint" summary="preferred stride"/>
     </event>
 
     <request name="copy">
@@ -120,8 +114,8 @@
         correct size, see zwlr_screencopy_frame_v1.buffer. The buffer needs to
         have a supported format.
 
-        If the frame is successfully copied, a ready event is sent. Otherwise,
+        If the frame is successfully copied, a "flags" and a "ready" events are
-        an abort event is sent.
+        sent. Otherwise, a "failed" event is sent.
       </description>
       <arg name="buffer" type="object" interface="wl_buffer"/>
     </request>
@@ -129,9 +123,8 @@
     <enum name="error">
       <entry name="already_used" value="0"
         summary="the object has already been used to copy a wl_buffer"/>
-      <entry name="invalid_format" value="1" summary="format not supported"/>
+      <entry name="invalid_buffer" value="1"
-      <entry name="invalid_dimensions" value="2"
+        summary="buffer attributes are invalid"/>
-        summary="invalid width or height"/>
     </enum>
 
     <enum name="flags" bitfield="true">

types/wlr_screencopy_v1.c

@@ -28,8 +28,8 @@ static void frame_handle_output_swap_buffers(struct wl_listener *listener,
 	wl_list_remove(&frame->output_swap_buffers.link);
 	wl_list_init(&frame->output_swap_buffers.link);
 
-	int x = frame->buffer_box.x;
+	int x = frame->box.x;
-	int y = frame->buffer_box.y;
+	int y = frame->box.y;
 
 	struct wl_shm_buffer *buffer = frame->buffer;
 	assert(buffer != NULL);
@@ -66,27 +66,24 @@ static void frame_handle_copy(struct wl_client *client,
 		struct wl_resource *buffer_resource) {
 	struct wlr_screencopy_frame_v1 *frame = frame_from_resource(frame_resource);
 	struct wlr_output *output = frame->output;
-	struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend);
 
 	struct wl_shm_buffer *buffer = wl_shm_buffer_get(buffer_resource);
 	if (buffer == NULL) {
-		zwlr_screencopy_frame_v1_send_failed(frame_resource);
+		wl_resource_post_error(frame->resource,
+			ZWLR_SCREENCOPY_FRAME_V1_ERROR_INVALID_BUFFER,
+			"unsupported buffer type");
 		return;
 	}
 
 	enum wl_shm_format fmt = wl_shm_buffer_get_format(buffer);
-	if (!wlr_renderer_format_supported(renderer, fmt)) {
+	int32_t width = wl_shm_buffer_get_width(buffer);
+	int32_t height = wl_shm_buffer_get_height(buffer);
+	int32_t stride = wl_shm_buffer_get_stride(buffer);
+	if (fmt != frame->format || width != frame->box.width ||
+			height != frame->box.height || stride != frame->stride) {
 		wl_resource_post_error(frame->resource,
-			ZWLR_SCREENCOPY_FRAME_V1_ERROR_INVALID_FORMAT,
+			ZWLR_SCREENCOPY_FRAME_V1_ERROR_INVALID_BUFFER,
-			"unsupported format %"PRIu32, fmt);
+			"invalid buffer attributes");
-		return;
-	}
-
-	if (frame->buffer_box.width != wl_shm_buffer_get_width(buffer) ||
-			frame->buffer_box.height != wl_shm_buffer_get_height(buffer)) {
-		wl_resource_post_error(frame->resource,
-			ZWLR_SCREENCOPY_FRAME_V1_ERROR_INVALID_DIMENSIONS,
-			"invalid width or height");
 		return;
 	}
 
@@ -165,7 +162,6 @@ static void capture_output(struct wl_client *client,
 		return;
 	}
 	frame->manager = manager;
-
 	frame->output = output;
 
 	frame->resource = wl_resource_create(client,
@@ -182,10 +178,11 @@ static void capture_output(struct wl_client *client,
 
 	wl_list_init(&frame->output_swap_buffers.link);
 
-	frame->buffer_box = buffer_box;
+	frame->format = WL_SHM_FORMAT_XRGB8888;
-	zwlr_screencopy_frame_v1_send_buffer(frame->resource,
+	frame->box = buffer_box;
-		frame->buffer_box.width, frame->buffer_box.height,
+	frame->stride = 4 * buffer_box.width;
-		WL_SHM_FORMAT_XRGB8888, 4 * frame->buffer_box.width);
+	zwlr_screencopy_frame_v1_send_buffer(frame->resource, frame->format,
+		buffer_box.width, buffer_box.height, frame->stride);
 }
 
 static void manager_handle_capture_output(struct wl_client *client,