[Mesa-dev] [PATCH 02/11] mesa: Split out the format code from update_array()

Ian Romanick idr at freedesktop.org
Mon Nov 4 13:05:26 PST 2013


On 10/28/2013 03:33 PM, Fredrik Höglund wrote:
> Split out the code for updating the array format into a new function
> called update_array_format(). This function will be called by both
> update_array() and the new glVertexAttrib*Format() entry points in
> ARB_vertex_attrib_binding.

Other than the one nit below,

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> ---
>  src/mesa/main/varray.c |  144 ++++++++++++++++++++++++++++++------------------
>  1 file changed, 90 insertions(+), 54 deletions(-)
> 
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index 24cd324..f3cd928 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -103,52 +103,33 @@ type_to_bit(const struct gl_context *ctx, GLenum type)
>  
>  
>  /**
> - * Do error checking and update state for glVertex/Color/TexCoord/...Pointer
> - * functions.
> + * Does error checking and updates the format in an attrib array.
>   *
> - * \param func  name of calling function used for error reporting
> - * \param attrib  the attribute array index to update
> - * \param legalTypes  bitmask of *_BIT above indicating legal datatypes
> - * \param sizeMin  min allowable size value
> - * \param sizeMax  max allowable size value (may also be BGRA_OR_4)
> - * \param size  components per element (1, 2, 3 or 4)
> - * \param type  datatype of each component (GL_FLOAT, GL_INT, etc)
> - * \param stride  stride between elements, in elements
> - * \param normalized  are integer types converted to floats in [-1, 1]?
> - * \param integer  integer-valued values (will not be normalized to [-1,1])
> - * \param ptr  the address (or offset inside VBO) of the array data
> + * Called by update_array().
> + *
> + * \param func        Name of calling function used for error reporting
> + * \param attrib      The index of the attribute array
> + * \param legalTypes  Bitmask of *_BIT above indicating legal datatypes
> + * \param sizeMin     Min allowable size value
> + * \param sizeMax     Max allowable size value (may also be BGRA_OR_4)
> + * \param size        Components per element (1, 2, 3 or 4)
> + * \param type        Datatype of each component (GL_FLOAT, GL_INT, etc)
> + * \param normalized  Whether integer types are converted to floats in [-1, 1]
> + * \param integer     Integer-valued values (will not be normalized to [-1, 1])
>   */
> -static void
> -update_array(struct gl_context *ctx,
> -             const char *func,
> -             GLuint attrib, GLbitfield legalTypesMask,
> -             GLint sizeMin, GLint sizeMax,
> -             GLint size, GLenum type, GLsizei stride,
> -             GLboolean normalized, GLboolean integer,
> -             const GLvoid *ptr)
> +static GLboolean

Use bool, true, and false for things that aren't visible to the GL API.

> +update_array_format(struct gl_context *ctx,
> +                    const char *func,
> +                    GLuint attrib, GLbitfield legalTypesMask,
> +                    GLint sizeMin, GLint sizeMax,
> +                    GLint size, GLenum type,
> +                    GLboolean normalized, GLboolean integer)

In spite of the previous suggestion, these should stay GLboolean since
they come directly from the API.

>  {
>     struct gl_client_array *array;
>     GLbitfield typeBit;
> -   GLsizei elementSize;
> +   GLuint elementSize;
>     GLenum format = GL_RGBA;
>  
> -   /* Page 407 (page 423 of the PDF) of the OpenGL 3.0 spec says:
> -    *
> -    *     "Client vertex arrays - all vertex array attribute pointers must
> -    *     refer to buffer objects (section 2.9.2). The default vertex array
> -    *     object (the name zero) is also deprecated. Calling
> -    *     VertexAttribPointer when no buffer object or no vertex array object
> -    *     is bound will generate an INVALID_OPERATION error..."
> -    *
> -    * The check for VBOs is handled below.
> -    */
> -   if (ctx->API == API_OPENGL_CORE
> -       && (ctx->Array.ArrayObj == ctx->Array.DefaultArrayObj)) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(no array object bound)",
> -                  func);
> -      return;
> -   }
> -
>     if (_mesa_is_gles(ctx)) {
>        legalTypesMask &= ~(FIXED_GL_BIT | DOUBLE_BIT);
>  
> @@ -186,7 +167,7 @@ update_array(struct gl_context *ctx,
>     if (typeBit == 0x0 || (typeBit & legalTypesMask) == 0x0) {
>        _mesa_error(ctx, GL_INVALID_ENUM, "%s(type = %s)",
>                    func, _mesa_lookup_enum_by_nr(type));
> -      return;
> +      return GL_FALSE;
>     }
>  
>     /* Do size parameter checking.
> @@ -219,13 +200,13 @@ update_array(struct gl_context *ctx,
>        if (bgra_error) {
>           _mesa_error(ctx, GL_INVALID_OPERATION, "%s(size=GL_BGRA and type=%s)",
>                       func, _mesa_lookup_enum_by_nr(type));
> -         return;
> +         return GL_FALSE;
>        }
>  
>        if (!normalized) {
>           _mesa_error(ctx, GL_INVALID_OPERATION,
>                       "%s(size=GL_BGRA and normalized=GL_FALSE)", func);
> -         return;
> +         return GL_FALSE;
>        }
>  
>        format = GL_BGRA;
> @@ -233,18 +214,82 @@ update_array(struct gl_context *ctx,
>     }
>     else if (size < sizeMin || size > sizeMax || size > 4) {
>        _mesa_error(ctx, GL_INVALID_VALUE, "%s(size=%d)", func, size);
> -      return;
> +      return GL_FALSE;
>     }
>  
>     if (ctx->Extensions.ARB_vertex_type_2_10_10_10_rev &&
>         (type == GL_UNSIGNED_INT_2_10_10_10_REV ||
>          type == GL_INT_2_10_10_10_REV) && size != 4) {
>        _mesa_error(ctx, GL_INVALID_OPERATION, "%s(size=%d)", func, size);
> -      return;
> +      return GL_FALSE;
>     }
>  
>     ASSERT(size <= 4);
>  
> +   elementSize = _mesa_bytes_per_vertex_attrib(size, type);
> +   assert(elementSize != -1);
> +
> +   array = &ctx->Array.ArrayObj->VertexAttrib[attrib];
> +   array->Size = size;
> +   array->Type = type;
> +   array->Format = format;
> +   array->Normalized = normalized;
> +   array->Integer = integer;
> +   array->_ElementSize = elementSize;
> +
> +   return GL_TRUE;
> +}
> +
> +
> +/**
> + * Do error checking and update state for glVertex/Color/TexCoord/...Pointer
> + * functions.
> + *
> + * \param func  name of calling function used for error reporting
> + * \param attrib  the attribute array index to update
> + * \param legalTypes  bitmask of *_BIT above indicating legal datatypes
> + * \param sizeMin  min allowable size value
> + * \param sizeMax  max allowable size value (may also be BGRA_OR_4)
> + * \param size  components per element (1, 2, 3 or 4)
> + * \param type  datatype of each component (GL_FLOAT, GL_INT, etc)
> + * \param stride  stride between elements, in elements
> + * \param normalized  are integer types converted to floats in [-1, 1]?
> + * \param integer  integer-valued values (will not be normalized to [-1,1])
> + * \param ptr  the address (or offset inside VBO) of the array data
> + */
> +static void
> +update_array(struct gl_context *ctx,
> +             const char *func,
> +             GLuint attrib, GLbitfield legalTypesMask,
> +             GLint sizeMin, GLint sizeMax,
> +             GLint size, GLenum type, GLsizei stride,
> +             GLboolean normalized, GLboolean integer,
> +             const GLvoid *ptr)
> +{
> +   struct gl_client_array *array;
> +
> +   /* Page 407 (page 423 of the PDF) of the OpenGL 3.0 spec says:
> +    *
> +    *     "Client vertex arrays - all vertex array attribute pointers must
> +    *     refer to buffer objects (section 2.9.2). The default vertex array
> +    *     object (the name zero) is also deprecated. Calling
> +    *     VertexAttribPointer when no buffer object or no vertex array object
> +    *     is bound will generate an INVALID_OPERATION error..."
> +    *
> +    * The check for VBOs is handled below.
> +    */
> +   if (ctx->API == API_OPENGL_CORE
> +       && (ctx->Array.ArrayObj == ctx->Array.DefaultArrayObj)) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(no array object bound)",
> +                  func);
> +      return;
> +   }
> +
> +   if (!update_array_format(ctx, func, attrib, legalTypesMask, sizeMin, sizeMax,
> +                            size, type, normalized, integer)) {
> +      return;
> +   }
> +
>     if (stride < 0) {
>        _mesa_error( ctx, GL_INVALID_VALUE, "%s(stride=%d)", func, stride );
>        return;
> @@ -268,19 +313,10 @@ update_array(struct gl_context *ctx,
>        return;
>     }
>  
> -   elementSize = _mesa_bytes_per_vertex_attrib(size, type);
> -   assert(elementSize != -1);
> -
>     array = &ctx->Array.ArrayObj->VertexAttrib[attrib];
> -   array->Size = size;
> -   array->Type = type;
> -   array->Format = format;
>     array->Stride = stride;
> -   array->StrideB = stride ? stride : elementSize;
> -   array->Normalized = normalized;
> -   array->Integer = integer;
> +   array->StrideB = stride ? stride : array->_ElementSize;
>     array->Ptr = (const GLubyte *) ptr;
> -   array->_ElementSize = elementSize;
>  
>     _mesa_reference_buffer_object(ctx, &array->BufferObj,
>                                   ctx->Array.ArrayBufferObj);
> 



More information about the mesa-dev mailing list