[Mesa-dev] [PATCH 1/2] mesa: fix uniforms calculation in glGetProgramiv

[Timothy Arceri]

On Sat, 2015-10-31 at 06:52 +0200, Tapani Pälli wrote:
> On 10/30/2015 05:57 PM, Ilia Mirkin wrote:
> > On Fri, Oct 30, 2015 at 8:30 AM, Tapani Pälli <tapani.palli at intel.com>
> > wrote:
> > > Since introduction of SSBO, UniformStorage contains not just uniforms
> > > but also buffer variables, this needs to be taken in to account when
> > > calculating active uniforms with GL_ACTIVE_UNIFORMS and
> > > GL_ACTIVE_UNIFORM_MAX_LENGTH.
> > > 
> > > No Piglit regressions.
> > > 
> > > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > > ---
> > >   src/mesa/main/shaderapi.c | 14 ++++++++++++--
> > >   1 file changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> > > index 765602e..ac40891 100644
> > > --- a/src/mesa/main/shaderapi.c
> > > +++ b/src/mesa/main/shaderapi.c
> > > @@ -630,9 +630,16 @@ get_programiv(struct gl_context *ctx, GLuint
> > > program, GLenum pname,
> > >      case GL_ACTIVE_ATTRIBUTE_MAX_LENGTH:
> > >         *params = _mesa_longest_attribute_name_length(shProg);
> > >         return;
> > > -   case GL_ACTIVE_UNIFORMS:
> > > -      *params = shProg->NumUniformStorage - shProg->NumHiddenUniforms;
> > > +   case GL_ACTIVE_UNIFORMS: {
> > > +      unsigned i;
> > > +      const unsigned num_uniforms =
> > > +         shProg->NumUniformStorage - shProg->NumHiddenUniforms;
> > Are the hidden uniforms guaranteed to be at the end of that list? If
> > not, the subtraction needs to happen at the end. Also didn't I see
> 
> Good point, I don't think that's guaranteed. 

It should be guaranteed see: assign_hidden_uniform_slot_id() in
link_uniforms.cpp

> I'll need to to move the 
> subtraction part or perhaps easier just skip hidden ones in the loop.
> 
> > some patches making dedicated lists for UBOs and SSBOs?
> 
> UBO and SSBO index spaces were separated but they both still utilize 
> gl_uniform_storage for their contents, uniforms and buffer variables.
> 

Right good point.

> > > +      for (*params = 0, i = 0; i < num_uniforms; i++) {
> > > +         if (!shProg->UniformStorage[i].is_shader_storage)
> > > +            (*params)++;
> > > +      }
> > >         return;
> > > +   }
> > >      case GL_ACTIVE_UNIFORM_MAX_LENGTH: {
> > >         unsigned i;
> > >         GLint max_len = 0;
> > > @@ -640,6 +647,9 @@ get_programiv(struct gl_context *ctx, GLuint
> > > program, GLenum pname,
> > >            shProg->NumUniformStorage - shProg->NumHiddenUniforms;
> > > 
> > >         for (i = 0; i < num_uniforms; i++) {
> > > +         if (shProg->UniformStorage[i].is_shader_storage)
> > > +            continue;
> > > +
> > >           /* Add one for the terminating NUL character for a non-array,
> > > and
> > >            * 4 for the "[0]" and the NUL for an array.
> > >            */
> > > --
> > > 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