[Piglit] [PATCH] drawpix-format-errors: test error checking in glDraw/ReadPixels, glTexImage

Brian Paul brianp at vmware.com
Fri Feb 3 07:25:56 PST 2012


On 02/03/2012 06:25 AM, nobled wrote:
> On Thu, Feb 2, 2012 at 9:09 AM, Brian Paul<brianp at vmware.com>  wrote:
>> Test that GL_INVALID_ENUM or GL_INVALID_OPERATION is generated when
>> expected by these functions.
>>
>> The NVIDIA driver fails for one case (out of ~60):
>>
>> Failure testing glReadPixels:
>>   format=GL_RGBA, type=GL_BITMAP
>>   expected error GL_INVALID_ENUM, found error GL_INVALID_OPERATION
>>
>> Mesa (at commit cff0eac70) has quite a few failures but they'll be
>> fixed with some new patches.
>> ---
>>   tests/all.tests                       |    1 +
>>   tests/general/CMakeLists.gl.txt       |    1 +
>>   tests/general/drawpix-format-errors.c |  227 +++++++++++++++++++++++++++++++++
>>   3 files changed, 229 insertions(+), 0 deletions(-)
>>   create mode 100644 tests/general/drawpix-format-errors.c
>>
>> diff --git a/tests/all.tests b/tests/all.tests
>> index d0f18b4..4dddd02 100644
>> --- a/tests/all.tests
>> +++ b/tests/all.tests
>> @@ -250,6 +250,7 @@ add_plain_test(general, 'draw-vertices')
>>   general['draw-vertices-user'] = PlainExecTest(['draw-vertices', '-auto', 'user'])
>>   add_plain_test(general, 'draw-vertices-half-float')
>>   general['draw-vertices-half-float-user'] = PlainExecTest(['draw-vertices-half-float', '-auto', 'user'])
>> +add_plain_test(general, 'drawpix-format-errors')
>>   add_plain_test(general, 'early-z')
>>   add_plain_test(general, 'fog-modes')
>>   add_plain_test(general, 'fragment-center')
>> diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt
>> index 12e2f9e..f716012 100644
>> --- a/tests/general/CMakeLists.gl.txt
>> +++ b/tests/general/CMakeLists.gl.txt
>> @@ -46,6 +46,7 @@ add_executable (draw-pixel-with-texture draw-pixel-with-texture.c)
>>   add_executable (draw-sync draw-sync.c)
>>   add_executable (draw-vertices draw-vertices.c)
>>   add_executable (draw-vertices-half-float draw-vertices-half-float.c)
>> +add_executable (drawpix-format-errors drawpix-format-errors.c)
>>   add_executable (fog-modes fog-modes.c)
>>   IF (UNIX)
>>         target_link_libraries (fog-modes m)
>> diff --git a/tests/general/drawpix-format-errors.c b/tests/general/drawpix-format-errors.c
>> new file mode 100644
>> index 0000000..014a7c7
>> --- /dev/null
>> +++ b/tests/general/drawpix-format-errors.c
>> @@ -0,0 +1,227 @@
>> +/*
>> + * Copyright 2012, VMware, Inc.
>> + *
>> + * 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.
>> + */
>> +
>> +/*
>> + * Test that bad format/type params to glDrawPixels, glReadPixels and
>> + * glTexImage generate the right error code.
>> + *
>> + * Brian Paul
>> + * 2 Feb 2012
>> + */
>> +
>> +
>> +#include<stdbool.h>
>> +#include "piglit-util.h"
>> +
>> +int piglit_width = 100, piglit_height = 100;
>> +int piglit_window_mode = GLUT_RGB;
>> +
>> +
>> +struct combo {
>> +       GLenum format, type, expected_error;
>> +};
>> +
>> +
>> +/* From the glRead/DrawPixels and glTexImage man pages:
>> + *
>> + * GL_INVALID_ENUM is generated if format or type is not an accepted value.
>> + *
>> + * GL_INVALID_ENUM is generated if type is GL_BITMAP and format is
>> + * not GL_COLOR_INDEX or GL_STENCIL_INDEX.
>> + *
>> + * GL_INVALID_OPERATION is generated if type is one of
>> + * GL_UNSIGNED_BYTE_3_3_2, GL_UNSIGNED_BYTE_2_3_3_REV,
>> + * GL_UNSIGNED_SHORT_5_6_5, or GL_UNSIGNED_SHORT_5_6_5_REV and format is
>> + * not GL_RGB.
>> + *
>> + * GL_INVALID_OPERATION is generated if type is one of
>> + * GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_SHORT_4_4_4_4_REV,
>> + * GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_SHORT_1_5_5_5_REV,
>> + * GL_UNSIGNED_INT_8_8_8_8, GL_UNSIGNED_INT_8_8_8_8_REV,
>> + * GL_UNSIGNED_INT_10_10_10_2, or GL_UNSIGNED_INT_2_10_10_10_REV and
>> + * format is neither GL_RGBA nor GL_BGRA.
>> + *
>> + */
>> +
>> +#define BOGUS GL_BLEND
>> +
>> +static const struct combo combos[] = {
>> +       /* valid cases */
>> +       { GL_RGBA, GL_UNSIGNED_BYTE, GL_NO_ERROR },
>> +       { GL_RGBA, GL_FLOAT, GL_NO_ERROR },
>> +       { GL_RGBA, GL_UNSIGNED_SHORT_4_4_4_4, GL_NO_ERROR },
>> +       { GL_BGRA, GL_UNSIGNED_SHORT_4_4_4_4, GL_NO_ERROR },
>> +       { GL_RGB, GL_UNSIGNED_BYTE_3_3_2, GL_NO_ERROR },
>> +       { GL_COLOR_INDEX, GL_BITMAP, GL_NO_ERROR },
>> +       { GL_RGBA, GL_UNSIGNED_INT_8_8_8_8, GL_NO_ERROR },
>> +       { GL_RGBA, GL_UNSIGNED_INT_8_8_8_8_REV, GL_NO_ERROR },
>> +
>> +       /* invalid enums */
>> +       { GL_RGBA, BOGUS, GL_INVALID_ENUM },
>> +       { BOGUS, GL_UNSIGNED_BYTE, GL_INVALID_ENUM },
>> +       { BOGUS, BOGUS, GL_INVALID_ENUM },
>> +       { GL_RGBA, GL_BITMAP, GL_INVALID_ENUM },
>> +       { GL_UNSIGNED_BYTE, GL_RGBA, GL_INVALID_ENUM },
>> +
>> +       /* invalid operations */
>> +       { GL_RGB, GL_UNSIGNED_INT_8_8_8_8, GL_INVALID_OPERATION },
>> +       { GL_LUMINANCE, GL_UNSIGNED_INT_8_8_8_8_REV, GL_INVALID_OPERATION },
>> +       { GL_RGBA, GL_UNSIGNED_SHORT_5_6_5, GL_INVALID_OPERATION },
>> +       { GL_RGB, GL_UNSIGNED_SHORT_4_4_4_4, GL_INVALID_OPERATION },
>> +       { GL_RGB, GL_UNSIGNED_SHORT_4_4_4_4_REV, GL_INVALID_OPERATION },
>> +       { GL_BGR, GL_UNSIGNED_SHORT_4_4_4_4, GL_INVALID_OPERATION },
>> +       { GL_RGB, GL_UNSIGNED_SHORT_5_5_5_1, GL_INVALID_OPERATION },
>> +       { GL_BGR, GL_UNSIGNED_BYTE_3_3_2, GL_INVALID_OPERATION }
>> +
>> +        /* Omit this case since it could generate invalid operation or
>> +         * invalid enum depending on the order that the arguments are
>> +         * evaluated.
>> +         */
>> +#if 0
>> +       { BOGUS, GL_UNSIGNED_INT_8_8_8_8, GL_INVALID_OPERATION }
>> +#endif
>> +};
>> +
>> +
>> +#define NUM_COMBOS ARRAY_SIZE(combos)
>> +
>> +
>> +/* XXX it would be nice to have a piglit util function for this */
>> +static const char *
>> +lookup_enum(GLenum val)
>> +{
>> +       static const struct {
>> +               GLenum val;
>> +               const char *name;
>> +       } tokens[] = {
>> +               { BOGUS, "bogus value" },
>> +               { GL_RGBA, "GL_RGBA" },
>> +               { GL_BGRA, "GL_BGRA" },
>> +               { GL_RGB, "GL_RGB" },
>> +               { GL_BGR, "GL_BGR" },
>> +               { GL_LUMINANCE, "GL_LUMINANCE" },
>> +               { GL_COLOR_INDEX, "GL_COLOR_INDEX" },
>> +               { GL_BITMAP, "GL_BITMAP" },
>> +               { GL_FLOAT, "GL_FLOAT" },
>> +               { GL_UNSIGNED_BYTE, "GL_UNSIGNED_BYTE" },
>> +               { GL_UNSIGNED_BYTE_3_3_2, "GL_UNSIGNED_BYTE_3_3_2" },
>> +               { GL_UNSIGNED_SHORT_4_4_4_4, "GL_UNSIGNED_SHORT_4_4_4_4" },
>> +               { GL_UNSIGNED_SHORT_4_4_4_4_REV, "GL_UNSIGNED_SHORT_4_4_4_4_REV" },
>> +               { GL_UNSIGNED_SHORT_5_6_5, "GL_UNSIGNED_SHORT_5_6_5" },
>> +               { GL_UNSIGNED_SHORT_5_6_5_REV, "GL_UNSIGNED_SHORT_5_6_5_REV" },
>> +               { GL_UNSIGNED_SHORT_5_5_5_1, "GL_UNSIGNED_SHORT_5_5_5_1" },
>> +               { GL_UNSIGNED_INT_8_8_8_8, "GL_UNSIGNED_INT_8_8_8_8" },
>> +               { GL_UNSIGNED_INT_8_8_8_8_REV, "GL_UNSIGNED_INT_8_8_8_8_REV" }
>> +       };
>> +
>> +       int i;
>> +       for (i = 0; i<  ARRAY_SIZE(tokens); i++) {
>> +               if (tokens[i].val == val)
>> +                       return tokens[i].name;
>> +       }
>> +       return "???";
>> +}
>> +
>> +
>> +enum functions {
>> +       DRAWPIX,
>> +       READPIX,
>> +       TEXIMAGE,
>> +       NUM_FUNCS
>> +};
>> +
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> +       GLubyte buffer[100];
>> +       bool pass = true;
>> +       GLenum err;
>> +       int i;
>> +       int func;
>> +       char *func_name = "";
>> +
>> +       memset(buffer, 0, sizeof(buffer));
>> +
>> +       for (func = 0; func<  NUM_FUNCS; func++) {
>> +               for (i = 0; i<  NUM_COMBOS; i++) {
>> +                       GLenum format = combos[i].format;
>> +                       GLenum type = combos[i].type;
>> +                       GLenum expected = combos[i].expected_error;
>> +
>> +                       piglit_reset_gl_error();
>> +
>> +                       switch (func) {
>> +                       case DRAWPIX:
>> +                               func_name = "glDrawPixels";
>> +                               glDrawPixels(2, 2, format, type, buffer);
>> +                               break;
>> +                       case READPIX:
>> +                               func_name = "glReadPixels";
>> +                               if (format == GL_COLOR_INDEX) {
>> +                                       /* special case / skip */
>> +                                       err = expected;
> Don't you want "continue;" here and below? Otherwise "err =
> glGetError()" will still be run after the switch statement,
> overwriting this.

Oops, yes.  I changed this a couple times during development and never 
finished that up.


>> +                               }
> (All the curly braces to wrap a single statement seem kind of
> gratuitous, is that supposed to be the piglit style?)

It's a habit.  It seems that I'm always adding printfs or assertions 
inside of if clauses and wind up having to add braces.  So I just put 
in the braces right away.

I always try to keep ease of debugging in mind when I'm writing code.

-Brian


More information about the Piglit mailing list