<div dir="ltr">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. <div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 23, 2015 at 4:29 AM, Eduardo Lima Mitev <<a href="mailto:elima@igalia.com" target="_blank">elima@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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, </blockquote><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> return GL_TRUE; } </blockquote><div>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. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + 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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div></div>