On 23.03.2018 12:18, Alejandro Piñeiro wrote:
On 23/03/18 09:58, Tapani Pälli wrote:

On 03/23/2018 07:54 AM, Lin, Johnson wrote:
So the solution will be query if EXT_color_buffer_half_float is
supported?

I think I found a proof that we don't actually need anything. Spec for
EXT_color_buffer_float adds following text:

--- 8< ---
An INVALID_OPERATION error is generated ... if the color buffer is a
floating-point format and type is not FLOAT, HALF FLOAT, or
UNSIGNED_INT_10F_11F_11F_REV.
--- 8< ---

This means that type can be HALF FLOAT when color buffer has
floating-point format.

(even though for some weird reason spec does not add it to earlier
sentence that says " "For floating-point rendering surfaces, the
combination format RGBA and type FLOAT is accepted.")

Since EXT_color_buffer_float is enabled, we can just use this patch.
Please remove the comment about EXT_color_buffer_half_float extension
though, that extension is not enabled and would need more work elsewhere.

Taking into account that the conclusion that we don't need a extension
check required some code+spec research, how about it we mention it
somewhere? In a comment at the code, or perhaps in the commit message is
enough.

True, commit message could include the part mentioning when INVALID_OPERATION is generated as that mentions both FLOAT and HALF FLOAT.





-----Original Message-----
From: Palli, Tapani
Sent: Friday, March 23, 2018 1:53 PM
To: Lin, Johnson <johnson....@intel.com>; Alejandro Piñeiro
<apinhe...@igalia.com>; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] mesa: readpixels add support for
GL_HALF_FLOAT


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 <apinhe...@igalia.com>; Lin, Johnson
<johnson....@intel.com>; mesa-dev@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....@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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


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

Reply via email to