[Mesa-dev] [PATCH 4/6] nir/lower_tex: support for lowering RECT textures

Fri Sep 18 11:07:39 PDT 2015

On Friday, September 18, 2015 12:36:21 PM Rob Clark wrote:
> On Fri, Sep 18, 2015 at 11:22 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > On Friday, September 18, 2015 10:55:07 AM Rob Clark wrote:
> >> From: Rob Clark <robclark at freedesktop.org>
> >>
> >> v2: comments/suggestions from Ilia and Eric, split out get_texture_size()
> >> helper so we can use it in the next commit for clamping RECT textures.
> >>
> >> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> >> ---
> >>  src/glsl/nir/nir.h           |  7 +++++
> >>  src/glsl/nir/nir_lower_tex.c | 62 +++++++++++++++++++++++++++++++++++++++++---
> >>  2 files changed, 66 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> >> index 4600fb0..3c908b9 100644
> >> --- a/src/glsl/nir/nir.h
> >> +++ b/src/glsl/nir/nir.h
> >> @@ -1843,6 +1843,13 @@ typedef struct nir_lower_tex_options {
> >>      * sampler types a texture projector is lowered.
> >>      */
> >>     unsigned lower_txp;
> >> +
> >> +   /**
> >> +    * If true, lower rect textures to 2D, using txs to fetch the
> >> +    * texture dimensions and dividing the texture coords by the
> >> +    * texture dims to normalize.
> >> +    */
> >> +   bool lower_rect;
> >>  } nir_lower_tex_options;
> >>
> >>  void nir_lower_tex(nir_shader *shader,
> >> diff --git a/src/glsl/nir/nir_lower_tex.c b/src/glsl/nir/nir_lower_tex.c
> >> index 281fc9f..a71a5c5 100644
> >> --- a/src/glsl/nir/nir_lower_tex.c
> >> +++ b/src/glsl/nir/nir_lower_tex.c
> >> @@ -22,9 +22,13 @@
> >>   */
> >>
> >>  /*
> >> - * This lowering pass converts the coordinate division for texture projection
> >> - * to be done in ALU instructions instead of asking the texture operation to
> >> - * do so.
> >> + * This lowering pass supports (as configured via nir_lower_tex_options)
> >> + * various texture related conversions:
> >> + *   + texture projector lowering: converts the coordinate division for
> >> + *     texture projection to be done in ALU instructions instead of
> >> + *     asking the texture operation to do so.
> >> + *   + lowering RECT: converts the un-normalized RECT texture coordinates
> >> + *     to normalized coordinates with txs plus ALU instructions
> >>   */
> >>
> >>  #include "nir.h"
> >> @@ -111,6 +115,55 @@ project_src(nir_builder *b, nir_tex_instr *tex)
> >>     tex->num_srcs--;
> >>  }
> >>
> >> +static nir_ssa_def *
> >> +get_texture_size(nir_builder *b, nir_tex_instr *tex)
> >> +{
> >> +   b->cursor = nir_before_instr(&tex->instr);
> >> +
> >> +   /* RECT textures should not be array: */
> >> +   assert(!tex->is_array);
> >> +
> >> +   nir_tex_instr *txs;
> >> +
> >> +   txs = nir_tex_instr_create(b->shader, 1);
> >> +   txs->op = nir_texop_txs;
> >> +   txs->sampler_dim = GLSL_SAMPLER_DIM_RECT;
> >> +   txs->sampler_index = tex->sampler_index;
> >> +
> >> +   /* only single src, the lod: */
> >> +   txs->src[0].src = nir_src_for_ssa(nir_imm_int(b, 0));
> >> +   txs->src[0].src_type = nir_tex_src_lod;
> >> +
> >> +   nir_ssa_dest_init(&txs->instr, &txs->dest, 3, NULL);
> >
> > Shouldn't this be 2? textureSize of 2DRect returns an ivec2...
> 
> yup
> 
> >> +   nir_builder_instr_insert(b, &txs->instr);
> >> +
> >> +   return nir_i2f(b, &txs->dest.ssa);
> >> +}
> >> +
> >> +static void
> >> +lower_rect(nir_builder *b, nir_tex_instr *tex)
> >> +{
> >> +   nir_ssa_def *txs = get_texture_size(b, tex);
> >> +
> >> +   nir_ssa_def *scalex = nir_frcp(b, nir_channel(b, txs, 0));
> >> +   nir_ssa_def *scaley = nir_frcp(b, nir_channel(b, txs, 1));
> >> +   nir_ssa_def *scale  = nir_vec2(b, scalex, scaley);
> >
> > Why not just do 1 / txs.xy directly?
> 
> I had assumed that frcp just operates on src.x (at least that it is
> how it is in tgsi and hw that I have seen).. if it operates on each
> component, then yes I could do it in one shot.  And I guess since txs
> is already a vec2, I could just:
> 
>   nir_ssa_def *scale = nir_frcp(b, txs);
> 

Good point, that's even nicer :)  frcp and other operations should
operate per component and support up vec1..vec4, so you should be able
to do this.

> > unsigned swizzle[4] = { SWIZZLE_X, SWIZZLE_Y, SWIZZLE_Y, SWIZZLE_Y };
> > nir_ssa_def *scale = nir_frcp(b, nir_swizzle(b, txs, swizzle, 2, false);
> >
> >> +
> >> +   /* Walk through the sources normalizing the requested arguments. */
> >> +   for (unsigned i = 0; i < tex->num_srcs; i++) {
> >> +      if (tex->src[i].src_type != nir_tex_src_coord)
> >> +         continue;
> >> +
> >> +      nir_ssa_def *coords =
> >> +         nir_ssa_for_src(b, tex->src[i].src, tex->coord_components);
> >> +      nir_instr_rewrite_src(&tex->instr,
> >> +                            &tex->src[i].src,
> >> +                            nir_src_for_ssa(nir_fmul(b, coords, scale)));
> >> +   }
> >> +
> >> +   tex->sampler_dim = GLSL_SAMPLER_DIM_2D;
> >> +}
> >> +
> >>  static bool
> >>  nir_lower_tex_block(nir_block *block, void *void_state)
> >>  {
> >> @@ -127,6 +180,9 @@ nir_lower_tex_block(nir_block *block, void *void_state)
> >>        if (lower_txp)
> >>           project_src(b, tex);
> >>
> >> +      if ((tex->sampler_dim == GLSL_SAMPLER_DIM_RECT) &&
> >
> > Please remove the bonus parens.
> 
> hmm, I'm the only one who *likes* bonus parens?  (Then again, I'd
> always liked lisp/scheme so maybe I have a paren fetish..)
> 
> BR,
> -R

No, Ian likes them too.  Ironically, my Lisp/Scheme/Racket background is
one of the reasons I dislike bonus parens...because in Scheme you have
to use exactly the right parens. :)  Not a big deal though, keep them if
you prefer.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150918/65e3fd8a/attachment-0001.sig>