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

[Tapani]

On 03/04/2015 05:07 PM, Ilia Mirkin wrote:
> 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? :)

IMO Arthur's interpretation is correct. I had to read this many many 
times while writing test for GetProgramResourceIndex to get it, the 
"a[1]" example seems misleading at first. GL_ARB_array_of_arrays is 
special case explained in GL_ARB_program_interface_query spec:

"For a uniform array such as: uniform vec4 a[5][4][3]; we enumerate 
twenty different entries ("a[0][0][0]" through "a[4][3][0]"), ..."


>    -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
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150304/3676e1a3/attachment.html>