[Mesa-dev] [PATCH 1/6] main: fix ACTIVE_UNIFORM_BLOCKS value

[Timothy Arceri]

On Mon, 2015-09-28 at 16:30 +0200, Samuel Iglesias Gonsálvez wrote:
> 
> On 28/09/15 14:23, Timothy Arceri wrote:
> > On Mon, 2015-09-28 at 12:46 +0200, Samuel Iglesias Gonsálvez wrote:
> > > 
> > > On 28/09/15 12:39, Samuel Iglesias Gonsálvez wrote:
> > > > 
> > > > 
> > > > On 27/09/15 23:15, Timothy Arceri wrote:
> > > > > On Fri, 2015-09-25 at 10:24 +0200, Samuel Iglesias Gonsalvez
> > > > > wrote:
> > > > > > NumUniformBlocks also counts shader storage blocks.
> > > > > > NumUniformBlocks variable will be renamed in a later patch
> > > > > > to
> > > > > > avoid
> > > > > > misunderstandings.
> > > > > > 
> > > > > > Signed-off-by: Samuel Iglesias Gonsalvez <
> > > > > > siglesias at igalia.com>
> > > > > > ---
> > > > > >  src/mesa/main/shaderapi.c | 9 +++++++--
> > > > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/src/mesa/main/shaderapi.c
> > > > > > b/src/mesa/main/shaderapi.c
> > > > > > index edc23bc..7866a20 100644
> > > > > > --- a/src/mesa/main/shaderapi.c
> > > > > > +++ b/src/mesa/main/shaderapi.c
> > > > > > @@ -725,12 +725,17 @@ get_programiv(struct gl_context *ctx,
> > > > > > GLuint
> > > > > > program, GLenum pname,
> > > > > >        *params = max_len;
> > > > > >        return;
> > > > > >     }
> > > > > > -   case GL_ACTIVE_UNIFORM_BLOCKS:
> > > > > > +   case GL_ACTIVE_UNIFORM_BLOCKS: {
> > > > > > +      unsigned i;
> > > > > > +
> > > > > >        if (!has_ubo)
> > > > > >           break;
> > > > > >  
> > > > > > -      *params = shProg->NumUniformBlocks;
> > > > > > +      for (i = 0, *params = 0; i < shProg
> > > > > > ->NumProgramResourceList;
> > > > > > i++)
> > > > > > +         if (shProg->ProgramResourceList[i].Type ==
> > > > > > GL_UNIFORM_BLOCK)
> > > > > > +            (*params)++;
> > > > > >        return;
> > > > > > +   }
> > > > > 
> > > > > Rather than loop through the entire resource list you could
> > > > > just
> > > > > looping over the uniform block list right?
> > > > > 
> > > > >    for (unsigned i = 0; i < shProg->NumUniformBlocks; i++) {
> > > > >       if (!shProg->UniformBlocks[i].IsShaderStorage) {
> > > > >          (*params)++;;
> > > > >       }
> > > > >    }
> > > > > 
> > > > 
> > > > Yeah, that works too. I will change it. Do you give R-b to this
> > > > change?
> > > 
> > > Mmm, looking at the OpenGL 4.3 spec, 7.6 Uniform Variables:
> > > 
> > > "Active uniform blocks are those that contain active uniforms
> > > after a
> > > program has been compiled and linked."
> > > 
> > > And active uniform blocks have been gathered in
> > > ProgramResourceList[].
> > > So I prefer my original patch even when we loop through the
> > > entire
> > > resource list.
> > > 
> > > What do you think?
> > 
> > UniformBlocks satisfies the spec also as far as I can tell. In fact
> >  ProgramResourceList is just filled with the values from
> > UniformBlocks
> > so I don't really see the problem.
> 
> True. I'll do the change then.

Great. With that change:

Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>

> 
> Sam
> > 
> > 
> > > 
> > > Sam
> > > 
> > > > 
> > > > Sam
> > > > 
> > > > > >     case GL_PROGRAM_BINARY_RETRIEVABLE_HINT:
> > > > > >        /* This enum isn't part of the OES extension for
> > > > > > OpenGL
> > > > > > ES
> > > > > > 2.0.  It is
> > > > > >         * only available with desktop OpenGL 3.0+ with the
> > > > > 
> >