Backport screenshooter fixes from the renderer redesign v1

This backports some changes to #319 to fix the screenshooter data
format. This also adds wlr_backend_get_renderer which will be
useful to support multiple renderers.

backend/backend.c

@@ -52,6 +52,13 @@ struct wlr_egl *wlr_backend_get_egl(struct wlr_backend *backend) {
 	return NULL;
 }
 
+struct wlr_renderer *wlr_backend_get_renderer(struct wlr_backend *backend) {
+	if (backend->impl->get_renderer) {
+		return backend->impl->get_renderer(backend);
+	}
+	return NULL;
+}
+
 static struct wlr_backend *attempt_wl_backend(struct wl_display *display) {
 	struct wlr_backend *backend = wlr_wl_backend_create(display, NULL);
 	if (backend) {

backend/drm/backend.c

@@ -50,10 +50,17 @@ static struct wlr_egl *wlr_drm_backend_get_egl(struct wlr_backend *backend) {
 	return &drm->renderer.egl;
 }
 
+static struct wlr_renderer *wlr_drm_backend_get_renderer(
+		struct wlr_backend *backend) {
+	struct wlr_drm_backend *drm = (struct wlr_drm_backend *)backend;
+	return drm->renderer.wlr_rend;
+}
+
 static struct wlr_backend_impl backend_impl = {
 	.start = wlr_drm_backend_start,
 	.destroy = wlr_drm_backend_destroy,
-	.get_egl = wlr_drm_backend_get_egl
+	.get_egl = wlr_drm_backend_get_egl,
+	.get_renderer = wlr_drm_backend_get_renderer,
 };
 
 bool wlr_backend_is_drm(struct wlr_backend *b) {

backend/headless/backend.c

@@ -1,6 +1,6 @@
 #include <stdlib.h>
-#include <EGL/egl.h>
-#include <EGL/eglext.h>
+#include <wlr/render/egl.h>
+#include <wlr/render/gles2.h>
 #include <wlr/util/log.h>
 #include <wlr/interfaces/wlr_output.h>
 #include <wlr/interfaces/wlr_input_device.h>
@@ -61,10 +61,18 @@ static struct wlr_egl *backend_get_egl(struct wlr_backend *wlr_backend) {
 	return &backend->egl;
 }
 
+static struct wlr_renderer *backend_get_renderer(
+		struct wlr_backend *wlr_backend) {
+	struct wlr_headless_backend *backend =
+		(struct wlr_headless_backend *)wlr_backend;
+	return backend->renderer;
+}
+
 static const struct wlr_backend_impl backend_impl = {
 	.start = backend_start,
 	.destroy = backend_destroy,
 	.get_egl = backend_get_egl,
+	.get_renderer = backend_get_renderer,
 };
 
 static void handle_display_destroy(struct wl_listener *listener, void *data) {
@@ -103,6 +111,11 @@ struct wlr_backend *wlr_headless_backend_create(struct wl_display *display) {
 		return NULL;
 	}
 
+	backend->renderer = wlr_gles2_renderer_create(&backend->backend);
+	if (backend->renderer == NULL) {
+		wlr_log(L_ERROR, "Failed to create renderer");
+	}
+
 	backend->display_destroy.notify = handle_display_destroy;
 	wl_display_add_destroy_listener(display, &backend->display_destroy);
 

backend/multi/backend.c

@@ -62,10 +62,24 @@ static struct wlr_egl *multi_backend_get_egl(struct wlr_backend *wlr_backend) {
 	return NULL;
 }
 
+static struct wlr_renderer *multi_backend_get_renderer(
+		struct wlr_backend *backend) {
+	struct wlr_multi_backend *multi = (struct wlr_multi_backend *)backend;
+	struct subbackend_state *sub;
+	wl_list_for_each(sub, &multi->backends, link) {
+		struct wlr_renderer *rend = wlr_backend_get_renderer(sub->backend);
+		if (rend != NULL) {
+			return rend;
+		}
+	}
+	return NULL;
+}
+
 struct wlr_backend_impl backend_impl = {
 	.start = multi_backend_start,
 	.destroy = multi_backend_destroy,
 	.get_egl = multi_backend_get_egl,
+	.get_renderer = multi_backend_get_renderer,
 };
 
 static void handle_display_destroy(struct wl_listener *listener, void *data) {

backend/wayland/backend.c

@@ -6,6 +6,7 @@
 #include <EGL/eglext.h>
 #include <wayland-server.h>
 #include <wlr/render/egl.h>
+#include <wlr/render/gles2.h>
 #include <wlr/backend/interface.h>
 #include <wlr/interfaces/wlr_output.h>
 #include <wlr/interfaces/wlr_input_device.h>
@@ -106,15 +107,22 @@ static void wlr_wl_backend_destroy(struct wlr_backend *_backend) {
 	free(backend);
 }
 
-static struct wlr_egl *wlr_wl_backend_get_egl(struct wlr_backend *_backend) {
-	struct wlr_wl_backend *backend = (struct wlr_wl_backend *)_backend;
+static struct wlr_egl *wlr_wl_backend_get_egl(struct wlr_backend *wlr_backend) {
+	struct wlr_wl_backend *backend = (struct wlr_wl_backend *)wlr_backend;
 	return &backend->egl;
 }
 
+static struct wlr_renderer *wlr_wl_backend_get_renderer(
+		struct wlr_backend *wlr_backend) {
+	struct wlr_wl_backend *backend = (struct wlr_wl_backend *)wlr_backend;
+	return backend->renderer;
+}
+
 static struct wlr_backend_impl backend_impl = {
 	.start = wlr_wl_backend_start,
 	.destroy = wlr_wl_backend_destroy,
-	.get_egl = wlr_wl_backend_get_egl
+	.get_egl = wlr_wl_backend_get_egl,
+	.get_renderer = wlr_wl_backend_get_renderer,
 };
 
 bool wlr_backend_is_wl(struct wlr_backend *b) {
@@ -191,7 +199,8 @@ struct wlr_backend *wlr_wl_backend_create(struct wl_display *display, const char
 		return false;
 	}
 
-	if (!(backend->registry = wl_display_get_registry(backend->remote_display))) {
+	backend->registry = wl_display_get_registry(backend->remote_display);
+	if (backend->registry == NULL) {
 		wlr_log_errno(L_ERROR, "Could not obtain reference to remote registry");
 		return false;
 	}
@@ -200,6 +209,11 @@ struct wlr_backend *wlr_wl_backend_create(struct wl_display *display, const char
 		backend->remote_display, NULL, WL_SHM_FORMAT_ARGB8888);
 	wlr_egl_bind_display(&backend->egl, backend->local_display);
 
+	backend->renderer = wlr_gles2_renderer_create(&backend->backend);
+	if (backend->renderer == NULL) {
+		wlr_log_errno(L_ERROR, "Could not create renderer");
+	}
+
 	backend->local_display_destroy.notify = handle_display_destroy;
 	wl_display_add_destroy_listener(display, &backend->local_display_destroy);
 

backend/x11/backend.c

@@ -16,6 +16,7 @@
 #include <wlr/backend/interface.h>
 #include <wlr/backend/x11.h>
 #include <wlr/render/egl.h>
+#include <wlr/render/gles2.h>
 #include <wlr/interfaces/wlr_output.h>
 #include <wlr/interfaces/wlr_input_device.h>
 #include <wlr/interfaces/wlr_keyboard.h>
@@ -272,16 +273,23 @@ static struct wlr_egl *wlr_x11_backend_get_egl(struct wlr_backend *backend) {
 	return &x11->egl;
 }
 
-bool wlr_backend_is_x11(struct wlr_backend *backend) {
-	return backend->impl == &backend_impl;
+static struct wlr_renderer *wlr_x11_backend_get_renderer(
+		struct wlr_backend *backend) {
+	struct wlr_x11_backend *x11 = (struct wlr_x11_backend *)backend;
+	return x11->renderer;
 }
 
 static struct wlr_backend_impl backend_impl = {
 	.start = wlr_x11_backend_start,
 	.destroy = wlr_x11_backend_destroy,
 	.get_egl = wlr_x11_backend_get_egl,
+	.get_renderer = wlr_x11_backend_get_renderer,
 };
 
+bool wlr_backend_is_x11(struct wlr_backend *backend) {
+	return backend->impl == &backend_impl;
+}
+
 static void handle_display_destroy(struct wl_listener *listener, void *data) {
 	struct wlr_x11_backend *x11 =
 		wl_container_of(listener, x11, display_destroy);
@@ -330,6 +338,11 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display,
 		goto error_event;
 	}
 
+	x11->renderer = wlr_gles2_renderer_create(&x11->backend);
+	if (x11->renderer == NULL) {
+		wlr_log(L_ERROR, "Failed to create renderer");
+	}
+
 	wlr_input_device_init(&x11->keyboard_dev, WLR_INPUT_DEVICE_KEYBOARD,
 		&input_device_impl, "X11 keyboard", 0, 0);
 	wlr_keyboard_init(&x11->keyboard, NULL);

examples/screenshot.c

@@ -74,7 +74,7 @@ static void output_handle_mode(void *data, struct wl_output *wl_output,
 }
 
 static void output_handle_done(void *data, struct wl_output *wl_output) {
-
+	// No-op
 }
 
 static const struct wl_output_listener output_listener = {
@@ -148,16 +148,6 @@ static struct wl_buffer *create_shm_buffer(int width, int height,
 	return buffer;
 }
 
-static void argb_to_rgba(uint32_t *data, size_t height, size_t stride) {
-	size_t n = height*stride/4;
-	for (size_t i = 0; i < n; ++i) {
-		uint32_t v = data[i];
-		uint32_t rgb = v & 0x00ffffff;
-		uint32_t a = (v & 0xff000000) >> 24;
-		data[i] = (rgb << 8) | a;
-	}
-}
-
 static void write_image(const char *filename, int width, int height) {
 	int buffer_stride = width * 4;
 
@@ -182,8 +172,6 @@ static void write_image(const char *filename, int width, int height) {
 		free(output);
 	}
 
-	argb_to_rgba(data, height, buffer_stride);
-
 	char size[10 + 1 + 10 + 2 + 1]; // int32_t are max 10 digits
 	sprintf(size, "%dx%d+0", width, height);
 
@@ -213,7 +201,9 @@ static void write_image(const char *filename, int width, int height) {
 			exit(EXIT_FAILURE);
 		}
 		close(fd[0]);
-		execlp("convert", "convert", "-depth", "8", "-size", size, "rgba:-",
+		// 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);
 		fprintf(stderr, "cannot execute convert\n");
 		exit(EXIT_FAILURE);

include/backend/headless.h

@@ -7,6 +7,7 @@
 struct wlr_headless_backend {
 	struct wlr_backend backend;
 	struct wlr_egl egl;
+	struct wlr_renderer *renderer;
 	struct wl_display *display;
 	struct wl_list outputs;
 	struct wl_list input_devices;

include/backend/wayland.h

@@ -6,6 +6,7 @@
 #include <wayland-server.h>
 #include <wayland-egl.h>
 #include <wlr/render/egl.h>
+#include <wlr/render.h>
 #include <wlr/backend/wayland.h>
 #include <wlr/types/wlr_box.h>
 #include <wayland-util.h>
@@ -19,6 +20,7 @@ struct wlr_wl_backend {
 	struct wl_list devices;
 	struct wl_list outputs;
 	struct wlr_egl egl;
+	struct wlr_renderer *renderer;
 	size_t requested_outputs;
 	struct wl_listener local_display_destroy;
 	/* remote state */

include/backend/x11.h

@@ -39,6 +39,7 @@ struct wlr_x11_backend {
 	struct wlr_input_device pointer_dev;
 
 	struct wlr_egl egl;
+	struct wlr_renderer *renderer;
 	struct wl_event_source *event_source;
 	struct wl_event_source *frame_timer;
 

include/wlr/backend.h

@@ -23,6 +23,7 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display);
 bool wlr_backend_start(struct wlr_backend *backend);
 void wlr_backend_destroy(struct wlr_backend *backend);
 struct wlr_egl *wlr_backend_get_egl(struct wlr_backend *backend);
+struct wlr_renderer *wlr_backend_get_renderer(struct wlr_backend *backend);
 
 uint32_t usec_to_msec(uint64_t usec);
 

include/wlr/backend/interface.h

@@ -9,6 +9,7 @@ struct wlr_backend_impl {
 	bool (*start)(struct wlr_backend *backend);
 	void (*destroy)(struct wlr_backend *backend);
 	struct wlr_egl *(*get_egl)(struct wlr_backend *backend);
+	struct wlr_renderer *(*get_renderer)(struct wlr_backend *backend);
 };
 
 void wlr_backend_init(struct wlr_backend *backend,

include/wlr/render.h

@@ -52,10 +52,17 @@ const enum wl_shm_format *wlr_renderer_get_formats(
 bool wlr_renderer_buffer_is_drm(struct wlr_renderer *renderer,
 	struct wl_resource *buffer);
 /**
- * Reads pixels and stores them in out_data as ARGB8888.
+ * Reads out of pixels of the currently bound surface into data. `stride` is in
+ * bytes.
  */
-void wlr_renderer_read_pixels(struct wlr_renderer *r, int x, int y,
-	int width, int height, void *out_data);
+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 src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y, void *data);
+/**
+ * Checks if a format is supported.
+ */
+bool wlr_renderer_format_supported(struct wlr_renderer *r,
+	enum wl_shm_format fmt);
 /**
  * Destroys this wlr_renderer. Textures must be destroyed separately.
  */

include/wlr/render/interface.h

@@ -28,8 +28,12 @@ struct wlr_renderer_impl {
 		struct wlr_renderer *renderer, size_t *len);
 	bool (*buffer_is_drm)(struct wlr_renderer *renderer,
 		struct wl_resource *buffer);
-	void (*read_pixels)(struct wlr_renderer *renderer, int x, int y, int width,
-		int height, void *out_data);
+	bool (*read_pixels)(struct wlr_renderer *renderer, enum wl_shm_format 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);
+	bool (*format_supported)(struct wlr_renderer *renderer,
+		enum wl_shm_format fmt);
 	void (*destroy)(struct wlr_renderer *renderer);
 };
 

include/wlr/types/wlr_screenshooter.h

@@ -4,7 +4,6 @@
 
 struct wlr_screenshooter {
 	struct wl_global *wl_global;
-	struct wlr_renderer *renderer;
 	struct wl_list screenshots; // wlr_screenshot::link
 
 	struct wl_listener display_destroy;
@@ -23,8 +22,7 @@ struct wlr_screenshot {
 	void* data;
 };
 
-struct wlr_screenshooter *wlr_screenshooter_create(struct wl_display *display,
-	struct wlr_renderer *renderer);
+struct wlr_screenshooter *wlr_screenshooter_create(struct wl_display *display);
 void wlr_screenshooter_destroy(struct wlr_screenshooter *screenshooter);
 
 #endif

render/gles2/pixel_format.c

@@ -2,7 +2,10 @@
 #include <GLES2/gl2ext.h>
 #include "render/gles2.h"
 
-// Adapted from weston
+/*
+* The wayland formats are little endian while the GL formats are big endian,
+* so WL_SHM_FORMAT_ARGB8888 is actually compatible with GL_BGRA_EXT.
+*/
 struct pixel_format formats[] = {
 	{
 		.wl_format = WL_SHM_FORMAT_ARGB8888,

render/gles2/renderer.c

@@ -211,20 +211,33 @@ static bool wlr_gles2_buffer_is_drm(struct wlr_renderer *_renderer,
 		EGL_TEXTURE_FORMAT, &format);
 }
 
-static void rgba_to_argb(uint32_t *data, size_t height, size_t stride) {
-	size_t n = height*stride/4;
-	for (size_t i = 0; i < n; ++i) {
-		uint32_t v = data[i];
-		uint32_t rgb = (v & 0xffffff00) >> 8;
-		uint32_t a = v & 0x000000ff;
-		data[i] = rgb | (a << 24);
+static bool wlr_gles2_read_pixels(struct wlr_renderer *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) {
+	const struct pixel_format *fmt = gl_format_for_wl_format(wl_fmt);
+	if (fmt == NULL) {
+		wlr_log(L_ERROR, "Cannot read pixels: unsupported pixel format");
+		return false;
+	}
+
+	// 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 * 4);
 	}
+
+	return true;
 }
 
-static void wlr_gles2_read_pixels(struct wlr_renderer *renderer, int x, int y,
-		int width, int height, void *out_data) {
-	glReadPixels(x, y, width, height, GL_RGBA, GL_UNSIGNED_BYTE, out_data);
-	rgba_to_argb(out_data, height, width*4);
+static bool wlr_gles2_format_supported(struct wlr_renderer *r,
+		enum wl_shm_format wl_fmt) {
+	return gl_format_for_wl_format(wl_fmt);
 }
 
 static struct wlr_renderer_impl wlr_renderer_impl = {
@@ -237,6 +250,7 @@ static struct wlr_renderer_impl wlr_renderer_impl = {
 	.formats = wlr_gles2_formats,
 	.buffer_is_drm = wlr_gles2_buffer_is_drm,
 	.read_pixels = wlr_gles2_read_pixels,
+	.format_supported = wlr_gles2_format_supported,
 };
 
 struct wlr_renderer *wlr_gles2_renderer_create(struct wlr_backend *backend) {

render/wlr_renderer.c

@@ -52,7 +52,15 @@ bool wlr_renderer_buffer_is_drm(struct wlr_renderer *r,
 	return r->impl->buffer_is_drm(r, buffer);
 }
 
-void wlr_renderer_read_pixels(struct wlr_renderer *r, int x, int y,
-		int width, int height, void *out_data) {
-	r->impl->read_pixels(r, x, y, width, height, out_data);
+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 src_x, uint32_t src_y, uint32_t dst_x, uint32_t dst_y,
+		void *data) {
+	return r->impl->read_pixels(r, fmt, stride, width, height, src_x, src_y,
+		dst_x, dst_y, data);
+}
+
+bool wlr_renderer_format_supported(struct wlr_renderer *r,
+		enum wl_shm_format fmt) {
+	return r->impl->format_supported(r, fmt);
 }

rootston/desktop.c

@@ -494,8 +494,7 @@ struct roots_desktop *desktop_create(struct roots_server *server,
 
 	desktop->gamma_control_manager = wlr_gamma_control_manager_create(
 		server->wl_display);
-	desktop->screenshooter = wlr_screenshooter_create(server->wl_display,
-		server->renderer);
+	desktop->screenshooter = wlr_screenshooter_create(server->wl_display);
 	desktop->server_decoration_manager =
 		wlr_server_decoration_manager_create(server->wl_display);
 	wlr_server_decoration_manager_set_default_mode(

rootston/main.c

@@ -8,7 +8,6 @@
 #include <wlr/backend/headless.h>
 #include <wlr/backend/multi.h>
 #include <wlr/render.h>
-#include <wlr/render/gles2.h>
 #include <wlr/util/log.h>
 #include "rootston/config.h"
 #include "rootston/server.h"
@@ -34,13 +33,12 @@ int main(int argc, char **argv) {
 	assert(server.wl_event_loop = wl_display_get_event_loop(server.wl_display));
 
 	server.backend = wlr_backend_autocreate(server.wl_display);
-
 	if (server.backend == NULL) {
 		wlr_log(L_ERROR, "could not start backend");
 		return 1;
 	}
 
-	assert(server.renderer = wlr_gles2_renderer_create(server.backend));
+	assert(server.renderer = wlr_backend_get_renderer(server.backend));
 	server.data_device_manager =
 		wlr_data_device_manager_create(server.wl_display);
 	wl_display_init_shm(server.wl_display);

types/wlr_screenshooter.c

@@ -3,24 +3,13 @@
 #include <string.h>
 #include <wayland-server.h>
 #include <wlr/render.h>
+#include <wlr/backend.h>
 #include <wlr/types/wlr_screenshooter.h>
 #include <wlr/types/wlr_output.h>
 #include <wlr/util/log.h>
 #include "screenshooter-protocol.h"
 
-static void copy_yflip(uint8_t *dst, uint8_t *src, int32_t height,
-		int32_t stride) {
-	uint8_t *end = dst + height * stride;
-	while (dst < end) {
-		memcpy(dst, src, stride);
-		dst += stride;
-		src -= stride;
-	}
-}
-
 struct screenshot_state {
-	int32_t width, height, stride;
-	uint8_t *pixels;
 	struct wl_shm_buffer *shm_buffer;
 	struct wlr_screenshot *screenshot;
 	struct wl_listener frame_listener;
@@ -41,27 +30,32 @@ static void handle_screenshot_resource_destroy(
 	}
 }
 
-static void output_frame_notify(struct wl_listener *listener, void *_data) {
+static void output_handle_frame(struct wl_listener *listener, void *_data) {
 	struct screenshot_state *state = wl_container_of(listener, state,
 		frame_listener);
-	struct wlr_renderer *renderer = state->screenshot->screenshooter->renderer;
 	struct wlr_output *output = state->screenshot->output;
+	struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend);
+	struct wl_shm_buffer *shm_buffer = state->shm_buffer;
 
-	wlr_output_make_current(output);
-	wlr_renderer_read_pixels(renderer, 0, 0, output->width, output->height,
-		state->pixels);
-
-	void *data = wl_shm_buffer_get_data(state->shm_buffer);
-	wl_shm_buffer_begin_access(state->shm_buffer);
-	copy_yflip(data, state->pixels + state->stride * (state->height - 1),
-		state->height, state->stride);
-	wl_shm_buffer_end_access(state->shm_buffer);
-
-	free(state->pixels);
-	wl_list_remove(&listener->link);
+	enum wl_shm_format format = wl_shm_buffer_get_format(shm_buffer);
+	int32_t width = wl_shm_buffer_get_width(shm_buffer);
+	int32_t height = wl_shm_buffer_get_height(shm_buffer);
+	int32_t stride = wl_shm_buffer_get_stride(shm_buffer);
+	void *data = wl_shm_buffer_get_data(shm_buffer);
+	wl_shm_buffer_begin_access(shm_buffer);
+	bool ok = wlr_renderer_read_pixels(renderer, format, stride, width, height,
+		0, 0, 0, 0, data);
+	wl_shm_buffer_end_access(shm_buffer);
+
+	if (!ok) {
+		wlr_log(L_ERROR, "Cannot read pixels");
+		goto cleanup;
+	}
 
 	orbital_screenshot_send_done(state->screenshot->resource);
 
+cleanup:
+	wl_list_remove(&listener->link);
 	free(state);
 }
 
@@ -72,35 +66,35 @@ static void screenshooter_shoot(struct wl_client *client,
 	struct wlr_screenshooter *screenshooter =
 		wl_resource_get_user_data(screenshooter_resource);
 	struct wlr_output *output = wl_resource_get_user_data(output_resource);
-	if (!wl_shm_buffer_get(buffer_resource)) {
-		wlr_log(L_ERROR, "Invalid buffer: not a shared memory buffer");
+
+	struct wlr_renderer *renderer = wlr_backend_get_renderer(output->backend);
+	if (renderer == NULL) {
+		wlr_log(L_ERROR, "Backend doesn't have a renderer");
 		return;
 	}
+
 	struct wl_shm_buffer *shm_buffer = wl_shm_buffer_get(buffer_resource);
+	if (shm_buffer == NULL) {
+		wlr_log(L_ERROR, "Invalid buffer: not a shared memory buffer");
+		return;
+	}
+
 	int32_t width = wl_shm_buffer_get_width(shm_buffer);
 	int32_t height = wl_shm_buffer_get_height(shm_buffer);
-	int32_t stride = wl_shm_buffer_get_stride(shm_buffer);
 	if (width < output->width || height < output->height) {
 		wlr_log(L_ERROR, "Invalid buffer: too small");
 		return;
 	}
 
 	uint32_t format = wl_shm_buffer_get_format(shm_buffer);
-	if (format != WL_SHM_FORMAT_XRGB8888) {
+	if (!wlr_renderer_format_supported(renderer, format)) {
 		wlr_log(L_ERROR, "Invalid buffer: unsupported format");
 		return;
 	}
 
-	uint8_t *pixels = malloc(stride * height);
-	if (pixels == NULL) {
-		wl_client_post_no_memory(client);
-		return;
-	}
-
 	struct wlr_screenshot *screenshot =
 		calloc(1, sizeof(struct wlr_screenshot));
 	if (!screenshot) {
-		free(pixels);
 		wl_resource_post_no_memory(screenshooter_resource);
 		return;
 	}
@@ -112,7 +106,6 @@ static void screenshooter_shoot(struct wl_client *client,
 		wl_resource_get_version(screenshooter_resource), id);
 	if (screenshot->resource == NULL) {
 		free(screenshot);
-		free(pixels);
 		wl_resource_post_no_memory(screenshooter_resource);
 		return;
 	}
@@ -127,17 +120,12 @@ static void screenshooter_shoot(struct wl_client *client,
 	if (!state) {
 		wl_resource_destroy(screenshot->resource);
 		free(screenshot);
-		free(pixels);
 		wl_resource_post_no_memory(screenshooter_resource);
 		return;
 	}
-	state->width = width;
-	state->height = height;
-	state->stride = stride;
-	state->pixels = pixels;
 	state->shm_buffer = shm_buffer;
 	state->screenshot = screenshot;
-	state->frame_listener.notify = output_frame_notify;
+	state->frame_listener.notify = output_handle_frame;
 	wl_signal_add(&output->events.swap_buffers, &state->frame_listener);
 }
 
@@ -179,14 +167,12 @@ static void handle_display_destroy(struct wl_listener *listener, void *data) {
 	wlr_screenshooter_destroy(screenshooter);
 }
 
-struct wlr_screenshooter *wlr_screenshooter_create(struct wl_display *display,
-		struct wlr_renderer *renderer) {
+struct wlr_screenshooter *wlr_screenshooter_create(struct wl_display *display) {
 	struct wlr_screenshooter *screenshooter =
 		calloc(1, sizeof(struct wlr_screenshooter));
 	if (!screenshooter) {
 		return NULL;
 	}
-	screenshooter->renderer = renderer;
 
 	wl_list_init(&screenshooter->screenshots);