[Mesa-dev] [PATCH 2/2] glsl: add packed varyings to program resource list

[Tapani Pälli]

On 09/11/2015 08:48 AM, Ilia Mirkin wrote:
> On Fri, Sep 11, 2015 at 1:43 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> This makes sure that user is still able to query properties about
>> variables that have gotten packed by lower_packed_varyings pass.
>>
>> Fixes following OpenGL ES 3.1 test:
>>     ES31-CTS.program_interface_query.separate-programs-vertex
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/glsl/linker.cpp | 60 +++++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 56 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index 94f847e..13d27ac 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -3141,6 +3141,20 @@ build_stageref(struct gl_shader_program *shProg, const char *name,
>>            if (var) {
>>               unsigned baselen = strlen(var->name);
>>
>> +            /* If a variable is a packed varying, it has a name like
>> +             * 'packed:a,b,c' where a, b and c are separate variables.
>> +             * Here we check if our variable is part of packed varying
>> +             * list.
>> +             */
>> +            if (strncmp(var->name, "packed:", 7) == 0) {
>> +               const char *name_in_list = strstr(var->name, name);
>> +               const char *end = name_in_list + strlen(name);
>> +               if (name_in_list && (*end == '\0' || *end == ',')) {
>
> What if you have names
>
> ab,b
>
> and you query b. It will find the b in ab, and will pass the test above.

Right, seems like I need to check both for head and tail. Will fix this, 
probably deserves a little helper function.

>> +                  stages |= (1 << i);
>> +                  break;
>> +               }
>> +            }
>> +
>>               /* Type needs to match if specified, otherwise we might
>>                * pick a variable with same name but different interface.
>>                */
>> @@ -3166,9 +3180,9 @@ build_stageref(struct gl_shader_program *shProg, const char *name,
>>
>>   static bool
>>   add_interface_variables(struct gl_shader_program *shProg,
>> -                        struct gl_shader *sh, GLenum programInterface)
>> +                        exec_list *ir, GLenum programInterface)
>>   {
>> -   foreach_in_list(ir_instruction, node, sh->ir) {
>> +   foreach_in_list(ir_instruction, node, ir) {
>>         ir_variable *var = node->as_variable();
>>         uint8_t mask = 0;
>>
>> @@ -3203,6 +3217,12 @@ add_interface_variables(struct gl_shader_program *shProg,
>>            continue;
>>         };
>>
>> +      /* Skip packed varyings, packed varyings are handled separately
>> +       * by add_packed_varyings.
>> +       */
>> +      if (strncmp(var->name, "packed:", 7) == 0)
>> +         continue;
>> +
>>         if (!add_program_resource(shProg, programInterface, var,
>>                                   build_stageref(shProg, var->name,
>>                                                  var->data.mode) | mask))
>> @@ -3211,6 +3231,33 @@ add_interface_variables(struct gl_shader_program *shProg,
>>      return true;
>>   }
>>
>> +static bool
>> +add_packed_varyings(struct gl_shader_program *shProg, int stage)
>> +{
>> +   struct gl_shader *sh = shProg->_LinkedShaders[stage];
>> +   GLenum iface;
>> +
>> +   if (!sh || !sh->packed_varyings)
>> +      return true;
>> +
>> +   foreach_in_list(ir_instruction, node, sh->packed_varyings) {
>> +      ir_variable *var = node->as_variable();
>> +      if (var) {
>> +         switch (var->data.mode) {
>> +         case ir_var_shader_in:
>> +            iface = GL_PROGRAM_INPUT;
>> +         case ir_var_shader_out:
>> +            iface = GL_PROGRAM_OUTPUT;
>> +         }
>> +         if (!add_program_resource(shProg, iface, var,
>> +                                   build_stageref(shProg, var->name,
>> +                                                  var->data.mode)))
>> +            return false;
>> +      }
>> +   }
>> +   return true;
>> +}
>> +
>>   /**
>>    * Builds up a list of program resources that point to existing
>>    * resource data.
>> @@ -3243,12 +3290,17 @@ build_program_resource_list(struct gl_shader_program *shProg)
>>      if (input_stage == MESA_SHADER_STAGES && output_stage == 0)
>>         return;
>>
>> +   if (!add_packed_varyings(shProg, input_stage))
>> +      return;
>> +   if (!add_packed_varyings(shProg, output_stage))
>> +      return;
>> +
>>      /* Add inputs and outputs to the resource list. */
>> -   if (!add_interface_variables(shProg, shProg->_LinkedShaders[input_stage],
>> +   if (!add_interface_variables(shProg, shProg->_LinkedShaders[input_stage]->ir,
>>                                   GL_PROGRAM_INPUT))
>>         return;
>>
>> -   if (!add_interface_variables(shProg, shProg->_LinkedShaders[output_stage],
>> +   if (!add_interface_variables(shProg, shProg->_LinkedShaders[output_stage]->ir,
>>                                   GL_PROGRAM_OUTPUT))
>>         return;
>>
>> --
>> 2.4.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev