[Mesa-dev] [PATCH] mesa: Fix computation of default vertex attrib stride for 2_10_10_10 formats.

[Brian Paul]

On 12/11/2012 09:25 PM, Paul Berry wrote:
> Previously, if the client program didn't specify a stride when setting
> up a vertex attribute, we used _mesa_sizeof_type() to compute the size
> of the type, and multiplied it by the number of components.
>
> This didn't work for the 2_10_10_10 formats, since _mesa_sizeof_type()
> returns -1 for those types, resulting in all kinds of havoc, since it
> was causing the hardware to be programmed with a negative stride
> value.
>
> This patch adds a new function _mesa_bytes_per_vertex_attrib(), which
> is similar to the existing function _mesa_bytes_per_pixel(), but which
> computes the size of a vertex attribute based on the type and the
> number of formats.  For packed formats (currently only the 2_10_10_10
> formats), it verifies that the number of components is correct and
> returns the size of the packed format.  For unpacked formats, it
> returns the size of the type times the number of components.
>
> In addition, this patch adds an assertion so that if we ever forget to
> update _mesa_bytes_per_vertex_attrib() when adding a new vertex
> format, we'll see the problem quickly rather than having to debug a
> subtle conformance test failure.
>
> Fixes GLES3 conformance tests
> vertex_type_2_10_10_10_rev_{conversion,divisor,stride_pointer}.test.
> ---
>   src/mesa/main/glformats.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>   src/mesa/main/glformats.h |  3 +++
>   src/mesa/main/varray.c    |  3 ++-
>   3 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index fefa9c4..b6b16ca 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -308,6 +308,48 @@ _mesa_bytes_per_pixel(GLenum format, GLenum type)
>
>
>   /**
> + * Get the number of bytes for a vertex attrib with the given number of
> + * components ant type.

ant -> and


> + *
> + * \param comps number of components.
> + * \param type data type.
> + *
> + * \return bytes per attribute, or -1 if a bad comps/type combination was given.
> + */
> +GLint
> +_mesa_bytes_per_vertex_attrib(GLint comps, GLenum type)
> +{
> +   switch (type) {
> +   case GL_BYTE:
> +   case GL_UNSIGNED_BYTE:
> +      return comps * sizeof(GLubyte);
> +   case GL_SHORT:
> +   case GL_UNSIGNED_SHORT:
> +      return comps * sizeof(GLshort);
> +   case GL_INT:
> +   case GL_UNSIGNED_INT:
> +      return comps * sizeof(GLint);
> +   case GL_FLOAT:
> +      return comps * sizeof(GLfloat);
> +   case GL_HALF_FLOAT_ARB:
> +      return comps * sizeof(GLhalfARB);
> +   case GL_DOUBLE:
> +      return comps * sizeof(GLdouble);
> +   case GL_FIXED:
> +      return comps * sizeof(GLfixed);
> +   case GL_INT_2_10_10_10_REV:
> +   case GL_UNSIGNED_INT_2_10_10_10_REV:
> +      if (comps == 4)
> +         return sizeof(GLuint);
> +      else
> +         return -1;
> +   default:
> +      return -1;
> +   }
> +}
> +
> +
> +/**
>    * Test if the given format is an integer (non-normalized) format.
>    */
>   GLboolean
> diff --git a/src/mesa/main/glformats.h b/src/mesa/main/glformats.h
> index 5d09951..ccfb5e1 100644
> --- a/src/mesa/main/glformats.h
> +++ b/src/mesa/main/glformats.h
> @@ -49,6 +49,9 @@ _mesa_components_in_format( GLenum format );
>   extern GLint
>   _mesa_bytes_per_pixel( GLenum format, GLenum type );
>
> +extern GLint
> +_mesa_bytes_per_vertex_attrib(GLint comps, GLenum type);
> +
>   extern GLboolean
>   _mesa_is_type_integer(GLenum type);
>
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index f770143..5e4d6c3 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -251,7 +251,8 @@ update_array(struct gl_context *ctx,
>         return;
>      }
>
> -   elementSize = _mesa_sizeof_type(type) * size;
> +   elementSize = _mesa_bytes_per_vertex_attrib(size, type);
> +   assert(elementSize != -1);
>
>      array =&ctx->Array.ArrayObj->VertexAttrib[attrib];
>      array->Size = size;

Reviewed-by: Brian Paul <brianp at vmware.com>