[Mesa-dev] [PATCH] mesa: RGB9_E5 invalid for CopyTexSubImage* in GLES

[Tapani Pälli]

On 03/16/2018 02:10 PM, Juan A. Suarez Romero wrote:
> On Fri, 2018-03-16 at 13:25 +0200, Tapani Pälli wrote:
>>
>> On 03/16/2018 01:13 PM, Tapani Pälli wrote:
>>>
>>>
>>> On 03/16/2018 01:01 PM, Juan A. Suarez Romero wrote:
>>>> On Fri, 2018-03-16 at 08:02 +0200, Tapani Pälli wrote:
>>>>> On 03/15/2018 02:04 PM, Juan A. Suarez Romero wrote:
>>>>>> According to OpenGL ES 3.2, section 8.6, CopyTexSubImage* should return
>>>>>> an INVALID_OPERATION if the internalformat of the texture is RGB9_E5.
>>>>>>
>>>>>> This fixes
>>>>>> dEQP-GLES31.functional.debug.negative_coverage.*.copytexsubimage2d_texture_internalformat.
>>>>>>
>>>>>
>>>>> These tests seem to be passing for me, I believe they fail in
>>>>> texture_format_error_check_gles. What kind of configuration are you
>>>>> running on?
>>>>>
>>>>
>>>> Correct. The tests as they are pass, but turns out the tests are
>>>> wrong, as they
>>>> are not testing what it is supposed to test.
>>>>
>>>> There's a patch to fix them:
>>>>
>>>> https://android-review.googlesource.com/c/platform/external/deqp/+/630411
>>>>
>>>> With the patch applied, now the tests would be failing, and the patch
>>>> I've sent
>>>> should fix them.
>>>>
>>>
>>> Thanks, this makes sense;
>>> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
>>
>> Having said that, it makes me wonder if we have errors in
>> _mesa_es3_error_check_format_and_type and should we try to fix this
>> there instead?
>>
> 
> 
> _mesa_es3_error_check_format_and_type() do error checking when creating the
> texture with glTex[Sub]Image.
> 
> In this case, creating the texture with that format is incorrect. What it is
> incorrect is trying to copy it later with glCopyTex[Sub]Image*D.
> 
> Hence why I'm not do ing the error checking there.

Ah right .. thanks, this fix is correct.


> 	J.A.
> 
>>>
>>>>      J.A.
>>>>
>>>>
>>>>>> ---
>>>>>>     src/mesa/main/teximage.c | 14 ++++++++++++++
>>>>>>     1 file changed, 14 insertions(+)
>>>>>>
>>>>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>>>>> index 928e50d472d..9c13e6d7cf6 100644
>>>>>> --- a/src/mesa/main/teximage.c
>>>>>> +++ b/src/mesa/main/teximage.c
>>>>>> @@ -2623,6 +2623,20 @@ copytexsubimage_error_check(struct gl_context
>>>>>> *ctx, GLuint dimensions,
>>>>>>           return GL_TRUE;
>>>>>>        }
>>>>>> +   /* From OpenGL ES 3.2 spec, section 8.6:
>>>>>> +    *
>>>>>> +    *     "An INVALID_OPERATION error is generated by
>>>>>> CopyTexSubImage3D,
>>>>>> +    *      CopyTexImage2D, or CopyTexSubImage2D if the
>>>>>> internalformat of the
>>>>>> +    *      texture image being (re)specified is RGB9_E5"
>>>>>> +    */
>>>>>> +   if (texImage->InternalFormat == GL_RGB9_E5 &&
>>>>>> +       !_mesa_is_desktop_gl(ctx)) {
>>>>>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>>>>>> +                  "%s(invalid internal format %s)", caller,
>>>>>> +                  _mesa_enum_to_string(texImage->InternalFormat));
>>>>>> +      return GL_TRUE;
>>>>>> +   }
>>>>>> +
>>>>>>        if (!_mesa_source_buffer_exists(ctx, texImage->_BaseFormat)) {
>>>>>>           _mesa_error(ctx, GL_INVALID_OPERATION,
>>>>>>                       "%s(missing readbuffer, format=%s)", caller,
>>>>>>
>>>>>
>>>>>
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>