[Mesa-dev] [PATCH] mesa: Make TexSubImage check negative dimensions sooner.
Anuj Phogat
anuj.phogat at gmail.com
Mon Jun 13 22:32:15 UTC 2016
On Mon, Jun 13, 2016 at 3:25 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> 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.
>
With this call dropped, patch is:
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>
>> > 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
>>
>
More information about the mesa-dev
mailing list