[Mesa-dev] [PATCH] mesa: fix error handling for glMaterial*

[Brian Paul]

On Mon, Sep 19, 2011 at 9:16 PM, Yuanhan Liu
<yuanhan.liu at linux.intel.com> wrote:
> Trigger GL_INVALID_ENUM error if the face paramter is not a valid value.
>
> Trigger GL_INVALID_VALUE error if the GL_SHININESS value is out side
> [0, ctx->Constant.MaxShiniess].
>
> v2: fix the max shininess value.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> ---
>  src/mesa/vbo/vbo_attrib_tmp.h |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
> index 65717eb..f5814f6 100644
> --- a/src/mesa/vbo/vbo_attrib_tmp.h
> +++ b/src/mesa/vbo/vbo_attrib_tmp.h
> @@ -795,6 +795,11 @@ TAG(VertexAttrib4fvNV)(GLuint index, const GLfloat * v)
>
>  #define MAT( ATTR, N, face, params )                   \
>  do {                                                   \
> +   if (face != GL_FRONT && face != GL_BACK &&           \
> +       face != GL_FRONT_AND_BACK) {                     \
> +      _mesa_error(ctx, GL_INVALID_ENUM, "Invalid value for parameter face"); \
> +      break;                                            \
> +   }                                                    \
>    if (face != GL_BACK)                                        \
>       MAT_ATTR( ATTR, N, params ); /* front */         \
>    if (face != GL_FRONT)                               \

I think that error checking of the face parameter could be done in the
next function instead so that the code isn't needlessly duplicated.
Also, the error message passed to the _mesa_error() call should be
something like "glMaterialfv(invalid face)".


> @@ -823,7 +828,11 @@ TAG(Materialfv)(GLenum face, GLenum pname,
>       MAT(VBO_ATTRIB_MAT_FRONT_SPECULAR, 4, face, params);
>       break;
>    case GL_SHININESS:
> -      MAT(VBO_ATTRIB_MAT_FRONT_SHININESS, 1, face, params);
> +      if (*params < 0 || *params > ctx->Const.MaxShininess)
> +         _mesa_error(ctx, GL_INVALID_VALUE,
> +                     "GL_SHININESS value out side [0,%f]", ctx->Const.MaxShininess);
> +      else
> +         MAT(VBO_ATTRIB_MAT_FRONT_SHININESS, 1, face, params);
>       break;
>    case GL_COLOR_INDEXES:
>       MAT(VBO_ATTRIB_MAT_FRONT_INDEXES, 3, face, params);
> --

The error string there should be something like "glMaterial(GL_SHININESS > %f)"

-Brian