[Piglit] [PATCH] [v3] Add test case on error check on glGetTexImage with type
Ian Romanick
idr at freedesktop.org
Fri Dec 16 16:24:14 PST 2011
On 12/14/2011 06:10 PM, jian.j.zhao at intel.com wrote:
> From: Jian Zhao<jian.j.zhao at intel.com>
>
> As glGetTexImage in OpenGL 4.2 Reference Pages:
> "GL_INVALID_OPERATION is returned if type is one of GL_UNSIGNED_BYTE_3_3_2,
> GL_UNSIGNED_BYTE_2_3_3_REV, GL_UNSIGNED_SHORT_5_6_5,
> GL_UNSIGNED_SHORT_5_6_5_REV, or GL_UNSIGNED_INT_10F_11F_11F_REV and format is
> not GL_RGB".
All of these formats should be tested.
> v2: change the test name to be more specific and change some code style to be
> easier to read (per Brian's suggest).
>
> v3: move the case to the spec directory and rename it to getteximage_error_check.c (per Brian's suggest)
>
> Signed-off-by: Jian Zhao<jian.j.zhao at intel.com>
> ---
> tests/spec/ext_packed_float/CMakeLists.gl.txt | 1 +
> .../ext_packed_float/getteximage-error-check.c | 84 ++++++++++++++++++++
> 2 files changed, 85 insertions(+), 0 deletions(-)
> create mode 100644 tests/spec/ext_packed_float/getteximage-error-check.c
>
> diff --git a/tests/spec/ext_packed_float/CMakeLists.gl.txt b/tests/spec/ext_packed_float/CMakeLists.gl.txt
> index 78bbcd1..b6f8356 100644
> --- a/tests/spec/ext_packed_float/CMakeLists.gl.txt
> +++ b/tests/spec/ext_packed_float/CMakeLists.gl.txt
> @@ -13,5 +13,6 @@ link_libraries (
> )
>
> add_executable (ext_packed_float-pack pack.c)
> +add_executable (getteximage-error-check getteximage-error-check.c)
>
> # vim: ft=cmake:
> diff --git a/tests/spec/ext_packed_float/getteximage-error-check.c b/tests/spec/ext_packed_float/getteximage-error-check.c
> new file mode 100644
> index 0000000..50608fa
> --- /dev/null
> +++ b/tests/spec/ext_packed_float/getteximage-error-check.c
> @@ -0,0 +1,84 @@
> +/*
> + * 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
The name should actually be descriptive. Use
getteximage-invalid-format-for-packed-type. This matches the naming
pattern of other tests. Try 'find tests -name "*invalid*.c"' from the
top-level piglit directory.
> + *
> + * Tests on error check on glGetTexImage with type
> + * GL_UNSIGNED_INT_10F_11F_11F_REV should be GL_INVALID_OPERATION.
Please replace the text above with the text below without modification.
It's important in API tests like this to include specific
justification from the OpenGL spec.
* Page 262 (page 282 of the PDF) of the OpenGL 4.2 Compatibility
* Profile spec says:
*
* "The number of components per packed pixel is fixed by the
* type, and must match the number of components per group
* indicated by the format parameter, as listed in table 3.8.
* The error INVALID_OPERATION is generated by any command
* processing pixel rectangles if a mismatch occurs."
*
* Table 3.8 says:
*
* "type Parameter Token Name ... Matching Pixel Formats"
* UNSIGNED_BYTE_3_3_2 RGB, RGB_INTEGER
* UNSIGNED_BYTE_2_3_3_REV RGB, RGB_INTEGER
* UNSIGNED_SHORT_5_6_5 RGB, RGB_INTEGER
* UNSIGNED_SHORT_5_6_5_REV RGB, RGB_INTEGER
* UNSIGNED_SHORT_4_4_4_4 RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_SHORT_4_4_4_4_REV RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_SHORT_5_5_5_1 RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_SHORT_1_5_5_5_REV RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_INT_8_8_8_8 RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_INT_8_8_8_8_REV RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_INT_10_10_10_2 RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_INT_2_10_10_10_REV RGBA, BGRA, RGBA_INTEGER,
* BGRA_INTEGER
* UNSIGNED_INT_24_8 DEPTH_STENCIL
* UNSIGNED_INT_10F_11F_11F_REV RGB, RGB_INTEGER
* UNSIGNED_INT_5_9_9_9_REV RGB, RGB_INTEGER
* FLOAT_32_UNSIGNED_INT_24_8_REV DEPTH_STENCIL"
> + */
> +
> +#include "piglit-util.h"
> +
> +int piglit_width = 100, piglit_height = 100;
> +int piglit_window_mode = GLUT_RGBA | GLUT_DOUBLE;
> +
> +const GLenum formatTypes[] = {
We usually make these static too.
> + GL_RGBA,
> + GL_RGB,
> + GL_RED,
> + GL_GREEN,
> + GL_BLUE,
> + GL_ALPHA,
> + GL_LUMINANCE,
> + GL_LUMINANCE_ALPHA,
> +};
> +
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + long rcvError, expError = GL_NO_ERROR;
> + GLfloat *pxBuffer;
> + int i;
> +
> + pxBuffer = (GLfloat *)malloc(10 * 10 * 10 * 4 * sizeof(GLfloat));
Why malloc a fixed-size buffer that is never freed? Since there's no
texture set, this shouldn't need any data at all.
> + if (!pxBuffer)
> + return PIGLIT_FAIL;
> +
> + for(i=0; i< ARRAY_SIZE(formatTypes); i++)
> + {
Whitespace errors.
> + glGetTexImage(GL_TEXTURE_2D, 0, formatTypes[i],
> + GL_UNSIGNED_INT_10F_11F_11F_REV_EXT,
Should also test the non-REV format.
> + pxBuffer);
> + rcvError = glGetError();
> + if ( formatTypes[i] == GL_RGB )
Whitespace errors.
> + expError = GL_NO_ERROR;
> + else
> + expError = GL_INVALID_OPERATION;
> +
> + if (rcvError != expError)
> + return PIGLIT_FAIL;
This whole body of this loop be:
GLubyte buf[4];
const GLenum expected = (formatTypes[i] == GL_RGB)
? GL_NO_ERROR : GL_INVALID_OPERATION;
glGetTexImage(GL_TEXTURE_2D, 0, formatTypes[i],
GL_UNSIGNED_INT_10F_11F_11F_REV_EXT,
buf);
piglit_check_gl_error(expected, PIGLIT_FAIL);
This will need some changes to support testing formats other than
GL_UNSIGNED_INT_10F_11F_11F_REV_EXT. An inner loop (that loops over the
packed formats) seems like the right way to go.
> + }
This code doesn't display anything, and it's not a rendering test. All
of the checks should be in piglit_init. There are a lot of other tests
that work like that. See
tests/spec/gl-3.0/api/clearbuffer-invalid-buffer.c for a recent example.
> +
> + return PIGLIT_PASS;
> +}
> +
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + piglit_require_extension("GL_EXT_packed_float");
> + glewInit();
There's no need to call glewInit. It's already called by the piglit
framework.
> +}
More information about the Piglit
mailing list