[Piglit] [PATCH] for gles v1 extension oes_query_matrix
Huang, JunX A
junx.a.huang at intel.com
Fri Feb 28 01:07:14 PST 2014
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
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: oes_query_matrix.c
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140228/9b21c9f9/attachment-0001.c>
More information about the Piglit
mailing list