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

[Timothy Arceri]

On Tue, 2015-10-20 at 13:55 +0300, Tapani Pälli wrote:
> 
> 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!

I've just sent a couple of patches to fix the AoA support :) Although
the test still fails as it looks like some compute shader support is
still missing.

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