[Mesa-dev] [PATCH] main: fix basename match's check if it's an array or struct

[Tapani Pälli]

On 10/28/2015 09:09 AM, Samuel Iglesias Gonsálvez wrote:
>
> On 28/10/15 06:53, Tapani Pälli wrote:
>> On 10/27/2015 04:04 PM, Samuel Iglesias Gonsalvez wrote:
>>> Commit 4565b6f did not update the basename match's check for
>>> the case that string would exactly match the name of the
>>> variable if the suffix "[0]" were appended to it.
>>>
>>> Fixes 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
>>>
>> These tests are passing already with commit 4565b6f right? I think the
>> 'found' boolean already takes care of this, I need to step through again
>> to make sure though.
> No, they are failing in those cases because 'found' is true but then for
> either Uniform block or shader storage block, we check if the name is an
> array or struct:
>
>         if (found) {
>           switch (programInterface) {
>           case GL_UNIFORM_BLOCK:
>           case GL_SHADER_STORAGE_BLOCK:
>              /* Basename match, check if array or struct. */
>              if (name[baselen] == '\0' ||
>                  name[baselen] == '[' ||
>                  name[baselen] == '.') {
>              [...]
>
> As 'found' can be true because of two different cases, we need to take
> care of both in this 'if' condition.
>
> $ ./deqp-gles31 -n
> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array*
> dEQP Core git-deb902685b9309a64b5e08c48157d3117cf27750 (0xdeb90268)
> starting..
>    target implementation = 'Default'
>
> Test case
> 'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array'..
> Compute shader compile time = 16.732000 ms
> Link time = 14.251000 ms
> Test case duration in microseconds = 43993 us
>    Fail (GetProgramResourceIndex returned unexpected values)
>
> Test case
> 'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element'..
> Compute shader compile time = 1.114000 ms
> Link time = 2.714000 ms
> Test case duration in microseconds = 8858 us
>    Fail (GetProgramResourceIndex returned unexpected values)
>
> DONE!
>
> Test run totals:
>    Passed:        0/2 (0.0%)
>    Failed:        2/2 (100.0%)
>    Not supported: 0/2 (0.0%)
>    Warnings:      0/2 (0.0%)
>
> $ glxinfo | grep git
>
>
> OpenGL core profile version string: 3.3 (Core Profile) Mesa 11.1.0-devel
> (git-4565b6f)
> OpenGL version string: 3.0 Mesa 11.1.0-devel (git-4565b6f)
> OpenGL ES profile version string: OpenGL ES 3.1 Mesa 11.1.0-devel
> (git-4565b6f)

That is strange, for me these pass fine, I can see that our deqp version 
differ:

[tpalli at tpalli-mobl2 gles31]$ mesa_run ./deqp-gles31 
--deqp-case=dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array*
dEQP Core git-7d804783aef83f8f2784d99812d98bcfcf21e95f (0x7d804783) 
starting..
   target implementation = 'Default'

Test case 
'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array'..
Compute shader compile time = 4.585000 ms
Link time = 3.678000 ms
Test case duration in microseconds = 12021 us
   Pass (Pass)

Test case 
'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element'..
Compute shader compile time = 0.822000 ms
Link time = 1.225000 ms
Test case duration in microseconds = 3682 us
   Pass (Pass)

DONE!

Test run totals:
   Passed:        2/2 (100.0%)
   Failed:        0/2 (0.0%)
   Not supported: 0/2 (0.0%)
   Warnings:      0/2 (0.0%)



This is with

MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader
MESA_GLES_VERSION_OVERRIDE=3.1

and current Mesa head (03c92ff), tested on Ivybridge laptop.


>
>   This function is becoming a bit of a pain because
>> different resources have different naming schemes, I'll have to see if
>> it could be somehow refactored simpler.
>>
> OK, thanks.
>
> Sam
>
>>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>> ---
>>>    src/mesa/main/shader_query.cpp | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/main/shader_query.cpp
>>> b/src/mesa/main/shader_query.cpp
>>> index 59ec3d7..6cc91de 100644
>>> --- a/src/mesa/main/shader_query.cpp
>>> +++ b/src/mesa/main/shader_query.cpp
>>> @@ -592,7 +592,8 @@ _mesa_program_resource_find_name(struct
>>> gl_shader_program *shProg,
>>>                /* Basename match, check if array or struct. */
>>>                if (name[baselen] == '\0' ||
>>>                    name[baselen] == '[' ||
>>> -                name[baselen] == '.') {
>>> +                name[baselen] == '.' ||
>>> +                rname_has_array_index_zero) {
>>>                   return res;
>>>                }
>>>                break;
>>