[Mesa-dev] [PATCH v2] main: Remove interface block array index for doing the name comparison

[Tapani Pälli]

On 10/26/2015 12:45 PM, Samuel Iglesias Gonsálvez wrote:
>
>
> On 26/10/15 11:38, Samuel Iglesias Gonsalvez wrote:
>>  From ARB_program_query_interface spec:
>>
>> "uint GetProgramResourceIndex(uint program, enum programInterface,
>>                                     const char *name);
>>   [...]
>>   If <name> exactly matches the name string of one of the active resources
>>   for <programInterface>, the index of the matched resource is returned.
>>   Additionally, if <name> would exactly match the name string of an active
>>   resource if "[0]" were appended to <name>, the index of the matched
>>   resource is returned. [...]"
>>
>> "A string provided to GetProgramResourceLocation or
>>   GetProgramResourceLocationIndex is considered to match an active variable
>>   if:
>> [...]
>>     * if the string identifies the base name of an active array, where the
>>       string would exactly match the name of the variable if the suffix
>>       "[0]" were appended to the string;
>> [...]
>> "
>>
>> But this is only happening on those ARB_program_query_interface's queries.
>> For the rest of specs we need to keep old behavior. For that reason,
>> arb_program_interface_query boolean is added to the affected functions.
>>
>
> Forgot to remove this paragraph ^ from commit log. I will do it before
> pushing it to master or before sending a v3 (if needed).

Was just about to comment on this, no booleans required. The problem 
here is that find_name function was broken for names which contained the 
array brackets.

This patch (with changes to commit message) is

Reviewed-by: Tapani Pälli <tapani.palli at intel.com>



> Sam
>
>> Fixes the following two dEQP-GLES31 tests:
>>
>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array
>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element
>>
>> v2:
>> - Add AoA support (Timothy)
>> - Apply it too for GetUniformLocation(), GetUniformName() and others
>>    because ARB_program_interface_query says that they are equivalent
>>    to GetProgramResourceLocation() and GetProgramResourceName() (Tapani)
>>
>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>> Cc: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/shader_query.cpp | 43 +++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index b0707a4..a0514b0 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -543,8 +543,49 @@ _mesa_program_resource_find_name(struct gl_shader_program *shProg,
>>         /* Resource basename. */
>>         const char *rname = _mesa_program_resource_name(res);
>>         unsigned baselen = strlen(rname);
>> +      unsigned baselen_without_array_index = baselen;
>> +      const char *rname_last_square_bracket = strrchr(rname, '[');
>> +      bool found = false;
>> +      bool rname_has_array_index_zero = false;
>> +      /* From ARB_program_interface_query spec:
>> +       *
>> +       * "uint GetProgramResourceIndex(uint program, enum programInterface,
>> +       *                               const char *name);
>> +       *  [...]
>> +       *  If <name> exactly matches the name string of one of the active
>> +       *  resources for <programInterface>, the index of the matched resource is
>> +       *  returned. Additionally, if <name> would exactly match the name string
>> +       *  of an active resource if "[0]" were appended to <name>, the index of
>> +       *  the matched resource is returned. [...]"
>> +       *
>> +       * "A string provided to GetProgramResourceLocation or
>> +       * GetProgramResourceLocationIndex is considered to match an active variable
>> +       * if:
>> +       *
>> +       *  * the string exactly matches the name of the active variable;
>> +       *
>> +       *  * if the string identifies the base name of an active array, where the
>> +       *    string would exactly match the name of the variable if the suffix
>> +       *    "[0]" were appended to the string; [...]"
>> +       */
>> +      /* Remove array's index from interface block name comparison only if
>> +       * array's index is zero and the resulting string length is the same
>> +       * than the provided name's length.
>> +       */
>> +      if (rname_last_square_bracket) {
>> +         baselen_without_array_index -= strlen(rname_last_square_bracket);
>> +         rname_has_array_index_zero =
>> +            (strncmp(rname_last_square_bracket, "[0]\0", 4) == 0) &&
>> +            (baselen_without_array_index == strlen(name));
>> +      }
>> +
>> +      if (strncmp(rname, name, baselen) == 0)
>> +         found = true;
>> +      else if (rname_has_array_index_zero &&
>> +               strncmp(rname, name, baselen_without_array_index) == 0)
>> +         found = true;
>>
>> -      if (strncmp(rname, name, baselen) == 0) {
>> +      if (found) {
>>            switch (programInterface) {
>>            case GL_UNIFORM_BLOCK:
>>            case GL_SHADER_STORAGE_BLOCK:
>>