[Mesa-dev] [PATCH v2 2/3] nir: Add a lowering pass for UYVY textures

[Kristian Høgsberg]

On Mon, Jun 19, 2017 at 7:07 PM, Lin, Johnson <johnson.lin at 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 at lists.freedesktop.org
> Cc: Lin, Johnson <johnson.lin at 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
>