[Mesa-dev] [PATCH 1/3] mesa: Improve validation of target, format and type of glTexSubImage[2, 3]D

[Laura Ekstrand]

I'm glad you found this target checking problem.  I have come across the
same problem in other texture functions, but I have been too busy to find
all instances of this.  It's kind of a natural, negative byproduct of
adding DSA.

On Mon, Mar 23, 2015 at 4:29 AM, Eduardo Lima Mitev <elima at igalia.com>
wrote:

> Currently, glTexSubImage[2,3]D attempt to resolve the texture object
> (by calling _mesa_get_current_tex_object()) before validating the given
> target. However, that method explicitly states that target must have been
> validated before calling it, so it never returns a user error.
>
> The target validation occurs later when texsubimage_error_check() is
> called.
> This patch moves the target validation out from that function and into
> a point before the texture object is resolved.
>
> It also adds a missing validation of format and type against the texture
> object's internal format, when profile is OpenGL-ES 3.0.
>
> Fixes 2 dEQP tests:
> * dEQP-GLES3.functional.negative_api.texture.texsubimage2d
> * dEQP-GLES3.functional.negative_api.texture.texsubimage3d
> ---
>  src/mesa/main/teximage.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index 8d9d7cf..9f7e10c 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -2530,6 +2530,24 @@ texsubimage_error_check(struct gl_context *ctx,
> GLuint dimensions,
>
I recommend getting rid of the legal_texsubimage_target check in
texsubimage_error_check and moving it up into texturesubimage, right after
the texObj has been retrieved. Otherwise in the texsubimage case, the
target will get checked twice, once in texsubimage and once in
texsubimage-error-check.  In general, checking hurts driver performance, so
we should aim to get rid of redundancies.


>        return GL_TRUE;
>     }
>
> This is not related to target checking and should be moved to a separate
commit (with a name like gles 3 support for texsubimage_error_check).  I
would also move this block up to about line 2508 so it can sit next to the
other format and type checks there.  It would be nice to have all of the
format_and_type_checks formatted into one if, else if, else block so it's
really obvious what's going on, too.

> +   if (_mesa_is_gles3(ctx)) {
> +      /* Validation of format and type for ES3 has to be done here
> +       * after the texture image is resolved, because the internal
> +       * format is needed for the verification
> +       */
> +      err = _mesa_es3_error_check_format_and_type(ctx, format, type,
> +
> texImage->InternalFormat);
> +      if (err != GL_NO_ERROR) {
> +         _mesa_error(ctx, err,
> +                     "%s(incompatible format = %s, type = %s, "
> +                     "internalformat = %s)",
> +                     callerName, _mesa_lookup_enum_by_nr(format),
> +                     _mesa_lookup_enum_by_nr(type),
> +                     _mesa_lookup_enum_by_nr(texImage->InternalFormat));
> +         return GL_TRUE;
> +      }
> +   }
> +
>     if (error_check_subtexture_dimensions(ctx, dimensions,
>                                           texImage, xoffset, yoffset,
> zoffset,
>                                           width, height, depth,
> callerName)) {
> @@ -3569,6 +3587,13 @@ texsubimage(struct gl_context *ctx, GLuint dims,
> GLenum target, GLint level,
>     struct gl_texture_object *texObj;
>     struct gl_texture_image *texImage;
>
> +   /* check target (proxies not allowed) */
> +   if (!legal_texsubimage_target(ctx, dims, target, false)) {
> +      _mesa_error(ctx, GL_INVALID_ENUM, "glTexSubImage%uD(target=%s)",
> +                  dims, _mesa_lookup_enum_by_nr(target));
> +      return;
> +   }
> +
>     texObj = _mesa_get_current_tex_object(ctx, target);
>     if (!texObj)
>        return;
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150323/88a56de8/attachment-0001.html>