[Mesa-dev] [PATCH v3 6/6] main: fix length of values written to glGetProgramResourceiv() for ACTIVE_VARIABLES

[Iago Toral]

On Thu, 2015-10-08 at 09:53 +1100, Timothy Arceri wrote:
> On Wed, 2015-10-07 at 09:21 +0200, Samuel Iglesias Gonsalvez wrote:
> > Return the number of values written.
> > 
> > Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> 
> Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>
> 
> Are you guys going to split SSBO into their own list? There was talk
> about this recently, but I don't recall what the outcome was.

I sent RFC patches with a proposal but there hasn't been much discussion
about that yet. Ilia wasn't sure that my proposal was enough. Maybe we
need to wait a bit so he can progress a bit further and confirm/reject
this. Ilia, do you have any new thoughts on this subject?

> If so we will need to remember to undo all these work arounds.

Yeah, it shouldn't be too problematic though. The way I thought about
this in my RFC patches was that we keep UBOs/SSBOs merged as they are
now (only renaming the array name) so we don't have the uniform linking
code in the compiler, and then, right after linking is finished, we add
two additional arrays with separate views of UBOs and SSBOs for backends
to use as they see fit. These two arrays only hold pointers to entries
in the combined array, so the extra memory required for this is
negligible.

After that and if that is really all we need, we can decide to keep the
combined array or drop it right after linking and splitting it in two.
If we go with the latter then we will need to fix all these queries (we
might want to do that anyway even if we don't drop it, but it wouldn't
be strictly necessary) and we will need to rewrite parts of the i965
backend to accommodate to the split array version (shouldn't be too much
work I think).

Iago

> > ---
> >  src/mesa/main/shader_query.cpp | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/mesa/main/shader_query.cpp
> > b/src/mesa/main/shader_query.cpp
> > index 3bf4311..ecae3de 100644
> > --- a/src/mesa/main/shader_query.cpp
> > +++ b/src/mesa/main/shader_query.cpp
> > @@ -1188,7 +1188,8 @@ get_buffer_property(struct gl_shader_program
> > *shProg,
> >              (*val)++;
> >           }
> >           return 1;
> > -      case GL_ACTIVE_VARIABLES:
> > +      case GL_ACTIVE_VARIABLES: {
> > +         unsigned num_values = 0;
> >           for (unsigned i = 0; i < RESOURCE_UBO(res)->NumUniforms;
> > i++) {
> >              const char *iname = RESOURCE_UBO(res)
> > ->Uniforms[i].IndexName;
> >              struct gl_program_resource *uni =
> > @@ -1198,8 +1199,10 @@ get_buffer_property(struct gl_shader_program
> > *shProg,
> >                 continue;
> >              *val++ =
> >                 _mesa_program_resource_index(shProg, uni);
> > +            num_values++;
> >           }
> > -         return RESOURCE_UBO(res)->NumUniforms;
> > +         return num_values;
> > +      }
> >        }
> >     } else if (res->Type == GL_SHADER_STORAGE_BLOCK) {
> >        switch (prop) {
> > @@ -1221,7 +1224,8 @@ get_buffer_property(struct gl_shader_program
> > *shProg,
> >              (*val)++;
> >           }
> >           return 1;
> > -      case GL_ACTIVE_VARIABLES:
> > +      case GL_ACTIVE_VARIABLES: {
> > +         unsigned num_values = 0;
> >           for (unsigned i = 0; i < RESOURCE_UBO(res)->NumUniforms;
> > i++) {
> >              const char *iname = RESOURCE_UBO(res)
> > ->Uniforms[i].IndexName;
> >              struct gl_program_resource *uni =
> > @@ -1231,8 +1235,10 @@ get_buffer_property(struct gl_shader_program
> > *shProg,
> >                 continue;
> >              *val++ =
> >                 _mesa_program_resource_index(shProg, uni);
> > +            num_values++;
> >           }
> > -         return RESOURCE_UBO(res)->NumUniforms;
> > +         return num_values;
> > +      }
> >        }
> >     } else if (res->Type == GL_ATOMIC_COUNTER_BUFFER) {
> >        switch (prop) {
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev