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 <[email protected]> > --- > 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; > } > _______________________________________________ Piglit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/piglit
