On Wed, 2016-06-29 at 16:29 +0200, Alejandro Piñeiro wrote:
> On 14/06/16 23:36, Andres Gomez wrote:
[snip]
> > diff --git a/tests/util/piglit-vbo.cpp b/tests/util/piglit-vbo.cpp
> > index 274779f..6744b1e 100644
> > --- a/tests/util/piglit-vbo.cpp
> > +++ b/tests/util/piglit-vbo.cpp
[snip]
> > @@ -130,76 +129,136 @@
> > #include <string>
> > #include <vector>
> > #include <errno.h>
> > +#include <limits.h>
> > #include <ctype.h>
> >
> > #include "piglit-util.h"
> > #include "piglit-util-gl.h"
> > #include "piglit-vbo.h"
> >
> > +
>
> Unneeded extra space.
OK.
>
> > /**
> > - * Currently all the attribute types we support (int, uint, and
> > float)
> > - * are 4 bytes in width.
> > + * Convert a type name string to a GLenum.
> > */
> > -const int ATTRIBUTE_SIZE = 4;
> > +static bool
> > +decode_type(const char *type,
> > + GLenum *gl_type, size_t *gl_type_size, GLenum
> > *glsl_type)
> > +{
> > + assert(type);
> > + assert(gl_type);
> > + assert(gl_type_size);
> > +
> > + static struct type_table_entry {
> > + const char *type; /* NULL means end of table */
> > + GLenum gl_type;
> > + size_t gl_type_size;
> > + GLenum glsl_type;
> > + } const type_table[] = {
> > + { "byte", GL_BYTE, 1, GL_
> > INT },
> > + { "ubyte", GL_UNSIGNED_BYTE, 1, GL
> > _UNSIGNED_INT },
> > + { "short", GL_SHORT, 2, GL
> > _INT },
> > + { "ushort", GL_UNSIGNED_SHORT, 2, G
> > L_UNSIGNED_INT },
> > + { "int", GL_INT, 4, GL_I
> > NT },
> > + { "uint", GL_UNSIGNED_INT, 4, GL_
> > UNSIGNED_INT },
> > + { "half", GL_HALF_FLOAT, 2, GL_FL
> > OAT },
> > + { "float", GL_FLOAT, 4, GL
> > _FLOAT },
> > + { "double", GL_DOUBLE, 8, G
> > L_DOUBLE },
> > + { NULL, 0, 0, 0
> > },
> > + };
> > +
> > +
> > + for (int i = 0; type_table[i].type; ++i) {
> > + if (0 == strcmp(type, type_table[i].type)) {
> > + *gl_type = type_table[i].gl_type;
> > + *gl_type_size =
> > type_table[i].gl_type_size;
> > + if (glsl_type)
> > + *glsl_type =
> > type_table[i].glsl_type;
> > + return true;
> > + }
> > + }
> > +
> > + return false;
> > +}
> >
> >
> > /**
> > - * Convert a type name string to a GLenum.
> > + * Convert a GLSL type name string to its basic GLenum type.
> > */
> > -GLenum
> > -decode_type(const char *type, size_t *rows, size_t *cols)
> > +static bool
> > +decode_glsl_type(const char *type,
> > + GLenum *glsl_type, size_t *rows, size_t *cols,
> > char **endptr)
> Not sure if this is due my email-client, but that GLenum doesn't seem
> properly placed, should be at the same level that const.
Piglit uses tabs for indentation. Hence, the effect.
> > {
> > + assert(glsl_type);
> > + assert(rows);
> > + assert(cols);
> > + assert(endptr);
> > +
> > + if (isdigit(type[0])) {
> > + *rows = strtoul(type, endptr, 10);
> > + *cols = 1;
> > + *glsl_type = 0;
> > + return true;
> > + }
> > +
> > static struct type_table_entry {
> > const char *type; /* NULL means end of table */
> > - GLenum enum_value;
> > + GLenum glsl_type;
> > } const type_table[] = {
> > - { "int", GL_INT },
> > - { "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 }
> > + { "int", GL_INT },
> > + { "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 }
>
> Unless Im missing something, you didn't made any functionality-
> related
> change here, but a reformatting (more space between the string name
> and
> the GLenum). I wouldn't do that on this commit.
It is actually applying tabs instead of spaces but, yeah, I will remove
this part.
> > };
> >
> > -
> > for (int i = 0; type_table[i].type; ++i) {
> > - if (0 == strncmp(type, type_table[i].type,
> > strlen(type_table[i].type))) {
> > + const size_t type_len =
> > strlen(type_table[i].type);
> > + if (0 == strncmp(type, type_table[i].type,
> > type_len)) {
> > + *endptr = const_cast<char
> > *>(&type[type_len]);
> > +
> > /* In case of vectors or matrices, let's
> > - * calculate rows and columns, if needed.
> > + * calculate rows and columns.
> > */
> > - if (i > 3 && (rows || cols)) {
> > - size_t type_len = strlen(type);
> > - size_t the_rows = type[type_len -
> > 1] - '0';
> > - if (rows)
> > - *rows = the_rows;
> > - if (cols) {
> > - /* In case of matrices,
> > let's
> > - * calculate the columns.
> > - */
> > - if (i > 7)
> > - *cols =
> > type[type_len - 2] == 'x' ?
> > - type[type_
> > len - 3] - '0' : the_rows;
> > - else
> > - *cols = 1;
> > + if (i > 3) {
> > + if (!isdigit(**endptr))
> > + goto cleanup;
> > + *rows = **endptr - '0';
> > + ++*endptr;
> > +
> > + /* In case of matrices, let's
> > + * calculate the rows.
> > + */
> > + if (i > 7) {
> > + *cols = *rows;
> > + if (**endptr == 'x') {
> > + if
> > (!isdigit(*(++*endptr)))
> > + goto
> > cleanup;
> > + *rows = **endptr -
> > '0';
> > + ++*endptr;
> > + }
> > + } else {
> > + *cols = 1;
> > }
> > } else {
> > - if (rows)
> > - *rows = 1;
> > - if (cols)
> > - *cols = 1;
> > + *rows = 1;
> > + *cols = 1;
> > }
> > - return type_table[i].enum_value;
> > + *glsl_type = type_table[i].glsl_type;
> > + return true;
> > }
> > }
> >
> > - printf("Unrecognized type: %s\n", type);
> > - piglit_report_result(PIGLIT_FAIL);
> > - return 0;
> > +cleanup:
> > +
> Unneeded extra space.
OK.
[snip]
> > @@ -368,6 +459,15 @@ vertex_attrib_description::parse_datum(const
> > char **text, void *data) const
> > char *endptr;
> > errno = 0;
> > switch (this->data_type) {
> > + case GL_HALF_FLOAT: {
> > + unsigned short value = strtohf_hex(*text,
> > &endptr);
> > + if (errno == ERANGE) {
> > + printf("Could not parse as half float\n");
> > + return false;
> > + }
> > + *((GLhalf *) data) = value;
> > + break;
> > + }
> > case GL_FLOAT: {
> > float value = strtof_hex(*text, &endptr);
> > if (errno == ERANGE) {
> > @@ -386,6 +486,42 @@ vertex_attrib_description::parse_datum(const
> > char **text, void *data) const
> > *((GLdouble *) data) = value;
> > break;
> > }
> > + case GL_BYTE: {
> > + long value = strtol_hex(*text, &endptr);
> > + if (errno == ERANGE || value < SCHAR_MIN || value
> > > SCHAR_MAX) {
> > + printf("Could not parse as signed
> > byte\n");
> > + return false;
> > + }
> > + *((GLbyte *) data) = (GLbyte) value;
> > + break;
> > + }
> > + case GL_UNSIGNED_BYTE: {
> > + unsigned long value = strtoul(*text, &endptr, 0);
> > + if (errno == ERANGE || value > UCHAR_MAX) {
> > + printf("Could not parse as unsigned
> > byte\n");
> > + return false;
> > + }
> > + *((GLubyte *) data) = (GLubyte) value;
> > + break;
> > + }
> > + case GL_SHORT: {
> > + long value = strtol_hex(*text, &endptr);
> > + if (errno == ERANGE || value < SHRT_MIN || value >
> > SHRT_MAX) {
> > + printf("Could not parse as signed
> > short\n");
> > + return false;
> > + }
> > + *((GLshort *) data) = (GLshort) value;
> > + break;
> > + }
> > + case GL_UNSIGNED_SHORT: {
> > + unsigned long value = strtoul(*text, &endptr, 0);
> > + if (errno == ERANGE || value > USHRT_MAX) {
> > + printf("Could not parse as unsigned
> > short\n");
> > + return false;
> > + }
> > + *((GLushort *) data) = (GLushort) value;
> > + break;
> > + }
>
> Looking at GL_UNSIGNED_BYTE, GL_SHORT and GL_UNSIGNED_SHORT, I realized
> that there aren't hex wrappers like for the other types. So that means
> for some types you can use hexadecimal format but not for the others. I
> will assume that this is on purpose and that there is a good reason for
> that. But in that case, I think that it would be good to mention on the
> documentation-like comment at the beginning of the file (although it is
> true that it is already somewhat thick as it is).
The types that don't have _hex equivalent are the unsigned ones:
GL_UNSIGNED_BYTE, GL_UNSIGNED_SHORT and GL_UNSIGNED_INT.
The reason is because the original stroul function already does the
job, not that you cannot pass an hex value.
--
Br,
Andres
_______________________________________________
Piglit mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/piglit
