[Piglit] [PATCH 1/7] arb_vertex_attrib_binding: Test error conditions
Eric Anholt
eric at anholt.net
Tue Oct 29 23:35:50 CET 2013
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.
> + /* 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 <eric at anholt.net>
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."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131029/6420ff05/attachment.pgp>
More information about the Piglit
mailing list