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

[Timothy Arceri]

On Fri, 2015-10-30 at 11:57 -0400, 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
> some patches making dedicated lists for UBOs and SSBOs?

Yeah I did see some too, but I don't think there was any feedback on them.

It seems like it would be less effort to get those patches reviewed and right
then chasing around and fixing up all these issues.

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