screencopy: add support for frame flags

backend/drm/drm.c

@@ -683,7 +683,7 @@ static bool drm_connector_set_cursor(struct wlr_output *output,
 		wlr_render_texture_with_matrix(rend, texture, matrix, 1.0);
 		wlr_renderer_end(rend);
 
-		wlr_renderer_read_pixels(rend, WL_SHM_FORMAT_ARGB8888, bo_stride,
+		wlr_renderer_read_pixels(rend, WL_SHM_FORMAT_ARGB8888, NULL, bo_stride,
 			plane->surf.width, plane->surf.height, 0, 0, 0, 0, bo_data);
 
 		swap_drm_surface_buffers(&plane->surf, NULL);

examples/screencopy.c

@@ -45,6 +45,7 @@ static struct {
 	struct wl_buffer *wl_buffer;
 	void *data;
 	int width, height, stride;
+	bool y_invert;
 } buffer;
 bool buffer_copy_done = false;
 
@@ -99,7 +100,7 @@ static struct wl_buffer *create_shm_buffer(int width, int height,
 
 static void frame_handle_buffer(void *data,
 		struct zwlr_screencopy_frame_v1 *frame, uint32_t width, uint32_t height,
-		uint32_t flags, uint32_t format, uint32_t stride) {
+		uint32_t format, uint32_t stride) {
 	buffer.width = width;
 	buffer.height = height;
 	buffer.wl_buffer =
@@ -112,6 +113,11 @@ static void frame_handle_buffer(void *data,
 	zwlr_screencopy_frame_v1_copy(frame, buffer.wl_buffer);
 }
 
+static void frame_handle_flags(void *data,
+		struct zwlr_screencopy_frame_v1 *frame, uint32_t flags) {
+	buffer.y_invert = flags & ZWLR_SCREENCOPY_FRAME_V1_FLAGS_Y_INVERT;
+}
+
 static void frame_handle_ready(void *data,
 		struct zwlr_screencopy_frame_v1 *frame, uint32_t tv_sec_hi,
 		uint32_t tv_sec_lo, uint32_t tv_nsec) {
@@ -126,6 +132,7 @@ static void frame_handle_failed(void *data,
 
 static const struct zwlr_screencopy_frame_v1_listener frame_listener = {
 	.buffer = frame_handle_buffer,
+	.flags = frame_handle_flags,
 	.ready = frame_handle_ready,
 	.failed = frame_handle_failed,
 };
@@ -153,8 +160,8 @@ static const struct wl_registry_listener registry_listener = {
 	.global_remove = handle_global_remove,
 };
 
-static void write_image(const char *filename, int width, int height, int stride,
-		void *data) {
+static void write_image(char *filename, int width, 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);
 
@@ -183,10 +190,19 @@ static void write_image(const char *filename, int width, int height, int stride,
 			exit(EXIT_FAILURE);
 		}
 		close(fd[0]);
+
 		// We requested WL_SHM_FORMAT_XRGB8888 in little endian, so that's BGRA
 		// in big endian.
-		execlp("convert", "convert", "-depth", "8", "-size", size, "bgra:-",
-			"-alpha", "opaque", filename, NULL);
+		char *argv[11] = {"convert", "-depth", "8", "-size", size, "bgra:-",
+			"-alpha", "opaque", filename, NULL};
+		if (y_invert) {
+			argv[8] = "-flip";
+			argv[9] = filename;
+			argv[10] = NULL;
+		}
+
+		execvp("convert", argv);
+
 		fprintf(stderr, "cannot execute convert\n");
 		exit(EXIT_FAILURE);
 	}
@@ -221,12 +237,12 @@ int main(int argc, char *argv[]) {
 		zwlr_screencopy_manager_v1_capture_output(screencopy_manager, 0, output);
 	zwlr_screencopy_frame_v1_add_listener(frame, &frame_listener, NULL);
 
-	while (!buffer_copy_done) {
-		wl_display_roundtrip(display);
+	while (!buffer_copy_done && wl_display_dispatch(display) != -1) {
+		// This space is intentionally left blank
 	}
 
 	write_image("wayland-screenshot.png", buffer.width, buffer.height,
-		buffer.stride, buffer.data);
+		buffer.stride, buffer.y_invert, buffer.data);
 	wl_buffer_destroy(buffer.wl_buffer);
 
 	return EXIT_SUCCESS;

include/wlr/render/interface.h

@@ -34,7 +34,7 @@ struct wlr_renderer_impl {
 	int (*get_dmabuf_modifiers)(struct wlr_renderer *renderer, int format,
 		uint64_t **modifiers);
 	bool (*read_pixels)(struct wlr_renderer *renderer, enum wl_shm_format fmt,
-		uint32_t stride, uint32_t width, uint32_t height,
+		uint32_t *flags, uint32_t stride, uint32_t width, uint32_t height,
 		uint32_t src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y,
 		void *data);
 	bool (*format_supported)(struct wlr_renderer *renderer,

include/wlr/render/wlr_renderer.h

@@ -7,6 +7,10 @@
 #include <wlr/render/wlr_texture.h>
 #include <wlr/types/wlr_box.h>
 
+enum wlr_renderer_read_pixels_flags {
+	WLR_RENDERER_READ_PIXELS_Y_INVERT = 1,
+};
+
 struct wlr_renderer_impl;
 
 struct wlr_renderer {
@@ -87,9 +91,12 @@ int wlr_renderer_get_dmabuf_modifiers(struct wlr_renderer *renderer, int format,
 /**
  * Reads out of pixels of the currently bound surface into data. `stride` is in
  * bytes.
+ *
+ * If `flags` is not NULl, the caller indicates that it accepts frame flags
+ * defined in `enum wlr_renderer_read_pixels_flags`.
  */
 bool wlr_renderer_read_pixels(struct wlr_renderer *r, enum wl_shm_format fmt,
-	uint32_t stride, uint32_t width, uint32_t height,
+	uint32_t *flags, uint32_t stride, uint32_t width, uint32_t height,
 	uint32_t src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y, void *data);
 /**
  * Checks if a format is supported.

protocol/wlr-screencopy-unstable-v1.xml

@@ -76,10 +76,6 @@
       destroy the frame.
     </description>
 
-    <enum name="flags" bitfield="true">
-      <entry name="y_invert" value="1" summary="contents are y-inverted"/>
-    </enum>
-
     <event name="buffer">
       <description summary="buffer information">
         Provides information about the frame's buffer. This event is sent once
@@ -91,7 +87,6 @@
       </description>
       <arg name="width" type="uint" summary="buffer width"/>
       <arg name="height" type="uint" summary="buffer height"/>
-      <arg name="flags" type="uint" enum="flags" summary="buffer flags"/>
       <arg name="format" type="uint" summary="preferred DRM_FORMAT"/>
       <arg name="stride" type="uint" summary="preferred stride"/>
     </event>
@@ -116,6 +111,18 @@
         summary="invalid width or height"/>
     </enum>
 
+    <enum name="flags" bitfield="true">
+      <entry name="y_invert" value="1" summary="contents are y-inverted"/>
+    </enum>
+
+    <event name="flags">
+      <description summary="frame flags">
+        Provides flags about the frame. This event is sent once before the
+        "ready" event.
+      </description>
+      <arg name="flags" type="uint" enum="flags" summary="frame flags"/>
+    </event>
+
     <event name="ready">
       <description summary="indicates frame is available for reading">
         Called as soon as the frame is copied, indicating it is available

render/gles2/renderer.c

@@ -250,9 +250,9 @@ static int gles2_get_dmabuf_modifiers(struct wlr_renderer *wlr_renderer,
 }
 
 static bool gles2_read_pixels(struct wlr_renderer *wlr_renderer,
-		enum wl_shm_format wl_fmt, uint32_t stride, uint32_t width,
-		uint32_t height, uint32_t src_x, uint32_t src_y, uint32_t dst_x,
-		uint32_t dst_y, void *data) {
+		enum wl_shm_format wl_fmt, uint32_t *flags, uint32_t stride,
+		uint32_t width, uint32_t height, uint32_t src_x, uint32_t src_y,
+		uint32_t dst_x, uint32_t dst_y, void *data) {
 	gles2_get_renderer_in_context(wlr_renderer);
 
 	const struct wlr_gles2_pixel_format *fmt = get_gles2_format_from_wl(wl_fmt);
@@ -266,12 +266,24 @@ static bool gles2_read_pixels(struct wlr_renderer *wlr_renderer,
 	// Make sure any pending drawing is finished before we try to read it
 	glFinish();
 
-	// Unfortunately GLES2 doesn't support GL_PACK_*, so we have to read
-	// the lines out row by row
 	unsigned char *p = data + dst_y * stride;
-	for (size_t i = src_y; i < src_y + height; ++i) {
-		glReadPixels(src_x, src_y + height - i - 1, width, 1, fmt->gl_format,
-			fmt->gl_type, p + i * stride + dst_x * fmt->bpp / 8);
+	uint32_t pack_stride = width * fmt->bpp / 8;
+	if (pack_stride == stride && dst_x == 0 && flags != NULL) {
+		// Under these particular conditions, we can read the pixels with only
+		// one glReadPixels call
+		glReadPixels(src_x, src_y, width, height, fmt->gl_format,
+			fmt->gl_type, p);
+		*flags = WLR_RENDERER_READ_PIXELS_Y_INVERT;
+	} else {
+		// Unfortunately GLES2 doesn't support GL_PACK_*, so we have to read
+		// the lines out row by row
+		for (size_t i = src_y; i < src_y + height; ++i) {
+			glReadPixels(src_x, src_y + height - i - 1, width, 1, fmt->gl_format,
+				fmt->gl_type, p + i * stride + dst_x * fmt->bpp / 8);
+		}
+		if (flags != NULL) {
+			*flags = 0;
+		}
 	}
 
 	POP_GLES2_DEBUG;

render/wlr_renderer.c

@@ -139,14 +139,14 @@ int wlr_renderer_get_dmabuf_modifiers(struct wlr_renderer *r, int format,
 }
 
 bool wlr_renderer_read_pixels(struct wlr_renderer *r, enum wl_shm_format fmt,
-		uint32_t stride, uint32_t width, uint32_t height,
+		uint32_t *flags, uint32_t stride, uint32_t width, uint32_t height,
 		uint32_t src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y,
 		void *data) {
 	if (!r->impl->read_pixels) {
 		return false;
 	}
-	return r->impl->read_pixels(r, fmt, stride, width, height, src_x, src_y,
-		dst_x, dst_y, data);
+	return r->impl->read_pixels(r, fmt, flags, stride, width, height,
+		src_x, src_y, dst_x, dst_y, data);
 }
 
 bool wlr_renderer_format_supported(struct wlr_renderer *r,

types/wlr_screencopy_v1.c

@@ -43,8 +43,9 @@ static void frame_handle_output_swap_buffers(struct wl_listener *listener,
 
 	wl_shm_buffer_begin_access(buffer);
 	void *data = wl_shm_buffer_get_data(buffer);
-	bool ok = wlr_renderer_read_pixels(renderer, fmt, stride, width, height,
-		0, 0, 0, 0, data);
+	uint32_t flags = 0;
+	bool ok = wlr_renderer_read_pixels(renderer, fmt, &flags, stride,
+		width, height, 0, 0, 0, 0, data);
 	wl_shm_buffer_end_access(buffer);
 
 	if (!ok) {
@@ -52,6 +53,8 @@ static void frame_handle_output_swap_buffers(struct wl_listener *listener,
 		return;
 	}
 
+	zwlr_screencopy_frame_v1_send_flags(frame->resource, flags);
+
 	uint32_t tv_sec_hi = event->when->tv_sec >> 32;
 	uint32_t tv_sec_lo = event->when->tv_sec & 0xFFFFFFFF;
 	zwlr_screencopy_frame_v1_send_ready(frame->resource,
@@ -172,7 +175,7 @@ static void manager_handle_capture_output(struct wl_client *client,
 	frame->height = output->height;
 	// TODO: don't send zero
 	zwlr_screencopy_frame_v1_send_buffer(frame->resource,
-		frame->width, frame->height, 0, 0, 0);
+		frame->width, frame->height, 0, 0);
 }
 
 static void manager_handle_destroy(struct wl_client *client,

types/wlr_screenshooter.c

@@ -50,8 +50,8 @@ static void output_handle_frame(struct wl_listener *listener, void *_data) {
 	int32_t stride = wl_shm_buffer_get_stride(shm_buffer);
 	wl_shm_buffer_begin_access(shm_buffer);
 	void *data = wl_shm_buffer_get_data(shm_buffer);
-	bool ok = wlr_renderer_read_pixels(renderer, format, stride, width, height,
-		0, 0, 0, 0, data);
+	bool ok = wlr_renderer_read_pixels(renderer, format, NULL, stride,
+		width, height, 0, 0, 0, 0, data);
 	wl_shm_buffer_end_access(shm_buffer);
 
 	if (!ok) {