[Mesa-dev] [PATCH] mesa: Make TexSubImage check negative dimensions sooner.

[Kenneth Graunke]

On Monday, June 13, 2016 2:19:36 PM PDT Anuj Phogat wrote:
> On Wed, Jun 8, 2016 at 2:11 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > Two dEQP tests expect INVALID_VALUE errors for negative width/height
> > parameters, but get INVALID_OPERATION because they haven't actually
> > created a destination image.  This is arguably not a bug in Mesa, as
> > there's no specified ordering of error conditions.
> >
> > However, it's also really easy to make the tests pass, and there's
> > no real harm in doing these checks earlier.
> >
> > Fixes:
> > dEQP-GLES3.functional.negative_api.texture.texsubimage3d_neg_width_height
> > dEQP-GLES31.functional.debug.negative_coverage.get_error.texture.texsubimage3d_neg_width_height
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/main/teximage.c | 68 ++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 49 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> > index 58b7f27..d4f8278 100644
> > --- a/src/mesa/main/teximage.c
> > +++ b/src/mesa/main/teximage.c
> > @@ -1102,6 +1102,32 @@ _mesa_legal_texture_dimensions(struct gl_context *ctx, GLenum target,
> >     }
> >  }
> >
> > +static bool
> > +error_check_subtexture_negative_dimensions(struct gl_context *ctx,
> > +                                           GLuint dims,
> > +                                           GLsizei subWidth,
> > +                                           GLsizei subHeight,
> > +                                           GLsizei subDepth,
> > +                                           const char *func)
> > +{
> > +   /* Check size */
> > +   if (subWidth < 0) {
> > +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(width=%d)", func, subWidth);
> > +      return true;
> > +   }
> > +
> > +   if (dims > 1 && subHeight < 0) {
> > +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(height=%d)", func, subHeight);
> > +      return true;
> > +   }
> > +
> > +   if (dims > 2 && subDepth < 0) {
> > +      _mesa_error(ctx, GL_INVALID_VALUE, "%s(depth=%d)", func, subDepth);
> > +      return true;
> > +   }
> > +
> > +   return false;
> > +}
> >
> >  /**
> >   * Do error checking of xoffset, yoffset, zoffset, width, height and depth
> > @@ -1119,25 +1145,6 @@ error_check_subtexture_dimensions(struct gl_context *ctx, GLuint dims,
> >     const GLenum target = destImage->TexObject->Target;
> >     GLuint bw, bh, bd;
> >
> > -   /* Check size */
> > -   if (subWidth < 0) {
> > -      _mesa_error(ctx, GL_INVALID_VALUE,
> > -                  "%s(width=%d)", func, subWidth);
> > -      return GL_TRUE;
> > -   }
> > -
> > -   if (dims > 1 && subHeight < 0) {
> > -      _mesa_error(ctx, GL_INVALID_VALUE,
> > -                  "%s(height=%d)", func, subHeight);
> > -      return GL_TRUE;
> > -   }
> > -
> > -   if (dims > 2 && subDepth < 0) {
> > -      _mesa_error(ctx, GL_INVALID_VALUE,
> > -                  "%s(depth=%d)", func, subDepth);
> > -      return GL_TRUE;
> > -   }
> > -
> >     /* check xoffset and width */
> >     if (xoffset < - (GLint) destImage->Border) {
> >        _mesa_error(ctx, GL_INVALID_VALUE, "%s(xoffset)", func);
> > @@ -2104,6 +2111,12 @@ texsubimage_error_check(struct gl_context *ctx, GLuint dimensions,
> >        return GL_TRUE;
> >     }
> >
> > +   if (error_check_subtexture_negative_dimensions(ctx, dimensions,
> > +                                                  width, height, depth,
> > +                                                  callerName)) {
> > +      return GL_TRUE;
> > +   }
> > +
> >     texImage = _mesa_select_tex_image(texObj, target, level);
> >     if (!texImage) {
> >        /* non-existant texture level */
> > @@ -2140,6 +2153,12 @@ texsubimage_error_check(struct gl_context *ctx, GLuint dimensions,
> >        return GL_TRUE;
> >     }
> >
> > +   if (error_check_subtexture_negative_dimensions(ctx, dimensions,
> > +                                                  width, height, depth,
> > +                                                  callerName)) {
> > +      return GL_TRUE;
> > +   }
> > +
> It's not obvious why we need to check twice for the negative dimensions
> in texsubimage_error_check(). Adding a comment here will avoid confusion.

This was a mistake - I accidentally added a second call.  I've dropped
this copy (the second call) in my local patch.

> >     if (error_check_subtexture_dimensions(ctx, dimensions,
> >                                           texImage, xoffset, yoffset, zoffset,
> >                                           width, height, depth, callerName)) {
> > @@ -2497,6 +2516,11 @@ copytexsubimage_error_check(struct gl_context *ctx, GLuint dimensions,
> >        return GL_TRUE;
> >     }
> >
> > +   if (error_check_subtexture_negative_dimensions(ctx, dimensions,
> > +                                                  width, height, 1, caller)) {
> > +      return GL_TRUE;
> > +   }
> > +
> >     if (error_check_subtexture_dimensions(ctx, dimensions, texImage,
> >                                           xoffset, yoffset, zoffset,
> >                                           width, height, 1, caller)) {
> > @@ -4387,6 +4411,12 @@ compressed_subtexture_error_check(struct gl_context *ctx, GLint dims,
> >        return GL_TRUE;
> >     }
> >
> > +   if (error_check_subtexture_negative_dimensions(ctx, dims,
> > +                                                  width, height, depth,
> > +                                                  callerName)) {
> > +      return GL_TRUE;
> > +   }
> > +
> Same here.

I only see one copy here.

> >     if (error_check_subtexture_dimensions(ctx, dims,
> >                                           texImage, xoffset, yoffset, zoffset,
> >                                           width, height, depth,
> > --
> > 2.8.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160613/2f43cb3e/attachment-0001.sig>