Re: [Mesa-dev] [PATCH v3] mesa: readpixels add support for GL_HALF_FLOAT
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
[Mesa-dev] [PATCH v3] mesa: readpixels add support for GL_HALF_FLOAT
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> --- 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
[Mesa-dev] [PATCH v2] mesa: readpixels add support for GL_HALF_FLOAT
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> --- 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
Re: [Mesa-dev] [PATCH] mesa: readpixels add support for GL_HALF_FLOAT
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 usin
Re: [Mesa-dev] [PATCH] mesa: readpixels add support for GL_HALF_FLOAT
So the solution will be query if EXT_color_buffer_half_float is supported? -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
Re: [Mesa-dev] [PATCH] mesa: readpixels add support for GL_HALF_FLOAT
Hi, Thanks for the comments. I just noticed it does not check the extension support for EXT_color_buffer_float neither? -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] [PATCH] mesa: readpixels add support for GL_HALF_FLOAT
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 (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
Re: [Mesa-dev] [PATCH v4 1/3] dri: Add UYVY as available format
@Kristian
-Original Message-
From: Lin, Johnson
Sent: Thursday, June 22, 2017 11:28 AM
To: mesa-dev@lists.freedesktop.org
Cc: Lin, Johnson <johnson@intel.com>
Subject: [PATCH v4 1/3] dri: Add UYVY as available format
UYVY is diffrent with YUYV in byte order.
YUYV is already declared in dri_interface.h, this CL add the difinitions for
UYVY.
Drivers can add UYVY as supported format
---
include/GL/internal/dri_interface.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/GL/internal/dri_interface.h
b/include/GL/internal/dri_interface.h
index fc2d4bbe22ef..6992da16d5f8 100644
--- a/include/GL/internal/dri_interface.h
+++ b/include/GL/internal/dri_interface.h
@@ -1211,6 +1211,7 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_FOURCC_NV120x3231564e
#define __DRI_IMAGE_FOURCC_NV160x3631564e
#define __DRI_IMAGE_FOURCC_YUYV0x56595559
+#define __DRI_IMAGE_FOURCC_UYVY0x59565955
#define __DRI_IMAGE_FOURCC_YVU410 0x39555659
#define __DRI_IMAGE_FOURCC_YVU411 0x31315659
@@ -1224,7 +1225,7 @@ struct __DRIdri2ExtensionRec {
* RGB and RGBA are may be usable directly as images but its still
* recommended to call fromPlanar with plane == 0.
*
- * Y_U_V, Y_UV and Y_XUXV all requires call to fromPlanar to create
+ * Y_U_V, Y_UV,Y_XUXV and Y_UXVX all requires call to fromPlanar to
+ create
* usable sub-images, sampling from images return raw YUV data and
* color conversion needs to be done in the shader.
*
@@ -1236,6 +1237,7 @@ struct __DRIdri2ExtensionRec {
#define __DRI_IMAGE_COMPONENTS_Y_U_V 0x3003
#define __DRI_IMAGE_COMPONENTS_Y_UV0x3004
#define __DRI_IMAGE_COMPONENTS_Y_XUXV 0x3005
+#define __DRI_IMAGE_COMPONENTS_Y_UXVX 0x3008
#define __DRI_IMAGE_COMPONENTS_R 0x3006
#define __DRI_IMAGE_COMPONENTS_RG 0x3007
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] FW: [PATCH v3 2/3] nir: Add a lowering pass for UYVY textures
Kristian,
Thanks for reviewing. You are right. I had a swap in DRI formats which I have
fixed in patch3. So the order is corrected here.
-Original Message-
From: Lin, Johnson
Sent: Wednesday, June 21, 2017 11:24 AM
To: mesa-dev@lists.freedesktop.org
Cc: Lin, Johnson <johnson@intel.com>
Subject: [PATCH v3 2/3] nir: Add a lowering pass for UYVY textures
Similar with support for YUYV but with byte order difference in sampler
---
src/compiler/nir/nir.h | 1 +
src/compiler/nir/nir_lower_tex.c | 18 ++
2 files changed, 19 insertions(+)
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index
ab7ba14303b7..1b4e47058d4d 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2449,6 +2449,7 @@ typedef struct nir_lower_tex_options {
unsigned lower_y_uv_external;
unsigned lower_y_u_v_external;
unsigned lower_yx_xuxv_external;
+ unsigned lower_xy_uxvx_external;
/**
* To emulate certain texture wrap modes, this can be used diff --git
a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
index 4ef81955513e..65681decb1c0 100644
--- a/src/compiler/nir/nir_lower_tex.c
+++ b/src/compiler/nir/nir_lower_tex.c
@@ -301,6 +301,20 @@ lower_yx_xuxv_external(nir_builder *b, nir_tex_instr *tex)
nir_channel(b, xuxv, 3)); }
+static void
+lower_xy_uxvx_external(nir_builder *b, nir_tex_instr *tex) {
+ b->cursor = nir_after_instr(>instr);
+
+ nir_ssa_def *y = sample_plane(b, tex, 0); nir_ssa_def *uxvx =
+ sample_plane(b, tex, 1);
+
+ convert_yuv_to_rgb(b, tex,
+ nir_channel(b, y, 1),
+ nir_channel(b, uxvx, 0),
+ nir_channel(b, uxvx, 2)); }
+
/*
* Emits a textureLod operation used to replace an existing
* textureGrad instruction.
@@ -760,6 +774,10 @@ nir_lower_tex_block(nir_block *block, nir_builder *b,
progress = true;
}
+ if ((1 << tex->texture_index) & options->lower_xy_uxvx_external) {
+ lower_xy_uxvx_external(b, tex);
+ progress = true;
+ }
if (sat_mask) {
saturate_src(b, tex, sat_mask);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/3] nir: Add a lowering pass for UYVY textures
Thanks for the comments. I don't have a swap in other places in the patches.
But I think your point is good, from the code that is a swap of U/V channels. I
am suspecting there might be some bug somewhere else. Need to continue to
investigate.
-Original Message-
From: hoegsb...@gmail.com [mailto:hoegsb...@gmail.com] On Behalf Of Kristian
Høgsberg
Sent: Wednesday, June 21, 2017 12:50 AM
To: Lin, Johnson <johnson@intel.com>
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH v2 2/3] nir: Add a lowering pass for UYVY textures
On Mon, Jun 19, 2017 at 7:07 PM, Lin, Johnson <johnson@intel.com> wrote:
> Kristian,
>
> Thanks for the review comments. By my tests, UYVY buffer can be sampled and
> rendered correctly. So there is no swap of U/V channel here.
Just saying "the test pass, all is fine" isn't good enough. See below.
>
> -Original Message-
> From: Lin, Johnson
> Sent: Tuesday, June 20, 2017 9:43 AM
> To: mesa-dev@lists.freedesktop.org
> Cc: Lin, Johnson <johnson@intel.com>
> Subject: [PATCH v2 2/3] nir: Add a lowering pass for UYVY textures
>
> Similar with support for YUYV but with byte order difference in
> sampler
> ---
> src/compiler/nir/nir.h | 1 +
> src/compiler/nir/nir_lower_tex.c | 16
> 2 files changed, 17 insertions(+)
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index
> ab7ba14303b7..1b4e47058d4d 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2449,6 +2449,7 @@ typedef struct nir_lower_tex_options {
> unsigned lower_y_uv_external;
> unsigned lower_y_u_v_external;
> unsigned lower_yx_xuxv_external;
> + unsigned lower_xy_uxvx_external;
>
> /**
> * To emulate certain texture wrap modes, this can be used diff
> --git a/src/compiler/nir/nir_lower_tex.c
> b/src/compiler/nir/nir_lower_tex.c
> index 4ef81955513e..5593f9890b28 100644
> --- a/src/compiler/nir/nir_lower_tex.c
> +++ b/src/compiler/nir/nir_lower_tex.c
> @@ -301,6 +301,18 @@ lower_yx_xuxv_external(nir_builder *b, nir_tex_instr
> *tex)
>nir_channel(b, xuxv, 3)); }
>
> +static void lower_xy_uxvx_external(nir_builder *b, nir_tex_instr
> +*tex) {
> + b->cursor = nir_after_instr(>instr);
> +
> + nir_ssa_def *y = sample_plane(b, tex, 0); nir_ssa_def *uxvx =
> + sample_plane(b, tex, 1);
> +
> + convert_yuv_to_rgb(b, tex,
> + nir_channel(b, y, 1),
> + nir_channel(b, uxvx, 2),
> + nir_channel(b, uxvx, 0)); }
Let's look at convert_yuv_to_rgb:
static void
convert_yuv_to_rgb(nir_builder *b, nir_tex_instr *tex,
nir_ssa_def *y, nir_ssa_def *u, nir_ssa_def *v)
Notice how it takes the channels in y, u and v order. But you're passing in y,
then nir_channel(b, uxvx, 2), which extracts the second channels, which is v
and then nir_channel(b, uxvx, 0) which is u. If the tests pass you probably
have another swap somewhere else in the patch that cancels out this problem.
You need to fix this.
Kristian
> +
> /*
> * Emits a textureLod operation used to replace an existing
> * textureGrad instruction.
> @@ -760,6 +772,10 @@ nir_lower_tex_block(nir_block *block, nir_builder *b,
> progress = true;
>}
>
> + if ((1 << tex->texture_index) & options->lower_xy_uxvx_external) {
> + lower_xy_uxvx_external(b, tex);
> + progress = true;
> + }
>
>if (sat_mask) {
> saturate_src(b, tex, sat_mask);
> --
> 1.9.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/3] nir: Add a lowering pass for UYVY textures
Kristian,
Thanks for the review comments. By my tests, UYVY buffer can be sampled and
rendered correctly. So there is no swap of U/V channel here.
-Original Message-
From: Lin, Johnson
Sent: Tuesday, June 20, 2017 9:43 AM
To: mesa-dev@lists.freedesktop.org
Cc: Lin, Johnson <johnson@intel.com>
Subject: [PATCH v2 2/3] nir: Add a lowering pass for UYVY textures
Similar with support for YUYV but with byte order difference in sampler
---
src/compiler/nir/nir.h | 1 +
src/compiler/nir/nir_lower_tex.c | 16
2 files changed, 17 insertions(+)
diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index
ab7ba14303b7..1b4e47058d4d 100644
--- a/src/compiler/nir/nir.h
+++ b/src/compiler/nir/nir.h
@@ -2449,6 +2449,7 @@ typedef struct nir_lower_tex_options {
unsigned lower_y_uv_external;
unsigned lower_y_u_v_external;
unsigned lower_yx_xuxv_external;
+ unsigned lower_xy_uxvx_external;
/**
* To emulate certain texture wrap modes, this can be used diff --git
a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
index 4ef81955513e..5593f9890b28 100644
--- a/src/compiler/nir/nir_lower_tex.c
+++ b/src/compiler/nir/nir_lower_tex.c
@@ -301,6 +301,18 @@ lower_yx_xuxv_external(nir_builder *b, nir_tex_instr *tex)
nir_channel(b, xuxv, 3)); }
+static void lower_xy_uxvx_external(nir_builder *b, nir_tex_instr *tex)
+{
+ b->cursor = nir_after_instr(>instr);
+
+ nir_ssa_def *y = sample_plane(b, tex, 0); nir_ssa_def *uxvx =
+ sample_plane(b, tex, 1);
+
+ convert_yuv_to_rgb(b, tex,
+ nir_channel(b, y, 1),
+ nir_channel(b, uxvx, 2),
+ nir_channel(b, uxvx, 0)); }
+
/*
* Emits a textureLod operation used to replace an existing
* textureGrad instruction.
@@ -760,6 +772,10 @@ nir_lower_tex_block(nir_block *block, nir_builder *b,
progress = true;
}
+ if ((1 << tex->texture_index) & options->lower_xy_uxvx_external) {
+ lower_xy_uxvx_external(b, tex);
+ progress = true;
+ }
if (sat_mask) {
saturate_src(b, tex, sat_mask);
--
1.9.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] fix minor error in YUV2RGB matrix used in shader
Thanks for the comments and have updated the patch! -Original Message- From: Matt Turner [mailto:matts...@gmail.com] Sent: Wednesday, May 3, 2017 2:37 AM To: Lin, Johnson <johnson@intel.com> Cc: mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [PATCH] fix minor error in YUV2RGB matrix used in shader On Tue, May 2, 2017 at 7:24 AM, Johnson Lin <johnson@intel.com> wrote: > The matrix used for YCbCr to RGB is listed in Wiki > https://en.wikipedia.org/ wiki/YCbCr; There is minor error in the > matrix constant: 0.0625=16/256 should be 16.0/255,and 0.5=128.0/256 should > be 128.0/255. > Note that conversion from a 0-255 byte number to 0-1.0 float is to > divide by > 255 instead of 256. That's we get 255=1.0f. > By the constant change we can see the CSC result is bit aligned with > Wiki conversion result and FFMPeg result.Otherwise in some situation, > there will be one bit difference > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100854 > --- > src/compiler/nir/nir_lower_tex.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/compiler/nir/nir_lower_tex.c > b/src/compiler/nir/nir_lower_tex.c > index 352d1499bc8d..f20425e84aab 100644 > --- a/src/compiler/nir/nir_lower_tex.c > +++ b/src/compiler/nir/nir_lower_tex.c > @@ -244,9 +244,9 @@ convert_yuv_to_rgb(nir_builder *b, nir_tex_instr *tex, > nir_ssa_def *yuv = >nir_vec4(b, > nir_fmul(b, nir_imm_float(b, 1.16438356f), > -nir_fadd(b, y, nir_imm_float(b, -0.0625f))), > - nir_channel(b, nir_fadd(b, u, nir_imm_float(b, -0.5f)), 0), > - nir_channel(b, nir_fadd(b, v, nir_imm_float(b, -0.5f)), 0), > +nir_fadd(b, y, nir_imm_float(b, -16.0f/255))), > + nir_channel(b, nir_fadd(b, u, nir_imm_float(b, -128.0f/255)), > 0), > + nir_channel(b, nir_fadd(b, v, nir_imm_float(b, > + -128.0f/255)), 0), Other feedback which wasn't taken into account: spaces around operators (/), and use a floating point constant (255.0f) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] fix minor error in YUV2RGB matrix used in shader
Yes, We can. Will do it -Original Message- From: Eric Anholt [mailto:e...@anholt.net] Sent: Saturday, April 29, 2017 6:02 AM To: Lin, Johnson <johnson@intel.com>; mesa-dev@lists.freedesktop.org Cc: Lin, Johnson <johnson@intel.com> Subject: Re: [Mesa-dev] [PATCH] fix minor error in YUV2RGB matrix used in shader Johnson Lin <johnson@intel.com> writes: > The matrix used for YCbCr to RGB is listed in Wiki > https://en.wikipedia.org/wiki/YCbCr; > There is minor error in the matrix constant: 0.0625=16/256 should be > 16.0/255, and 0.5=128.0/256 should be 128.0/255. > Note that conversion from a 0-255 byte number to 0-1.0 float is to > divide by 255 instead of 256. That's we get 255=1.0f. > By the constant change we can see the CSC result is bit aligned with > Wiki conversion result and FFMPeg result. > Otherwise in some situation, there will be one bit difference > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100854 > --- > src/compiler/nir/nir_lower_tex.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/compiler/nir/nir_lower_tex.c > b/src/compiler/nir/nir_lower_tex.c > index 352d1499bc8d..385739a56a71 100644 > --- a/src/compiler/nir/nir_lower_tex.c > +++ b/src/compiler/nir/nir_lower_tex.c > @@ -244,9 +244,9 @@ convert_yuv_to_rgb(nir_builder *b, nir_tex_instr *tex, > nir_ssa_def *yuv = >nir_vec4(b, > nir_fmul(b, nir_imm_float(b, 1.16438356f), > -nir_fadd(b, y, nir_imm_float(b, -0.0625f))), > - nir_channel(b, nir_fadd(b, u, nir_imm_float(b, -0.5f)), 0), > - nir_channel(b, nir_fadd(b, v, nir_imm_float(b, -0.5f)), 0), > +nir_fadd(b, y, nir_imm_float(b, -0.0627451f))), > + nir_channel(b, nir_fadd(b, u, nir_imm_float(b, > -0.50196078431f)), 0), > + nir_channel(b, nir_fadd(b, v, nir_imm_float(b, > + -0.50196078431f)), 0), > nir_imm_float(b, 0.0)); Could we use 16.0/255.0 and 128.0/255.0, instead of magic-looking numbers? With that, it will be: Reviewed-by: Eric Anholt <e...@anholt.net> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
