[Piglit] [PATCH] util: probe pixel rectangles as ubyte if possible

[Brian Paul]

On 04/11/2016 04:47 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> ---
>   tests/util/piglit-util-gl.c | 154 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 154 insertions(+)
>
> diff --git a/tests/util/piglit-util-gl.c b/tests/util/piglit-util-gl.c
> index dbdfb13..99a396b 100644
> --- a/tests/util/piglit-util-gl.c
> +++ b/tests/util/piglit-util-gl.c
> @@ -1060,6 +1060,23 @@ piglit_read_pixels_float(GLint x, GLint y, GLsizei width, GLsizei height,
>   	return pixels;
>   }
>
> +static bool
> +piglit_can_probe_ubyte()
> +{
> +	int r,g,b,a;
> +
> +	glGetIntegerv(GL_RED_BITS, &r);
> +	glGetIntegerv(GL_GREEN_BITS, &g);
> +	glGetIntegerv(GL_BLUE_BITS, &b);
> +	glGetIntegerv(GL_ALPHA_BITS, &a);
> +
> +	/* it could be LUMINANCE32F, etc. */
> +	if (!r && !g && !b && !a)
> +		return false;
> +
> +	return r <= 8 && g <= 8 && b <= 8 && a <= 8;
> +}
> +
>   int
>   piglit_probe_pixel_rgb_silent(int x, int y, const float* expected, float *out_probe)
>   {
> @@ -1158,6 +1175,57 @@ piglit_probe_pixel_rgba(int x, int y, const float* expected)
>   	return 0;
>   }
>
> +static void
> +piglit_array_float_to_ubyte(int n, const float *f, GLubyte *b)
> +{
> +	int i;
> +
> +	for (i = 0; i < n; i++)
> +		b[i] = f[i] * 255;
> +}
> +
> +static void
> +piglit_array_float_to_ubyte_roundup(int n, const float *f, GLubyte *b)
> +{
> +	int i;
> +
> +	for (i = 0; i < n; i++)
> +		b[i] = ceil(f[i] * 255);
> +}
> +
> +static int
> +piglit_probe_rect_rgb_ubyte_silent(int x, int y, int w, int h, const float *fexpected)

I know the public probe functions return int, but these new ones could 
return bool.  The public ones should probably be changed to bool someday 
too.


> +{
> +	int i, j, p;
> +	GLubyte *probe;
> +	GLubyte *pixels;
> +	GLubyte tolerance[3];
> +	GLubyte expected[3];
> +
> +	piglit_array_float_to_ubyte_roundup(3, piglit_tolerance, tolerance);
> +	piglit_array_float_to_ubyte(3, fexpected, expected);
> +
> +	/* RGBA readbacks are likely to be faster */
> +	pixels = malloc(w*h*4);
> +	glReadPixels(x, y, w, h, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
> +
> +	for (j = 0; j < h; j++) {
> +		for (i = 0; i < w; i++) {
> +			probe = &pixels[(j*w+i)*4];
> +
> +			for (p = 0; p < 3; ++p) {
> +				if (abs((int)probe[p] - (int)expected[p]) >= tolerance[p]) {
> +					free(pixels);
> +					return 0;
> +				}
> +			}
> +		}
> +	}
> +
> +	free(pixels);
> +	return 1;
> +}
> +
>   int
>   piglit_probe_rect_rgb_silent(int x, int y, int w, int h, const float *expected)
>   {
> @@ -1165,6 +1233,9 @@ piglit_probe_rect_rgb_silent(int x, int y, int w, int h, const float *expected)
>   	GLfloat *probe;
>   	GLfloat *pixels;
>
> +	if (piglit_can_probe_ubyte())
> +		return piglit_probe_rect_rgb_ubyte_silent(x, y, w, h, expected);
> +
>   	pixels = piglit_read_pixels_float(x, y, w, h, GL_RGB, NULL);
>
>   	for (j = 0; j < h; j++) {
> @@ -1216,6 +1287,83 @@ piglit_probe_rect_r_ubyte(int x, int y, int w, int h, GLubyte expected)
>   	return 1;
>   }
>
> +static int
> +piglit_probe_rect_rgb_ubyte(int x, int y, int w, int h, const float *fexpected)
> +{
> +	int i, j, p;
> +	GLubyte *probe;
> +	GLubyte *pixels;
> +	GLubyte tolerance[3];
> +	GLubyte expected[3];
> +
> +	piglit_array_float_to_ubyte_roundup(3, piglit_tolerance, tolerance);
> +	piglit_array_float_to_ubyte(3, fexpected, expected);
> +
> +	/* RGBA readbacks are likely to be faster */
> +	pixels = malloc(w*h*4);
> +	glReadPixels(x, y, w, h, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
> +
> +	for (j = 0; j < h; j++) {
> +		for (i = 0; i < w; i++) {
> +			probe = &pixels[(j*w+i)*4];
> +
> +			for (p = 0; p < 3; ++p) {
> +				if (abs((int)probe[p] - (int)expected[p]) >= tolerance[p]) {
> +					printf("Probe color at (%i,%i)\n", x+i, y+j);
> +					printf("  Expected: %u %u %u\n",
> +					       expected[0], expected[1], expected[2]);
> +					printf("  Observed: %u %u %u\n",
> +					       probe[0], probe[1], probe[2]);
> +
> +					free(pixels);
> +					return 0;
> +				}
> +			}
> +		}
> +	}
> +
> +	free(pixels);
> +	return 1;
> +}
> +
> +static int
> +piglit_probe_rect_rgba_ubyte(int x, int y, int w, int h, const float *fexpected)
> +{
> +	int i, j, p;
> +	GLubyte *probe;
> +	GLubyte *pixels;
> +	GLubyte tolerance[4];
> +	GLubyte expected[4];
> +
> +	piglit_array_float_to_ubyte_roundup(4, piglit_tolerance, tolerance);
> +	piglit_array_float_to_ubyte(4, fexpected, expected);
> +
> +	pixels = malloc(w*h*4);
> +	glReadPixels(x, y, w, h, GL_RGBA, GL_UNSIGNED_BYTE, pixels);
> +
> +	for (j = 0; j < h; j++) {
> +		for (i = 0; i < w; i++) {
> +			probe = &pixels[(j*w+i)*4];
> +
> +			for (p = 0; p < 4; ++p) {
> +				if (abs((int)probe[p] - (int)expected[p]) >= tolerance[p]) {
> +					printf("Probe color at (%i,%i)\n", x+i, y+j);
> +					printf("  Expected: %u %u %u %u\n",
> +					       expected[0], expected[1], expected[2], expected[3]);
> +					printf("  Observed: %u %u %u %u\n",
> +					       probe[0], probe[1], probe[2], probe[3]);
> +
> +					free(pixels);
> +					return 0;
> +				}
> +			}
> +		}
> +	}
> +
> +	free(pixels);
> +	return 1;
> +}

Seems like a lot of nearly identical code duplicated.  Couldn't you have 
one function that takes a bool 'silent' flag and a 'numComponents' (3 or 
4) parameter?

Looks OK otherwise.  I presume no piglit regressions.

-Brian


> +
>   int
>   piglit_probe_rect_rgb(int x, int y, int w, int h, const float *expected)
>   {
> @@ -1223,6 +1371,9 @@ piglit_probe_rect_rgb(int x, int y, int w, int h, const float *expected)
>   	GLfloat *probe;
>   	GLfloat *pixels;
>
> +	if (piglit_can_probe_ubyte())
> +		return piglit_probe_rect_rgb_ubyte(x, y, w, h, expected);
> +
>   	pixels = piglit_read_pixels_float(x, y, w, h, GL_RGBA, NULL);
>
>   	for (j = 0; j < h; j++) {
> @@ -1290,6 +1441,9 @@ piglit_probe_rect_rgba(int x, int y, int w, int h, const float *expected)
>   	GLfloat *probe;
>   	GLfloat *pixels;
>
> +	if (piglit_can_probe_ubyte())
> +		return piglit_probe_rect_rgba_ubyte(x, y, w, h, expected);
> +
>   	pixels = piglit_read_pixels_float(x, y, w, h, GL_RGBA, NULL);
>
>   	for (j = 0; j < h; j++) {
>