Re: [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 <e...@anholt.net>; Lin, Johnson <johnson@intel.com>; 
mesa-dev@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@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.pa...@intel.com>
Signed-off-by: Lin Johnson <johnson@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


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Lin, Johnson]

Guy,

Then how can we deal with
android.view.cts.PixelCopyTest #TestWindowProducerCopyToRGBA16F
It is blocked by this error check..



-Original Message-
From: Palli, Tapani 
Sent: Tuesday, April 3, 2018 1:42 PM
To: Eric Anholt <e...@anholt.net>; Lin, Johnson <johnson@intel.com>; 
mesa-dev@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@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.pa...@intel.com>
>> Signed-off-by: Lin Johnson <johnson@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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Tapani Pälli]

Hi Eric;

On 03/30/2018 11:25 PM, Eric Anholt wrote:

Lin Johnson  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 
Signed-off-by: Lin Johnson 


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

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

[Eric Anholt]

Lin Johnson  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 
> Signed-off-by: Lin Johnson 

I've been looking into CTS issues on VC5, and I think this patch should
be reverted.

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


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Tapani Pälli]

Thanks, I'll modify the commit message a bit and push this in.

On 03/26/2018 05:13 PM, 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
v2: remove commtens of Ext_color_buffer_half_float.
As Ext_color_buffer__float can use type GL_HALF_FLOAT

Reviewed-by:   Palli, Tapani 
Signed-off-by: Lin Johnson 
---
  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..4407f13289e2 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;
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Lin Johnson]

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 
Signed-off-by: Lin Johnson 
---
 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..4407f13289e2 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;
   if (type == GL_UNSIGNED_BYTE && data_type == GL_UNSIGNED_NORMALIZED)
  return GL_NO_ERROR;
   if (internalFormat == GL_RGB10_A2 &&
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev