[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