[Piglit] [PATCH 1/7] arb_vertex_attrib_binding: Test error conditions
Fredrik Höglund
fredrik at kde.org
Fri Nov 1 11:45:26 PDT 2013
On Friday 01 November 2013, Eric Anholt wrote:
> Fredrik Höglund <fredrik at kde.org> writes:
>
> > On Tuesday 29 October 2013, Eric Anholt wrote:
> >> Sorry we all dropped this on the floor. I'm taking a look at your
> >> series now.
> >>
> >> Fredrik Höglund <fredrik at kde.org> writes:
> >> > diff --git a/tests/spec/arb_vertex_attrib_binding/errors.c b/tests/spec/arb_vertex_attrib_binding/errors.c
> >> > new file mode 100644
> >> > index 0000000..adac098
> >> > --- /dev/null
> >> > +++ b/tests/spec/arb_vertex_attrib_binding/errors.c
> >>
> >> > +#define MIN(a, b) (a < b ? a : b)
> >>
> >> There's already a MIN2 macro you can use.
> >
> > This test doesn't actually use the macro. I guess it's a left over from the
> > test I based it on.
> >
> >> > + /* uncreached */
> >>
> >> "unreached"
> >>
> >> > +void
> >> > +piglit_init(int argc, char **argv)
> >> > +{
> >> > + GLint maxBindings, maxAttribs, maxRelativeOffset;
> >> > + GLuint vbo, vao;
> >> > + int i;
> >> > +
> >> > + piglit_require_extension("GL_ARB_vertex_attrib_binding");
> >> > + piglit_require_extension("GL_ARB_vertex_array_bgra");
> >> > + piglit_require_extension("GL_ARB_vertex_type_2_10_10_10_rev");
> >> > + piglit_require_extension("GL_ARB_ES2_compatibility");
> >>
> >> Would be nice to see the extra extensions moved into
> >> piglit_is_extension_supported() checks for executing those tests.
> >> Though I suppose since a lot of the code for them is expecting
> >> INVALID_ENUM, you can execute that with or without the extension
> >> providing them :)
> >
> > Yeah, I've been meaning to do that, but I wanted to hear what others had
> > to say about the ARB_vertex_attrib_binding dependencies first.
> >
> >> > +
> >> > + /* Create and bind a vertex array object */
> >> > + glGenVertexArrays(1, &vao);
> >> > + glBindVertexArray(vao);
> >> > +
> >> > + glGenBuffers(1, &vbo);
> >> > + glBindBuffer(GL_ARRAY_BUFFER, vbo);
> >> > +
> >> > +
> >> > + /* Query limits */
> >> > + glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &maxAttribs);
> >> > + glGetIntegerv(GL_MAX_VERTEX_ATTRIB_BINDINGS, &maxBindings);
> >> > + glGetIntegerv(GL_MAX_VERTEX_ATTRIB_RELATIVE_OFFSET, &maxRelativeOffset);
> >> > +
> >> > + /* Clear the error state */
> >> > + while (glGetError() != GL_NO_ERROR) {}
> >>
> >> What error state are you clearing here? If you really do expect to have
> >> something you're ignoring, piglit_reset_gl_error() is the helper for
> >> this.
> >
> > There shouldn't be any errors at this point, it's just make sure we
> > start with a clean slate.
> >
> > Other piglit tests seem to do this as well.
>
> They shouldn't, if there shouldn't have been an error produced. It's
> like the extra glFinish()es some tests still have -- I haven't
> successfully kept them all from sneaking in.
>
> On the other hand, I would like to see the framework catch leftover GL
> errors after test completion and throw a FAIL, but that would require
> testing to fix whatever tests might leave one around currently.
Okay, I'll remove the error clearing.
Fredrik
More information about the Piglit
mailing list