[Mesa-stable] [Mesa-dev] [PATCH 3/5] mesa: Fix add_index_to_name logic

[Timothy Arceri]

On Tue, 2016-05-31 at 11:52 -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> Our piglit tests for geometry and tessellation shader inputs were
> incorrect.  Array shader inputs and output should have '[0]' on the
> end
> regardless of stage.  In addtion, transform feedback varyings should
> not.

Is there a spec quote for this? It doesn't seem right to me since for
arrays of arrays would that mean we should end up with gs inputs like
this

input_name[0][0]
input_name[...][0]
input_name[num_vertices-1][0]

otherwise

in vec4 input1[];
and
in vec4 input2[][3];

Would both end up as:
input1[0]
input2[0]

> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: "12.0" <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/main/shader_query.cpp | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/src/mesa/main/shader_query.cpp
> b/src/mesa/main/shader_query.cpp
> index eec933c..f4b7243 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -696,20 +696,17 @@ _mesa_program_resource_find_index(struct
> gl_shader_program *shProg,
>  static bool
>  add_index_to_name(struct gl_program_resource *res)
>  {
> -   bool add_index = !((res->Type == GL_PROGRAM_INPUT &&
> -                       res->StageReferences & (1 <<
> MESA_SHADER_GEOMETRY |
> -                                               1 <<
> MESA_SHADER_TESS_CTRL |
> -                                               1 <<
> MESA_SHADER_TESS_EVAL)) ||
> -                      (res->Type == GL_PROGRAM_OUTPUT &&
> -                       res->StageReferences & 1 <<
> MESA_SHADER_TESS_CTRL));
> -
> -   /* Transform feedback varyings have array index already appended
> -    * in their names.
> -    */
> -   if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING)
> -      add_index = false;
> +   if (res->Type != GL_PROGRAM_INPUT && res->Type !=
> GL_PROGRAM_OUTPUT)
> +      return res->Type != GL_TRANSFORM_FEEDBACK_VARYING;

I'm slighlty confused by this. When does this return true? And for
transform feedback wont this always end up us false?

So isn't it just 

   if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING)
      return false;

> +
> +   const gl_shader_variable *const var = RESOURCE_VAR(res);
>  
> -   return add_index;
> +   assert(var->type->is_array());
> +
> +   if (var->interface_type != NULL && var->interface_type-
> >is_array())
> +      return var->type->fields.array->is_array();
> +

So I'm assuming your doing this since after lowering block[3].foo we
end up with foo[3]. However something like block[3].foo[2] will end up
as foo[3][2] and blocks can also be arrays of arrays so I'm not sure
this will work.


> +   return true;
>  }
>  
>  /* Get name length of a program resource. This consists of