[Mesa-dev] [PATCH 1/2] softpipe: clean up lod computation

Brian Paul brianp at vmware.com
Fri Feb 8 07:22:28 PST 2013


On 02/06/2013 10:59 AM, sroland at vmware.com wrote:
> From: Roland Scheidegger<sroland at vmware.com>
>
> This should handle the new lod_zero modifier more correctly.
> The runtime-conditional is a bit more complex however we now also do
> scalar lod computation when appropriate which should more than make up for it.
> The refactoring should also fix an issue with explicit lods
> (lod clamp wasn't applied to them).
> Also, always pass lod as the 5th element from tgsi executor, which simplifies
> things (get rid of annoying conditionals later).
> ---
>   src/gallium/auxiliary/tgsi/tgsi_exec.c       |   64 ++++++------
>   src/gallium/auxiliary/tgsi/tgsi_exec.h       |    5 +-
>   src/gallium/drivers/softpipe/sp_tex_sample.c |  137 ++++++++++++++------------
>   3 files changed, 109 insertions(+), 97 deletions(-)
>
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> index 6d04cdc..2f5db82 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> @@ -1718,6 +1718,7 @@ fetch_texel( struct tgsi_sampler *sampler,
>      uint j;
>      float rgba[TGSI_NUM_CHANNELS][TGSI_QUAD_SIZE];
>
> +   /* FIXME: handle explicit derivs, offsets */
>      sampler->get_samples(sampler, s->f, t->f, p->f, c0->f, c1->f, control, rgba);
>
>      for (j = 0; j<  4; j++) {
> @@ -1750,9 +1751,11 @@ exec_tex(struct tgsi_exec_machine *mach,
>      const uint unit = inst->Src[sampler].Register.Index;
>      union tgsi_exec_channel r[4], cubearraycomp, cubelod;
>      const union tgsi_exec_channel *lod =&ZeroVec;
> -   enum tgsi_sampler_control control;
> +   enum tgsi_sampler_control control =  tgsi_sampler_lod_none;
>      uint chan;
>
> +   assert(modifier != TEX_MODIFIER_LEVEL_ZERO);
> +
>      if (modifier != TEX_MODIFIER_NONE&&  (sampler == 1)) {
>         FETCH(&r[3], 0, TGSI_CHAN_W);
>         if (modifier != TEX_MODIFIER_PROJECTED) {
> @@ -1762,7 +1765,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>
>      if (modifier == TEX_MODIFIER_EXPLICIT_LOD) {
>         control = tgsi_sampler_lod_explicit;
> -   } else {
> +   } else if (modifier == TEX_MODIFIER_LOD_BIAS){
>         control = tgsi_sampler_lod_bias;
>      }
>
> @@ -1775,7 +1778,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&ZeroVec,&ZeroVec, lod,&ZeroVec, /* S, T, P, LOD */
> +&r[0],&ZeroVec,&ZeroVec,&ZeroVec, lod, /* S, T, P, LOD */

I think the /* S, T, P, LOD */ comments should be updated while you're 
at it.  There's 5 arguments on that line, but the comment only has 4 
names.  That's been wrong for a while.


>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);     /* R, G, B, A */
>         break;
> @@ -1788,7 +1791,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&ZeroVec,&r[2], lod,&ZeroVec, /* S, T, P, LOD */
> +&r[0],&ZeroVec,&r[2],&ZeroVec, lod, /* S, T, P, LOD */
>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);     /* R, G, B, A */
>         break;
> @@ -1808,7 +1811,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&r[1],&r[2], lod,&ZeroVec,    /* S, T, P, LOD */
> +&r[0],&r[1],&r[2],&ZeroVec, lod,    /* S, T, P, LOD */
>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);  /* outputs */
>         break;
> @@ -1822,7 +1825,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&r[1],&ZeroVec, lod,&ZeroVec,    /* S, T, P, LOD */
> +&r[0],&r[1],&ZeroVec,&ZeroVec, lod,   /* S, T, P, LOD */
>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);  /* outputs */
>         break;
> @@ -1836,7 +1839,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&r[1],&r[2], lod,&ZeroVec,    /* S, T, P, LOD */
> +&r[0],&r[1],&r[2],&ZeroVec, lod,   /* S, T, P, LOD */
>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);  /* outputs */
>         break;
> @@ -1852,7 +1855,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&r[1],&r[2], lod,&ZeroVec,    /* S, T, P, LOD */
> +&r[0],&r[1],&r[2],&ZeroVec, lod,   /* S, T, P, LOD */
>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);  /* outputs */
>         break;
> @@ -1898,7 +1901,7 @@ exec_tex(struct tgsi_exec_machine *mach,
>         }
>
>         fetch_texel(mach->Samplers[unit],
> -&r[0],&r[1],&r[2], lod,&ZeroVec,
> +&r[0],&r[1],&r[2],&ZeroVec, lod,
>                     control,
>                     &r[0],&r[1],&r[2],&r[3]);
>         break;
> @@ -2127,31 +2130,28 @@ exec_sample(struct tgsi_exec_machine *mach,
>   {
>      const uint resource_unit = inst->Src[1].Register.Index;
>      const uint sampler_unit = inst->Src[2].Register.Index;
> -   union tgsi_exec_channel r[4];
> +   union tgsi_exec_channel r[4], c1;
>      const union tgsi_exec_channel *lod =&ZeroVec;
> -   enum tgsi_sampler_control control;
> +   enum tgsi_sampler_control control = tgsi_sampler_lod_none;
>      uint chan;
>
>      assert(modifier != TEX_MODIFIER_PROJECTED);
>
>      if (modifier != TEX_MODIFIER_NONE) {
>         if (modifier == TEX_MODIFIER_LOD_BIAS) {
> -         FETCH(&r[3], 3, TGSI_CHAN_X);
> -         lod =&r[3];
> +         FETCH(&c1, 3, TGSI_CHAN_X);
> +         lod =&c1;
> +         control = tgsi_sampler_lod_bias;
>         }
>         else if (modifier == TEX_MODIFIER_EXPLICIT_LOD) {
> -         FETCH(&r[3], 0, TGSI_CHAN_W);
> -         lod =&r[3];
> +         FETCH(&c1, 0, TGSI_CHAN_W);
> +         lod =&c1;
> +         control = tgsi_sampler_lod_explicit;
>         }
> -      else
> +      else {
>            assert(modifier == TEX_MODIFIER_LEVEL_ZERO);
> -   }
> -
> -   if (modifier == TEX_MODIFIER_EXPLICIT_LOD ||
> -       modifier == TEX_MODIFIER_LEVEL_ZERO) {
> -      control = tgsi_sampler_lod_explicit;
> -   } else {
> -      control = tgsi_sampler_lod_bias;
> +         control = tgsi_sampler_lod_zero;
> +      }
>      }
>
>      FETCH(&r[0], 0, TGSI_CHAN_X);
> @@ -2161,13 +2161,13 @@ exec_sample(struct tgsi_exec_machine *mach,
>         if (compare) {
>            FETCH(&r[2], 3, TGSI_CHAN_X);
>            fetch_texel(mach->Samplers[sampler_unit],
> -&r[0],&ZeroVec,&r[2], lod,&ZeroVec,  /* S, T, P, LOD */
> +&r[0],&ZeroVec,&r[2],&ZeroVec, lod, /* S, T, P, LOD */
>                        control,
>                        &r[0],&r[1],&r[2],&r[3]);     /* R, G, B, A */
>         }
>         else {
>            fetch_texel(mach->Samplers[sampler_unit],
> -&r[0],&ZeroVec,&ZeroVec, lod,&ZeroVec,  /* S, T, P, LOD */
> +&r[0],&ZeroVec,&ZeroVec,&ZeroVec, lod, /* S, T, P, LOD */
>                        control,
>                        &r[0],&r[1],&r[2],&r[3]);     /* R, G, B, A */
>         }
> @@ -2180,13 +2180,13 @@ exec_sample(struct tgsi_exec_machine *mach,
>         if (compare) {
>            FETCH(&r[2], 3, TGSI_CHAN_X);
>            fetch_texel(mach->Samplers[sampler_unit],
> -&r[0],&r[1],&r[2], lod,&ZeroVec,     /* S, T, P, LOD */
> +&r[0],&r[1],&r[2],&ZeroVec, lod,    /* S, T, P, LOD */
>                        control,
>                        &r[0],&r[1],&r[2],&r[3]);  /* outputs */
>         }
>         else {
>            fetch_texel(mach->Samplers[sampler_unit],
> -&r[0],&r[1],&ZeroVec, lod,&ZeroVec,     /* S, T, P, LOD */
> +&r[0],&r[1],&ZeroVec,&ZeroVec, lod,    /* S, T, P, LOD */
>                        control,
>                        &r[0],&r[1],&r[2],&r[3]);  /* outputs */
>         }
> @@ -2200,13 +2200,13 @@ exec_sample(struct tgsi_exec_machine *mach,
>         if(compare) {
>            FETCH(&r[3], 3, TGSI_CHAN_X);
>            fetch_texel(mach->Samplers[sampler_unit],
> -&r[0],&r[1],&r[2],&r[3],&ZeroVec,
> +&r[0],&r[1],&r[2],&r[3], lod,
>                        control,
>                        &r[0],&r[1],&r[2],&r[3]);
>         }
>         else {
>            fetch_texel(mach->Samplers[sampler_unit],
> -&r[0],&r[1],&r[2], lod,&ZeroVec,
> +&r[0],&r[1],&r[2],&ZeroVec, lod,
>                        control,
>                        &r[0],&r[1],&r[2],&r[3]);
>         }
> @@ -2217,12 +2217,6 @@ exec_sample(struct tgsi_exec_machine *mach,
>         FETCH(&r[2], 0, TGSI_CHAN_Z);
>         FETCH(&r[3], 0, TGSI_CHAN_W);
>         if(compare) {
> -         assert(modifier == TEX_MODIFIER_NONE);
> -         /*
> -          * FIXME: lod bias and explicit lod are prohibited but
> -          * for sample_c_lz we pass the level zero info as explicit
> -          * lod 0.
> -          */
>            FETCH(&r[4], 3, TGSI_CHAN_X);
>            fetch_texel(mach->Samplers[sampler_unit],
>                        &r[0],&r[1],&r[2],&r[3],&r[4],
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.h b/src/gallium/auxiliary/tgsi/tgsi_exec.h
> index fbd28a2..911432a 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.h
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.h
> @@ -89,8 +89,11 @@ struct tgsi_interp_coef
>   };
>
>   enum tgsi_sampler_control {
> +   tgsi_sampler_lod_none,
>      tgsi_sampler_lod_bias,
> -   tgsi_sampler_lod_explicit
> +   tgsi_sampler_lod_explicit,
> +   tgsi_sampler_lod_zero
> +   /* FIXME: tgsi_sampler_derivs_explicit */
>   };
>
>   /**
> diff --git a/src/gallium/drivers/softpipe/sp_tex_sample.c b/src/gallium/drivers/softpipe/sp_tex_sample.c
> index e9575da..60c1f92 100644
> --- a/src/gallium/drivers/softpipe/sp_tex_sample.c
> +++ b/src/gallium/drivers/softpipe/sp_tex_sample.c
> @@ -1647,15 +1647,77 @@ img_filter_3d_linear(struct tgsi_sampler *tgsi_sampler,
>    */
>   static INLINE void
>   compute_lod(const struct pipe_sampler_state *sampler,
> +            enum tgsi_sampler_control control,
>               const float biased_lambda,
> -            const float lodbias[TGSI_QUAD_SIZE],
> +            const float lod_in[TGSI_QUAD_SIZE],
>               float lod[TGSI_QUAD_SIZE])

Could you update the comments on that function to explain all the 
different lod parameters?  It's kind of confusing.


>   {
> +   float min_lod = sampler->min_lod;
> +   float max_lod = sampler->max_lod;
>      uint i;
>
> -   for (i = 0; i<  TGSI_QUAD_SIZE; i++) {
> -      lod[i] = biased_lambda + lodbias[i];
> -      lod[i] = CLAMP(lod[i], sampler->min_lod, sampler->max_lod);
> +   if (control == tgsi_sampler_lod_none ||
> +       control == tgsi_sampler_lod_zero) {
> +      float lod_scalar = CLAMP(biased_lambda, min_lod, max_lod);
> +      lod[0] = lod[1] = lod[2] = lod[3] = lod_scalar;
> +   }
> +   else if (control == tgsi_sampler_lod_bias) {
> +      for (i = 0; i<  TGSI_QUAD_SIZE; i++) {
> +         lod[i] = biased_lambda + lod_in[i];
> +         lod[i] = CLAMP(lod[i], min_lod, max_lod);
> +      }
> +   }
> +   else {
> +      assert(control == tgsi_sampler_lod_explicit);
> +      for (i = 0; i<  TGSI_QUAD_SIZE; i++) {
> +         lod[i] = CLAMP(lod_in[i], min_lod, max_lod);
> +      }
> +   }

Not a big deal, but I'd probably use a switch instead of if/else.


> +}
> +
> +
> +/* Calculate level of detail for every fragment.

What are the c0, c1 parameters?


> + */
> +static INLINE void
> +compute_lambda_lod(struct sp_sampler_variant *samp,
> +                   const float s[TGSI_QUAD_SIZE],
> +                   const float t[TGSI_QUAD_SIZE],
> +                   const float p[TGSI_QUAD_SIZE],
> +                   const float c0[TGSI_QUAD_SIZE],
> +                   const float c1[TGSI_QUAD_SIZE],
> +                   enum tgsi_sampler_control control,
> +                   float lod[TGSI_QUAD_SIZE])
> +{
> +   const struct pipe_sampler_state *sampler = samp->sampler;
> +   float lod_bias = sampler->lod_bias;
> +   float min_lod = sampler->min_lod;
> +   float max_lod = sampler->max_lod;
> +   uint i;
> +
> +   if (control == tgsi_sampler_lod_none) {
> +      float lambda = samp->compute_lambda(samp, s, t, p) + lod_bias;
> +      float lod_scalar = CLAMP(lambda, min_lod, max_lod);
> +      lod[0] = lod[1] = lod[2] = lod[3] = lod_scalar;
> +   }
> +   else if (control == tgsi_sampler_lod_bias) {
> +      float lambda = samp->compute_lambda(samp, s, t, p) + lod_bias;
> +
> +      for (i = 0; i<  TGSI_QUAD_SIZE; i++) {
> +         lod[i] = lambda + c1[i];
> +         lod[i] = CLAMP(lod[i], min_lod, max_lod);
> +      }
> +   }
> +   else if (control == tgsi_sampler_lod_zero) {
> +      /* this is all static state in the sampler really need clamp here? */
> +      float lod_scalar = CLAMP(lod_bias, min_lod, max_lod);
> +      lod[0] = lod[1] = lod[2] = lod[3] = lod_scalar;
> +   }
> +   else {
> +      assert(control == tgsi_sampler_lod_explicit);
> +
> +      for (i = 0; i<  TGSI_QUAD_SIZE; i++) {
> +         lod[i] = CLAMP(c1[i], min_lod, max_lod);
> +      }
>      }

Another switch?


>   }
>
> @@ -1675,21 +1737,7 @@ mip_filter_linear(struct tgsi_sampler *tgsi_sampler,
>      int j;
>      float lod[TGSI_QUAD_SIZE];
>
> -   if (control == tgsi_sampler_lod_bias) {
> -      float lambda = samp->compute_lambda(samp, s, t, p) + samp->sampler->lod_bias;
> -      if (samp->key.bits.target == PIPE_TEXTURE_CUBE_ARRAY)
> -         compute_lod(samp->sampler, lambda, c1, lod);
> -      else
> -         compute_lod(samp->sampler, lambda, c0, lod);
> -   } else {
> -      assert(control == tgsi_sampler_lod_explicit);
> -
> -      if (samp->key.bits.target == PIPE_TEXTURE_CUBE_ARRAY)
> -         memcpy(lod, c1, sizeof(lod));
> -      else
> -         memcpy(lod, c0, sizeof(lod));
> -
> -   }
> +   compute_lambda_lod(samp, s, t, p, c0, c1, control, lod);
>
>      for (j = 0; j<  TGSI_QUAD_SIZE; j++) {
>         int level0 = samp->view->u.tex.first_level + (int)lod[j];
> @@ -1744,20 +1792,7 @@ mip_filter_nearest(struct tgsi_sampler *tgsi_sampler,
>      float lod[TGSI_QUAD_SIZE];
>      int j;
>
> -   if (control == tgsi_sampler_lod_bias) {
> -      float lambda = samp->compute_lambda(samp, s, t, p) + samp->sampler->lod_bias;
> -      if (samp->key.bits.target == PIPE_TEXTURE_CUBE_ARRAY)
> -         compute_lod(samp->sampler, lambda, c1, lod);
> -      else
> -         compute_lod(samp->sampler, lambda, c0, lod);
> -   } else {
> -      assert(control == tgsi_sampler_lod_explicit);
> -
> -      if (samp->key.bits.target == PIPE_TEXTURE_CUBE_ARRAY)
> -         memcpy(lod, c1, sizeof(lod));
> -      else
> -         memcpy(lod, c0, sizeof(lod));
> -   }
> +   compute_lambda_lod(samp, s, t, p, c0, c1, control, lod);
>
>      for (j = 0; j<  TGSI_QUAD_SIZE; j++) {
>         if (lod[j]<  0.0)
> @@ -1791,20 +1826,7 @@ mip_filter_none(struct tgsi_sampler *tgsi_sampler,
>      float lod[TGSI_QUAD_SIZE];
>      int j;
>
> -   if (control == tgsi_sampler_lod_bias) {
> -      float lambda = samp->compute_lambda(samp, s, t, p) + samp->sampler->lod_bias;
> -      if (samp->key.bits.target == PIPE_TEXTURE_CUBE_ARRAY)
> -         compute_lod(samp->sampler, lambda, c1, lod);
> -      else
> -         compute_lod(samp->sampler, lambda, c0, lod);
> -   } else {
> -      assert(control == tgsi_sampler_lod_explicit);
> -
> -      if (samp->key.bits.target == PIPE_TEXTURE_CUBE_ARRAY)
> -         memcpy(lod, c1, sizeof(lod));
> -      else
> -         memcpy(lod, c0, sizeof(lod));
> -   }
> +   compute_lambda_lod(samp, s, t, p, c0, c1, control, lod);
>
>      for (j = 0; j<  TGSI_QUAD_SIZE; j++) {
>         if (lod[j]<  0.0) {
> @@ -2077,7 +2099,8 @@ mip_filter_linear_aniso(struct tgsi_sampler *tgsi_sampler,
>      float dvdx = (t[QUAD_BOTTOM_RIGHT] - t[QUAD_BOTTOM_LEFT]) * t_to_v;
>      float dvdy = (t[QUAD_TOP_LEFT]     - t[QUAD_BOTTOM_LEFT]) * t_to_v;
>
> -   if (control == tgsi_sampler_lod_bias) {
> +   if (control == tgsi_sampler_lod_bias ||
> +       control == tgsi_sampler_lod_none) {
>         /* note: instead of working with Px and Py, we will use the
>          * squared length instead, to avoid sqrt.
>          */
> @@ -2114,12 +2137,12 @@ mip_filter_linear_aniso(struct tgsi_sampler *tgsi_sampler,
>          * this since 0.5*log(x) = log(sqrt(x))
>          */
>         lambda = 0.5F * util_fast_log2(Pmin2) + samp->sampler->lod_bias;
> -      compute_lod(samp->sampler, lambda, c0, lod);
> +      compute_lod(samp->sampler, control, lambda, c1, lod);
>      }
>      else {
> -      assert(control == tgsi_sampler_lod_explicit);
> -
> -      memcpy(lod, c0, sizeof(lod));
> +      assert(control == tgsi_sampler_lod_explicit ||
> +             control == tgsi_sampler_lod_zero);
> +      compute_lod(samp->sampler, control, samp->sampler->lod_bias, c1, lod);
>      }
>
>      /* XXX: Take into account all lod values.
> @@ -2168,17 +2191,9 @@ mip_filter_linear_2d_linear_repeat_POT(
>      struct sp_sampler_variant *samp = sp_sampler_variant(tgsi_sampler);
>      const struct pipe_resource *texture = samp->view->texture;
>      int j;
> -   float lambda;
>      float lod[TGSI_QUAD_SIZE];
>
> -   if (control == tgsi_sampler_lod_bias) {
> -      lambda = samp->compute_lambda(samp, s, t, p) + samp->sampler->lod_bias;
> -      compute_lod(samp->sampler, lambda, c0, lod);
> -   } else {
> -      assert(control == tgsi_sampler_lod_explicit);
> -
> -      memcpy(lod, c0, sizeof(lod));
> -   }
> +   compute_lambda_lod(samp, s, t, p, c0, c1, control, lod);
>
>      for (j = 0; j<  TGSI_QUAD_SIZE; j++) {
>         int level0 = samp->view->u.tex.first_level + (int)lod[j];

Looks good otherwise!

Reviewed-by: Brian Paul <brianp at vmware.com>


More information about the mesa-dev mailing list