[Mesa-dev] [PATCH 3/3] gallivm: honor explicit derivatives values for cube maps.

Jose Fonseca jfonseca at vmware.com
Wed Apr 3 07:08:25 PDT 2013


I only had time to skim through your changes but the series looks great, Roland.

If it's not too much trouble, could you please add a piglit test for explicit cube derivatives? Also, please confirm there are no piglit regressions.

Jose

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> This is trivial now, though need to make sure we pass all the necessary
> derivative values (which is 3 each for ddx/ddy not 2).
> Untested (no piglit test) however since the transform works the same
> as implicit derivatives this should probably work correctly.
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_sample.c     |   10 ++--
>  src/gallium/auxiliary/gallivm/lp_bld_sample.h     |    1 +
>  src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |    2 +-
>  src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c   |   66
>  ++++++++++++++-------
>  4 files changed, 52 insertions(+), 27 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample.c
> b/src/gallium/auxiliary/gallivm/lp_bld_sample.c
> index 5d50921..cc04a70 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample.c
> @@ -1287,6 +1287,7 @@ lp_build_cube_lookup(struct lp_build_sample_context
> *bld,
>                       LLVMValueRef s,
>                       LLVMValueRef t,
>                       LLVMValueRef r,
> +                     const struct lp_derivatives *derivs, /* optional */
>                       LLVMValueRef *face,
>                       LLVMValueRef *face_s,
>                       LLVMValueRef *face_t,
> @@ -1296,7 +1297,6 @@ lp_build_cube_lookup(struct lp_build_sample_context
> *bld,
>     LLVMBuilderRef builder = bld->gallivm->builder;
>     struct gallivm_state *gallivm = bld->gallivm;
>     LLVMValueRef si, ti, ri;
> -   boolean implicit_derivs = TRUE;
>     boolean need_derivs = TRUE;
>  
>     if (1 || coord_bld->type.length > 4) {
> @@ -1334,9 +1334,9 @@ lp_build_cube_lookup(struct lp_build_sample_context
> *bld,
>        assert(PIPE_TEX_FACE_NEG_Z == PIPE_TEX_FACE_POS_Z + 1);
>  
>        /*
> -       * TODO do this only when needed, and implement explicit derivs
> (trivial).
> +       * TODO do this only when needed.
>         */
> -      if (need_derivs && implicit_derivs) {
> +      if (need_derivs && !derivs) {
>           LLVMValueRef ddx_ddy[2], tmp[2];
>           /*
>            * This isn't quite the same as the "ordinary" path since
> @@ -1374,9 +1374,9 @@ lp_build_cube_lookup(struct lp_build_sample_context
> *bld,
>           dmax[2] = lp_build_max(coord_bld, tmp[0], tmp[1]);
>        }
>        else if (need_derivs) {
> -         /* dmax[0] = lp_build_max(coord_bld, derivs->ddx[0],
> derivs->ddy[0]);
> +         dmax[0] = lp_build_max(coord_bld, derivs->ddx[0], derivs->ddy[0]);
>           dmax[1] = lp_build_max(coord_bld, derivs->ddx[1], derivs->ddy[1]);
> -         dmax[2] = lp_build_max(coord_bld, derivs->ddx[2], derivs->ddy[2]);
> */
> +         dmax[2] = lp_build_max(coord_bld, derivs->ddx[2], derivs->ddy[2]);
>        }
>  
>        si = LLVMBuildBitCast(builder, s, lp_build_vec_type(gallivm,
>        intctype), "");
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample.h
> b/src/gallium/auxiliary/gallivm/lp_bld_sample.h
> index 5026b0a..72af813 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample.h
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample.h
> @@ -433,6 +433,7 @@ lp_build_cube_lookup(struct lp_build_sample_context *bld,
>                       LLVMValueRef s,
>                       LLVMValueRef t,
>                       LLVMValueRef r,
> +                     const struct lp_derivatives *derivs, /* optional */
>                       LLVMValueRef *face,
>                       LLVMValueRef *face_s,
>                       LLVMValueRef *face_t,
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index 3b950ea..d2cc0f3 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -1102,7 +1102,7 @@ lp_build_sample_common(struct lp_build_sample_context
> *bld,
>      */
>     if (target == PIPE_TEXTURE_CUBE) {
>        LLVMValueRef face, face_s, face_t;
> -      lp_build_cube_lookup(bld, *s, *t, *r, &face, &face_s, &face_t,
> &cube_rho);
> +      lp_build_cube_lookup(bld, *s, *t, *r, derivs, &face, &face_s, &face_t,
> &cube_rho);
>        *s = face_s; /* vec */
>        *t = face_t; /* vec */
>        /* use 'r' to indicate cube face */
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> index facfc82..007e3c9 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> @@ -1276,8 +1276,7 @@ emit_tex( struct lp_build_tgsi_soa_context *bld,
>     LLVMValueRef offsets[3] = { NULL };
>     struct lp_derivatives derivs;
>     struct lp_derivatives *deriv_ptr = NULL;
> -   unsigned num_coords;
> -   unsigned dims;
> +   unsigned num_coords, num_derivs, num_offsets;
>     unsigned i;
>  
>     if (!bld->sampler) {
> @@ -1291,37 +1290,52 @@ emit_tex( struct lp_build_tgsi_soa_context *bld,
>     switch (inst->Texture.Texture) {
>     case TGSI_TEXTURE_1D:
>        num_coords = 1;
> -      dims = 1;
> +      num_offsets = 1;
> +      num_derivs = 1;
>        break;
>     case TGSI_TEXTURE_1D_ARRAY:
>        num_coords = 2;
> -      dims = 1;
> +      num_offsets = 1;
> +      num_derivs = 1;
>        break;
>     case TGSI_TEXTURE_2D:
>     case TGSI_TEXTURE_RECT:
>        num_coords = 2;
> -      dims = 2;
> +      num_offsets = 2;
> +      num_derivs = 2;
>        break;
>     case TGSI_TEXTURE_SHADOW1D:
>     case TGSI_TEXTURE_SHADOW1D_ARRAY:
>        num_coords = 3;
> -      dims = 1;
> +      num_offsets = 1;
> +      num_derivs = 1;
>        break;
>     case TGSI_TEXTURE_SHADOW2D:
>     case TGSI_TEXTURE_SHADOWRECT:
>     case TGSI_TEXTURE_2D_ARRAY:
> +      num_coords = 3;
> +      num_offsets = 2;
> +      num_derivs = 2;
> +      break;
>     case TGSI_TEXTURE_CUBE:
>        num_coords = 3;
> -      dims = 2;
> +      num_offsets = 2;
> +      num_derivs = 3;
>        break;
>     case TGSI_TEXTURE_3D:
>        num_coords = 3;
> -      dims = 3;
> +      num_offsets = 3;
> +      num_derivs = 3;
>        break;
>     case TGSI_TEXTURE_SHADOW2D_ARRAY:
> +      num_coords = 4;
> +      num_offsets = 2;
> +      num_derivs = 2;
> +      break;
>     case TGSI_TEXTURE_SHADOWCUBE:
>        num_coords = 4;
> -      dims = 2;
> +      num_offsets = 2;
> +      num_derivs = 3;
>        break;
>     default:
>        assert(0);
> @@ -1361,20 +1375,20 @@ emit_tex( struct lp_build_tgsi_soa_context *bld,
>  
>     if (modifier == LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV) {
>        unsigned dim;
> -      for (dim = 0; dim < dims; ++dim) {
> +      for (dim = 0; dim < num_derivs; ++dim) {
>           derivs.ddx[dim] = lp_build_emit_fetch( &bld->bld_base, inst, 1, dim
>           );
>           derivs.ddy[dim] = lp_build_emit_fetch( &bld->bld_base, inst, 2, dim
>           );
>        }
>        deriv_ptr = &derivs;
>        unit = inst->Src[3].Register.Index;
> -   }  else {
> +   } else {
>        unit = inst->Src[1].Register.Index;
>     }
>  
>     /* some advanced gather instructions (txgo) would require 4 offsets */
>     if (inst->Texture.NumOffsets == 1) {
>        unsigned dim;
> -      for (dim = 0; dim < dims; dim++) {
> +      for (dim = 0; dim < num_offsets; dim++) {
>           offsets[dim] = lp_build_emit_fetch_texoffset(&bld->bld_base, inst,
>           0, dim );
>        }
>     }
> @@ -1405,7 +1419,7 @@ emit_sample(struct lp_build_tgsi_soa_context *bld,
>     LLVMValueRef offsets[3] = { NULL };
>     struct lp_derivatives derivs;
>     struct lp_derivatives *deriv_ptr = NULL;
> -   unsigned num_coords, dims;
> +   unsigned num_coords, num_offsets, num_derivs;
>     unsigned i;
>  
>     if (!bld->sampler) {
> @@ -1431,29 +1445,39 @@ emit_sample(struct lp_build_tgsi_soa_context *bld,
>     switch (bld->sv[texture_unit].Resource) {
>     case TGSI_TEXTURE_1D:
>        num_coords = 1;
> -      dims = 1;
> +      num_offsets = 1;
> +      num_derivs = 1;
>        break;
>     case TGSI_TEXTURE_1D_ARRAY:
>        num_coords = 2;
> -      dims = 1;
> +      num_offsets = 1;
> +      num_derivs = 1;
>        break;
>     case TGSI_TEXTURE_2D:
>     case TGSI_TEXTURE_RECT:
>        num_coords = 2;
> -      dims = 2;
> +      num_offsets = 2;
> +      num_derivs = 2;
>        break;
>     case TGSI_TEXTURE_2D_ARRAY:
> +      num_coords = 3;
> +      num_offsets = 2;
> +      num_derivs = 2;
> +      break;
>     case TGSI_TEXTURE_CUBE:
>        num_coords = 3;
> -      dims = 2;
> +      num_offsets = 2;
> +      num_derivs = 3;
>        break;
>     case TGSI_TEXTURE_3D:
>        num_coords = 3;
> -      dims = 3;
> +      num_offsets = 3;
> +      num_derivs = 3;
>        break;
>     case TGSI_TEXTURE_CUBE_ARRAY:
>        num_coords = 4;
> -      dims = 3;
> +      num_offsets = 2;
> +      num_derivs = 3;
>        break;
>     default:
>        assert(0);
> @@ -1504,7 +1528,7 @@ emit_sample(struct lp_build_tgsi_soa_context *bld,
>  
>     if (modifier == LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV) {
>        unsigned dim;
> -      for (dim = 0; dim < dims; ++dim) {
> +      for (dim = 0; dim < num_derivs; ++dim) {
>           derivs.ddx[dim] = lp_build_emit_fetch( &bld->bld_base, inst, 3, dim
>           );
>           derivs.ddy[dim] = lp_build_emit_fetch( &bld->bld_base, inst, 4, dim
>           );
>        }
> @@ -1514,7 +1538,7 @@ emit_sample(struct lp_build_tgsi_soa_context *bld,
>     /* some advanced gather instructions (txgo) would require 4 offsets */
>     if (inst->Texture.NumOffsets == 1) {
>        unsigned dim;
> -      for (dim = 0; dim < dims; dim++) {
> +      for (dim = 0; dim < num_offsets; dim++) {
>           offsets[dim] = lp_build_emit_fetch_texoffset(&bld->bld_base, inst,
>           0, dim );
>        }
>     }
> --
> 1.7.9.5
> 


More information about the mesa-dev mailing list