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

[Tapani Pälli]

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?


> -----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