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

[Tapani Pälli]

On 03/22/2018 07:53 AM, Tapani Pälli wrote:
> 
> 
> On 03/22/2018 04:43 AM, Lin, Johnson wrote:
>> Hi,  Thanks for the comments.
>>
>> I just noticed it does not check the extension support for 
>> EXT_color_buffer_float neither?
> 
> That is probably because it is enabled as 'dummy_true' (see 
> extensions_table.h) so it's always enabled on any driver. I wonder if we 
> can just go and do the same for EXT_color_buffer_half_float? Is there 
> any driver that would not support this?

Took a brief look and no, we can't simply toggle it on. There is also 
some API interaction defined by the spec that would need to be enabled, 
querying component type via glGetFramebufferAttachmentParameteriv and so on.

> 
>> -----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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev