screenshooter: Grab pixel data directly before buffer swap

clients/screenshot.c

@@ -41,6 +41,7 @@
 static struct wl_shm *shm;
 static struct screenshooter *screenshooter;
 static struct wl_list output_list;
+int buffer_copy_done;
 
 struct screenshooter_output {
 	struct wl_output *output;
@@ -93,6 +94,16 @@ static const struct wl_output_listener output_listener = {
 	display_handle_mode
 };
 
+static void
+screenshot_done(void *data, struct screenshooter *screenshooter)
+{
+	buffer_copy_done = 1;
+}
+
+static const struct screenshooter_listener screenshooter_listener = {
+	screenshot_done
+};
+
 static void
 handle_global(struct wl_display *display, uint32_t id,
 	      const char *interface, uint32_t version, void *data)
@@ -188,6 +199,9 @@ int main(int argc, char *argv[])
 		return -1;
 	}
 
+	screenshooter_add_listener(screenshooter, &screenshooter_listener, screenshooter);
+
+
 	wl_list_for_each(output, &output_list, link) {
 		width = MAX(width, output->offset_x + output->width);
 		height = MAX(height, output->offset_y + output->height);
@@ -197,11 +211,12 @@ int main(int argc, char *argv[])
 
 	wl_list_for_each_safe(output, next, &output_list, link) {
 		screenshooter_shoot(screenshooter, output->output, buffer);
+		buffer_copy_done = 0;
+		while (!buffer_copy_done)
+			wl_display_roundtrip(display);
 		free(output);
 	}
 
-	wl_display_roundtrip(display);
-
 	write_png(width, height, data);
 
 	return 0;

protocol/screenshooter.xml

@@ -5,6 +5,8 @@
       <arg name="output" type="object" interface="wl_output"/>
       <arg name="buffer" type="object" interface="wl_buffer"/>
     </request>
+    <event name="done">
+    </event>
   </interface>
 
 </protocol>

src/compositor-drm.c

@@ -215,6 +215,8 @@ drm_output_render(struct drm_output *output, pixman_region32_t *damage)
 	wl_list_for_each_reverse(surface, &compositor->base.surface_list, link)
 		weston_surface_draw(surface, &output->base, damage);
 
+	weston_output_do_read_pixels(&output->base);
+
 	eglSwapBuffers(compositor->base.display, output->egl_surface);
 	output->next_bo = gbm_surface_lock_front_buffer(output->surface);
 	if (!output->next_bo) {
@@ -1190,21 +1192,6 @@ drm_set_dpms(struct weston_output *output_base, enum dpms_enum level)
 	drmModeFreeConnector(connector);
 }
 
-static void
-drm_output_read_pixels(struct weston_output *output_base, void *data)
-{
-	struct drm_output *output = (struct drm_output *) output_base;
-	struct drm_compositor *compositor =
-		(struct drm_compositor *) output->base.compositor;
-
-	eglMakeCurrent(compositor->base.display, output->egl_surface,
-			output->egl_surface, compositor->base.context);
-
-	glReadPixels(0, 0, output_base->current->width,
-		     output_base->current->height,
-		     compositor->base.read_format, GL_UNSIGNED_BYTE, data);
-}
-
 static int
 create_output_for_connector(struct drm_compositor *ec,
 			    drmModeRes *resources,
@@ -1313,7 +1300,6 @@ create_output_for_connector(struct drm_compositor *ec,
 	output->base.repaint = drm_output_repaint;
 	output->base.destroy = drm_output_destroy;
 	output->base.assign_planes = drm_assign_planes;
-	output->base.read_pixels = drm_output_read_pixels;
 	output->base.set_dpms = drm_set_dpms;
 	output->base.switch_mode = drm_output_switch_mode;
 

src/compositor-wayland.c

@@ -357,6 +357,8 @@ wayland_output_repaint(struct weston_output *output_base,
 
 	draw_border(output);
 
+	weston_output_do_read_pixels(&output->base);
+
 	eglSwapBuffers(compositor->base.display, output->egl_surface);
 	callback = wl_surface_frame(output->parent.surface);
 	wl_callback_add_listener(callback, &frame_listener, output);
@@ -377,21 +379,6 @@ wayland_output_destroy(struct weston_output *output_base)
 	return;
 }
 
-static void
-wayland_output_read_pixels(struct weston_output *output_base, void *data)
-{
-	struct wayland_output *output = (struct wayland_output *) output_base;
-	struct wayland_compositor *compositor =
-		(struct wayland_compositor *) output->base.compositor;
-
-	eglMakeCurrent(compositor->base.display, output->egl_surface,
-			output->egl_surface, compositor->base.context);
-
-	glReadPixels(0, 0, output_base->current->width,
-		     output_base->current->height,
-		     compositor->base.read_format, GL_UNSIGNED_BYTE, data);
-}
-
 static int
 wayland_compositor_create_output(struct wayland_compositor *c,
 				 int width, int height)
@@ -460,7 +447,6 @@ wayland_compositor_create_output(struct wayland_compositor *c,
 	output->base.repaint = wayland_output_repaint;
 	output->base.destroy = wayland_output_destroy;
 	output->base.assign_planes = NULL;
-	output->base.read_pixels = wayland_output_read_pixels;
 	output->base.set_backlight = NULL;
 	output->base.set_dpms = NULL;
 	output->base.switch_mode = NULL;

src/compositor-x11.c

@@ -212,6 +212,8 @@ x11_output_repaint(struct weston_output *output_base,
 	wl_list_for_each_reverse(surface, &compositor->base.surface_list, link)
 		weston_surface_draw(surface, &output->base, damage);
 
+	weston_output_do_read_pixels(&output->base);
+
 	eglSwapBuffers(compositor->base.display, output->egl_surface);
 
 	wl_event_source_timer_update(output->finish_frame_timer, 10);
@@ -344,21 +346,6 @@ x11_output_set_icon(struct x11_compositor *c,
 	pixman_image_unref(image);
 }
 
-static void
-x11_output_read_pixels(struct weston_output *output_base, void *data)
-{
-	struct x11_output *output = (struct x11_output *) output_base;
-	struct x11_compositor *compositor =
-		(struct x11_compositor *) output->base.compositor;
-
-	eglMakeCurrent(compositor->base.display, output->egl_surface,
-			output->egl_surface, compositor->base.context);
-
-	glReadPixels(0, 0, output_base->current->width,
-		     output_base->current->height,
-		     compositor->base.read_format, GL_UNSIGNED_BYTE, data);
-}
-
 static int
 x11_compositor_create_output(struct x11_compositor *c, int x, int y,
 			     int width, int height, int fullscreen)
@@ -470,7 +457,6 @@ x11_compositor_create_output(struct x11_compositor *c, int x, int y,
 	output->base.repaint = x11_output_repaint;
 	output->base.destroy = x11_output_destroy;
 	output->base.assign_planes = NULL;
-	output->base.read_pixels = x11_output_read_pixels;
 	output->base.set_backlight = NULL;
 	output->base.set_dpms = NULL;
 	output->base.switch_mode = NULL;

src/compositor.c

@@ -2389,6 +2389,7 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
 	output->mm_width = width;
 	output->mm_height = height;
 	output->dirty = 1;
+	wl_list_init(&output->read_pixels_list);
 
 	output->zoom.active = 0;
 	output->zoom.increment = 0.05;
@@ -2412,6 +2413,20 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
 				      output, bind_output);
 }
 
+WL_EXPORT void
+weston_output_do_read_pixels(struct weston_output *output)
+{
+	struct weston_read_pixels *r, *next;
+
+	glPixelStorei(GL_PACK_ALIGNMENT, 1);
+	wl_list_for_each_safe(r, next, &output->read_pixels_list, link) {
+		glReadPixels(r->x, r->y, r->width, r->height,
+			     output->compositor->read_format,
+			     GL_UNSIGNED_BYTE, r->data);
+		r->done(r, output);
+	}
+}
+
 static void
 compositor_bind(struct wl_client *client,
 		void *data, uint32_t version, uint32_t id)

src/compositor.h

@@ -44,6 +44,7 @@ struct weston_transform {
 
 struct weston_surface;
 struct weston_input_device;
+struct weston_output;
 
 struct weston_mode {
 	uint32_t flags;
@@ -72,6 +73,14 @@ enum dpms_enum {
 	WESTON_DPMS_OFF
 };
 
+struct weston_read_pixels {
+	void *data;
+	int x, y, width, height;
+	void (*done)(struct weston_read_pixels *read_pixels,
+		     struct weston_output *output);
+	struct wl_list link;
+};
+
 struct weston_output {
 	uint32_t id;
 
@@ -90,6 +99,7 @@ struct weston_output {
 	int repaint_scheduled;
 	struct weston_output_zoom zoom;
 	int dirty;
+	struct wl_list read_pixels_list;
 
 	char *make, *model;
 	uint32_t subpixel;
@@ -102,7 +112,6 @@ struct weston_output {
 			pixman_region32_t *damage);
 	void (*destroy)(struct weston_output *output);
 	void (*assign_planes)(struct weston_output *output);
-	void (*read_pixels)(struct weston_output *output, void *data);
 	int (*switch_mode)(struct weston_output *output, struct weston_mode *mode);
 
 	/* backlight values are on 0-255 range, where higher is brighter */
@@ -424,6 +433,8 @@ weston_output_finish_frame(struct weston_output *output, int msecs);
 void
 weston_output_damage(struct weston_output *output);
 void
+weston_output_do_read_pixels(struct weston_output *output);
+void
 weston_compositor_repick(struct weston_compositor *compositor);
 void
 weston_compositor_schedule_repaint(struct weston_compositor *compositor);

src/screenshooter.c

@@ -36,6 +36,12 @@ struct screenshooter {
 	struct wl_listener destroy_listener;
 };
 
+struct screenshooter_read_pixels {
+	struct weston_read_pixels base;
+	struct wl_buffer *buffer;
+	struct wl_resource *resource;
+};
+
 static void
 copy_bgra_yflip(uint8_t *dst, uint8_t *src, int height,
 		int dst_stride, int src_stride)
@@ -81,6 +87,43 @@ copy_rgba_yflip(uint8_t *dst, uint8_t *src, int height,
 	}
 }
 
+static void
+screenshooter_read_pixels_done(struct weston_read_pixels *base,
+			       struct weston_output *output)
+{
+	struct screenshooter_read_pixels *r =
+		(struct screenshooter_read_pixels *) base;
+	int32_t buffer_stride, output_stride;
+	uint8_t *d, *s;
+
+	buffer_stride = wl_shm_buffer_get_stride(r->buffer);
+	output_stride = output->current->width * 4;
+
+	d = wl_shm_buffer_get_data(r->buffer) +
+		output->y * buffer_stride + output->x * 4;
+	s = r->base.data + output_stride * (output->current->height - 1);
+
+	switch (output->compositor->read_format) {
+	case GL_BGRA_EXT:
+		copy_bgra_yflip(d, s, output->current->height,
+				buffer_stride, output_stride);
+		break;
+	case GL_RGBA:
+		copy_rgba_yflip(d, s, output->current->height,
+				buffer_stride, output_stride);
+		break;
+	default:
+		break;
+	}
+
+	wl_list_remove(&r->base.link);
+
+	screenshooter_send_done(r->resource);
+	free(r->base.data);
+	free(r);
+
+}
+
 static void
 screenshooter_shoot(struct wl_client *client,
 		    struct wl_resource *resource,
@@ -88,9 +131,9 @@ screenshooter_shoot(struct wl_client *client,
 		    struct wl_resource *buffer_resource)
 {
 	struct weston_output *output = output_resource->data;
+	struct screenshooter_read_pixels *r;
 	struct wl_buffer *buffer = buffer_resource->data;
-	uint8_t *tmp, *d, *s;
+	int32_t stride;
-	int32_t buffer_stride, output_stride;
 
 	if (!wl_buffer_is_shm(buffer))
 		return;
@@ -99,35 +142,30 @@ screenshooter_shoot(struct wl_client *client,
 	    buffer->height < output->current->height)
 		return;
 
-	buffer_stride = wl_shm_buffer_get_stride(buffer);
+	r = malloc(sizeof *r);
-	output_stride = output->current->width * 4;
+	if (r == NULL) {
-	tmp = malloc(output_stride * output->current->height);
-	if (tmp == NULL) {
 		wl_resource_post_no_memory(resource);
 		return;
 	}
 
-	glPixelStorei(GL_PACK_ALIGNMENT, 1);
+	r->base.x = 0;
-	output->read_pixels(output, tmp);
+	r->base.y = 0;
-
+	r->base.width = output->current->width;
-	d = wl_shm_buffer_get_data(buffer) + output->y * buffer_stride +
+	r->base.height = output->current->height;
-							output->x * 4;
+	r->base.done = screenshooter_read_pixels_done;
-	s = tmp + output_stride * (output->current->height - 1);
+	r->buffer = buffer;
-
+	r->resource = resource;
-	switch (output->compositor->read_format) {
+	stride = buffer->width * 4;
-	case GL_BGRA_EXT:
+	r->base.data = malloc(stride * buffer->height);
-		copy_bgra_yflip(d, s, output->current->height,
+
-				buffer_stride, output_stride);
+	if (r->base.data == NULL) {
-		break;
+		free(r);
-	case GL_RGBA:
+		wl_resource_post_no_memory(resource);
-		copy_rgba_yflip(d, s, output->current->height,
+		return;
-				buffer_stride, output_stride);
-		break;
-	default:
-		break;
 	}
 
-	free(tmp);
+	wl_list_insert(output->read_pixels_list.prev, &r->base.link);
+	weston_compositor_schedule_repaint(output->compositor);
 }
 
 struct screenshooter_interface screenshooter_implementation = {