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(&tex->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
