[Mesa-dev] [PATCH 01/12] mesa: Refactor element type checking into its own function

[Kenneth Graunke]

On 08/22/2012 07:26 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> This consolidates the tests and makes the emitted error message
> consistent.
> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/mesa/main/api_validate.c |   46 +++++++++++++++++++++---------------------
>  1 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index eaf614b..e8c8333 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -262,6 +262,25 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
>     return GL_TRUE;
>  }
>  
> +/**
> + * Verify that the element type is valid.
> + *
> + * Generates \c GL_INVALID_ENUM and returns \c false if it is not.
> + */
> +static bool
> +_mesa_valid_element_type(struct gl_context *ctx, GLenum type, const char *name)
> +{

Could we call this _mesa_valid_draw_elements_type()?  When I first saw
this, I was thinking more broadly..."elements of what?", "only unsigned
elements are allowed?", and so on.

> +   switch (type) {
> +   case GL_UNSIGNED_BYTE:
> +   case GL_UNSIGNED_SHORT:
> +   case GL_UNSIGNED_INT:
> +      return true;
> +
> +   default:
> +      _mesa_error(ctx, GL_INVALID_ENUM, "%s(type = 0x%04x)", name, type);

Could we also please do:

      _mesa_error(ctx, GL_INVALID_ENUM, "%s(type = %s)", name,
                  _mesa_lookup_enum_by_nr(type));

May as well make it a bit friendlier, right?  These enums are
unambiguous so the string will be correct.

Otherwise, I like the idea.
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +      return false;
> +   }
> +}
>  
>  /**
>   * Error checking for glDrawElements().  Includes parameter checking
> @@ -286,13 +305,8 @@ _mesa_validate_DrawElements(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   if (type != GL_UNSIGNED_INT &&
> -       type != GL_UNSIGNED_BYTE &&
> -       type != GL_UNSIGNED_SHORT)
> -   {
> -      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawElements(type)" );
> +   if (!_mesa_valid_element_type(ctx, type, "glDrawElements"))
>        return GL_FALSE;
> -   }
>  
>     if (!check_valid_to_render(ctx, "glDrawElements"))
>        return GL_FALSE;
> @@ -348,13 +362,8 @@ _mesa_validate_MultiDrawElements(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   if (type != GL_UNSIGNED_INT &&
> -       type != GL_UNSIGNED_BYTE &&
> -       type != GL_UNSIGNED_SHORT)
> -   {
> -      _mesa_error(ctx, GL_INVALID_ENUM, "glMultiDrawElements(type)" );
> +   if (!_mesa_valid_element_type(ctx, type, "glMultiDrawElements"))
>        return GL_FALSE;
> -   }
>  
>     if (!check_valid_to_render(ctx, "glMultiDrawElements"))
>        return GL_FALSE;
> @@ -419,12 +428,8 @@ _mesa_validate_DrawRangeElements(struct gl_context *ctx, GLenum mode,
>        return GL_FALSE;
>     }
>  
> -   if (type != GL_UNSIGNED_INT &&
> -       type != GL_UNSIGNED_BYTE &&
> -       type != GL_UNSIGNED_SHORT) {
> -      _mesa_error(ctx, GL_INVALID_ENUM, "glDrawRangeElements(type)" );
> +   if (!_mesa_valid_element_type(ctx, type, "glDrawRangeElements"))
>        return GL_FALSE;
> -   }
>  
>     if (!check_valid_to_render(ctx, "glDrawRangeElements"))
>        return GL_FALSE;
> @@ -548,13 +553,8 @@ _mesa_validate_DrawElementsInstanced(struct gl_context *ctx,
>        return GL_FALSE;
>     }
>  
> -   if (type != GL_UNSIGNED_INT &&
> -       type != GL_UNSIGNED_BYTE &&
> -       type != GL_UNSIGNED_SHORT) {
> -      _mesa_error(ctx, GL_INVALID_ENUM,
> -                  "glDrawElementsInstanced(type=0x%x)", type);
> +   if (!_mesa_valid_element_type(ctx, type, "glDrawElementsInstanced"))
>        return GL_FALSE;
> -   }
>  
>     if (numInstances <= 0) {
>        if (numInstances < 0)
>