Fredrik Höglund <[email protected]> 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 <[email protected]> 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.
pgpR98xtQoQus.pgp
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
