[Mesa-dev] [PATCH 3/3] st/mesa: add texture gather support. (v2)

[Ilia Mirkin]

Just one (probably) trivial comment below:

On Mon, Feb 10, 2014 at 8:43 PM, Dave Airlie <airlied at gmail.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
>
> This adds support for GL_ARB_texture_gather, and one step of
> support for GL_ARB_gpu_shader5.
>
> This adds support for passing the TG4 instruction, along
> with non-constant texture offsets, and tracking them for the
> optimisation passes.
>
> This doesn't support native textureGatherOffsets hw, to do that
> you'd need to add a CAP and if set disable the lowering pass,
> and bump the MAX offsets to 4, then do the i0,j0 sampling using
> those.
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
>  src/mesa/state_tracker/st_extensions.c     |   4 +
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 117 ++++++++++++++++++++++-------
>  2 files changed, 94 insertions(+), 27 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 610fc68..0db770e 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -4079,7 +4128,7 @@ struct st_translate {
>     struct ureg_dst address[2];
>     struct ureg_src samplers[PIPE_MAX_SAMPLERS];
>     struct ureg_src systemValues[SYSTEM_VALUE_MAX];
> -
> +   struct tgsi_texture_offset tex_offsets[MAX_GLSL_TEXTURE_OFFSET];
>     unsigned array_sizes[MAX_ARRAYS];
>
>     const GLuint *inputMapping;
> @@ -4379,22 +4428,34 @@ translate_src(struct st_translate *t, const st_src_reg *src_reg)
>
>  static struct tgsi_texture_offset
>  translate_tex_offset(struct st_translate *t,
> -                     const struct tgsi_texture_offset *in_offset)
> +                     const st_src_reg *in_offset, int idx)

idx does not appear to get used... did you mean to use it, or
left-over code change?

>  {
>     struct tgsi_texture_offset offset;
>     struct ureg_src imm_src;
>
> -   assert(in_offset->File == PROGRAM_IMMEDIATE);
> -   imm_src = t->immediates[in_offset->Index];
> -
> -   offset.File = imm_src.File;
> -   offset.Index = imm_src.Index;
> -   offset.SwizzleX = imm_src.SwizzleX;
> -   offset.SwizzleY = imm_src.SwizzleY;
> -   offset.SwizzleZ = imm_src.SwizzleZ;
> -   offset.File = TGSI_FILE_IMMEDIATE;
> -   offset.Padding = 0;
> -
> +   switch (in_offset->file) {
> +   case PROGRAM_IMMEDIATE:
> +      imm_src = t->immediates[in_offset->index];
> +
> +      offset.File = imm_src.File;
> +      offset.Index = imm_src.Index;
> +      offset.SwizzleX = imm_src.SwizzleX;
> +      offset.SwizzleY = imm_src.SwizzleY;
> +      offset.SwizzleZ = imm_src.SwizzleZ;
> +      offset.Padding = 0;
> +      break;
> +   case PROGRAM_TEMPORARY:
> +      imm_src = ureg_src(t->temps[in_offset->index]);
> +      offset.File = imm_src.File;
> +      offset.Index = imm_src.Index;
> +      offset.SwizzleX = GET_SWZ(in_offset->swizzle, 0);
> +      offset.SwizzleY = GET_SWZ(in_offset->swizzle, 1);
> +      offset.SwizzleZ = GET_SWZ(in_offset->swizzle, 2);
> +      offset.Padding = 0;
> +      break;
> +   default:
> +      break;
> +   }
>     return offset;
>  }
>
> @@ -4450,9 +4511,10 @@ compile_tgsi_instruction(struct st_translate *t,
>     case TGSI_OPCODE_TEX2:
>     case TGSI_OPCODE_TXB2:
>     case TGSI_OPCODE_TXL2:
> +   case TGSI_OPCODE_TG4:
>        src[num_src++] = t->samplers[inst->sampler];
>        for (i = 0; i < inst->tex_offset_num_offset; i++) {
> -         texoffsets[i] = translate_tex_offset(t, &inst->tex_offsets[i]);
> +         texoffsets[i] = translate_tex_offset(t, &inst->tex_offsets[i], i);

and so this new parameter wouldn't be needed either.

  -ilia