[Mesa-dev] [PATCH] gallivm: already pass coords in the right place in the sampler interface

[Roland Scheidegger]

Am 15.08.2013 10:16, schrieb Michel Dänzer:
> On Mit, 2013-08-14 at 18:35 +0200, sroland at vmware.com wrote:
>> From: Roland Scheidegger <sroland at vmware.com>
>>
>> This makes things a bit nicer, and more importantly it fixes an issue
>> where a "downgraded" array texture (due to view reduced to 1 layer and
>> addressed with (non-array) samplec instruction) would use the wrong
>> coord as shadow reference value. (This could also be fixed by passing
>> target through the sampler interface much the same way as is done for
>> size queries, might do this eventually anyway.)
>> And if we'd ever want to support (shadow) cube map arrays, we'd need
>> 5 coords in any case.
>>
>> v2: fix bugs (texel fetch using wrong layer coord for 1d, shadow tex
>> using wrong shadow coord for 2d...). Plus need to project the shadow
>> coord, and just for fun keep projecting the layer coord too.
> 
> [...]
> 
>> @@ -1631,55 +1632,58 @@ emit_tex( struct lp_build_tgsi_soa_context *bld,
>>     }
>>  
>>     switch (inst->Texture.Texture) {
>> -   case TGSI_TEXTURE_1D:
>> -      num_coords = 1;
>> -      num_offsets = 1;
>> -      num_derivs = 1;
>> -      break;
>>     case TGSI_TEXTURE_1D_ARRAY:
>> -      num_coords = 2;
>> +      layer_coord = 1;
>> +      /* fallthrough */
>> +   case TGSI_TEXTURE_1D:
>>        num_offsets = 1;
>>        num_derivs = 1;
>>        break;
>> +   case TGSI_TEXTURE_2D_ARRAY:
>> +      layer_coord = 2;
>> +      /* fallthrough */
>>     case TGSI_TEXTURE_2D:
>>     case TGSI_TEXTURE_RECT:
>> -      num_coords = 2;
>>        num_offsets = 2;
>>        num_derivs = 2;
>>        break;
>> -   case TGSI_TEXTURE_SHADOW1D:
>>     case TGSI_TEXTURE_SHADOW1D_ARRAY:
>> -      num_coords = 3;
>> +      layer_coord = 1;
>> +      /* fallthrough */
>> +   case TGSI_TEXTURE_SHADOW1D:
>> +      shadow_coord = 2;
>>        num_offsets = 1;
>>        num_derivs = 1;
>>        break;
>> +   case TGSI_TEXTURE_SHADOW2D_ARRAY:
>> +      layer_coord = 2;
>> +      shadow_coord = 3;
>> +      num_offsets = 2;
>> +      num_derivs = 2;
>> +      break;
>>     case TGSI_TEXTURE_SHADOW2D:
>>     case TGSI_TEXTURE_SHADOWRECT:
>> -   case TGSI_TEXTURE_2D_ARRAY:
>> -      num_coords = 3;
>> +      shadow_coord = 2;
>>        num_offsets = 2;
>>        num_derivs = 2;
>>        break;
>>     case TGSI_TEXTURE_CUBE:
>> -      num_coords = 3;
>>        num_offsets = 2;
>>        num_derivs = 3;
>>        break;
>>     case TGSI_TEXTURE_3D:
>> -      num_coords = 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;
>> +      shadow_coord = 3;
>>        num_offsets = 2;
>>        num_derivs = 3;
>>        break;
>> +   case TGSI_TEXTURE_CUBE_ARRAY:
>> +   case TGSI_TEXTURE_SHADOWCUBE_ARRAY:
>> +   case TGSI_TEXTURE_2D_MSAA:
>> +   case TGSI_TEXTURE_2D_ARRAY_MSAA:
>>     default:
>>        assert(0);
>>        return;
> 
> Couldn't some of this code be simplified using
> tgsi_util_get_texture_coord_dim()?
> 
> 

Hmm yes this code is a bunch of crap but whenever I try to simplify it
it just gets more complicated.
Maybe it was a mistake to adjust layer coord position here, could do
that in the sampling code itself, which would mean
tgsi_util_get_texture_coord_dim() would fit quite well. The different
handling of derivs/offsets though sort of looks required to not fetch
these components for explicit derivatives, texel offsets as for instance
cube maps have 3 coords but only 2 offsets etc. Though I wonder if it's
possible that these can end up as invalid pointers, I guess not so could
probably stop caring about them and just fetch more components than
potentially necessary (after all llvm will immediately drop them anyway
as they won't get used). That definitely would clean things up.

Roland