[Mesa-dev] [PATCH 1/3] glsl: skip buffer variables when filling UniformRemapTable

[Tapani Pälli]

On 10/20/2015 01:40 PM, Lofstedt, Marta wrote:
>> -----Original Message-----
>> From: Palli, Tapani
>> Sent: Tuesday, October 20, 2015 12:25 PM
>> To: Lofstedt, Marta; mesa-dev at lists.freedesktop.org
>> Subject: Re: [Mesa-dev] [PATCH 1/3] glsl: skip buffer variables when filling
>> UniformRemapTable
>>
>>
>>
>> On 10/20/2015 01:11 PM, Lofstedt, Marta wrote:
>>>
>>> If you also remove the uniforms[i].array_elements in the same manner,
>> the assert in:
>>> ES31-CTS.explicit_uniform_location.uniform-loc-arrays-of-arrays
>>> Is also avoided.
>>
>> Are you referring to one of the loops iterating uniforms in this same place the
>> patch is touching?
>
> Yes, I hit the same assert in "uniform-loc-structs" and "uniform-loc-arrays-of-arrays", with your patchset I no longer hit the assert for the "uniform-loc-structs" tests.
>
> If I also remove array_elements I also avoid the assert in "uniform-loc-arrays-of-arrays". This is just an observation, not a suggestion of a fix.

Right, I was not sure at first what you meant by 'removing 
array_elements'. I was hoping arrays-of-arrays to be the same case as 
with loc-structs but unfortunately seems quite different, we just lack 
some support for arrays of arrays here. Will dig more!


>>
>> For me it looks like the problem with arrays-of-arrays test is that it will assign
>> overlapping locations currently for uniforms if we have 'arrays of arrays'
>> situation going on.
>>
>> For example with
>>
>> layout(location = 0) uniform float uni[2][2]
>>
>> uni[0] gets location 0, but uni[1] has gets location 0 too. I haven't quite
>> figured out yet where the issue is but the location counter gets reset for
>> each array item and starts again from 0 while it should actually maintain its
>> count ..
>>
>>
>>>> -----Original Message-----
>>>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
>>>> Behalf Of Tapani Pälli
>>>> Sent: Tuesday, October 20, 2015 11:24 AM
>>>> To: mesa-dev at lists.freedesktop.org
>>>> Subject: [Mesa-dev] [PATCH 1/3] glsl: skip buffer variables when
>>>> filling UniformRemapTable
>>>>
>>>> UniformRemapTable is used only for remapping user specified uniform
>>>> locations to driver internally used ones, shader storage buffer
>>>> variables should not utilize uniform locations.
>>>>
>>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>>> ---
>>>>    src/glsl/link_uniforms.cpp | 7 +++++--
>>>>    1 file changed, 5 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>>>> index
>>>> fe00aa3..f7b87a1 100644
>>>> --- a/src/glsl/link_uniforms.cpp
>>>> +++ b/src/glsl/link_uniforms.cpp
>>>> @@ -1180,7 +1180,8 @@ link_assign_uniform_locations(struct
>>>> gl_shader_program *prog,
>>>>
>>>>       /* Reserve all the explicit locations of the active uniforms. */
>>>>       for (unsigned i = 0; i < num_uniforms; i++) {
>>>> -      if (uniforms[i].type->is_subroutine())
>>>> +      if (uniforms[i].type->is_subroutine() ||
>>>> +          uniforms[i].is_shader_storage)
>>>>             continue;
>>>>
>>>>          if (uniforms[i].remap_location != UNMAPPED_UNIFORM_LOC) { @@
>>>> -
>>>> 1200,8 +1201,10 @@ link_assign_uniform_locations(struct
>>>> gl_shader_program *prog,
>>>>       /* Reserve locations for rest of the uniforms. */
>>>>       for (unsigned i = 0; i < num_uniforms; i++) {
>>>>
>>>> -      if (uniforms[i].type->is_subroutine())
>>>> +      if (uniforms[i].type->is_subroutine() ||
>>>> +          uniforms[i].is_shader_storage)
>>>>             continue;
>>>> +
>>>>          /* Built-in uniforms should not get any location. */
>>>>          if (uniforms[i].builtin)
>>>>             continue;
>>>> --
>>>> 2.4.3
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev