[Piglit] [PATCH] for gles v1 extension oes_query_matrix

Huang, JunX A junx.a.huang at intel.com
Tue Mar 4 17:36:31 PST 2014


Soft reminder.

-----Original Message-----
From: piglit-bounces at lists.freedesktop.org [mailto:piglit-bounces at lists.freedesktop.org] On Behalf Of Huang, JunX A
Sent: Friday, February 28, 2014 5:07 PM
To: Ian Romanick; Brian Paul; piglit at lists.freedesktop.org
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:idr at freedesktop.org]
Sent: Tuesday, December 24, 2013 4:54 AM
To: Brian Paul; Huang, JunX A; piglit at lists.freedesktop.org
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];
                  ^ 16

>     for (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
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
> 



More information about the Piglit mailing list