On Monday, June 13, 2016 2:19:36 PM PDT Anuj Phogat wrote: > On Wed, Jun 8, 2016 at 2:11 PM, Kenneth Graunke <[email protected]> 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 <[email protected]> > > --- > > 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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
