[Mesa-dev] [PATCH] mesa: readpixels add support for GL_HALF_FLOAT

[Lin, Johnson]

Hi,  Thanks for the comments.

I just noticed it does not check the extension support for EXT_color_buffer_float neither?

-----Original Message-----
From: Palli, Tapani 
Sent: Wednesday, March 21, 2018 6:57 PM
To: Alejandro Piñeiro <apinheiro at igalia.com>; Lin, Johnson <johnson.lin at intel.com>; mesa-dev at lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] mesa: readpixels add support for GL_HALF_FLOAT



On 21.03.2018 12:45, Tapani Pälli wrote:
> 
> 
> On 21.03.2018 08:52, Alejandro Piñeiro wrote:
>> On 21/03/18 06:57, Lin Johnson wrote:
>>> Ext_color_buffer_half_float is using type GL_HALF_FLOAT and 
>>> data_type GL_FLOAT. This fix Android CTS test 
>>> android.view.cts.PixelCopyTest #TestWindowProducerCopyToRGBA16F
>>>
>>> Signed-off-by: Lin Johnson <johnson.lin at intel.com>
>>> ---
>>>   src/mesa/main/readpix.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c index 
>>> 6ce340ddf9bb..51331dd095ab 100644
>>> --- a/src/mesa/main/readpix.c
>>> +++ b/src/mesa/main/readpix.c
>>> @@ -920,6 +920,8 @@ read_pixels_es3_error_check(GLenum format, 
>>> GLenum type,
>>>      case GL_RGBA:
>>>         if (type == GL_FLOAT && data_type == GL_FLOAT)
>>>            return GL_NO_ERROR; /* EXT_color_buffer_float */
>>> +      if (type == GL_HALF_FLOAT && data_type == GL_FLOAT)
>>> +         return GL_NO_ERROR; /* EXT_color_buffer_half_float */
>>
>> If this combination is allowed thanks to that extension, what would 
>> happen if that extension is not supported? shouldn't include a 
>> extension check? Or that is checked in a different place?
> 
> I was thinking the same. Having seen the test it does not seem to make 
> any kind of checks what is supported (like asking for extension, or 
> maybe asking for GL_IMPLEMENTATION_COLOR_READ_TYPE)  but attempts 
> glReadPixels using GL_HALF_FLOAT type, I think it should verify first 
> that such reads are supported. Currently we don't seem to support this 
> extension.

... but probably support the functionality (OpenGL ES 3.2), so maybe some checks needed for ES version (?)


> 
> 
>>>         if (type == GL_UNSIGNED_BYTE && data_type == 
>>> GL_UNSIGNED_NORMALIZED)
>>>            return GL_NO_ERROR;
>>>         if (internalFormat == GL_RGB10_A2 &&
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev