[Piglit] [PATCH] shader_runner: fix uniform array name lookups

[Ilia Mirkin]

On Wed, Mar 4, 2015 at 9:58 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Wed, Mar 4, 2015 at 4:53 AM, Arthur Huillet <arthur.huillet at free.fr> wrote:
>> From: Arthur Huillet <ahuillet at nvidia.com>
>>
>> Don't look up uniform names for non-zero array elements, as this is illegal per GL4.5.
>>
>> From the discussion of GetProgramResourceIndex in the GL4.5 spec:
>>
>>        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. Otherwise, name
>>     is considered not to be the name of an active resource, and INVALID_INDEX is
>>     returned. Note that if an interface enumerates a single active resource list entry for
>>     an array variable (e.g., "a[0]"), a name identifying any array element other than
>>     the first (e.g., "a[1]") is not considered to match.
>
> Hm, and the spec goes on to define GetUniformIndices in terms of
> GetProgramResourceIndex...
>
> What does this say about a[3].b[75].c[2] ? Do they all have to be 0
> and you have to retrieve all 3 strides? If so, your implementation
> isn't quite doing that.

Also I'm having a ton of trouble parsing the meaning of

"""
Note that if an interface enumerates a single active resource list entry for
an array variable (e.g., "a[0]"), a name identifying any array element
other than
the first (e.g., "a[1]") is not considered to match.
"""

It's unclear to me that it means what you claim it means in the first
place. Ian, as the resident GL and UBO expert, care to comment? :)

  -ilia

>
>>
>> Instead, strip the "[xxx]" part of the name for the lookup.
>>
>> The NVIDIA proprietary driver enforces this rule of the specification, while Mesa
>> is more permissive. Piglit shouldn't rely on the implementation being lax.
>>
>> Signed-off-by: Arthur Huillet <ahuillet at nvidia.com>
>> ---
>>  tests/shaders/shader_runner.c | 33 ++++++++++++++++++++++++++-------
>>  1 file changed, 26 insertions(+), 7 deletions(-)
>>
>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>> index c193de9..22a9828 100644
>> --- a/tests/shaders/shader_runner.c
>> +++ b/tests/shaders/shader_runner.c
>> @@ -1227,11 +1227,12 @@ check_double_support(void)
>>   * the data.  If the uniform is not in a uniform block, returns false.
>>   */
>>  bool
>> -set_ubo_uniform(const char *name, const char *type, const char *line, int ubo_array_index)
>> +set_ubo_uniform(char *name, const char *type, const char *line, int ubo_array_index)
>>  {
>>         GLuint uniform_index;
>>         GLint block_index;
>>         GLint offset;
>> +       GLint array_index;
>>         char *data;
>>         float f[16];
>>         double d[16];
>> @@ -1242,7 +1243,29 @@ set_ubo_uniform(const char *name, const char *type, const char *line, int ubo_ar
>>         if (!num_uniform_blocks)
>>                 return false;
>>
>> -       glGetUniformIndices(prog, 1, &name, &uniform_index);
>> +       /* if the uniform is an array, strip the index, as GL
>> +          prevents non-zero indexes from matching a name */
>> +       if (name[name_len - 1] == ']') {
>> +               int i;
>> +
>> +               for (i = name_len - 1; (i > 0) && isdigit(name[i-1]); --i)
>> +                       /* empty */;
>> +
>> +               array_index = strtol(&name[i], NULL, 0);
>> +
>> +               if (i) {
>> +                       i--;
>> +                       if (name[i] != '[') {
>> +                               printf("cannot parse uniform \"%s\"\n", name);
>> +                               piglit_report_result(PIGLIT_FAIL);
>> +                       }
>> +                       name[i] = 0;
>> +               }
>> +
>> +       }
>> +
>> +
>> +       glGetUniformIndices(prog, 1, (const char **)&name, &uniform_index);
>>         if (uniform_index == GL_INVALID_INDEX) {
>>                 printf("cannot get index of uniform \"%s\"\n", name);
>>                 piglit_report_result(PIGLIT_FAIL);
>> @@ -1265,14 +1288,10 @@ set_ubo_uniform(const char *name, const char *type, const char *line, int ubo_ar
>>
>>         if (name[name_len - 1] == ']') {
>>                 GLint stride;
>> -               int i;
>> -
>> -               for (i = name_len - 1; (i > 0) && isdigit(name[i-1]); --i)
>> -                       /* empty */;
>>
>>                 glGetActiveUniformsiv(prog, 1, &uniform_index,
>>                                       GL_UNIFORM_ARRAY_STRIDE, &stride);
>> -               offset += stride * strtol(&name[i], NULL, 0);
>> +               offset += stride * array_index;
>>         }
>>
>>         glBindBuffer(GL_UNIFORM_BUFFER,
>> --
>> 2.3.0
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit