[PATCH v1 weston 06/11] tests: Add screenshot recording to weston-test

[Pekka Paalanen]

On Wed, 19 Nov 2014 15:06:21 -0800
Bryce Harrington <bryce at osg.samsung.com> wrote:

> From: Derek Foreman <derekf at osg.samsung.com>
> 
> Adds wl_test_record_screenshot() to weston test.  This commit also
> adds a dependency on cairo to weston-test to use it for writing PNG
> files.
> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=83981
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>

Hi,

could we use a wl_shm-based wl_buffer instead of a file, please?

That way we can simply relay the pixels as is to the test client, which
can then compare without compressing and decompressing from PNG first.

For an example how to do this, see Weston's screenshooter protocol and
implementation.

I think you also want to specify in the record_screenshot request:
- which output to capture (in case there are multiple)
- the rectangular sub-region that must be contained in the output
  (which you already do with the clip coords in a later patch, so that
  should be squashed here)

Synchronization cannot be done with wl_display_roundtrip, so you need
an explicit event for that. The best would be to create a new protocol
object on record_screenshot request, which then delivers the event and
self-destructs, like wl_callback. You could just use wl_callback, but
in principle we should avoid new use cases for wl_callback due to
the design issues.

Please also document carefully in which orientation the screenshot is
taken and how the clip coords are interpreted, in case some
output_scale/transform are in effect. Or if you rely on the renderer's
read_pixels() convention, I hope that is documented somewhere...

Btw. there is a small danger in linking Cairo to the compositor.
GL-renderer uses GLESv2, but if CAIRO_LIBS contains cairo-gl.so linking
to libGL, it might explode... or maybe not because of no RTLD_GLOBAL...
or maybe it could, I don't know. So I'd just avoid that. :-)

I see the test interface has been extended before without bumping the
revision... but we probably should bump it, to not give a bad example.


Thanks,
pq

> ---
>  Makefile.am               |  4 +--
>  protocol/wayland-test.xml |  3 +++
>  tests/weston-test.c       | 68 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 1e7cc81..26dd473 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -881,7 +881,7 @@ noinst_PROGRAMS +=			\
>  	matrix-test
>  
>  test_module_ldflags = \
> -	-module -avoid-version -rpath $(libdir) $(COMPOSITOR_LIBS)
> +	-module -avoid-version -rpath $(libdir) $(COMPOSITOR_LIBS) $(CAIRO_LIBS)
>  
>  surface_global_test_la_SOURCES = tests/surface-global-test.c
>  surface_global_test_la_LDFLAGS = $(test_module_ldflags)
> @@ -893,7 +893,7 @@ surface_test_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
>  
>  weston_test_la_LIBADD = $(COMPOSITOR_LIBS) libshared.la
>  weston_test_la_LDFLAGS = $(test_module_ldflags)
> -weston_test_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS)
> +weston_test_la_CFLAGS = $(GCC_CFLAGS) $(COMPOSITOR_CFLAGS) $(CAIRO_CFLAGS)
>  weston_test_la_SOURCES = tests/weston-test.c
>  nodist_weston_test_la_SOURCES =			\
>  	protocol/wayland-test-protocol.c	\
> diff --git a/protocol/wayland-test.xml b/protocol/wayland-test.xml
> index 18b6625..a22a6ac 100644
> --- a/protocol/wayland-test.xml
> +++ b/protocol/wayland-test.xml
> @@ -58,5 +58,8 @@
>      <event name="n_egl_buffers">
>        <arg name="n" type="uint"/>
>      </event>
> +    <request name="record_screenshot">
> +      <arg name="basename" type="string"/>
> +    </request>
>    </interface>
>  </protocol>
> diff --git a/tests/weston-test.c b/tests/weston-test.c
> index f1e45c1..16f20c6 100644
> --- a/tests/weston-test.c
> +++ b/tests/weston-test.c
> @@ -35,6 +35,8 @@
>  #include <EGL/eglext.h>
>  #endif /* ENABLE_EGL */
>  
> +#include <cairo.h>
> +
>  struct weston_test {
>  	struct weston_compositor *compositor;
>  	struct weston_layer layer;
> @@ -235,6 +237,71 @@ get_n_buffers(struct wl_client *client, struct wl_resource *resource)
>  	wl_test_send_n_egl_buffers(resource, n_buffers);
>  }
>  
> +static void
> +dump_image(const char *filename, int x, int y, uint32_t *image)
> +{
> +	cairo_surface_t *surface, *flipped;
> +	cairo_t *cr;
> +
> +	surface = cairo_image_surface_create_for_data((unsigned char *)image,
> +						      CAIRO_FORMAT_ARGB32,
> +						      x, y, x * 4);
> +	flipped = cairo_surface_create_similar_image(surface, CAIRO_FORMAT_ARGB32, x, y);
> +
> +	cr = cairo_create(flipped);
> +	cairo_translate(cr, 0.0, y);
> +	cairo_scale(cr, 1.0, -1.0);
> +	cairo_set_source_surface(cr, surface, 0, 0);
> +	cairo_paint(cr);
> +	cairo_destroy(cr);
> +	cairo_surface_destroy(surface);
> +
> +	cairo_surface_write_to_png(flipped, filename);
> +	cairo_surface_destroy(flipped);
> +}
> +
> +static void
> +record_screenshot(struct wl_client *client, struct wl_resource *resource,
> +		  const char *basename)
> +{
> +	struct weston_output *o;
> +	struct weston_test *test = wl_resource_get_user_data(resource);
> +	char *filename;
> +	uint32_t *buffer;
> +	int w, h, head = 0;
> +
> +	wl_list_for_each(o, &test->compositor->output_list, link) {
> +		switch (o->transform) {
> +		case WL_OUTPUT_TRANSFORM_90:
> +		case WL_OUTPUT_TRANSFORM_270:
> +		case WL_OUTPUT_TRANSFORM_FLIPPED_90:
> +		case WL_OUTPUT_TRANSFORM_FLIPPED_270:
> +			w = o->height;
> +			h = o->width;
> +			break;
> +		default:
> +			w = o->width;
> +			h = o->height;
> +			break;
> +		}
> +		buffer = malloc(w * h * 4);
> +		if (!buffer)
> +			return;
> +
> +		test->compositor->renderer->read_pixels(o,
> +					      o->compositor->read_format,
> +					      buffer, 0, 0, w, h);
> +
> +		if (asprintf(&filename, "%s-%d.png", basename, head) < 0)
> +			return;
> +
> +		dump_image(filename, w, h, buffer);
> +		free(filename);
> +		free(buffer);
> +		head++;
> +	}
> +}
> +
>  static const struct wl_test_interface test_implementation = {
>  	move_surface,
>  	move_pointer,
> @@ -242,6 +309,7 @@ static const struct wl_test_interface test_implementation = {
>  	activate_surface,
>  	send_key,
>  	get_n_buffers,
> +	record_screenshot
>  };
>  
>  static void