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

[Roland Scheidegger]

I was wrong that there is no test for this in piglit.
spec/glsl-1.30/execution/fs-texelFetch-2D.c (and the offset version)
only test 2d samplers, but texturing/shaders/texelFetch.c can test all
samplers, in both fs and vs.
It currently crashes though with llvmpipe (using glsl and GL version
overrides) due to integer texture formats being used, so I think adding
another test is not necessary and we can just use this one later.

Roland


Am 12.12.2012 21:46, schrieb 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
>>
>>