[PATCH weston 6/7] tests: add subsurface-shot test

Quentin Glidic sardemff7+wayland at sardemff7.net
Tue Feb 7 11:14:03 UTC 2017


On 27/01/2017 17:30, Emilio Pozuelo Monfort wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> This is marked as a FAIL_TEST, because the last image comparison fails
> due to a bug in Weston.
> 
> Jointly authored by Pekka and Emilio.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo at collabora.co.uk>

A few comments inline.


> ---
>   Makefile.am                               |  12 +-
>   tests/reference/subsurface_z_order-00.png | Bin 0 -> 825 bytes
>   tests/reference/subsurface_z_order-01.png | Bin 0 -> 858 bytes
>   tests/reference/subsurface_z_order-02.png | Bin 0 -> 889 bytes
>   tests/reference/subsurface_z_order-03.png | Bin 0 -> 903 bytes
>   tests/reference/subsurface_z_order-04.png | Bin 0 -> 902 bytes

Do we want even smaller screenshots? Do we have guidelines about it?


>   tests/subsurface-shot-test.c              | 262 ++++++++++++++++++++++++++++++
>   7 files changed, 273 insertions(+), 1 deletion(-)
>   create mode 100644 tests/reference/subsurface_z_order-00.png
>   create mode 100644 tests/reference/subsurface_z_order-01.png
>   create mode 100644 tests/reference/subsurface_z_order-02.png
>   create mode 100644 tests/reference/subsurface_z_order-03.png
>   create mode 100644 tests/reference/subsurface_z_order-04.png
>   create mode 100644 tests/subsurface-shot-test.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3932095a..458dabb1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1213,6 +1213,7 @@ weston_tests =					\
>   	viewporter.weston			\
>   	roles.weston				\
>   	subsurface.weston			\
> +	subsurface-shot.weston			\
>   	devices.weston
>   
>   ivi_tests =
> @@ -1373,6 +1374,10 @@ subsurface_weston_SOURCES = tests/subsurface-test.c
>   subsurface_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
>   subsurface_weston_LDADD = libtest-client.la
>   
> +subsurface_shot_weston_SOURCES = tests/subsurface-shot-test.c
> +subsurface_shot_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
> +subsurface_shot_weston_LDADD = libtest-client.la
> +
>   presentation_weston_SOURCES = 			\
>   	tests/presentation-test.c		\
>   	shared/helpers.h
> @@ -1492,7 +1497,12 @@ EXTRA_DIST +=							\
>   	tests/weston-tests-env					\
>   	tests/internal-screenshot.ini				\
>   	tests/reference/internal-screenshot-bad-00.png		\
> -	tests/reference/internal-screenshot-good-00.png
> +	tests/reference/internal-screenshot-good-00.png		\
> +	tests/reference/subsurface_z_order-00.png		\
> +	tests/reference/subsurface_z_order-01.png		\
> +	tests/reference/subsurface_z_order-02.png		\
> +	tests/reference/subsurface_z_order-03.png		\
> +	tests/reference/subsurface_z_order-04.png

We should put weston-tests-env (or other file that will never change) at 
the end of the list, it’d avoid one line diff each time. [Cosmetics thought]

Or a specific variables to list screenshots (with "$(null)" at the end 
for the one line diff too)?


>   
>   BUILT_SOURCES +=				\
>   	protocol/weston-test-protocol.c	\
> diff --git a/tests/reference/subsurface_z_order-00.png b/tests/reference/subsurface_z_order-00.png
> new file mode 100644
> index 0000000000000000000000000000000000000000..f127154cb68edf8e0148ac0de00ea5ca360b371b
> GIT binary patch
> literal 825
> zcmeAS at N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P at Hb9Ck$=lt9;Xep2*t>i(0|V1L
> zPZ!6KiaBquI`T3(inusVQ%IX=T;|45HlbP8=FQXRsjEf4JYCvf{FeQ~mpO6_G3p5%
> z!Uipz5{FoL5}Dcz7 at cP{DEb at _P@})_w^?!lpMKXZQ%~7*ecrqRZn;6=rU3{BqO9ET
> rIHtOaxnLsufW{b6PNij%+F<YbhV6JNuW&msFEDtz`njxgN at xNAsH5fN
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/reference/subsurface_z_order-01.png b/tests/reference/subsurface_z_order-01.png
> new file mode 100644
> index 0000000000000000000000000000000000000000..d356528105a9c1407d2b5abe5afa4ccb31e24e92
> GIT binary patch
> literal 858
> zcmeAS at N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P at Hb9Ck$=lt9;Xep2*t>i(0|PU^
> zr;B4q#hkZyHu4^F5Mg!f(Kt5cahZ>wwO)IX!Xn1x!l)T1{i{yCN;f}yx{7Jmxy9 at Z
> zzd0oivG61^wHYuv&uCEeIUt~xz#(kVG62G{Zu}aRIo~e|8?8HSRdp!7pph)+d{)0U
> z-Dmmp7o7LEzOA>?-~PDlkA?W4 at g&W{b$5?kKvC?qzx_ZDADGQeR+Qc_ssD6d*kB5I
> hx(7J<V1(>9rfODhL8pZOKY^Kr!PC{xWt~$(69BBr`2GL@
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/reference/subsurface_z_order-02.png b/tests/reference/subsurface_z_order-02.png
> new file mode 100644
> index 0000000000000000000000000000000000000000..7f3d4cce79f4a7a0af1662bd2c5e1cb37b77b729
> GIT binary patch
> literal 889
> zcmeAS at N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P at Hb9Ck$=lt9;Xep2*t>i(0|T?3
> zr;B4q#hkZyHs&675MgyZ6<`=*Sz;NYv;A<<n!G0`cm<gFwa-0a-;?rj^X at aJtr&gd
> z7qc_`=9D<Z!js6<X29q?qe0Q<fPh*8hp<7*00_sr^K1OPTj%C~Vu$YQ7o5u<Fp=e=
> zXZ~xNW0qZi!Fl}FH~YEMfB!D4u at EQEll4iWbKY35eg5Uf-?D#p*XyJ71|(p}iHqwO
> znZ5E(@B07o+dkP3=UK^$-ZS$f*k?}v{cijDmmiO*)(DXoiW^SvS+m%j!FuNaCM}E*
> b{Kl^FLtuNr)T9_-USjZc^>bP0l+XkKnq&~~
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/reference/subsurface_z_order-03.png b/tests/reference/subsurface_z_order-03.png
> new file mode 100644
> index 0000000000000000000000000000000000000000..80c9cf2280f06e1af86dc68eefc7658f82ebce7d
> GIT binary patch
> literal 903
> zcmeAS at N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P at Hb9Ck$=lt9;Xep2*t>i(0|T>-
> zr;B4q#hkZyHx?dp5O4^5vQT2DOdwxhp=ChdRbIXr#zTjHM!$b*p7SYvvaI3R(?1zZ
> z_V&s#-0(Xfpq9WPY|z3fafpQ{k*Upq(RoILq7VIr-&j;NOj>{Y`3pg#d(%I^5G*^$
> zMwVmFm~V_rvAW~l{~>DoTmR=zAAf)L<9QBFKi<Bpdadg2KmO*+!TbN*d9Ao?Zfrm6
> zO`P7}w%c1r&ftYWwrz~r{<;3Yf6bf5JD|Zf05OGT<?)Nmdv4WO^~*g>eErp^ishe%
> rG|sfNtny{78lye#=oo;M_?wNPMCbxfR^kz0&SLO%^>bP0l+XkKHT4$5
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/reference/subsurface_z_order-04.png b/tests/reference/subsurface_z_order-04.png
> new file mode 100644
> index 0000000000000000000000000000000000000000..e02ce2ae03f78308f113f6bc28d82d5eb85c8455
> GIT binary patch
> literal 902
> zcmeAS at N?(olHy`uVBq!ia0y~yU~~YoKX5Ps$$$P at Hb9Ck$=lt9;Xep2*t>i(0|T?Q
> zr;B4q#hkZyHx?dp5O4^5vQT2DOdwxhp=ChdRbG~L2O2EvU+;}EUj5*_ntA!<Ju>AE
> zmv6sdZ1~M7afpQ{k*Upq(RoILqR#;VwFC}fgO&jhe(TO3 at Tv5tjIGnL-$gREhtzJ6
> z=a`+s-SHda{L1UC_2vENJgMK at h|60!#p)ZceP-6@@jE}8dEP-bT!FCR?A#-dT-SZC
> z-}O#zhyUvhYkzOa8K_VqIbyyoX4jaxZn1jB*0X;L`go}y*V}ep=X`#+ at C%3iwt4=G
> u-5>OC!X4nVrr$fM@?bU%TtR-aH9yKGqT9D%rwA}(F?hQAxvX<aXaWEN$Q5`1
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/subsurface-shot-test.c b/tests/subsurface-shot-test.c
> new file mode 100644
> index 00000000..29b03c41
> --- /dev/null
> +++ b/tests/subsurface-shot-test.c
> @@ -0,0 +1,262 @@
> +/*
> + * Copyright © 2015 Samsung Electronics Co., Ltd
> + * Copyright © 2016 Collabora, Ltd.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +
> +#include "weston-test-client-helper.h"
> +
> +char *server_parameters = "--use-pixman --width=320 --height=240"
> +	" --shell=weston-test-desktop-shell.so";

Would there be a way to tag a test as screenshot so it would happen with 
--use-pixman and the test shell?


> +
> +static struct wl_subcompositor *
> +get_subcompositor(struct client *client)
> +{
> +	struct global *g;
> +	struct global *global_sub = NULL;
> +	struct wl_subcompositor *sub;
> +
> +	wl_list_for_each(g, &client->global_list, link) {
> +		if (strcmp(g->interface, "wl_subcompositor"))
> +			continue;
> +
> +		if (global_sub)
> +			assert(0 && "multiple wl_subcompositor objects");
> +
> +		global_sub = g;
> +	}
> +
> +	assert(global_sub && "no wl_subcompositor found");
> +
> +	assert(global_sub->version == 1);
> +
> +	sub = wl_registry_bind(client->wl_registry, global_sub->name,
> +			       &wl_subcompositor_interface, 1);
> +	assert(sub);
> +
> +	return sub;
> +}
> +
> +static void
> +fill_color(pixman_image_t *image, pixman_color_t *color)
> +{
> +	pixman_image_t *solid;
> +	int width;
> +	int height;
> +
> +	width = pixman_image_get_width(image);
> +	height = pixman_image_get_height(image);
> +
> +	solid = pixman_image_create_solid_fill(color);
> +	pixman_image_composite32(PIXMAN_OP_SRC,
> +				 solid, /* src */
> +				 NULL, /* mask */
> +				 image, /* dst */
> +				 0, 0, /* src x,y */
> +				 0, 0, /* mask x,y */
> +				 0, 0, /* dst x,y */
> +				 width, height);
> +	pixman_image_unref(solid);
> +}
> +
> +static pixman_color_t *
> +color(pixman_color_t *tmp, uint8_t r, uint8_t g, uint8_t b)
> +{
> +	tmp->alpha = 65535;
> +	tmp->red = (r << 8) + r;
> +	tmp->green = (g << 8) + g;
> +	tmp->blue = (b << 8) + b;
> +
> +	return tmp;
> +}
> +
> +static void
> +write_visual_diff(pixman_image_t *ref_image,
> +		  struct buffer *shot,
> +		  const struct rectangle *clip,
> +		  const char *test_name,
> +		  int seq_no)
> +{
> +	char *fname;
> +	char *ext_test_name;
> +	pixman_image_t *diff;
> +	int ret;
> +
> +	ret = asprintf(&ext_test_name, "%s-diff", test_name);
> +	assert(ret >= 0);
> +
> +	fname = screenshot_output_filename(ext_test_name, seq_no);
> +	diff = visualize_image_difference(shot->image, ref_image, clip);
> +	write_image_as_png(diff, fname);
> +
> +	pixman_image_unref(diff);
> +	free(fname);
> +	free(ext_test_name);
> +}
> +
> +static int
> +check_screen(struct client *client,
> +	     const char *ref_image,
> +	     int ref_seq_no,
> +	     const struct rectangle *clip,
> +	     int seq_no)
> +{
> +	const char *test_name = get_test_name();
> +	struct buffer *shot;
> +	pixman_image_t *ref;
> +	char *ref_fname;
> +	char *shot_fname;
> +	bool match;
> +
> +	ref_fname = screenshot_reference_filename(ref_image, ref_seq_no);
> +	shot_fname = screenshot_output_filename(test_name, seq_no);
> +
> +	ref = load_image_from_png(ref_fname);
> +	assert(ref);
> +
> +	shot = capture_screenshot_of_output(client);
> +	assert(shot);
> +
> +	match = check_images_match(shot->image, ref, clip);
> +	printf("ref %s vs. shot %s: %s\n", ref_fname, shot_fname,
> +	       match ? "ok" : "FAIL");

PASS/FAIL?


> +
> +	write_image_as_png(shot->image, shot_fname);
> +	write_visual_diff(ref, shot, clip, test_name, seq_no);

Why write diff on PASS?


> +
> +	buffer_destroy(shot);
> +	pixman_image_unref(ref);
> +	free(ref_fname);
> +	free(shot_fname);
> +
> +	return match ? 0 : -1;
> +}
> +
> +static struct buffer *
> +surface_commit_color(struct client *client, struct wl_surface *surface,
> +		     pixman_color_t *color, int width, int height)
> +{
> +	struct buffer *buf;
> +
> +	buf = create_shm_buffer_a8r8g8b8(client, width, height);
> +	fill_color(buf->image, color);
> +	wl_surface_attach(surface, buf->proxy, 0, 0);
> +	wl_surface_damage(surface, 0, 0, width, height);
> +	wl_surface_commit(surface);
> +
> +	return buf;
> +}
> +
> +FAIL_TEST(subsurface_z_order)
> +{
> +	const char *test_name = get_test_name();
> +	struct client *client;
> +	struct wl_subcompositor *subco;
> +	struct buffer *bufs[5] = { 0 };
> +	struct wl_surface *surf[5] = { 0 };
> +	struct wl_subsurface *sub[5] = { 0 };
> +	struct rectangle clip = { 40, 40, 280, 200 };
> +	int fail = 0;
> +	unsigned i;
> +	pixman_color_t red;
> +	pixman_color_t blue;
> +	pixman_color_t cyan;
> +	pixman_color_t green;
> +
> +	color(&red, 255, 0, 0);
> +	color(&blue, 0, 0, 255);
> +	color(&cyan, 0, 255, 255);
> +	color(&green, 0, 255, 0);
> +
> +	client = create_client_and_test_surface(100, 50, 100, 100);
> +	assert(client);
> +	subco = get_subcompositor(client);
> +
> +	/* move the pointer clearly away from our screenshooting area */
> +	weston_test_move_pointer(client->test->weston_test, 2, 30);
> +
> +	/* make the parent surface red */
> +	surf[0] = client->surface->wl_surface;
> +	bufs[0] = surface_commit_color(client, surf[0], &red, 100, 100);
> +	/* sub[0] is not used */
> +
> +	fail += check_screen(client, test_name, 0, &clip, 0);
> +
> +	/* create a blue sub-surface above red */
> +	surf[1] = wl_compositor_create_surface(client->wl_compositor);
> +	sub[1] = wl_subcompositor_get_subsurface(subco, surf[1], surf[0]);
> +	bufs[1] = surface_commit_color(client, surf[1], &blue, 100, 100);
> +
> +	wl_subsurface_set_position(sub[1], 20, 20);
> +	wl_surface_commit(surf[0]);
> +
> +	fail += check_screen(client, test_name, 1, &clip, 1);
> +
> +	/* create a cyan sub-surface above blue */
> +	surf[2] = wl_compositor_create_surface(client->wl_compositor);
> +	sub[2] = wl_subcompositor_get_subsurface(subco, surf[2], surf[1]);
> +	bufs[2] = surface_commit_color(client, surf[2], &cyan, 100, 100);
> +
> +	wl_subsurface_set_position(sub[2], 20, 20);
> +	wl_surface_commit(surf[1]);
> +	wl_surface_commit(surf[0]);
> +
> +	fail += check_screen(client, test_name, 2, &clip, 2);
> +
> +	/* create a green sub-surface above blue, sibling to cyan */
> +	surf[3] = wl_compositor_create_surface(client->wl_compositor);
> +	sub[3] = wl_subcompositor_get_subsurface(subco, surf[3], surf[1]);
> +	bufs[3] = surface_commit_color(client, surf[3], &green, 100, 100);
> +
> +	wl_subsurface_set_position(sub[3], -40, 10);
> +	wl_surface_commit(surf[1]);
> +	wl_surface_commit(surf[0]);
> +
> +	fail += check_screen(client, test_name, 3, &clip, 3);
> +
> +	/* stack blue below red, which brings also cyan and green below red */
> +	wl_subsurface_place_below(sub[1], surf[0]);
> +	wl_surface_commit(surf[0]);
> +
> +	fail += check_screen(client, test_name, 4, &clip, 4);
> +
> +	assert(fail == 0);

Nit: move it after the destroys maybe?


> +
> +	for (i = 0; i < ARRAY_LENGTH(sub); i++)
> +		if (sub[i])
> +			wl_subsurface_destroy(sub[i]);
> +
> +	for (i = 0; i < ARRAY_LENGTH(surf); i++)
> +		if (surf[i])
> +			wl_surface_destroy(surf[i]);
> +
> +	for (i = 0; i < ARRAY_LENGTH(bufs); i++)
> +		if (bufs[i])
> +			buffer_destroy(bufs[i]);
> +}
> 

I’m not knowledgeable enough on subsurface commit order to check the 
“real” stuff, but the rest looks pretty good. I can at least give:
Acked-by: Quentin Glidic <sardemff7+git at sardemff7.net>

Cheers,

-- 

Quentin “Sardem FF7” Glidic


More information about the wayland-devel mailing list