[Piglit] [PATCH] for gles v1 extension oes_query_matrix
Brian Paul
brianp at vmware.com
Fri Mar 28 12:21:33 PDT 2014
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; piglit at lists.freedesktop.org
> Subject: RE: [Piglit] [PATCH] for gles v1 extension oes_query_matrix
>
> 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
>> 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
>>
>
More information about the Piglit
mailing list