On Tue, Sep 15, 2015 at 3:01 PM, Eduardo Lima Mitev <el...@igalia.com> wrote:
> 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.
I could easily see the test being wrong here. As per table 3.2 in the
ES 3.0 spec, you can't use GL_RGB with GL_UNSIGNED_BYTE and GL_RGBA.
Just switching the format to GL_RGBA or the internalFormat to GL_RGB8
in the test would probably fix it. I think the best path forward
there would be to make a patch to the test and try to get that
upstream to dEQP. I'm not sure what the procedure is there.
--Jason
>>> ---
>>> 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
