[Mesa-dev] [PATCH] mesa: fix GetProgramiv/GetActiveAttrib regression

[Tapani Pälli]

Ping, could someone ack/review this? If not, I will revert the patches 
that caused this for now and figure out alternative solution with more 
time for packed varyings. It is one of the CTS tests that will fail with 
revert and nothing else gets hurt since it's a corner-case with SSO. I 
think currently any game using Unity engine is broken because of this issue.

// Tapani


On 09/28/2015 12:12 PM, Tapani Pälli wrote:
> Commit 4639cea2921669527eb43dcb49724c05afb27e8e added packed varyings
> as part of program resource list. We need to take this to account with
> all functions dealing with active input attributes.
>
> Patch fixes the issue by adding additional check to is_active_attrib
> and iterating active attribs explicitly in GetActiveAttrib function.
>
> I've tested that fix works for Assault Android Cactus (demo version)
> and does not cause Piglit or CTS regressions in glGetProgramiv tests.
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92122
> ---
>   src/mesa/main/shader_query.cpp | 44 +++++++++++++++++++++++++++++++++---------
>   1 file changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 99d9e10..d023504 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -101,15 +101,34 @@ _mesa_BindAttribLocation(GLhandleARB program, GLuint index,
>       */
>   }
>
> +/* Function checks if given input variable is packed varying by checking
> + * if there also exists output variable with the same name.
> + */
> +static bool
> +is_packed_varying(struct gl_shader_program *shProg, const ir_variable *input)
> +{
> +   assert(input->data.mode == ir_var_shader_in);
> +   unsigned array_index = 0;
> +   struct gl_program_resource *out =
> +      _mesa_program_resource_find_name(shProg, GL_PROGRAM_OUTPUT, input->name,
> +                                       &array_index);
> +   if (out)
> +      return true;
> +   return false;
> +}
> +
>   static bool
> -is_active_attrib(const ir_variable *var)
> +is_active_attrib(struct gl_shader_program *shProg,
> +                 const ir_variable *var)
>   {
>      if (!var)
>         return false;
>
>      switch (var->data.mode) {
>      case ir_var_shader_in:
> -      return var->data.location != -1;
> +      return (var->data.location != -1 &&
> +              var->data.location_frac == 0 &&
> +              !is_packed_varying(shProg, var));
>
>      case ir_var_system_value:
>         /* From GL 4.3 core spec, section 11.1.1 (Vertex Attributes):
> @@ -154,19 +173,25 @@ _mesa_GetActiveAttrib(GLhandleARB program, GLuint desired_index,
>         return;
>      }
>
> -   struct gl_program_resource *res =
> -      _mesa_program_resource_find_index(shProg, GL_PROGRAM_INPUT,
> -                                        desired_index);
> +   struct gl_program_resource *res = shProg->ProgramResourceList;
> +   unsigned index = -1;
> +   for (unsigned i = 0; i < shProg->NumProgramResourceList; i++, res++) {
> +      if (res->Type != GL_PROGRAM_INPUT ||
> +          !is_active_attrib(shProg, RESOURCE_VAR(res)))
> +         continue;
> +      if (++index == desired_index)
> +         break;
> +   }
>
>      /* User asked for index that does not exist. */
> -   if (!res) {
> +   if (!res || index != desired_index) {
>         _mesa_error(ctx, GL_INVALID_VALUE, "glGetActiveAttrib(index)");
>         return;
>      }
>
>      const ir_variable *const var = RESOURCE_VAR(res);
>
> -   if (!is_active_attrib(var))
> +   if (!is_active_attrib(shProg, var))
>         return;
>
>      const char *var_name = var->name;
> @@ -252,7 +277,7 @@ _mesa_count_active_attribs(struct gl_shader_program *shProg)
>      for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
>         if (res->Type == GL_PROGRAM_INPUT &&
>             res->StageReferences & (1 << MESA_SHADER_VERTEX) &&
> -          is_active_attrib(RESOURCE_VAR(res)))
> +          is_active_attrib(shProg, RESOURCE_VAR(res)))
>            count++;
>      }
>      return count;
> @@ -271,7 +296,8 @@ _mesa_longest_attribute_name_length(struct gl_shader_program *shProg)
>      size_t longest = 0;
>      for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
>         if (res->Type == GL_PROGRAM_INPUT &&
> -          res->StageReferences & (1 << MESA_SHADER_VERTEX)) {
> +          res->StageReferences & (1 << MESA_SHADER_VERTEX) &&
> +          is_active_attrib(shProg, RESOURCE_VAR(res))) {
>
>             const size_t length = strlen(RESOURCE_VAR(res)->name);
>             if (length >= longest)
>