[Mesa-stable] [Mesa-dev] [PATCH] mesa: fix glGetInteger/Float/etc queries for vertex arrays attribs

Jordan Justen jordan.l.justen at intel.com
Thu May 10 07:01:34 UTC 2018


On 2018-05-09 18:51:04, Brian Paul wrote:
> The vertex array Size and Stride attributes are now ubyte and short,
> respectively.  The glGet code needed to be updated to handle those
> types, but wasn't.
> 
> Fixes the new piglit test gl-1.5-get-array-attribs test.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106450
> Fixes: d5f42f96e16 ("mesa: shrink size of gl_array_attributes (v2)")
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/mesa/main/get.c              | 77 ++++++++++++++++++++++++++++++++++++++--
>  src/mesa/main/get_hash_params.py | 24 ++++++-------
>  2 files changed, 86 insertions(+), 15 deletions(-)
> 
> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
> index 44b7b83..441eac4 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -105,6 +105,8 @@ enum value_type {
>     TYPE_ENUM,
>     TYPE_ENUM_2,
>     TYPE_BOOLEAN,
> +   TYPE_UBYTE,
> +   TYPE_SHORT,
>     TYPE_BIT_0,
>     TYPE_BIT_1,
>     TYPE_BIT_2,
> @@ -188,6 +190,8 @@ union value {
>     GLint value_int_4[4];
>     GLint64 value_int64;
>     GLenum value_enum;
> +   GLubyte value_ubyte;
> +   GLshort value_short;
>  
>     /* Sigh, see GL_COMPRESSED_TEXTURE_FORMATS_ARB handling */
>     struct {
> @@ -235,10 +239,13 @@ union value {
>  #define CONTEXT_MATRIX(field) CONTEXT_FIELD(field, TYPE_MATRIX)
>  #define CONTEXT_MATRIX_T(field) CONTEXT_FIELD(field, TYPE_MATRIX_T)
>  
> +/* Vertex array fields */
>  #define ARRAY_INT(field) ARRAY_FIELD(field, TYPE_INT)
>  #define ARRAY_ENUM(field) ARRAY_FIELD(field, TYPE_ENUM)
>  #define ARRAY_ENUM16(field) ARRAY_FIELD(field, TYPE_ENUM16)
>  #define ARRAY_BOOL(field) ARRAY_FIELD(field, TYPE_BOOLEAN)
> +#define ARRAY_UBYTE(field) ARRAY_FIELD(field, TYPE_UBYTE)
> +#define ARRAY_SHORT(field) ARRAY_FIELD(field, TYPE_SHORT)
>  
>  #define EXT(f)                                 \
>     offsetof(struct gl_extensions, f)
> @@ -1520,6 +1527,10 @@ get_value_size(enum value_type type, const union value *v)
>        return sizeof(GLenum) * 2;
>     case TYPE_BOOLEAN:
>        return sizeof(GLboolean);
> +   case TYPE_UBYTE:
> +      return sizeof(GLubyte);
> +   case TYPE_SHORT:
> +      return sizeof(GLshort);
>     case TYPE_BIT_0:
>     case TYPE_BIT_1:
>     case TYPE_BIT_2:
> @@ -1628,7 +1639,15 @@ _mesa_GetBooleanv(GLenum pname, GLboolean *params)
>        break;
>  
>     case TYPE_BOOLEAN:
> -      params[0] = ((GLboolean*) p)[0];
> +      params[0] = ((GLboolean *) p)[0];

I see 7 cases of 'GLboolean*' in get.c currently. Maybe you should
just split this out into a separate style cleanup patch? Or ... not
bother changing it. :)

If you want to cleanup them all in a separate patch, feel free to
include my Reviewed-by: Jordan Justen <jordan.l.justen at intel.com> on
that patch.

> +      break;
> +
> +   case TYPE_UBYTE:
> +      params[0] = ((GLubyte *) p)[0] != 0;
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = ((GLshort *) p)[0] != 0;

Seems like maybe INT_TO_BOOLEAN should be used in these two cases.

>        break;
>  
>     case TYPE_MATRIX:
> @@ -1735,7 +1754,15 @@ _mesa_GetFloatv(GLenum pname, GLfloat *params)
>        break;
>  
>     case TYPE_BOOLEAN:
> -      params[0] = BOOLEAN_TO_FLOAT(*(GLboolean*) p);
> +      params[0] = BOOLEAN_TO_FLOAT(*(GLboolean *) p);
> +      break;
> +
> +   case TYPE_UBYTE:
> +      params[0] = (GLfloat) ((GLubyte *) p)[0];
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = (GLfloat) ((GLshort *) p)[0];
>        break;
>  
>     case TYPE_MATRIX:
> @@ -1845,6 +1872,14 @@ _mesa_GetIntegerv(GLenum pname, GLint *params)
>        params[0] = BOOLEAN_TO_INT(*(GLboolean*) p);
>        break;
>  
> +   case TYPE_UBYTE:
> +      params[0] = ((GLubyte *) p)[0];
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = ((GLshort *) p)[0];
> +      break;
> +
>     case TYPE_MATRIX:
>        m = *(GLmatrix **) p;
>        for (i = 0; i < 16; i++)
> @@ -2065,6 +2100,14 @@ _mesa_GetDoublev(GLenum pname, GLdouble *params)
>        params[0] = *(GLboolean*) p;
>        break;
>  
> +   case TYPE_UBYTE:
> +      params[0] = ((GLubyte *) p)[0];
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = ((GLshort *) p)[0];
> +      break;
> +
>     case TYPE_MATRIX:
>        m = *(GLmatrix **) p;
>        for (i = 0; i < 16; i++)
> @@ -2144,6 +2187,8 @@ _mesa_GetUnsignedBytevEXT(GLenum pname, GLubyte *data)
>     case TYPE_ENUM:
>     case TYPE_ENUM_2:
>     case TYPE_BOOLEAN:
> +   case TYPE_UBYTE:
> +   case TYPE_SHORT:
>     case TYPE_FLOAT:
>     case TYPE_FLOATN:
>     case TYPE_FLOAT_2:
> @@ -2793,6 +2838,14 @@ _mesa_GetFloati_v(GLenum pname, GLuint index, GLfloat *params)
>        params[0] = BOOLEAN_TO_FLOAT(v.value_bool);
>        break;
>  
> +   case TYPE_UBYTE:
> +      params[0] = (GLfloat) v.value_ubyte;
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = (GLfloat) v.value_short;
> +      break;
> +
>     case TYPE_MATRIX:
>        m = *(GLmatrix **) &v;
>        for (i = 0; i < 16; i++)
> @@ -2876,6 +2929,14 @@ _mesa_GetDoublei_v(GLenum pname, GLuint index, GLdouble *params)
>        params[0] = (GLdouble) BOOLEAN_TO_FLOAT(v.value_bool);
>        break;
>  
> +   case TYPE_UBYTE:
> +      params[0] = (GLdouble) v.value_bool;
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = (GLdouble) v.value_bool;

Should this use value_ubyte and value_short?

With those changes:

Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> +      break;
> +
>     case TYPE_MATRIX:
>        m = *(GLmatrix **) &v;
>        for (i = 0; i < 16; i++)
> @@ -2928,6 +2989,8 @@ _mesa_GetUnsignedBytei_vEXT(GLenum target, GLuint index, GLubyte *data)
>     case TYPE_ENUM:
>     case TYPE_ENUM_2:
>     case TYPE_BOOLEAN:
> +   case TYPE_UBYTE:
> +   case TYPE_SHORT:
>     case TYPE_FLOAT:
>     case TYPE_FLOATN:
>     case TYPE_FLOAT_2:
> @@ -3018,7 +3081,15 @@ _mesa_GetFixedv(GLenum pname, GLfixed *params)
>        break;
>  
>     case TYPE_BOOLEAN:
> -      params[0] = BOOLEAN_TO_FIXED(((GLboolean*) p)[0]);
> +      params[0] = BOOLEAN_TO_FIXED(((GLboolean *) p)[0]);
> +      break;
> +
> +   case TYPE_UBYTE:
> +      params[0] = INT_TO_FIXED(((GLubyte *) p)[0]);
> +      break;
> +
> +   case TYPE_SHORT:
> +      params[0] = INT_TO_FIXED(((GLshort *) p)[0]);
>        break;
>  
>     case TYPE_MATRIX:
> diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py
> index de5fed6..755d33a 100644
> --- a/src/mesa/main/get_hash_params.py
> +++ b/src/mesa/main/get_hash_params.py
> @@ -212,20 +212,20 @@ descriptor=[
>    [ "TEXTURE_MATRIX", "LOC_CUSTOM, TYPE_MATRIX, 0, extra_valid_texture_unit" ],
>    [ "TEXTURE_STACK_DEPTH", "LOC_CUSTOM, TYPE_INT, 0, extra_valid_texture_unit" ],
>    [ "VERTEX_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_POS].Enabled), NO_EXTRA" ],
> -  [ "VERTEX_ARRAY_SIZE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_POS].Size), NO_EXTRA" ],
> +  [ "VERTEX_ARRAY_SIZE", "ARRAY_UBYTE(VertexAttrib[VERT_ATTRIB_POS].Size), NO_EXTRA" ],
>    [ "VERTEX_ARRAY_TYPE", "ARRAY_ENUM16(VertexAttrib[VERT_ATTRIB_POS].Type), NO_EXTRA" ],
> -  [ "VERTEX_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_POS].Stride), NO_EXTRA" ],
> +  [ "VERTEX_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_POS].Stride), NO_EXTRA" ],
>    [ "NORMAL_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_NORMAL].Enabled), NO_EXTRA" ],
>    [ "NORMAL_ARRAY_TYPE", "ARRAY_ENUM16(VertexAttrib[VERT_ATTRIB_NORMAL].Type), NO_EXTRA" ],
> -  [ "NORMAL_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_NORMAL].Stride), NO_EXTRA" ],
> +  [ "NORMAL_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_NORMAL].Stride), NO_EXTRA" ],
>    [ "COLOR_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR0].Enabled), NO_EXTRA" ],
>    [ "COLOR_ARRAY_SIZE", "LOC_CUSTOM, TYPE_INT, 0, NO_EXTRA" ],
>    [ "COLOR_ARRAY_TYPE", "ARRAY_ENUM16(VertexAttrib[VERT_ATTRIB_COLOR0].Type), NO_EXTRA" ],
> -  [ "COLOR_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_COLOR0].Stride), NO_EXTRA" ],
> +  [ "COLOR_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_COLOR0].Stride), NO_EXTRA" ],
>    [ "TEXTURE_COORD_ARRAY", "LOC_CUSTOM, TYPE_BOOLEAN, offsetof(struct gl_array_attributes, Enabled), NO_EXTRA" ],
> -  [ "TEXTURE_COORD_ARRAY_SIZE", "LOC_CUSTOM, TYPE_INT, offsetof(struct gl_array_attributes, Size), NO_EXTRA" ],
> +  [ "TEXTURE_COORD_ARRAY_SIZE", "LOC_CUSTOM, TYPE_UBYTE, offsetof(struct gl_array_attributes, Size), NO_EXTRA" ],
>    [ "TEXTURE_COORD_ARRAY_TYPE", "LOC_CUSTOM, TYPE_ENUM16, offsetof(struct gl_array_attributes, Type), NO_EXTRA" ],
> -  [ "TEXTURE_COORD_ARRAY_STRIDE", "LOC_CUSTOM, TYPE_INT, offsetof(struct gl_array_attributes, Stride), NO_EXTRA" ],
> +  [ "TEXTURE_COORD_ARRAY_STRIDE", "LOC_CUSTOM, TYPE_SHORT, offsetof(struct gl_array_attributes, Stride), NO_EXTRA" ],
>  
>  # GL_ARB_multitexture
>    [ "MAX_TEXTURE_UNITS", "CONTEXT_INT(Const.MaxTextureUnits), NO_EXTRA" ],
> @@ -255,7 +255,7 @@ descriptor=[
>  # OES_point_size_array
>    [ "POINT_SIZE_ARRAY_OES", "ARRAY_FIELD(VertexAttrib[VERT_ATTRIB_POINT_SIZE].Enabled, TYPE_BOOLEAN), NO_EXTRA" ],
>    [ "POINT_SIZE_ARRAY_TYPE_OES", "ARRAY_FIELD(VertexAttrib[VERT_ATTRIB_POINT_SIZE].Type, TYPE_ENUM16), NO_EXTRA" ],
> -  [ "POINT_SIZE_ARRAY_STRIDE_OES", "ARRAY_FIELD(VertexAttrib[VERT_ATTRIB_POINT_SIZE].Stride, TYPE_INT), NO_EXTRA" ],
> +  [ "POINT_SIZE_ARRAY_STRIDE_OES", "ARRAY_FIELD(VertexAttrib[VERT_ATTRIB_POINT_SIZE].Stride, TYPE_SHORT), NO_EXTRA" ],
>    [ "POINT_SIZE_ARRAY_BUFFER_BINDING_OES", "LOC_CUSTOM, TYPE_INT, 0, NO_EXTRA" ],
>  ]},
>  
> @@ -790,11 +790,11 @@ descriptor=[
>    [ "COLOR_ARRAY_COUNT_EXT", "CONST(0), NO_EXTRA" ],
>    [ "INDEX_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR_INDEX].Enabled), NO_EXTRA" ],
>    [ "INDEX_ARRAY_TYPE", "ARRAY_ENUM16(VertexAttrib[VERT_ATTRIB_COLOR_INDEX].Type), NO_EXTRA" ],
> -  [ "INDEX_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_COLOR_INDEX].Stride), NO_EXTRA" ],
> +  [ "INDEX_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_COLOR_INDEX].Stride), NO_EXTRA" ],
>    [ "INDEX_ARRAY_COUNT_EXT", "CONST(0), NO_EXTRA" ],
>    [ "TEXTURE_COORD_ARRAY_COUNT_EXT", "CONST(0), NO_EXTRA" ],
>    [ "EDGE_FLAG_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_EDGEFLAG].Enabled), NO_EXTRA" ],
> -  [ "EDGE_FLAG_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_EDGEFLAG].Stride), NO_EXTRA" ],
> +  [ "EDGE_FLAG_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_EDGEFLAG].Stride), NO_EXTRA" ],
>    [ "EDGE_FLAG_ARRAY_COUNT_EXT", "CONST(0), NO_EXTRA" ],
>  
>  # GL_ARB_texture_compression
> @@ -824,14 +824,14 @@ descriptor=[
>    [ "CURRENT_SECONDARY_COLOR", "CONTEXT_FIELD(Current.Attrib[VERT_ATTRIB_COLOR1][0], TYPE_FLOATN_4), extra_flush_current" ],
>    [ "SECONDARY_COLOR_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_COLOR1].Enabled), NO_EXTRA" ],
>    [ "SECONDARY_COLOR_ARRAY_TYPE", "ARRAY_ENUM16(VertexAttrib[VERT_ATTRIB_COLOR1].Type), NO_EXTRA" ],
> -  [ "SECONDARY_COLOR_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_COLOR1].Stride), NO_EXTRA" ],
> -  [ "SECONDARY_COLOR_ARRAY_SIZE", "LOC_CUSTOM, TYPE_INT, 0, NO_EXTRA" ],
> +  [ "SECONDARY_COLOR_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_COLOR1].Stride), NO_EXTRA" ],
> +  [ "SECONDARY_COLOR_ARRAY_SIZE", "LOC_CUSTOM, TYPE_UBYTE, 0, NO_EXTRA" ],
>  
>  # GL_EXT_fog_coord
>    [ "CURRENT_FOG_COORDINATE", "CONTEXT_FLOAT(Current.Attrib[VERT_ATTRIB_FOG][0]), extra_flush_current" ],
>    [ "FOG_COORDINATE_ARRAY", "ARRAY_BOOL(VertexAttrib[VERT_ATTRIB_FOG].Enabled), NO_EXTRA" ],
>    [ "FOG_COORDINATE_ARRAY_TYPE", "ARRAY_ENUM16(VertexAttrib[VERT_ATTRIB_FOG].Type), NO_EXTRA" ],
> -  [ "FOG_COORDINATE_ARRAY_STRIDE", "ARRAY_INT(VertexAttrib[VERT_ATTRIB_FOG].Stride), NO_EXTRA" ],
> +  [ "FOG_COORDINATE_ARRAY_STRIDE", "ARRAY_SHORT(VertexAttrib[VERT_ATTRIB_FOG].Stride), NO_EXTRA" ],
>    [ "FOG_COORDINATE_SOURCE", "CONTEXT_ENUM16(Fog.FogCoordinateSource), NO_EXTRA" ],
>  
>  # GL_NV_fog_distance
> -- 
> 2.7.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-stable mailing list