[PATCH weston 04/10] tests: Add surface checks

[Daniel Stone]

Hi Bryce,
A couple of nitpicks follow ...

On 7 May 2015 at 01:44, Bryce Harrington <bryce at osg.samsung.com> wrote:
> @@ -39,7 +39,7 @@ tests/weston-ivi.ini : $(srcdir)/ivi-shell/weston.ini.in
>
>  all-local : weston.ini ivi-shell/weston.ini
>
> -AM_CFLAGS = $(GCC_CFLAGS)
> +AM_CFLAGS = $(GCC_CFLAGS)  $(CAIRO_CFLAGS)
>
>  AM_CPPFLAGS =                                  \
>         -I$(top_srcdir)/src                     \
> @@ -987,6 +987,7 @@ noinst_LTLIBRARIES +=                       \
>
>  noinst_PROGRAMS +=                     \
>         $(setbacklight)                 \
> +       $(internal_tests)               \
>         $(shared_tests)                 \
>         $(weston_tests)                 \
>         $(ivi_tests)                    \
> @@ -1040,59 +1041,59 @@ libtest_client_la_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
>  libtest_client_la_LIBADD = $(TEST_CLIENT_LIBS) libshared.la libtest-runner.la
>
>  bad_buffer_weston_SOURCES = tests/bad-buffer-test.c
> -bad_buffer_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS)
> -bad_buffer_weston_LDADD = libtest-client.la
> +bad_buffer_weston_CFLAGS = $(AM_CFLAGS) $(TEST_CLIENT_CFLAGS) $(CAIRO_CFLAGS)
> +bad_buffer_weston_LDADD = libtest-client.la $(CAIRO_LIBS)

Just making Cairo part of the TEST_CLIENT checks would avoid the need
for all of these, and people tripping up later when forgetting to add
them. It should also make the AM_CFLAGS change redundant (do we really
want to link _everything_ in the build against Cairo?).

> +/**
> + * check_surfaces_equal() - tests if two surfaces are pixel-identical
> + *
> + * Returns true if surface buffers have all the same byte values,
> + * false if the surfaces don't match or can't be compared due to
> + * different dimensions.
> + */
> +bool
> +check_surfaces_equal(const struct surface *a, cairo_surface_t *b)
> +{
> +       if (a == NULL || b == NULL)
> +               return false;
> +       if (a->width != cairo_image_surface_get_width(b) ||
> +           a->height != cairo_image_surface_get_height(b))
> +               return false;
> +
> +       if (memcmp(a->data, cairo_image_surface_get_data(b), a->width * a->height) != 0)
> +               return false;
> +
> +       return true;
> +}

This will break on different strides, and also the bpp seems to be
missing when calculating offsets and lengths:
    {
        unsigned int a_stride = a->width * a->bpp; /* bytes */
        unsigned int b_stride = cairo_image_surface_get_stride(b); /* bytes */
        unsigned char *b_data = cairo_image_surface_get_data(b);

        if (a_stride == b_stride) {
            return (memcmp(a->data, b_data, a_stride * a->height) == 0);
        }
        else {
            int y;
            for (y = 0; y < a->height; y++) {
                if (memcmp(a->data + (y * a_stride), b_data + (y *
b_stride), a_stride) != 0)
                    return false;
            }
            return true;
        }

> +/**
> + * check_surfaces_match_in_clip() - tests if a given region within two
> + * surfaces are pixel-identical.
> + *
> + * Returns true if the two surfaces have the same byte values within the
> + * given clipping region, or false if they don't match or the surfaces
> + * can't be compared.
> + */
> +bool
> +check_surfaces_match_in_clip(const struct surface *a, cairo_surface_t *b, const struct rectangle *clip_rect)

This one also doesn't take bpp into account when doing offsets.
Testing with non-solid surfaces (e.g. the simple-shm test is a pretty
excellently bonkers pattern) should show this up I think?

Cheers,
Daniel