[Mesa-dev] [PATCH 4/6] nir/lower_tex: add lowering for texture gradient on shadow samplers

[Iago Toral]

On Mon, 2016-12-05 at 20:39 -0800, Kenneth Graunke wrote:
> On Thursday, December 1, 2016 8:53:19 AM PST Iago Toral Quiroga
> wrote:
> > 
> > This is ported from the Intel lowering pass that we use with GLSL
> > IR.
> > This takes care of lowering texture gradients on shadow samplers
> > other
> > than cube maps. Intel hardware requires this for gen < 8.
> > ---
> >  src/compiler/nir/nir.h           |  7 +++++++
> >  src/compiler/nir/nir_lower_tex.c | 40
> > ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index ed388c6..d494d5f 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -2437,6 +2437,13 @@ typedef struct nir_lower_tex_options {
> >      * If true, lower nir_texop_txd on cube maps with
> > nir_texop_txl.
> >      */
> >     bool lower_txd_cube_map;
> > +
> > +   /**
> > +    * If true, lower nir_texop_txd on shadow samplers (except cube
> > maps)
> > +    * with nir_texop_txl. Notice that cube map shadow samplers are
> > lowered
> > +    * with lower_txd_cube_map.
> > +    */
> > +   bool lower_txd_shadow;
> >  } 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 c7c3db2..d5ea509 100644
> > --- a/src/compiler/nir/nir_lower_tex.c
> > +++ b/src/compiler/nir/nir_lower_tex.c
> > @@ -556,6 +556,40 @@ lower_gradient_cube_map(nir_builder *b,
> > nir_tex_instr *tex)
> >  }
> >  
> >  static void
> > +lower_gradient_shadow(nir_builder *b, nir_tex_instr *tex)
> > +{
> > +   assert(tex->sampler_dim != GLSL_SAMPLER_DIM_CUBE);
> > +   assert(tex->is_shadow);
> > +   assert(tex->op == nir_texop_txd);
> > +   assert(tex->dest.is_ssa);
> > +
> > +   /* Use textureSize() to get the width and height of LOD 0 */
> > +   nir_ssa_def *size = get_texture_size(b, tex);
> > +
> > +   /* Scale the gradients by width and height.  Effectively, the
> > incoming
> > +    * gradients are s'(x,y), t'(x,y), and r'(x,y) from equation
> > 3.19 in the
> > +    * GL 3.0 spec; we want u'(x,y), which is w_t * s'(x,y).
> > +    */
> > +   nir_ssa_def *dPdx = nir_fmul(b, tex->src[2].src.ssa, size);
> > +   nir_ssa_def *dPdy = nir_fmul(b, tex->src[3].src.ssa, size);
> NIR texture sources aren't guaranteed to be in any particular order
> (it's kind of annoying)...you should instead do:
> 
>     nir_ssa_def *dPdx;
>     nir_ssa_def *dPdy;
> 
>     for (unsigned i = 0; i < tex->num_srcs; i++) {
>        switch (tex->src[i].src_type) {
>        case nir_tex_src_ddx:
>           dPdx = nir_fmul(b, tex->src[i].src.ssa, size);
>           break;
>        case nir_tex_src_ddy:
>           dPdy = nir_fmul(b, tex->src[i].src.ssa, size);
>           break;
>        default:
>           break;
>        }
>     }
> 
> Same comment on patch two.  Maybe make a get_ddx_ddy() helper?

Oh right, I forgot about this here and then copy pasted this into the
other patch. Yep, a helper seems like a good idea, I'll do that.

> > 
> > +
> > +   nir_ssa_def *rho;
> > +   if (dPdx->num_components == 1) {
> > +      rho = nir_fmax(b, nir_fabs(b, dPdx), nir_fabs(b, dPdy));
> > +   } else {
> > +      rho = nir_fmax(b,
> > +                     nir_fsqrt(b, nir_fdot(b, dPdx, dPdx)),
> > +                     nir_fsqrt(b, nir_fdot(b, dPdy, dPdy)));
> > +   }
> > +
> > +   /* lod = log2(rho).  We're ignoring GL state biases for now. */
> > +   nir_ssa_def *lod = nir_flog2(b, rho);
> > +
> > +   /* Replace the gradient instruction with an equivalent lod
> > instruction */
> > +   replace_gradient_with_lod(b, lod, tex);
> > +}
> > +
> > +static void
> >  saturate_src(nir_builder *b, nir_tex_instr *tex, unsigned
> > sat_mask)
> >  {
> >     b->cursor = nir_before_instr(&tex->instr);
> > @@ -786,6 +820,12 @@ nir_lower_tex_block(nir_block *block,
> > nir_builder *b,
> >           lower_gradient_cube_map(b, tex);
> >           progress = true;
> >        }
> > +
> > +      if (tex->op == nir_texop_txd && options->lower_txd_shadow &&
> > +          tex->is_shadow && tex->sampler_dim !=
> > GLSL_SAMPLER_DIM_CUBE) {
> > +         lower_gradient_shadow(b, tex);
> > +         progress = true;
> > +      }
> This seems a bit odd to me - if a driver sets options-
> >lower_txd_shadow
> but not options->lower_txd_cube_map, then
> textureGrad(samplerCubeShadow, ...)
> would not be lowered.  I would sort of expect that case to be lowered
> if
> either option is set.
> 
> I suppose it's not a problem in practice, as we set both, but it
> might
> be nice to fix this anyway.

Yep, that is a bit confusing. I'll change the patch to also lower
shadow cube samplers is lower_txd_shadow is set.

> With the ddx/ddy source finding fixed in both patches, the series is:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>


Thanks!

> > 
> >     }
> >  
> >     return progress;
> >