On Friday 01 November 2013, Eric Anholt wrote: > 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.
Okay, I'll remove the error clearing. Fredrik _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
