[PATCH weston 07/11] tests: rewrite check_surfaces_*() API

Pekka Paalanen ppaalanen at gmail.com
Thu Jun 16 14:13:26 UTC 2016


From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

check_surfaces_geometry() is removed as it was not used by anything, and
unlikely would be.

check_surfaces_equal() is merged into check_surfaces_match_in_clip(),
passing a NULL clip means to compare whole images.

check_surfaces_match_in_clip() is converted to work on pixman_image_t
instead of struct surface. The function is only concerned about
comparing images in memory, and does not care about a wl_buffer or a
wl_surface.

The verbosity of image comparisons is greatly reduced. An image mismatch
no longer prints a flood of raw pixel values. This will be replaced
later with a function writing out an error image instead.

Degenerate comparisons are no longer accepted, be that clip outside
images or zero area. Those are an indication of a programmer error.

The pixel format assumptions are made more visible in the code.

A new internal helper image_check_get_roi() computes and verifies the
area to be compared. Image iterator helper makes it simpler to write
manual pixel-poking loops.

Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
---
 tests/internal-screenshot-test.c  |  12 +--
 tests/weston-test-client-helper.c | 177 +++++++++++++++++++++-----------------
 tests/weston-test-client-helper.h |   9 +-
 3 files changed, 107 insertions(+), 91 deletions(-)

diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c
index aee146c..c584d26 100644
--- a/tests/internal-screenshot-test.c
+++ b/tests/internal-screenshot-test.c
@@ -102,16 +102,17 @@ TEST(internal_screenshot)
 	reference_bad = load_surface_from_png(fname);
 	assert(reference_bad);
 
-	/* Test check_surfaces_equal()
+	/* Test check_images_match() without a clip.
 	 * We expect this to fail since we use a bad reference image
 	 */
-	match = check_surfaces_equal(screenshot, reference_bad);
+	match = check_images_match(screenshot->buffer->image,
+				   reference_bad->buffer->image, NULL);
 	printf("Screenshot %s reference image\n", match? "equal to" : "different from");
 	assert(!match);
 	buffer_destroy(reference_bad->buffer);
 	free(reference_bad);
 
-	/* Test check_surfaces_match_in_clip()
+	/* Test check_images_match() with clip.
 	 * Alpha-blending and other effects can cause irrelevant discrepancies, so look only
 	 * at a small portion of the solid-colored background
 	 */
@@ -120,8 +121,9 @@ TEST(internal_screenshot)
 	clip.width = 100;
 	clip.height = 100;
 	printf("Clip: %d,%d %d x %d\n", clip.x, clip.y, clip.width, clip.height);
-	match = check_surfaces_match_in_clip(screenshot, reference_good,
-					     &clip);
+	match = check_images_match(screenshot->buffer->image,
+				   reference_good->buffer->image,
+				   &clip);
 	printf("Screenshot %s reference image in clipped area\n", match? "matches" : "doesn't match");
 	buffer_destroy(reference_good->buffer);
 	free(reference_good);
diff --git a/tests/weston-test-client-helper.c b/tests/weston-test-client-helper.c
index 76b5cdd..53b8d80 100644
--- a/tests/weston-test-client-helper.c
+++ b/tests/weston-test-client-helper.c
@@ -968,106 +968,125 @@ screenshot_reference_filename(const char *basename, uint32_t seq)
 }
 
 /**
- * check_surfaces_geometry() - verifies two surfaces are same size
+ * Compute the ROI for image comparisons
  *
- * @returns true if surfaces have the same width and height, or false
- * if not, or if there is no actual data.
+ * \param img_a An image.
+ * \param img_b Another image.
+ * \param clip_rect Explicit ROI, or NULL for using the whole
+ * image area.
+ *
+ * \return The region of interest (ROI) that is guaranteed to be inside both
+ * images.
+ *
+ * If clip_rect is given, it must fall inside of both images.
+ * If clip_rect is NULL, the images must be of the same size.
+ * If any precondition is violated, this function aborts with an error.
+ *
+ * The ROI is given as pixman_box32_t, where x2,y2 are non-inclusive.
  */
-bool
-check_surfaces_geometry(const struct surface *a, const struct surface *b)
+static pixman_box32_t
+image_check_get_roi(pixman_image_t *img_a, pixman_image_t *img_b,
+		   const struct rectangle *clip_rect)
 {
-	if (a == NULL || b == NULL) {
-		printf("Undefined surfaces\n");
-		return false;
-	}
-	else if (a->buffer == NULL || b->buffer == NULL) {
-		printf("Undefined data\n");
-		return false;
+	int width_a;
+	int width_b;
+	int height_a;
+	int height_b;
+	pixman_box32_t box;
+
+	width_a = pixman_image_get_width(img_a);
+	height_a = pixman_image_get_height(img_a);
+
+	width_b = pixman_image_get_width(img_b);
+	height_b = pixman_image_get_height(img_b);
+
+	if (clip_rect) {
+		box.x1 = clip_rect->x;
+		box.y1 = clip_rect->y;
+		box.x2 = clip_rect->x + clip_rect->width;
+		box.y2 = clip_rect->y + clip_rect->height;
+	} else {
+		box.x1 = 0;
+		box.y1 = 0;
+		box.x2 = max(width_a, width_b);
+		box.y2 = max(height_a, height_b);
 	}
-	else if (a->width != b->width || a->height != b->height) {
-		printf("Mismatched dimensions:  %d,%d != %d,%d\n",
-		       a->width, a->height, b->width, b->height);
-		return false;
-	}
-	return true;
+
+	assert(box.x1 >= 0);
+	assert(box.y1 >= 0);
+	assert(box.x2 > box.x1);
+	assert(box.y2 > box.y1);
+	assert(box.x2 <= width_a);
+	assert(box.x2 <= width_b);
+	assert(box.y2 <= height_a);
+	assert(box.y2 <= height_b);
+
+	return box;
 }
 
-/**
- * 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, const struct surface *b)
+struct image_iterator {
+	char *data;
+	int stride; /* bytes */
+};
+
+static void
+image_iter_init(struct image_iterator *it, pixman_image_t *image)
 {
-	int bpp = 4;  /* Assumes ARGB */
-	void *data_a;
-	void *data_b;
+	pixman_format_code_t fmt;
 
-	if (!check_surfaces_geometry(a, b))
-		return false;
+	it->stride = pixman_image_get_stride(image);
+	it->data = (void *)pixman_image_get_data(image);
 
-	data_a = pixman_image_get_data(a->buffer->image);
-	data_b = pixman_image_get_data(b->buffer->image);
+	fmt = pixman_image_get_format(image);
+	assert(PIXMAN_FORMAT_BPP(fmt) == 32);
+}
 
-	return (memcmp(data_a, data_b, bpp * a->width * a->height) == 0);
+static uint32_t *
+image_iter_get_row(struct image_iterator *it, int y)
+{
+	return (uint32_t *)(it->data + y * it->stride);
 }
 
 /**
- * check_surfaces_match_in_clip() - tests if a given region within two
- * surfaces are pixel-identical.
+ * Test if a given region within two images are pixel-identical.
+ *
+ * Returns true if the two images pixel-wise identical, and false otherwise.
  *
- * 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.
+ * \param img_a First image.
+ * \param img_b Second image.
+ * \param clip_rect The region of interest, or NULL for comparing the whole
+ * images.
+ *
+ * This function hard-fails if clip_rect is not inside both images. If clip_rect
+ * is given, the images do not have to match in size, otherwise size mismatch
+ * will be a hard failure.
  */
 bool
-check_surfaces_match_in_clip(const struct surface *a, const struct surface *b, const struct rectangle *clip_rect)
+check_images_match(pixman_image_t *img_a, pixman_image_t *img_b,
+		   const struct rectangle *clip_rect)
 {
-	int i, j;
-	int x0, y0, x1, y1;
-	void *p, *q;
-	int bpp = 4;  /* Assumes ARGB */
-	void *data_a;
-	void *data_b;
-
-	if (!check_surfaces_geometry(a, b) || clip_rect == NULL)
-		return false;
+	struct image_iterator it_a;
+	struct image_iterator it_b;
+	pixman_box32_t box;
+	int x, y;
+	uint32_t *pix_a;
+	uint32_t *pix_b;
 
-	if (clip_rect->x > a->width || clip_rect->y > a->height) {
-		printf("Clip outside image boundaries\n");
-		return true;
-	}
+	box = image_check_get_roi(img_a, img_b, clip_rect);
 
-	x0 = max(0, clip_rect->x);
-	y0 = max(0, clip_rect->y);
-	x1 = min(a->width,  clip_rect->x + clip_rect->width);
-	y1 = min(a->height, clip_rect->y + clip_rect->height);
+	image_iter_init(&it_a, img_a);
+	image_iter_init(&it_b, img_b);
 
-	if (x0 == x1 || y0 == y1) {
-		printf("Degenerate comparison\n");
-		return true;
-	}
+	for (y = box.y1; y < box.y2; y++) {
+		pix_a = image_iter_get_row(&it_a, y) + box.x1;
+		pix_b = image_iter_get_row(&it_b, y) + box.x1;
+
+		for (x = box.x1; x < box.x2; x++) {
+			if (*pix_a != *pix_b)
+				return false;
 
-	data_a = pixman_image_get_data(a->buffer->image);
-	data_b = pixman_image_get_data(b->buffer->image);
-
-	printf("Bytewise comparison inside clip\n");
-	for (i=y0; i<y1; i++) {
-		p = data_a + i * a->width * bpp + x0 * bpp;
-		q = data_b + i * b->width * bpp + x0 * bpp;
-		if (memcmp(p, q, (x1-x0)*bpp) != 0) {
-			/* Dump the bad row */
-			printf("Mismatched image on row %d\n", i);
-			for (j=0; j<(x1-x0)*bpp; j++) {
-				char a_char = *((char*)(p+j*bpp));
-				char b_char = *((char*)(q+j*bpp));
-				printf("%d,%d: %8x %8x %s\n", i, j, a_char, b_char,
-				       (a_char != b_char)? " <---": "");
-			}
-			return false;
+			pix_a++;
+			pix_b++;
 		}
 	}
 
diff --git a/tests/weston-test-client-helper.h b/tests/weston-test-client-helper.h
index 3e1cfcd..4910136 100644
--- a/tests/weston-test-client-helper.h
+++ b/tests/weston-test-client-helper.h
@@ -198,13 +198,8 @@ char *
 screenshot_reference_filename(const char *basename, uint32_t seq);
 
 bool
-check_surfaces_geometry(const struct surface *a, const struct surface *b);
-
-bool
-check_surfaces_equal(const struct surface *a, const struct surface *b);
-
-bool
-check_surfaces_match_in_clip(const struct surface *a, const struct surface *b, const struct rectangle *clip);
+check_images_match(pixman_image_t *img_a, pixman_image_t *img_b,
+		   const struct rectangle *clip);
 
 bool
 write_surface_as_png(const struct surface *weston_surface, const char *fname);
-- 
2.7.3



More information about the wayland-devel mailing list