[Mesa-dev] [PATCH v2 2/7] nir/lower_tex: add lowering for texture gradient on cube maps

[Iago Toral]

On Mon, 2016-12-12 at 23:14 -0800, Kenneth Graunke wrote:
> On Monday, December 12, 2016 2:11:43 PM PST Iago Toral Quiroga wrote:
> > 
> > This is ported from the Intel lowering pass that we use with GLSL
> > IR.
> > The NIR pass only handles cube maps, not shadow samplers, which are
> > also lowered for gen < 8 on Intel hardware. We will add support for
> > that in a later patch, at which point we should be able to remove
> > the GLSL IR lowering pass.
> > 
> > v2:
> > - added a helper to retrieve ddx/ddy parameters (Ken)
> > - No need to make size.z=1.0, we are only using component x anyway
> > (Iago)
> > ---
> >  src/compiler/nir/nir.h           |   5 +
> >  src/compiler/nir/nir_lower_tex.c | 265
> > +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 270 insertions(+)
> > 
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index 544d4ba..600e3d6 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -2394,6 +2394,11 @@ typedef struct nir_lower_tex_options {
> >      * of the texture are lowered to linear.
> >      */
> >     unsigned lower_srgb;
> > +
> > +   /**
> > +    * If true, lower nir_texop_txd on cube maps with
> > nir_texop_txl.
> > +    */
> > +   bool lower_txd_cube_map;
> >  } nir_lower_tex_options;
> >  
> >  bool nir_lower_tex(nir_shader *shader,
> > diff --git a/src/compiler/nir/nir_lower_tex.c
> > b/src/compiler/nir/nir_lower_tex.c
> > index ccca59b..da024e2 100644
> > --- a/src/compiler/nir/nir_lower_tex.c
> > +++ b/src/compiler/nir/nir_lower_tex.c
> > @@ -305,6 +305,265 @@ lower_yx_xuxv_external(nir_builder *b,
> > nir_tex_instr *tex)
> >  }
> >  
> >  static void
> > +get_ddx_ddy(nir_tex_instr *tex, nir_ssa_def **ddx, nir_ssa_def
> > **ddy)
> > +{
> > +   for (int i = 0; i < tex->num_srcs; i++) {
> > +      switch (tex->src[i].src_type) {
> > +      case nir_tex_src_ddx:
> > +         *ddx = tex->src[i].src.ssa;
> > +         break;
> > +      case nir_tex_src_ddy:
> > +         *ddy = tex->src[i].src.ssa;
> > +         break;
> > +      default:
> > +         break;
> > +      }
> > +   }
> > +}
> > +
> > +/*
> > + * Emits a textureLod operation used to replace an existing
> > + * textureGrad instruction.
> > + */
> > +static void
> > +replace_gradient_with_lod(nir_builder *b, nir_ssa_def *lod,
> > nir_tex_instr *tex)
> > +{
> > +   /* Check whether we need projector, shadow comparitor or offset
> > */
> > +   int comparitor_index = -1;
> > +   int projector_index = -1;
> > +   int offset_index = -1;
> > +   int extra_srcs = 0;
> > +   for (int i = 0; i < tex->num_srcs; i++) {
> > +      switch (tex->src[i].src_type) {
> > +      case nir_tex_src_projector:
> > +         projector_index = i;
> > +         extra_srcs++;
> > +         break;
> > +      case nir_tex_src_offset:
> > +         offset_index = i;
> > +         extra_srcs++;
> > +         break;
> > +      case nir_tex_src_comparitor:
> > +         comparitor_index = i;
> > +         extra_srcs++;
> > +         break;
> > +      default:
> > +         break;
> > +      }
> > +   }
> This looks correct now, but I liked Eric's suggestion from last time
> -
> you can actually just do:
> 
>    int comparitor_idx = nir_tex_instr_src_index(tex,
> nir_tex_src_comparitor);
>    int projector_idx = nir_tex_instr_src_index(tex,
> nir_tex_src_projector);
>    int offset_idx = nir_tex_instr_src_index(tex, nir_tex_src_offset);
> 
>    /* we replace ddx and ddy with a single value: lod */
>    int num_srcs = tex->num_srcs - 1;
> 
>    nir_ssa_def *dPdx =
>       tex->src[nir_tex_src_index(tex, nir_tex_src_ddx)].src.ssa;
>    nir_ssa_def *dPdy =
>       tex->src[nir_tex_src_index(tex, nir_tex_src_ddy)].src.ssa;
> 
> Either way, this series is:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> I don't need to see a third posting - feel free to fix up any last
> things before pushing.  Thanks for doing this!

I have pushed the series with two additional changes:

1. I added a 'continue' statement when the lowerings are successful
because the lowering replaces the original instruction, so it is not
safe to do anything else with it afterwards. This was not a problem
because the txd lowering was the last thing we do in the loop, but
prevents issues in the future if we add more texture lowerings after
the txd lowering.

2. Instead of tracking down the original projector, offset and
comparator parameters, I simply copy all the txd sources into our
texturelod (except ddx/ddy) and then add the lod at the end. This
simplifies a bit the code and makes sure we copy everything we might
possibly need.

Iago