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. > + /* 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 :) > + > + /* 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. > + /* index >= GL_MAX_VERTEX_ATTRIB_BINDINGS */ > + glBindVertexBuffer(maxBindings, vbo, 0, 0); > + if (!piglit_check_gl_error(GL_INVALID_VALUE)) > + piglit_report_result(PIGLIT_FAIL); Instead of piglit_report_result(PIGLIT_FAIL) immediately, please set pass = false, and only at the end of the test do piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL); That way somebody writing a new implementation gets to see what's still failing as they incrementally make things work. Once these comments are fixed, this test is: Reviewed-by: Eric Anholt <[email protected]> Errors I don't see covered: "An INVALID_OPERATION error is generated if no vertex array object is bound." (When in core profile for BindVertexbuffer, VertexAttribBinding, VertexBindingDivisor) " [Core profile only:] An INVALID_OPERATION error is generated if buffer is not zero or a name returned from a previous call to GenBuffers, or if such a name has since been deleted with DeleteBuffers." So it seems like we might want a core profile-only extra errors test. A non-error I noticed not explicitly touched in this series: "If <buffer> is zero, any buffer object attached to this bindpoint is detached."
pgpGGGjBuMdf9.pgp
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
