Cool. Thanks. Will update the patch. -----Original Message----- From: Palli, Tapani Sent: Friday, March 23, 2018 4:59 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/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. > -----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
