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

[Tapani Pälli]

On 04/03/2018 08:59 AM, Lin, Johnson wrote:
> Guy,
> 
> Then how can we deal with
> android.view.cts.PixelCopyTest #TestWindowProducerCopyToRGBA16F
> It is blocked by this error check..

It looks like that test is broken (behaving against GL ES conformance 
test) and needs to be fixed.

> 
> 
> -----Original Message-----
> From: Palli, Tapani
> Sent: Tuesday, April 3, 2018 1:42 PM
> To: Eric Anholt <eric at anholt.net>; Lin, Johnson <johnson.lin at intel.com>; mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH v3] mesa: readpixels add support for GL_HALF_FLOAT
> 
> Hi Eric;
> 
> On 03/30/2018 11:25 PM, Eric Anholt wrote:
>> Lin Johnson <johnson.lin at intel.com> writes:
>>
>>> 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
>>> v2: remove commtens of Ext_color_buffer_half_float.
>>> As Ext_color_buffer__float can use type GL_HALF_FLOAT
>>>
>>> Reviewed-by:   Palli, Tapani <tapani.palli at intel.com>
>>> Signed-off-by: Lin Johnson <johnson.lin at intel.com>
>>
>> I've been looking into CTS issues on VC5, and I think this patch
>> should be reverted.
> 
> After some inspection of the packed_pixels test I do agree, will revert this. It's too bad dEQP does not have similar test. It bothers me a bit though that 'reading pixels' section has hints that type could be GL_HALF_FLOAT but no spec seems to add this capability, maybe that is then only possible via IMPLEMENTATION_COLOR_READ_FORMAT?
> 
> 
>> The issue is that the text above that in the spec disallows this case.
>>   From GLES 3.0.2:
>>
>>       Only two combinations of format and type are accepted in most
>>       cases. The first varies depending on the format of the currently
>>       bound rendering surface. For normalized fixed-point rendering
>>       surfaces, the combination format RGBA and type UNSIGNED_BYTE is
>>       accepted. For signed integer rendering surfaces, the combina- tion
>>       format RGBA_INTEGER and type INT is accepted. For unsigned integer
>>       rendering surfaces, the combination format RGBA_INTEGER and type
>>       UNSIGNED_INT is accepted.
>>
>>       [...]
>>
>>       ReadPixels generates an INVALID_OPERATION error if the combination
>>       of format and type is unsupported.
>>
>> and this spec adds on to that first paragraph:
>>
>>       For floating-point rendering surfaces, the combination
>>       format RGBA and type FLOAT is accepted.
>>
>> The second allowed combo is:
>>
>>       The second is an implementation-chosen format from among those
>>       defined in table 3.2, excluding formats DEPTH_COMPONENT and
>>       DEPTH_STENCIL . The values of format and type for this format may be
>>       determined by calling Get- Integerv with the symbolic constants
>>       IMPLEMENTATION_COLOR_READ_FORMAT and IMPLEMENTATION_COLOR_READ_TYPE
>>       , respectively.
>>
>> The failing cases are in the CTS's packed_pixels testsuite, such as:
>>
>> KHR-GLES3.packed_pixels.pbo_rectangle.r11f_g11f_b10f
>>
>>
> 
> // Tapani
>