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

[Lofstedt, Marta]

> -----Original Message-----
> From: Palli, Tapani
> Sent: Tuesday, October 20, 2015 12:55 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: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!

The arrays test is another problem...
This patch-set solves the problem it set out to solve, so I consider the whole patchset:

Reviewed-by: Marta Lofstedt <marta.lofstedt at intel.com>

> 
> 
> >>
> >> 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