Is the objective of the test to thoroughly test glQueryMatrixOES() or just spot-check a few potentially problematic cases?
-Brian On 03/28/2014 02:32 AM, Huang, JunX A wrote:
Soft reminder, same attached. -----Original Message----- From: Huang, JunX A Sent: Wednesday, March 05, 2014 9:37 AM To: Huang, JunX A; Ian Romanick; Brian Paul; [email protected] Subject: RE: [Piglit] [PATCH] for gles v1 extension oes_query_matrix Soft reminder. -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Huang, JunX A Sent: Friday, February 28, 2014 5:07 PM To: Ian Romanick; Brian Paul; [email protected] Subject: Re: [Piglit] [PATCH] for gles v1 extension oes_query_matrix Importance: High Hi Paul, The reason for using snprintf(), strcat() is that I wanted to output the whole matrix as 4*4 table when any position went wrong, but you are right, it was too complicate, so I change it as your suggestion, only print the fail position. And I also add some comment in tests function, as your suggestion too. Hi Ian, I cannot set a #define for glQueryMatrixxOES because there is already a define in my local driver file "glext.h", so I use piglit_QueryMatrixxOES directly. Thanks for your suggestion! And I make some other small changes, as new file attached. Thanks again~ Jun -----Original Message----- From: Ian Romanick [mailto:[email protected]] Sent: Tuesday, December 24, 2013 4:54 AM To: Brian Paul; Huang, JunX A; [email protected] Subject: Re: [Piglit] [PATCH] for gles v1 extension oes_query_matrix Importance: High On 12/18/2013 07:54 AM, Brian Paul wrote: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:I would like some flavor of Brian's suggestion.GLfloat mat[4];^ 16for (k = 0; k < 16; k++) {^^ ARRAY_SIZE(mat)mat[k] = 1.0 * fMan[k] * (exp2 (iExp[k]) / (1 << 16));Or mat[k] = ldexp(fMan[k], iExp[k] - 16); ...assuming ldexp is available in MSVC.} 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");Do *NOT* name the function pointer the same as the function. Use piglit_QueryMatrixxOES and have a #define for glQueryMatrixxOES instead.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); }_______________________________________________ Piglit mailing list [email protected] https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/piglit&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=HgXHUmRsUv2za9vOTp0rZW0PL2I1V7r8WXA4k0dv5%2BM%3D%0A&s=d666fccb8dc37b0f67dd07af7dd5f65ebd3a8ded235957cd02089da0eaebc7f0
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
