[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: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.

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