On 09/15/2015 09:23 PM, Jason Ekstrand wrote: > On Tue, Sep 15, 2015 at 4:47 AM, Eduardo Lima Mitev <el...@igalia.com> wrote: >> For consistency and efficiency, the (sub)texture dimension error check >> should go before the validation of format, type and internal format. > > You mentioned in another patch that this fixes a bug or, at the very > least, prevents one. What bug is that? I ask because my personal > inclination would be to keep them in the order they were before. > --Jason >
These tests fail without this patch, because they fail the validation of format+type+internalFormat (INVALID_OPERATION) which happens before the check for texture dimensions (INVALID_VALUE, the expected error). dEQP-GLES2.functional.negative_api.texture.texsubimage2d_neg_offset dEQP-GLES2.functional.negative_api.texture.texsubimage2d_offset_allowed dEQP-GLES2.functional.negative_api.texture.texsubimage2d_neg_wdt_hgt In all 3, the failing combination is: format = GL_RGB, type = GL_UNSIGNED_BYTE, internalFormat = GL_RGBA Following the spec, the effective internal format should be GL_RGB8 (per table 3.12, page 128 of OpenGL-ES 3.0.4). So, it is actually failing the double-check I added in the previous patch, in which I obtain again the base format from the effective internal format to compare it with the original internal format in base form passed to the function. So GL_RGB8 -> GL_RGB which != GL_RGBA. Now I'm puzzled. Is the combination above valid? As I interpret the spec, it isn't. Which means the tests would be wrong, but that's strange too. >> --- >> src/mesa/main/teximage.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c >> index d9453e3..09040d5 100644 >> --- a/src/mesa/main/teximage.c >> +++ b/src/mesa/main/teximage.c >> @@ -2117,6 +2117,12 @@ texsubimage_error_check(struct gl_context *ctx, >> GLuint dimensions, >> return GL_TRUE; >> } >> >> + if (error_check_subtexture_dimensions(ctx, dimensions, >> + texImage, xoffset, yoffset, >> zoffset, >> + width, height, depth, callerName)) >> { >> + return GL_TRUE; >> + } >> + >> err = _mesa_error_check_format_and_type(ctx, format, type); >> if (err != GL_NO_ERROR) { >> _mesa_error(ctx, err, >> @@ -2145,12 +2151,6 @@ texsubimage_error_check(struct gl_context *ctx, >> GLuint dimensions, >> return GL_TRUE; >> } >> >> - if (error_check_subtexture_dimensions(ctx, dimensions, >> - texImage, xoffset, yoffset, >> zoffset, >> - width, height, depth, callerName)) >> { >> - return GL_TRUE; >> - } >> - >> if (_mesa_is_format_compressed(texImage->TexFormat)) { >> if (_mesa_format_no_online_compression(ctx, >> texImage->InternalFormat)) { >> _mesa_error(ctx, GL_INVALID_OPERATION, >> -- >> 2.4.6 >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev