[Mesa-dev] [PATCH] gallivm: fix texel fetch for array textures

[Jose Fonseca]

Looks good AFAICT.

Maybe we should add a test for this in piglit?

Jose

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Since we don't call lp_build_sample_common() in the texel fetch path
> we missed
> the layer fixup code. If someone would have tried to do texelFetch
> with array
> textures it would have crashed for sure.
> Not really tested (no overlap of texelFetch and array texture tests
> in piglit).
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c |   55
>  ++++++++++++++-------
>  1 file changed, 38 insertions(+), 17 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> index ba265b2..701e8c1 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
> @@ -973,6 +973,28 @@ lp_build_sample_mipmap(struct
> lp_build_sample_context *bld,
>     }
>  }
>  
> +
> +/**
> + * Clamp layer coord to valid values.
> + */
> +static LLVMValueRef
> +lp_build_layer_coord(struct lp_build_sample_context *bld,
> +                     unsigned unit,
> +                     LLVMValueRef layer)
> +{
> +   LLVMValueRef maxlayer;
> +
> +   layer = lp_build_iround(&bld->coord_bld, layer);
> +   maxlayer = bld->dynamic_state->depth(bld->dynamic_state,
> +                                        bld->gallivm, unit);
> +   maxlayer = lp_build_sub(&bld->int_bld, maxlayer,
> bld->int_bld.one);
> +   maxlayer = lp_build_broadcast_scalar(&bld->int_coord_bld,
> maxlayer);
> +   return lp_build_clamp(&bld->int_coord_bld, layer,
> +                         bld->int_coord_bld.zero, maxlayer);
> +
> +}
> +
> +
>  /**
>   * Calculate cube face, lod, mip levels.
>   */
> @@ -1018,23 +1040,11 @@ lp_build_sample_common(struct
> lp_build_sample_context *bld,
>        face_derivs.ddx_ddy[1] = NULL;
>        derivs = &face_derivs;
>     }
> -   else if (target == PIPE_TEXTURE_1D_ARRAY ||
> -            target == PIPE_TEXTURE_2D_ARRAY) {
> -      LLVMValueRef layer, maxlayer;
> -
> -      if (target == PIPE_TEXTURE_1D_ARRAY) {
> -         layer = *t;
> -      }
> -      else {
> -         layer = *r;
> -      }
> -      layer = lp_build_iround(&bld->coord_bld, layer);
> -      maxlayer = bld->dynamic_state->depth(bld->dynamic_state,
> -                                           bld->gallivm, unit);
> -      maxlayer = lp_build_sub(&bld->int_bld, maxlayer,
> bld->int_bld.one);
> -      maxlayer = lp_build_broadcast_scalar(&bld->int_coord_bld,
> maxlayer);
> -      *r = lp_build_clamp(&bld->int_coord_bld, layer,
> -                          bld->int_coord_bld.zero, maxlayer);
> +   else if (target == PIPE_TEXTURE_1D_ARRAY) {
> +      *r = lp_build_layer_coord(bld, unit, *t);
> +   }
> +   else if (target == PIPE_TEXTURE_2D_ARRAY) {
> +      *r = lp_build_layer_coord(bld, unit, *r);
>     }
>  
>     /*
> @@ -1205,6 +1215,7 @@ lp_build_fetch_texel(struct
> lp_build_sample_context *bld,
>     struct lp_build_context *perquadi_bld = &bld->perquadi_bld;
>     struct lp_build_context *int_coord_bld = &bld->int_coord_bld;
>     unsigned dims = bld->dims, chan;
> +   unsigned target = bld->static_state->target;
>     LLVMValueRef size, ilevel;
>     LLVMValueRef row_stride_vec = NULL, img_stride_vec = NULL;
>     LLVMValueRef x = coords[0], y = coords[1], z = coords[2];
> @@ -1227,6 +1238,16 @@ lp_build_fetch_texel(struct
> lp_build_sample_context *bld,
>     lp_build_extract_image_sizes(bld, &bld->int_size_bld,
>     int_coord_bld->type,
>                                  size, &width, &height, &depth);
>  
> +   if (target == PIPE_TEXTURE_1D_ARRAY ||
> +       target == PIPE_TEXTURE_2D_ARRAY) {
> +      if (target == PIPE_TEXTURE_1D_ARRAY) {
> +         z = lp_build_layer_coord(bld, unit, y);
> +      }
> +      else {
> +         z = lp_build_layer_coord(bld, unit, z);
> +      }
> +   }
> +
>     /* This is a lot like border sampling */
>     if (offsets[0]) {
>        /* XXX coords are really unsigned, offsets are signed */
> --
> 1.7.9.5
> 
>