[Piglit] [PATCH] for gles v1 extension oes_query_matrix

Brian Paul brianp at vmware.com
Wed Dec 18 07:54:48 PST 2013


On 12/17/2013 07:06 PM, Huang, JunX A wrote:
> Hi Paul,
>
> Thanks for your correcting!
> But I still can not follow your following saying, could you be more specific?
> "Also, maybe it should be structured so that the floating point matrix is first constructed from the mantisa & exponent arrays first.  Then, compare that matrix to the expected result."
> And here is the update file:

See below.


>
> /*
>   * Copyright © 2013 Intel Corporation
>   *
>   * 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.
>   */
>
> /**
>   * @file
>   * @brief Test GL_OES_query_matrix with types of operation in OpenGL ES 1.1.
>   *
>   * It uses the GL_FIXED data type.
>   *
>   */
>
> #include "piglit-util-gl-common.h"
> #include "piglit-util-egl.h"
>
>
> struct sub_test
> {
>      const char *name;
>      const GLfloat matrix[16];
> };
>
> PIGLIT_GL_TEST_CONFIG_BEGIN config.supports_gl_es_version = 11;
> PIGLIT_GL_TEST_CONFIG_END static PFNGLQUERYMATRIXXOESPROC glQueryMatrixxOES;
> static char sOut[256] = {0}, sTmp[256] = {0};
>
> static bool
> verifyQuery(struct sub_test
>   test, GLbitfield status)

Unneeded line break.


> {
> 	const GLfloat * matrix = test.matrix;
> 	GLfloat (*m)[4] = (void *) matrix;

That cast looks suspicious.  I don't know why you need 'm' at all.  See 
below.


> 	GLint i, j, k;
> 	bool functionPass = true;
> 	GLbitfield querystatus;
> 	GLfixed fMan[16];
> 	GLint iExp[16];
> 	GLfloat fQueryresult, fLimit, fDiff;

We usually put a blank line here between the declarations and code.

> 	sOut[0] = '\0';
> 	strcat(sOut, sTmp);
> 	querystatus = glQueryMatrixxOES(fMan, iExp);
> 	functionPass = piglit_check_gl_error(GL_NO_ERROR) && functionPass;
> 	snprintf(sTmp, sizeof(sTmp), "0x%x,0x%x, VerifyQuery:\n",
> 			status, querystatus);
> 	strcat(sOut, sTmp);
>
> 	for(i = 0; i < 4; i++){
> 		for(j = 0; j < 4; j++){
> 			k = j * 4 + i;
> 			strcat(sOut, "\t\t");
> 			fQueryresult = 1.0 * fMan[k] * (exp2 (iExp[k]) / (1 << 16));
> 			fLimit = 0.01 * (fabs(fQueryresult) + 1);
> 			fDiff = m[i][j] - fQueryresult;
> 			if((querystatus & (1 << k)) != 0 && (status & (1 << k)) != 0)
> 				strcat(sOut, "sameNA");
> 			else if(abs(fDiff) <= fLimit){
> 				strcat(sOut, "sameValue");
> 			}else{
> 				if(!(fDiff <= fLimit)){
> 					snprintf(sTmp, sizeof(sTmp),
> 							"should:%f<%f", (float) fDiff, (float) fLimit);
> 					strcat(sOut, sTmp);
> 				}else if(!(fDiff >= (-fLimit))){
> 					snprintf(sTmp, sizeof(sTmp),
> 							"should:%f>%f", (float) fDiff, (float) (-fLimit));
> 					strcat(sOut, sTmp);
> 				}else{
> 					snprintf(sTmp, sizeof(sTmp),
> 							"should:%f=%f,%i=%i", fQueryresult, m[i][j],
> 							querystatus & (1 << k), status & (1 << k));
> 					strcat(sOut, sTmp);
> 				}
> 				functionPass = false;
> 			}
> 		}
> 		strcat(sOut, "\n");
> 	}

I'd replace the above code with two loops.  First, construct the matrix 
from the mantissa/exponent info:


	GLfloat mat[4];
	for (k = 0; k < 16; k++) {
		mat[k] = 1.0 * fMan[k] * (exp2 (iExp[k]) / (1 << 16));
	}

Then, do the loops which check the values in the matrix:

	for (k = 0; k < 16; k++) {
		fLimit = .01 * (fabs(mat[k]) + 1);
		fDiff = matrix[k] - mat[k];
		if (fDiff indicates wrong value) {
			printf("Bad matrix value at position %d,"
				" expected %f, found %f\n",
				k, mat[k], matrix[k]);
			pass = false;
		}
	}

Also, all the snprintf(), strcat() code seems complicated.  What's the 
purpose of that?  Why not use a simple printf() when a problem is found?




> 	strcat(sOut, "\n");
> 	sTmp[0] = '\0';
> 	piglit_report_subtest_result(
> 					functionPass ? PIGLIT_PASS : PIGLIT_FAIL, test.name);
> 	return functionPass ? true : false;
> }
>
>
> /* new window size or exposure */
> static bool
> tests()

tests(void)


> {
> 	bool pass = true;
> 	GLint maxexp;
> 	GLfixed maxman;
> 	struct sub_test basedm = {

const?

> 		"based",
> 		{
> 			1, 0, 0, 0 ,
> 			0, 1, 0, 0,
> 			0, 0, 1, 0,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm1 = {
> 		"transformedm1",
> 		{
> 			6.666626, 0, 0, 0,
> 			0, 5, 0, 0,
> 			0, 0, -1.181793, -10.908936,
> 			0, 0, -1, 0
> 		}},
>
> 		transformedm2 = {
> 		"transformedm2",
> 		{
> 			0.444443, 0, 0, 0,
> 			0, 0.333328, 0, 0,
> 			0, 0, -0.666656, 0.333328,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm3 = {
> 		"transformedm3",
> 		{
> 			2, 0, 0,0,
> 			0, 3, 0, 0,
> 			0, 0, 0.500000, 0,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm4 = {
> 		"transformedm4",
> 		{
> 			2, 0, 0, 2,
> 			0, 3, 0, 6,
> 			0, 0, 0.500000, -3,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm5 = {
> 		"transformedm5",
> 		{
> 			1.999939, -0.002892, 0.005826, 0,
> 			0.002926, 2.999878, -0.017427, 0,
> 			-0.002905, 0.002913, 0.499977, 0,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm6 = {
> 		"transformedm6",
> 		{
> 			2, 0, 0, 0 ,
> 			0, 0, 0, 0,
> 			0, 0, 3.4E38, 0,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm7 = {
> 		"transformedm7",
> 		{
> 			1, 0, 0, 2,
> 			0, 1, 0, 0,
> 			0, 0, 1, 3.4E38,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm8 = {
> 		"transformedm8",
> 		{
> 			1, 0, 0, 2,
> 			0, 1, 0, 0,
> 			0, 0, 1, 3.4E38,
> 			0, 0, 0, 1
> 		}},
>
> 		transformedm9 = {
> 		"transformedm9",
> 		{
> 			2, 0, 0, 2,
> 			0, 0, 0, 0,
> 			0, 0, 3.4E38, 3.4E38,
> 			0, 0, 0, 1
> 		}};
>
> 	GLfloat ar = 3.0f / 4.0f;
>
> 	glMatrixMode(GL_PROJECTION);
> 	glLoadIdentity();
> 	glPushMatrix();
>
> 	snprintf(sTmp, sizeof(sTmp), "Identity:\n");
> 	pass = verifyQuery(basedm, 0) && pass;
>
> 	glFrustumf(-ar, ar, -1, 1, 5.0, 60.0);
> 	snprintf(sTmp, sizeof(sTmp), "Frustum:\n");
> 	pass = verifyQuery(transformedm1, 0) && pass;
>
>
>
> 	glPopMatrix();
> 	pass = verifyQuery(basedm, 0) && pass;
>
> 	glOrthof(-3.0 * ar, 3.0 * ar, -3.0, 3.0, -2.0, 1.0);
>
> 	pass = verifyQuery(transformedm2, 0) && pass;
>
>
> 	glMatrixMode(GL_MODELVIEW);
> 	glLoadIdentity();
> 	glScalef(2.0, 3.0, 0.5);
>
> 	pass = verifyQuery(transformedm3, 0) && pass;
>
> 	snprintf(sTmp, sizeof(sTmp), "Push,Translate:\n");

What is all the snprintf() code for?  The sTmp variable is never used 
AFAICT.


> 	glPushMatrix();
> 	glTranslatef(1.0, 2.0, -6.0);
>
> 	pass = verifyQuery(transformedm4, 0) && pass;
>
>
> 	snprintf(sTmp, sizeof(sTmp), "pop:%5Cn");
> 	glPopMatrix();
> 	pass = verifyQuery(transformedm3, 0) && pass;
>
>
> 	snprintf(sTmp, sizeof(sTmp), "Rotate:\n");
> 	glRotatef(0.5, 1.0, 1.0, 0.5);
>
> 	pass = verifyQuery(transformedm5, 0) && pass;
>
>

The following code needs comments to explain what it's doing.  In fact, 
the whole test has no comments at all.  Please look at other piglit 
tests to get an idea of how comments should be used.


> 	snprintf(sTmp, sizeof(sTmp), "overflow by max of GLfixed:\n");
> 	glLoadIdentity();
> 	maxexp = (1 << (8 * sizeof(GLint) - 1)) ^ (GLint) (-1);
> 	maxman = (1 << (8 * sizeof(GLfixed) - 1)) ^ (GLfixed) (-1);
> 	glTranslatef(1.0, 2.0, 1.0 * maxman * (exp2 (maxexp) / (1 << 16)));
>
> 	pass = verifyQuery(basedm, 0xf000) && pass;
>
> 	snprintf(sTmp, sizeof(sTmp), "invalid bits still:\n");
> 	glScalef(2.0, 3.0, 0.5);
>
> 	pass = verifyQuery(transformedm3, 0xf000) && pass;
>
> 	snprintf(sTmp, sizeof(sTmp),
> 					"revalid bits. glScalef with max and min of GLfloat:\n");

revalid?


> 	glLoadIdentity();
> 	glScalef(2.0, 3.4E-38, 3.4E38);
> 	pass = verifyQuery(transformedm6, 0) && pass;
>
>
> 	snprintf(sTmp, sizeof(sTmp),
> 						"glTranslatef with max and min of GLfloat:\n");
> 	glLoadIdentity();
> 	glTranslatef(2.0, 3.4E-38, 3.4E38);
> 	pass = verifyQuery(transformedm7, 0) && pass;
>
>
> 	snprintf(sTmp, sizeof(sTmp), "glRotatef a circle:\n");
> 	glRotatef(360, 0, 0, 0);
>
> 	pass = verifyQuery(transformedm7, 0) && pass;
>
>
> 	snprintf(sTmp, sizeof(sTmp), "glRotatef with max and min of GLfloat:\n");
> 	glRotatef(360, 2.0, 3.4E-38, 3.4E38);
> 	pass = verifyQuery(transformedm8, 0) && pass;
>
>
> 	snprintf(sTmp, sizeof(sTmp),
> 						"glScalef again with max and min of GLfloat:\n");
> 	glScalef(2.0, 3.4E-38, 3.4E38);
> 	pass = verifyQuery(transformedm9, 0) && pass;
>
>
> 	if(pass != true)

if (!pass)


> 		fprintf (stderr, "%s\nVerify Query Fail\n", sOut);
> 	return pass;
> }
>
> enum piglit_result
> piglit_display(void)
> {
> 	/* UNREACHED */
> 	return PIGLIT_FAIL;
> }
>
> void
> piglit_init(int argc, char **argv)
> {
> 	piglit_require_extension("GL_OES_query_matrix");
> 	glQueryMatrixxOES = (PFNGLQUERYMATRIXXOESPROC)
> 				eglGetProcAddress("glQueryMatrixxOES");
> 	if(!glQueryMatrixxOES)
> 		piglit_report_result(PIGLIT_FAIL);
> 	if(!piglit_check_gl_error(GL_NO_ERROR)){
> 		/* Should be no error at this point.  If there is, report failure */
> 		piglit_report_result(PIGLIT_FAIL);
> 	}
> 	piglit_report_result(tests() ? PIGLIT_PASS : PIGLIT_FAIL);
> }
>
>



More information about the Piglit mailing list