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

[Yuanhan Liu]

On Mon, Sep 19, 2011 at 09:25:54PM -0600, Brian Paul wrote:
> 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)".

Nice suggestion, thanks.

> 
> 
> > @@ -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)"
Well, I guess something like this:
  glMaterial(invalid shininess: -1.000000 out range [0, 128.000000]
would be clear enough.

A updated patch sent.

Thanks,
Yuanhan Liu