[Piglit] [PATCH 2/6] Add vbo support to shader_runner.
Paul Berry
stereotype441 at gmail.com
Mon Oct 24 15:40:01 PDT 2011
On 24 October 2011 14:56, Eric Anholt <eric at anholt.net> wrote:
> On Thu, 20 Oct 2011 14:12:24 -0700, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > This patch adds the ability for shader_runner tests to include a
> > "[vertex data]" section containing data in columnar format, for example:
> >
> > [vertex data]
> > vertex/float/3 foo/uint/1 bar/int/2
> > 0.0 0.0 0.0 0xe0000000 0 0
> > 0.0 1.0 0.0 0x70000000 1 1
> > 1.0 1.0 0.0 0x00000000 0 1
> >
> > Each column header is of the form ATTRNAME/TYPE/COUNT, where ATTRNAME
> > is the name of the vertex attribute to be bound to this column, TYPE
> > is the type of data that follows ("float", "int", or "uint"), and
> > COUNT is the vector length of the data (e.g. "3" for vec3 data).
> >
> > To send vertex data to the shader, use the new shader_runner command
> > "draw arrays". The parameters are the same as for the glDrawArrays()
> > command, so for example to draw triangle primitives using 3 elements
> > from the vertex data array starting at element 0, use the command:
> >
> > draw arrays GL_TRIANGLES 0 3
> >
> > More detailed examples can be found in the tests/shaders/vbo
> > directory.
> >
> > The implementation is largely in a new file, piglit-vbo.cpp, so that
> > it can be re-used by other piglit tests if necessary.
>
> For the other commits,
> Reviewed-by: Eric Anholt <eric at anholt.net>
>
> For this one, comments inline.
>
> > + } else if (sscanf(line, "draw arrays %31s %d %d", s, &x,
> &y)) {
> > + GLenum mode = decode_mode(s);
> > + int first = x;
> > + size_t count = (size_t) y;
> > + if (first < 0) {
> > + printf("draw arrays 'first' must be >=
> 0\n");
> > + piglit_report_result(PIGLIT_FAIL);
> > + } else if ((size_t) first >= num_vbo_rows) {
> > + printf("draw arrays 'first' must be <
> %lu\n",
> > + num_vbo_rows);
> > + piglit_report_result(PIGLIT_FAIL);
> > + }
> > + if (count <= 0) {
> > + printf("draw arrays 'count' must be >
> 0\n");
> > + piglit_report_result(PIGLIT_FAIL);
> > + } else if (count > num_vbo_rows - (size_t) first) {
> > + printf("draw arrays cannot draw beyond
> %lu\n",
> > + num_vbo_rows);
> > + piglit_report_result(PIGLIT_FAIL);
> > + }
> > + /* TODO: wrapper? */
> > + glDrawArrays(mode, first, count);
> > } else if (string_match("disable", line)) {
> > do_enable_disable(line + 7, false);
> > } else if (string_match("enable", line)) {
>
> Not sure what the TODO is about here.
>
Whoops, that was a reminder to myself to see if I needed to call a
piglit_DrawArrays() wrapper function instead of calling glDrawArrays()
directly. I'm pretty sure I don't, since this function is part of OpenGL
2.1 and every implementation that's worth testing these days supports OpenGL
2.1. I'll remove the comment.
>
> > @@ -1523,4 +1600,7 @@ piglit_init(int argc, char **argv)
> >
> > process_test_script(argv[1]);
> > link_and_use_shaders();
> > + if (vertex_data_start != NULL)
> > + num_vbo_rows = setup_vbo_from_text(prog, vertex_data_start,
> > + vertex_data_end);
> > }
> > diff --git a/tests/shaders/vbo/vbo-generic-float.shader_test
> b/tests/shaders/vbo/vbo-generic-float.shader_test
> > new file mode 100644
> > index 0000000..8982211
> > --- /dev/null
> > +++ b/tests/shaders/vbo/vbo-generic-float.shader_test
>
> The 3 new testcases in here surprised me -- I expected that patch 6/6
> was going to be the new testcases, though I suspect you wrote these for
> testing this shader_runner code.
>
> I would think that vbo-generic-uint would be part of
> tests/spec/glsl-1.30/ and the others part of tests/spec/glsl-1.10/? But
> given the other interpolation tests, it seems like generic-uint is the
> only interesting one here.
>
Yes, you're right--the other tests don't really verify any interesting
functionality of the GL implementation. I only wrote them to help me verify
that the new shader_runner functionality worked correctly. I was
considering dropping them entirely, but they seemed like a nice source of
extra documentation for how this new shader_runner feature works. That was
my primary motivation for putting them in tests/shaders/vbo--so they would
be nearer to the shader_runner.c code they were illustrating.
But perhaps I'm overthinking things. I'd be just as happy moving
vbo-generic-uint to tests/spec/glsl-1.30/ and dropping the others.
>
> > diff --git a/tests/util/piglit-vbo.cpp b/tests/util/piglit-vbo.cpp
> > new file mode 100644
> > index 0000000..fdba6b1
> > --- /dev/null
> > +++ b/tests/util/piglit-vbo.cpp
>
> > +/**
> > + * \file piglit-vbo.cpp
> > + *
> > + * This file adds the facility for specifying vertex data to piglit
> > + * tests using a columnar text format, for example:
> > + *
> > + * \verbatim
> > + * vertex/float/3 foo/uint/1 bar/int/2
> > + * 0.0 0.0 0.0 10 0 0 # comment
> > + * 0.0 1.0 0.0 5 1 1
> > + * 1.0 1.0 0.0 0 0 1
> > + * \endverbatim
> > + *
> > + * The format consists of a row of column headers followed by any
> > + * number of rows of data. Each column header has the form
> > + * "ATTRNAME/TYPE/COUNT", where ATTRNAME is the name of the vertex
> > + * attribute to be bound to this column, TYPE is the type of data that
> > + * follows ("float", "int", or "uint"), and COUNT is the vector length
> > + * of the data (e.g. "3" for vec3 data).
> > + *
> > + * The data follows the column headers in space-separated form. "#"
> > + * can be used for comments, as in shell scripts.
> > + *
> > + * To process textual vertex data, call the function
> > + * setup_vbo_from_text(), passing the int identifying the linked
> > + * program, and the string containing the vertex data. The return
> > + * value is the number of rows of vertex data found.
> > + *
> > + * If an error occurs, setup_vbo_from_text() will print out a
> > + * description of the error and exit with PIGLIT_FAIL.
> > + *
> > + * For the example above, the call to setup_vbo_from_text() is roughly
> > + * equivalent to the following GL operations:
> > + *
> > + * \code
> > + * struct vertex_attributes {
> > + * GLfloat vertex[3];
> > + * GLuint foo;
> > + * GLint bar[2];
> > + * } vertex_data[] = {
> > + * { { 0.0, 0.0, 0.0 }, 10, { 0, 0 } },
> > + * { { 0.0, 1.0, 0.0 }, 5, { 1, 1 } },
> > + * { { 1.0, 1.0, 0.0 }, 0, { 0, 1 } }
> > + * };
> > + * size_t stride = sizeof(vertex_data[0]);
> > + * GLint vertex_index = glGetAttribLocation(prog, "vertex");
> > + * GLint foo_index = glGetAttribLocation(prog, "foo");
> > + * GLint bar_index = glGetAttribLocation(prog, "bar");
> > + * GLuint buffer_handle;
> > + * glGenBuffers(1, &buffer_handle);
> > + * glBindBuffer(GL_ARRAY_BUFFER, buffer_handle);
> > + * glBufferData(GL_ARRAY_BUFFER, sizeof(vertex_data), &vertex_data,
> > + * GL_STATIC_DRAW);
> > + * glVertexAttribPointer(vertex_index, 3, GL_FLOAT, GL_FALSE, stride,
> > + * (void *) offsetof(vertex_attributes, vertex));
> > + * glVertexAttribIPointer(foo_index, 3, GL_UNSIGNED_INT, stride,
> > + * (void *) offsetof(vertex_attributes, foo));
> > + * glVertexAttribIPointer(bar_index, 3, GL_INT, stride,
> > + * (void *) offsetof(vertex_attributes, bar));
> > + * glEnableVertexAttribArray(vertex_index);
> > + * glEnableVertexAttribArray(foo_index);
> > + * glEnableVertexAttribArray(bar_index);
> > + * \endcode
> > + */
>
> Don't we need to do something to ensure that our vertex data ends up as
> attribute 0? Or that one of them does? Or is this something that is
> guaranteed by the API?
>
I don't think it matters whether our vertex data (or any of the vertex
attributes) ends up as attribute 0. The only things that are special about
attribute 0 are that (a) it aliases gl_Vertex, and (b) when using the
begin/end paradigm, it is the attribute that triggers the vertex shader to
execute. (a) shouldn't matter because this feature is intended to be used
with shaders that don't use gl_Vertex (perhaps I should document that more
clearly). (b) shouldn't matter because this feature doesn't use the
begin/end paradigm.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20111024/a5995ae1/attachment-0001.htm>
More information about the Piglit
mailing list