[Mesa-dev] [PATCH 1/2] tgsi: texture types

[Roland Scheidegger]

Am 08.06.2015 um 20:15 schrieb Rob Clark:
> From: Rob Clark <robclark at freedesktop.org>
> 
> Freedreno needs sampler type information to deal with int/uint textures.
> To accomplish this, start creating sampler-view declarations, as
> suggested here:
> 
>  http://lists.freedesktop.org/archives/mesa-dev/2014-November/071583.html
> 
> create a sampler-view with index matching the sampler, to encode the
> texture type (ie. SINT/UINT/FLOAT).  Ie:
> 
>    DCL SVIEW[n], 2D, UINT
>    DCL SAMP[n]
>    TEX OUT[1], IN[1], SAMP[n]
> 
> For tgsi texture instructions which do not take an explicit SVIEW
> argument, the SVIEW index is implied by the SAMP index.
I wonder if there should be some doc update somewhere.

> 
> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 0e60d95..ce8f2ea 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -239,6 +239,7 @@ public:
>     st_src_reg sampler; /**< sampler register */
>     int sampler_array_size; /**< 1-based size of sampler array, 1 if not array */
>     int tex_target; /**< One of TEXTURE_*_INDEX */
> +   glsl_base_type tex_type;
>     GLboolean tex_shadow;
>  
>     st_src_reg tex_offsets[MAX_GLSL_TEXTURE_OFFSET];
> @@ -345,6 +346,8 @@ public:
>  
>     int num_address_regs;
>     int samplers_used;
> +   glsl_base_type sampler_types[PIPE_MAX_SAMPLERS];
> +   int sampler_targets[PIPE_MAX_SAMPLERS];   /**< One of TGSI_TEXTURE_* */
>     bool indirect_addr_consts;
>     int wpos_transform_const;
>  
> @@ -3323,6 +3326,8 @@ glsl_to_tgsi_visitor::visit(ir_texture *ir)
>        assert(!"Should not get here.");
>     }
>  
> +   inst->tex_type = ir->type->base_type;
> +
>     this->result = result_src;
>  }
>  
> @@ -3471,6 +3476,11 @@ count_resources(glsl_to_tgsi_visitor *v, gl_program *prog)
>           for (int i = 0; i < inst->sampler_array_size; i++) {
>              v->samplers_used |= 1 << (inst->sampler.index + i);
>  
> +            debug_assert(i < (int)ARRAY_SIZE(v->sampler_types));
> +            v->sampler_types[i] = inst->tex_type;
> +            v->sampler_targets[i] =
> +               st_translate_texture_target(inst->tex_target, inst->tex_shadow);
> +
>              if (inst->tex_shadow) {
>                 prog->ShadowSamplers |= 1 << (inst->sampler.index + i);
>              }
> @@ -5529,7 +5539,26 @@ st_translate_program(
>     /* texture samplers */
>     for (i = 0; i < ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits; i++) {
>        if (program->samplers_used & (1 << i)) {
> +         unsigned type;
> +
>           t->samplers[i] = ureg_DECL_sampler(ureg, i);
> +
> +         switch (program->sampler_types[i]) {
> +         case GLSL_TYPE_INT:
> +            type = TGSI_RETURN_TYPE_SINT;
> +            break;
> +         case GLSL_TYPE_UINT:
> +            type = TGSI_RETURN_TYPE_UINT;
> +            break;
> +         case GLSL_TYPE_FLOAT:
> +            type = TGSI_RETURN_TYPE_FLOAT;
> +            break;
> +         default:
> +            unreachable("not reached");
> +         }
> +
> +         ureg_DECL_sampler_view( ureg, i, program->sampler_targets[i],
> +                                 type, type, type, type );
>        }
>     }
>  
> 

This indeed seems like a consistent solution. Hopefully drivers don't
assert when they see sampler view dcls...
I am also somewhat worried that this only changes the glsl-to-tgsi path,
not other paths. llvmpipe and draw rely on either having sview
declarations for all (sample) instructions or for none, and with draw
possibly inserting tex opcodes (for aalines etc.) on its own this could
probably break (these draw paths aren't used with d3d10 sample opcodes
as it's more or less illegal to use tex and sample opcodes in the same
shader, but that doesn't really matter here). I think using sview
declarations is the right thing to do but it probably should be illegal
then to NOT have them in some places.

Roland