Split out printing the results from eval_diff()

Separate the evalation of two colors given a format and the printing of
the result. This allows eval_diff() to be used elsewhere and also means
that we do not need to waste cycles generating unused strings for
eval_diff() [this accounts for as much time as composite_op!]. As part
of this process pass in the expected accuracy of the operation to
eval_diff().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

12 files changed, 177 insertions, 109 deletions

diff --git a/doc/TODO b/doc/TODO
index ee20732..de1787c 100644
--- a/doc/TODO
+++ b/doc/TODO
@@ -1,8 +1,6 @@
 - Check bilinear filtering
 - Check repeating transformed sources
 - Check coordinates for more different transformations
-- Get a useful number being produced for error values, and correct eval_diff's
-  limit.
 - Check source/mask pixels falling outside the drawable.
 - Check trapezoids!
 - get_pixel equivalent that doesn't involve round-trips per call.
diff --git a/rendercheck.h b/rendercheck.h
index 3e9966d..34d06c5 100644
--- a/rendercheck.h
+++ b/rendercheck.h
@@ -117,9 +117,26 @@ get_pixel_from_image(XImage *image,
 		     int x, int y,
 		     color4d *color);
 
-int
-eval_diff(char *name, color4d *expected, color4d *test, int x, int y,
-    Bool verbose);
+void
+accuracy(XRenderDirectFormat *result,
+	 const XRenderDirectFormat *a,
+	 const XRenderDirectFormat *b);
+
+double
+eval_diff(const XRenderDirectFormat *accuracy,
+	  const color4d *expected,
+	  const color4d *test);
+
+void print_fail(const char *name,
+		const color4d *expected,
+		const color4d *test,
+		int x, int y,
+		double d);
+
+void print_pass(const char *name,
+		const color4d *expected,
+		int x, int y,
+		double d);
 
 void
 argb_fill(Display *dpy, picture_info *p, int x, int y, int w, int h, float a,
@@ -154,7 +171,7 @@ composite_test(Display *dpy, picture_info *win, picture_info *dst,
 	       const picture_info **src_color, int num_src,
 	       const picture_info **mask_color, int num_mask,
 	       const picture_info **dst_color, int num_dst,
-	       Bool componentAlpha, Bool print_errors);
+	       Bool componentAlpha);
 
 Bool
 dstcoords_test(Display *dpy, picture_info *win, int op, picture_info *dst,
diff --git a/t_blend.c b/t_blend.c
index 7847d07..525ea09 100644
--- a/t_blend.c
+++ b/t_blend.c
@@ -72,10 +72,20 @@ blend_test(Display *dpy, picture_info *win, picture_info *dst,
 
 	    y = 0;
 	    while (k0 < k) {
+		XRenderDirectFormat dst_acc;
+
+		accuracy(&dst_acc,
+			 &dst->format->direct,
+			 &dst_color[k0]->format->direct);
+
 		tdst = dst_color[k0]->color;
 		color_correct(dst, &tdst);
 
 		for (j = 0; j < num_src; j++) {
+		    XRenderDirectFormat acc;
+
+		    accuracy(&acc, &src_color[j]->format->direct, &dst_acc);
+
 		    for (i = 0; i < num_op; i++) {
 			get_pixel_from_image(image, dst, i, y, &tested);
 
@@ -87,11 +97,13 @@ blend_test(Display *dpy, picture_info *win, picture_info *dst,
 				     FALSE);
 			color_correct(dst, &expected);
 
-			snprintf(testname, 20, "%s blend", ops[op[i]].name);
-			if (!eval_diff(testname, &expected, &tested, 0, 0, is_verbose)) {
+			if (eval_diff(&acc, &expected, &tested) > 3.) {
 			    char srcformat[20];
 
+			    snprintf(testname, 20, "%s blend", ops[op[i]].name);
 			    describe_format(srcformat, 20, src_color[j]->format);
+			    print_fail(testname, &expected, &tested, 0, 0,
+				       eval_diff(&acc, &expected, &tested));
 			    printf("src color: %.2f %.2f %.2f %.2f (%s)\n"
 				   "dst color: %.2f %.2f %.2f %.2f\n",
 				   src_color[j]->color.r, src_color[j]->color.g,
@@ -101,8 +113,6 @@ blend_test(Display *dpy, picture_info *win, picture_info *dst,
 				   dst_color[k0]->color.b, dst_color[k0]->color.a);
 			    printf("src: %s, dst: %s\n", src_color[j]->name, dst->name);
 			    return FALSE;
-			} else if (is_verbose) {
-			    printf("src: %s, dst: %s\n", src_color[j]->name, dst->name);
 			}
 		    }
 		    y++;
diff --git a/t_composite.c b/t_composite.c
index 315650b..d305d1e 100644
--- a/t_composite.c
+++ b/t_composite.c
@@ -33,7 +33,7 @@ composite_test(Display *dpy, picture_info *win, picture_info *dst,
 	       const picture_info **src_color, int num_src,
 	       const picture_info **mask_color, int num_mask,
 	       const picture_info **dst_color, int num_dst,
-	       Bool componentAlpha, Bool print_errors)
+	       Bool componentAlpha)
 {
 	color4d expected, tested, tdst, tmsk;
 	char testname[40];
@@ -44,6 +44,7 @@ composite_test(Display *dpy, picture_info *win, picture_info *dst,
 	    color_correct(dst, &tdst);
 
 	    for (m = 0; m < num_mask; m++) {
+		XRenderDirectFormat mask_acc;
 		XImage *image;
 
 		if (componentAlpha) {
@@ -100,7 +101,16 @@ composite_test(Display *dpy, picture_info *win, picture_info *dst,
 		} else
 		    tmsk = mask_color[m]->color;
 
+		accuracy(&mask_acc,
+			 &mask_color[m]->format->direct,
+			 &dst_color[d]->format->direct);
+		accuracy(&mask_acc, &mask_acc, &dst->format->direct);
+
 		for (s = 0; s < num_src; s++) {
+		    XRenderDirectFormat acc;
+
+		    accuracy(&acc, &mask_acc, &src_color[s]->format->direct);
+
 		    for (i = 0; i < num_op; i++) {
 			get_pixel_from_image(image, dst, i, s, &tested);
 
@@ -109,38 +119,33 @@ composite_test(Display *dpy, picture_info *win, picture_info *dst,
 				     &expected, componentAlpha);
 			color_correct(dst, &expected);
 
-			snprintf(testname, 40,
-				 "%s %scomposite", ops[op[i]].name,
-				 componentAlpha ? "CA " : "");
-			if (!eval_diff(testname, &expected, &tested, 0, 0,
-				       is_verbose && print_errors)) {
-			    if (print_errors)
-				printf("src color: %.2f %.2f %.2f %.2f\n"
-				       "msk color: %.2f %.2f %.2f %.2f\n"
-				       "dst color: %.2f %.2f %.2f %.2f\n",
-				       src_color[s]->color.r,
-				       src_color[s]->color.g,
-				       src_color[s]->color.b,
-				       src_color[s]->color.a,
-				       mask_color[m]->color.r,
-				       mask_color[m]->color.g,
-				       mask_color[m]->color.b,
-				       mask_color[m]->color.a,
-				       dst_color[d]->color.r,
-				       dst_color[d]->color.g,
-				       dst_color[d]->color.b,
-				       dst_color[d]->color.a);
+			if (eval_diff(&acc, &expected, &tested) > 3.) {
+			    snprintf(testname, 40,
+				     "%s %scomposite", ops[op[i]].name,
+				     componentAlpha ? "CA " : "");
+			    print_fail(testname, &expected, &tested, 0, 0,
+				       eval_diff(&acc, &expected, &tested));
+			    printf("src color: %.2f %.2f %.2f %.2f\n"
+				   "msk color: %.2f %.2f %.2f %.2f\n"
+				   "dst color: %.2f %.2f %.2f %.2f\n",
+				   src_color[s]->color.r,
+				   src_color[s]->color.g,
+				   src_color[s]->color.b,
+				   src_color[s]->color.a,
+				   mask_color[m]->color.r,
+				   mask_color[m]->color.g,
+				   mask_color[m]->color.b,
+				   mask_color[m]->color.a,
+				   dst_color[d]->color.r,
+				   dst_color[d]->color.g,
+				   dst_color[d]->color.b,
+				   dst_color[d]->color.a);
 			    printf("src: %s, mask: %s, dst: %s\n",
 				   src_color[s]->name,
 				   mask_color[m]->name,
 				   dst->name);
 			    XDestroyImage(image);
 			    return FALSE;
-			} else if (is_verbose) {
-			    printf("src: %s, mask: %s, dst: %s\n",
-				   src_color[s]->name,
-				   mask_color[m]->name,
-				   dst->name);
 			}
 		    }
 		}
diff --git a/t_dstcoords.c b/t_dstcoords.c
index f6adab7..ed91ecc 100644
--- a/t_dstcoords.c
+++ b/t_dstcoords.c
@@ -59,9 +59,12 @@ dstcoords_test(Display *dpy, picture_info *win, int op, picture_info *dst,
 				expected = bg->color;
 
 			color_correct(dst, &expected);
-			if (!eval_diff("dst coords", &expected, &tested, x, y,
-			    is_verbose))
+			if (eval_diff(&dst->format->direct, &expected, &tested) > 2.0) {
+			    print_fail("dst coords",
+				       &expected, &tested, x, y,
+				       eval_diff(&dst->format->direct, &expected, &tested));
 				failed = TRUE;
+			}
 		}
 	}
 
diff --git a/t_fill.c b/t_fill.c
index f367cb8..4dd80a6 100644
--- a/t_fill.c
+++ b/t_fill.c
@@ -35,10 +35,15 @@ fill_test(Display *dpy, picture_info *win, picture_info *src)
 	char name[20];
 
 	get_pixel(dpy, src, 0, 0, &tested);
-
 	copy_pict_to_win(dpy, src, win, win_width, win_height);
 
-	strcpy(name, "fill ");
-	describe_format(name, 20 - strlen(name), src->format);
-	return eval_diff(name, &src->color, &tested, 0, 0, is_verbose);
+	if (eval_diff(&src->format->direct, &src->color, &tested) > 2.) {
+	    strcpy(name, "fill ");
+	    describe_format(name, 20 - strlen(name), src->format);
+	    print_fail(name, &src->color, &tested, 0, 0,
+		       eval_diff(&src->format->direct, &src->color, &tested));
+	    return FALSE;
+	}
+
+	return TRUE;
 }
diff --git a/t_gradient.c b/t_gradient.c
index 379c163..4f84403 100644
--- a/t_gradient.c
+++ b/t_gradient.c
@@ -305,8 +305,11 @@ Bool linear_gradient_test(Display *dpy, picture_info *win,
                                  &expected, False);
                     color_correct(dst, &expected);
 
-                    snprintf(testname, 40, "%s linear gradient", ops[op].name);
-                    if (!eval_diff(testname, &expected, &tested, 0, 0, is_verbose)) {
+                    if (eval_diff(&dst->format->direct, &expected, &tested) > 3.) {
+			snprintf(testname, 40,
+				 "%s linear gradient", ops[op].name);
+			print_fail(testname, &expected, &tested, 0, 0,
+				   eval_diff(&dst->format->direct, &expected, &tested));
                         printf("gradient: %d stops: %d repeat: %d pos: %d/%d\n"
                                "src color: %.2f %.2f %.2f %.2f\n"
                                "dst color: %.2f %.2f %.2f %.2f\n",
diff --git a/t_repeat.c b/t_repeat.c
index 92b95ea..bd5cd38 100644
--- a/t_repeat.c
+++ b/t_repeat.c
@@ -63,6 +63,7 @@ repeat_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 		char name[40];
 		color4d tdst, c1expected, c2expected;
 		XRenderPictureAttributes pa;
+		XRenderDirectFormat acc;
 
 		pa.component_alpha = test_mask;
 		pa.repeat = TRUE;
@@ -108,6 +109,12 @@ repeat_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 		tdst = dst_color->color;
 		color_correct(dst, &tdst);
 
+		accuracy(&acc,
+			 &dst->format->direct,
+			 &dst_color->format->direct);
+		accuracy(&acc, &acc, &c1->format->direct);
+		accuracy(&acc, &acc, &c2->format->direct);
+
 		if (!test_mask) {
 			do_composite(ops[op].op, &c1->color, NULL, &tdst,
 			    &c1expected, FALSE);
@@ -122,8 +129,6 @@ repeat_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 		color_correct(dst, &c1expected);
 		color_correct(dst, &c2expected);
 
-		snprintf(name, 40, "%dx%d %s %s-repeat", w, h,
-		    ops[op].name, test_mask ? "mask" : "src");
 		for (x = 0; x < TEST_WIDTH; x++) {
 		    for (y = 0; y < TEST_HEIGHT; y++) {
 			int samplex = x % w;
@@ -137,9 +142,14 @@ repeat_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 			}
 			get_pixel(dpy, dst, x, y, &tested);
 
-			if (!eval_diff(name, expected, &tested, x, y,
-			    is_verbose))
+			if (eval_diff(&acc, expected, &tested) > 3.) {
+			    snprintf(name, 40, "%dx%d %s %s-repeat", w, h,
+				     ops[op].name, test_mask ? "mask" : "src");
+
+			    print_fail(name, expected, &tested, x, y,
+				       eval_diff(&acc, expected, &tested));
 				failed = TRUE;
+			}
 		    }
 		}
 		XRenderFreePicture(dpy, src.pict);
diff --git a/t_srccoords.c b/t_srccoords.c
index c29a960..6aff6d5 100644
--- a/t_srccoords.c
+++ b/t_srccoords.c
@@ -90,7 +90,6 @@ srccoords_test(Display *dpy, picture_info *win, picture_info *white,
 	}
 
 	for (i = 0; i < 25; i++) {
-		char name[20];
 		int x = i % 5, y = i / 5;
 
 		if (!test_mask)
@@ -123,13 +122,12 @@ srccoords_test(Display *dpy, picture_info *win, picture_info *white,
 		} else
 			tested_colors[x][y] = 9;
 
-		if (test_mask)
-			snprintf(name, 20, "mask coords");
-		else
-			snprintf(name, 20, "src coords");
-		if (!eval_diff(name, &expected, &tested, x, y,
-		    is_verbose))
-			failed = TRUE;
+		if (eval_diff(&win->format->direct, &expected, &tested) > 2.) {
+		    print_fail(test_mask ? "mask coords" : "src coords",
+			       &expected, &tested, x, y,
+			       eval_diff(&win->format->direct, &expected, &tested));
+		    failed = TRUE;
+		}
 	}
 	if (failed) {
 		int j;
diff --git a/t_triangles.c b/t_triangles.c
index 230c2ff..645f507 100644
--- a/t_triangles.c
+++ b/t_triangles.c
@@ -88,10 +88,10 @@ triangles_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 
 		get_pixel(dpy, dst, x, y, &tested);
 
-		if (!eval_diff("triangles", &expected, &tested, x, y,
-		    is_verbose))
-		{
-			success = FALSE;
+		if (eval_diff(&dst->format->direct, &expected, &tested) > 2.) {
+		    print_fail("triangles", &expected, &tested, x, y,
+			       eval_diff(&dst->format->direct, &expected, &tested));
+		    success = FALSE;
 		}
 	    }
 	}
@@ -154,9 +154,9 @@ trifan_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 
 		get_pixel(dpy, dst, x, y, &tested);
 
-		if (!eval_diff("triangles", &expected, &tested, x, y,
-		    is_verbose))
-		{
+		if (eval_diff(&dst->format->direct, &expected, &tested) > 2.) {
+			print_fail("triangles", &expected, &tested, x,y,
+				   eval_diff(&dst->format->direct, &expected, &tested));
 			success = FALSE;
 		}
 	    }
@@ -220,10 +220,10 @@ tristrip_test(Display *dpy, picture_info *win, picture_info *dst, int op,
 
 		get_pixel(dpy, dst, x, y, &tested);
 
-		if (!eval_diff("triangles", &expected, &tested, x, y,
-		    is_verbose))
-		{
-			success = FALSE;
+		if (eval_diff(&dst->format->direct, &expected, &tested) > 2.) {
+		    print_fail("triangles", &expected, &tested, x, y,
+			       eval_diff(&dst->format->direct, &expected, &tested));
+		    success = FALSE;
 		}
 	    }
 	}
diff --git a/t_tsrccoords2.c b/t_tsrccoords2.c
index 1bb3974..89e92b1 100644
--- a/t_tsrccoords2.c
+++ b/t_tsrccoords2.c
@@ -121,7 +121,6 @@ trans_srccoords_test_2(Display *dpy, picture_info *win, picture_info *white,
 	}
 
 	for (i = 0; i < 25; i++) {
-		char name[20];
 		int x = i % 5, y = i / 5, srcx, srcy;
 
 		get_pixel(dpy, win, x, y, &tested);
@@ -142,12 +141,12 @@ trans_srccoords_test_2(Display *dpy, picture_info *win, picture_info *white,
 		} else
 			tested_colors[y][x] = 9;
 
-		if (test_mask)
-			snprintf(name, 20, "mask coords");
-		else
-			snprintf(name, 20, "src coords");
-		if (!eval_diff(name, &expected, &tested, x, y, is_verbose))
+		if (eval_diff(&win->format->direct, &expected, &tested) > 2.) {
+		    print_fail(test_mask ? "mask coords" : "src coords",
+			       &expected, &tested, x, y,
+			       eval_diff(&win->format->direct, &expected, &tested));
 			failed = TRUE;
+		}
 	}
 	if (failed) {
 		int x, y;
diff --git a/tests.c b/tests.c
index 66a1115..f722bc6 100644
--- a/tests.c
+++ b/tests.c
@@ -156,43 +156,63 @@ get_pixel(Display *dpy,
 	XDestroyImage(image);
 }
 
-int
-eval_diff(char *name, color4d *expected, color4d *test, int x, int y,
-    Bool verbose)
+void
+accuracy(XRenderDirectFormat *result,
+	 const XRenderDirectFormat *a,
+	 const XRenderDirectFormat *b)
 {
-	double rscale, gscale, bscale, ascale;
-	double rdiff, gdiff, bdiff, adiff, diff;
+    result->redMask = min(a->redMask, b->redMask);
+    result->greenMask = min(a->greenMask, b->greenMask);
+    result->blueMask = min(a->blueMask, b->blueMask);
+    result->alphaMask = min(a->alphaMask, b->alphaMask);
+}
 
-	/* XXX: Need to be provided mask shifts so we can produce useful error
-	 * values.
-	 */
-	rscale = 1.0 * (1 << 5);
-	gscale = 1.0 * (1 << 6);
-	bscale = 1.0 * (1 << 5);
-	ascale = 1.0 * 32;
-	rdiff = fabs(test->r - expected->r) * rscale;
-	bdiff = fabs(test->g - expected->g) * gscale;
-	gdiff = fabs(test->b - expected->b) * bscale;
-	adiff = fabs(test->a - expected->a) * ascale;
-	/*rdiff = log2(1.0 + rdiff);
+double
+eval_diff(const XRenderDirectFormat *format,
+	  const color4d *expected,
+	  const color4d *test)
+{
+	double rdiff, gdiff, bdiff, adiff;
+
+	rdiff = fabs(test->r - expected->r) * format->redMask;
+	bdiff = fabs(test->g - expected->g) * format->greenMask;
+	gdiff = fabs(test->b - expected->b) * format->blueMask;
+	adiff = fabs(test->a - expected->a) * format->alphaMask;
+
+#if 0
+	rdiff = log2(1.0 + rdiff);
 	gdiff = log2(1.0 + gdiff);
 	bdiff = log2(1.0 + bdiff);
-	adiff = log2(1.0 + adiff);*/
-	diff = max(max(max(rdiff, gdiff), bdiff), adiff);
-	if (diff > 3.0) {
-		printf("%s test error of %.4f at (%d, %d) --\n"
-		    "           R    G    B    A\n"
-		    "got:       %.2f %.2f %.2f %.2f\n"
-		    "expected:  %.2f %.2f %.2f %.2f\n", name, diff, x, y,
-		    test->r, test->g, test->b, test->a,
-		    expected->r, expected->g, expected->b, expected->a);
-		return FALSE;
-	} else if (verbose) {
-		printf("%s test succeeded at (%d, %d) with %.4f: "
-		    "%.2f %.2f %.2f %.2f\n", name, x, y, diff,
-		    expected->r, expected->g, expected->b, expected->a);
-	}
-	return TRUE;
+	adiff = log2(1.0 + adiff);
+#endif
+
+	return max(max(max(rdiff, gdiff), bdiff), adiff);
+}
+
+void print_fail(const char *name,
+		const color4d *expected,
+		const color4d *test,
+		int x, int y,
+		double d)
+{
+    printf("%s test error of %.4f at (%d, %d) --\n"
+	   "           R     G     B     A\n"
+	   "got:       %.3f %.3f %.3f %.3f\n"
+	   "expected:  %.3f %.3f %.3f %.3f\n",
+	   name, d, x, y,
+	   test->r, test->g, test->b, test->a,
+	   expected->r, expected->g, expected->b, expected->a);
+}
+
+void print_pass(const char *name,
+		const color4d *expected,
+		int x, int y,
+		double d)
+{
+    printf("%s test succeeded at (%d, %d) with %.4f: "
+	   "%.2f %.2f %.2f %.2f\n",
+	   name, x, y, d,
+	   expected->r, expected->g, expected->b, expected->a);
 }
 
 void
@@ -578,7 +598,7 @@ do {								\
 					test_src, num_test_src,
 					test_mask, num_test_mask,
 					test_dst, num_test_dst,
-					FALSE, TRUE);
+					FALSE);
 		    RECORD_RESULTS();
 		}
 		if (group_ok)
@@ -603,7 +623,7 @@ do {								\
 					test_src, num_test_src,
 					test_mask, num_test_mask,
 					test_dst, num_test_dst,
-					TRUE, TRUE);
+					TRUE);
 		    RECORD_RESULTS();
 		}
 		if (group_ok)