[Piglit] [PATCH] Add test case on error check on glGetTexImage with type GL_UNSIGNED_INT_10F_11F_11F_REV should be GL_INVALID_OPERATION. As glGetTexImage in OpenGL 4.2 Reference Pages:

[Zhao, Jian J]

> -----Original Message-----
> From: Brian Paul [mailto:brianp at vmware.com]
> Sent: Tuesday, December 13, 2011 9:28 PM
> To: Zhao, Jian J
> Cc: piglit at lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH] Add test case on error check on glGetTexImage
> with type GL_UNSIGNED_INT_10F_11F_11F_REV should be
> GL_INVALID_OPERATION. As glGetTexImage in OpenGL 4.2 Reference Pages:
> 
> On 12/12/2011 10:47 PM, jian.j.zhao at intel.com wrote:
> > From: Jian Zhao<jian.j.zhao at intel.com>
> >
> > ---
> >   tests/general/CMakeLists.gl.txt |    1 +
> >   tests/general/getteximage.c     |   88
> +++++++++++++++++++++++++++++++++++++++
> 
> The test should be renamed to be more specific about what it does.
> getteximage.c is pretty generic.

OK. Is getteximage_arguments_mismatch_error.c OK? :) 

> 
> 
> >   2 files changed, 89 insertions(+), 0 deletions(-)
> >   create mode 100644 tests/general/getteximage.c
> >
> > diff --git a/tests/general/CMakeLists.gl.txt
> b/tests/general/CMakeLists.gl.txt
> > index 185f59d..e1d8f90 100644
> > --- a/tests/general/CMakeLists.gl.txt
> > +++ b/tests/general/CMakeLists.gl.txt
> > @@ -78,6 +78,7 @@ add_executable (pbo-readpixels-small
> pbo-readpixels-small.c)
> >   add_executable (pbo-teximage pbo-teximage.c)
> >   add_executable (pbo-teximage-tiling pbo-teximage-tiling.c)
> >   add_executable (pbo-teximage-tiling-2 pbo-teximage-tiling-2.c)
> > +add_executable (getteximage getteximage.c)
> >   add_executable (point-line-no-cull point-line-no-cull.c)
> >   add_executable (polygon-mode polygon-mode.c)
> >   add_executable (primitive-restart primitive-restart.c)
> > diff --git a/tests/general/getteximage.c b/tests/general/getteximage.c
> > new file mode 100644
> > index 0000000..7cf0cd8
> > --- /dev/null
> > +++ b/tests/general/getteximage.c
> > @@ -0,0 +1,88 @@
> > +/*
> > + * Copyright © 2009 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 getteximage_error_check.c
> > + *
> > + * Tests on error check on glGetTexImage with type
> > + * GL_UNSIGNED_INT_10F_11F_11F_REV should be
> GL_INVALID_OPERATION.
> > + */
> > +
> > +#include "piglit-util.h"
> > +
> > +int piglit_width = 100, piglit_height = 100;
> > +int piglit_window_mode = GLUT_RGBA | GLUT_DOUBLE;
> > +
> > +const GLenum formatTypes[] = {
> > +    GL_RGBA,
> > +    GL_RGB,
> > +    GL_RED,
> > +    GL_GREEN,
> > +    GL_BLUE,
> > +    GL_ALPHA,
> > +    GL_LUMINANCE,
> > +    GL_LUMINANCE_ALPHA,
> > +};
> > +
> > +
> > +enum piglit_result
> > +piglit_display(void)
> > +{
> > +	GLboolean pass = GL_FALSE;
> > +	long rcvError, expError = GL_NO_ERROR;
> > +	GLfloat *pxBuffer=0;
> 
> You don't have to initialize the pointer here.

OK. I can remove the initialization here because following have assigned it before using it. 

> 
> 
> > +	int i, j;
> > +
> > +	j = sizeof(formatTypes) / sizeof(GLenum);
> > +	pxBuffer = (GLfloat *)malloc(10 * 10 * 10 * 4 * sizeof(GLfloat));
> > +	if (!pxBuffer)
> > +		return PIGLIT_FAIL;
> > +
> > +	for(i=0; i<  j; i++)
> 
> I think this would be easier to read:
> 
> for (i = 0; i < ARRAY_SIZE(formatTypes); i++) {

Yes. I will use it. 

> 
> > +	{
> > +		glGetTexImage(GL_TEXTURE_2D, 0, formatTypes[i],
> > +			    GL_UNSIGNED_INT_10F_11F_11F_REV_EXT,
> > +			    pxBuffer);
> > +		rcvError = glGetError();
> > +		if ( formatTypes[i] == GL_RGB )
> > +			expError = GL_NO_ERROR;
> > +		else
> > +			expError = GL_INVALID_OPERATION;
> > +
> > +		if (rcvError == expError)
> > +			pass&= GL_TRUE;
> > +		else
> > +			return PIGLIT_FAIL;
> 
> I think you can simplify this to
> 
> if (rcvError != expError)
>         return PIGLIT_FAIL;
> 
> Then get rid of the pass variable entirely.

Exactly! I will change it. 

Thanks for your help! I will update the test case and send out soon. 

Best regards
ZhaoJian

> 
> 
> > +	}
> > +
> > +	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> > +}
> > +
> > +
> > +void
> > +piglit_init(int argc, char **argv)
> > +{
> > +	piglit_require_extension("GL_EXT_packed_float");
> > +	glewInit();
> > +}
> >
> >
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit