[Piglit] [PATCH] util: Add array_index support to [vertex data]
Alejandro Piñeiro
apinheiro at igalia.com
Mon May 2 09:07:04 UTC 2016
On 25/04/16 15:56, Andres Gomez wrote:
> This allows data to be set for arbitrary array sized attributes in
> shader runner.
>
> For example to set mat2x3[2]:
> attname[0]/mat2x3/3/1 attname[0]/mat2x3/3/2 attname[1]/mat2x3/3/1 attname[1]/mat2x3/3/2
>
> The syntax has been extended so the recommended type to specify in the
> [vertex data] header line is the GLSL corresponding one, not just
> "float", "double", "int" or "uint".
>
> In any case, the extended syntax is backward compatible so it is still
> possible to specify a vec3 attribute like:
> attname/float/3
>
> In addition to the now recommended format:
> attname/vec3/3
Nitpick: Are you sure that is the recommended instead of just the
extended one?
>
> Signed-off-by: Andres Gomez <agomez at igalia.com>
> ---
> tests/util/piglit-vbo.cpp | 154 +++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 118 insertions(+), 36 deletions(-)
>
> diff --git a/tests/util/piglit-vbo.cpp b/tests/util/piglit-vbo.cpp
> index 11a4adc..749a1ed 100644
> --- a/tests/util/piglit-vbo.cpp
> +++ b/tests/util/piglit-vbo.cpp
> @@ -1,5 +1,5 @@
> /*
> - * Copyright © 2011 Intel Corporation
> + * Copyright © 2011, 2016 Intel Corporation
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> * copy of this software and associated documentation files (the "Software"),
> @@ -28,29 +28,33 @@
> * 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
> + * vertex/vec3/3 foo/uint/1 bar[0]/int/1 bar[1]/int/1
> + * 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/MATRIX_COLUMN", 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"), COUNT is the
> - * vector length of the data (e.g. "3" for vec3 data) and
> - * MATRIX_COLUMN is the column number of the data in case of being a
> - * matrix. MATRIX_COLUMN can also be used for arrays of
> - * arrays. MATRIX_COLUMN doesn't need to be be specified if it is not
> - * a matrix column as in the example before. So another example, if
> - * you want to specify the data of a mat2x3:
> + * "ATTRNAME[ARRAY_INDEX]/TYPE/COUNT/MATRIX_COLUMN", where ATTRNAME is
> + * the name of the vertex attribute to be bound to this column,
> + * ARRAY_INDEX is the index, TYPE is the GLSL type of data that
> + * follows ("float", "double", "int", "uint" or any derived type like
> + * vec2, dmat4, etc.), COUNT is the vector length of the data
> + * (e.g. "3" for vec3 data or "2" for mat4x2) and MATRIX_COLUMN is the
> + * column number of the data in case of being a matrix. [ARRAY_INDEX]
> + * is optional and needs to be specified only in case of array
> + * attributes. COUNT is a redundant value that is already specified in
> + * the data type but that has been kept for backward
> + * compatibility.
Now that you mention backward compatibility, I think that it would be
good to include some examples with the old format. After all, is the
format of most tests right now.
> MATRIX_COLUMN doesn't need to be specified if it is
> + * not a matrix column as in the example before. So another example,
> + * if you want to specify the data of a mat2x3:
> *
> - * \verbatim
> - * foomatrix/float/3/0 foomatrix/float/3/1
> - * 0.0 1.0 2.0 3.0 4.0 5.0
> - * 6.0 7.0 8.0 9.0 10.0 11.0
> - * \endverbatim
> + * \verbatim
> + * foomatrix/mat2x3/3/0 foomatrix/mat2x3/3/1
> + * 0.0 1.0 2.0 3.0 4.0 5.0
> + * 6.0 7.0 8.0 9.0 10.0 11.0
> + * \endverbatim
> *
> * The data follows the column headers in space-separated form. "#"
> * can be used for comments, as in shell scripts.
> @@ -95,6 +99,23 @@
> * glEnableVertexAttribArray(foo_index);
> * glEnableVertexAttribArray(bar_index);
> * \endcode
> + *
> + * Which could correspond to a vertex shader of this type:
> + *
> + * \code
> + * in vec3 vertex;
> + * in uint foo;
> + * in int bar[2];
> + *
> + * void main()
> + * {
> + * if (bar[0] + bar[1] == foo) {
> + * gl_Position = vec4(vertex, 1.0);
> + * } else {
> + * gl_Position = vec4(1.0, vertex);
> + * }
> + * }
> + * \endcode
> */
>
> #include <string>
> @@ -116,7 +137,7 @@ const int ATTRIBUTE_SIZE = 4;
> * Convert a type name string to a GLenum.
> */
> GLenum
> -decode_type(const char *type)
> +decode_type(const char *type, size_t *rows, size_t *cols)
> {
> static struct type_table_entry {
> const char *type; /* NULL means end of table */
> @@ -126,13 +147,38 @@ decode_type(const char *type)
> { "uint", GL_UNSIGNED_INT },
> { "float", GL_FLOAT },
> { "double", GL_DOUBLE },
> + { "ivec", GL_INT },
> + { "uvec", GL_UNSIGNED_INT },
> + { "vec", GL_FLOAT },
> + { "dvec", GL_DOUBLE },
> + { "mat", GL_FLOAT },
> + { "dmat", GL_DOUBLE },
> { NULL, 0 }
> };
>
>
> for (int i = 0; type_table[i].type; ++i) {
> - if (0 == strcmp(type, type_table[i].type))
> + if (0 == strncmp(type, type_table[i].type, strlen(type_table[i].type))) {
> + if (i > 3 && (rows || cols))
What 3 is lacks somewhat a context, as you need to go back to type_table
to know what you want to do. Couldn't it be replaced by a kind of enum
or at least a comment?
> + size_t type_len = strlen(type);
> + size_t the_rows = type[type_len - 1] - '0';
> + if (rows)
> + *rows = the_rows;
> + if (cols) {
> + if (i > 7)
Ditto
> + *cols = type[type_len - 2] == 'x' ?
> + type[type_len - 3] - '0' : the_rows;
> + else
> + *cols = 1;
> + }
> + } else {
> + if (rows)
> + *rows = 1;
> + if (cols)
> + *cols = 1;
> + }
> return type_table[i].enum_value;
> + }
> }
>
> printf("Unrecognized type: %s\n", type);
> @@ -157,6 +203,11 @@ public:
> GLenum data_type;
>
> /**
> + * Number of cols in the data type of this attribute.
For the comment s/cols/columns
> + */
> + size_t data_type_cols;
> +
> + /**
> * Vector length of this attribute.
> */
> size_t count;
> @@ -167,6 +218,11 @@ public:
> size_t matrix_index;
>
> /**
> + * Index of the array for this attribute.
> + */
> + size_t array_index;
> +
> + /**
> * Index of this vertex attribute in the linked program.
> */
> GLuint index;
> @@ -184,25 +240,49 @@ public:
> vertex_attrib_description::vertex_attrib_description(GLuint prog,
> const char *text)
> {
> - /* Split the column header into name/type/count fields */
> + /* Split the column header into
> + * name[array_index]/type/count/matrix_column fields */
> const char *first_slash = strchr(text, '/');
> if (first_slash == NULL) {
> - printf("Column headers must be in the form name/type/count/matrix_column.\n"
> - "Note: matrix_column is optional.\n"
> + printf("Column headers must be in the form"
> + " name[array_index]/type/count/matrix_column.\n"
> + "Note: [array_index] and matrix_column are optional.\n"
> "Got: %s\n",
> text);
> piglit_report_result(PIGLIT_FAIL);
> }
> +
Is this extra line needed?
> std::string name(text, first_slash);
> +
> + /* If the attrib is an array, strip the index */
> + if (name[name.size() - 1] == ']') {
> + std::string::size_type n = name.find('[');
> + if (n == std::string::npos) {
> + printf("Column header looked like an array"
> + " but couldn't parse it.\n"
> + "Got: %s\n",
> + text);
> + piglit_report_result(PIGLIT_FAIL);
> + } else {
> + this->array_index = strtoul(name.substr(n + 1).c_str(), NULL, 0);
> + name.resize(n);
> + }
> + } else {
> + this->array_index = 0;
> + }
> +
> const char *second_slash = strchr(first_slash + 1, '/');
> if (second_slash == NULL) {
> - printf("Column headers must be in the form name/type/count/matrix_column.\n"
> + printf("Column headers must be in the form"
> + " name[array_index]/type/count/matrix_column.\n"
> + "Note: [array_index] and matrix_column are optional.\n"
> "Got: %s\n",
> text);
> piglit_report_result(PIGLIT_FAIL);
> }
> std::string type_str(first_slash + 1, second_slash);
> - this->data_type = decode_type(type_str.c_str());
> + this->data_type = decode_type(type_str.c_str(),
> + &this->count, &this->data_type_cols);
> char *endptr;
> this->count = strtoul(second_slash + 1, &endptr, 10);
>
> @@ -217,8 +297,9 @@ vertex_attrib_description::vertex_attrib_description(GLuint prog,
> }
>
> if (*endptr != '\0') {
> - printf("Column headers must be in the form name/type/matrix_column.\n"
> - "Note: matrix_column is optional.\n"
> + printf("Column headers must be in the form"
> + " name[array_index]/type/count/matrix_column.\n"
> + "Note: [array_index] and matrix_column are optional.\n"
> "Got: %s\n",
> text);
> piglit_report_result(PIGLIT_FAIL);
> @@ -323,9 +404,11 @@ void
> vertex_attrib_description::setup(size_t *offset, size_t stride) const
> {
> int attribute_size = ATTRIBUTE_SIZE;
> + GLuint actual_index = this->index + this->matrix_index
> + + this->array_index * this->data_type_cols;
> switch (this->data_type) {
> case GL_FLOAT:
> - glVertexAttribPointer(this->index + this->matrix_index, this->count,
> + glVertexAttribPointer(actual_index, this->count,
> this->data_type, GL_FALSE, stride,
> (void *) *offset);
> break;
> @@ -334,9 +417,9 @@ vertex_attrib_description::setup(size_t *offset, size_t stride) const
> fprintf(stderr,"vertex_attrib_description fail. no 64-bit float support\n");
> return;
> }
> - glVertexAttribLPointer(this->index + this->matrix_index, this->count,
> - this->data_type, stride,
> - (void *) *offset);
> + glVertexAttribLPointer(actual_index, this->count,
> + this->data_type, stride,
> + (void *) *offset);
> attribute_size *= 2;
> break;
> default:
> @@ -344,12 +427,11 @@ vertex_attrib_description::setup(size_t *offset, size_t stride) const
> fprintf(stderr,"vertex_attrib_description fail. no int support\n");
> return;
> }
> - glVertexAttribIPointer(this->index + this->matrix_index, this->count,
> + glVertexAttribIPointer(actual_index, this->count,
> this->data_type, stride,
> - (void *) *offset);
> - break;
> + (void *) *offset); break;
> }
> - glEnableVertexAttribArray(index + this->matrix_index);
> + glEnableVertexAttribArray(actual_index);
> *offset += attribute_size * this->count;
> }
>
More information about the Piglit
mailing list