[Mesa-dev] [PATCH] mesa: fix program resource queries for atomic counter buffers

[Lofstedt, Marta]

> -----Original Message-----
> From: Palli, Tapani
> Sent: Friday, October 30, 2015 11:43 AM
> To: Lofstedt, Marta; mesa-dev at lists.freedesktop.org
> Cc: currojerez at riseup.net
> Subject: Re: [PATCH] mesa: fix program resource queries for atomic counter
> buffers
> 
> 
> 
> On 10/30/2015 12:36 PM, Lofstedt, Marta wrote:
> >
> >> -----Original Message-----
> >> From: Palli, Tapani
> >> Sent: Friday, October 30, 2015 11:03 AM
> >> To: mesa-dev at lists.freedesktop.org
> >> Cc: Lofstedt, Marta; currojerez at riseup.net; Palli, Tapani
> >> Subject: [PATCH] mesa: fix program resource queries for atomic
> >> counter buffers
> >>
> >> gl_active_atomic_buffer contains index to UniformStorage, we need to
> >> calculate resource index for that gl_uniform_storage.
> >>
> >> Fixes following CTS tests:
> >>     ES31-CTS.program_interface_query.atomic-counters
> >>     ES31-CTS.program_interface_query.atomic-counters-one-buffer
> >>
> >> No Piglit regressions.
> >>
> >> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> >> ---
> >>   src/mesa/main/shader_query.cpp | 28
> ++++++++++++++++++++++++++--
> >>   1 file changed, 26 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/mesa/main/shader_query.cpp
> >> b/src/mesa/main/shader_query.cpp index 59ec3d7..6e0251a 100644
> >> --- a/src/mesa/main/shader_query.cpp
> >> +++ b/src/mesa/main/shader_query.cpp
> >> @@ -668,6 +668,20 @@ _mesa_program_resource_index(struct
> >> gl_shader_program *shProg,
> >>      }
> >>   }
> >>
> >> +/**
> >> + * Find a program resource that points to given data.
> >> + */
> >> +static struct gl_program_resource*
> >> +program_resource_find_data(struct gl_shader_program *shProg, void
> >> +*data) {
> >> +   struct gl_program_resource *res = shProg->ProgramResourceList;
> >> +   for (unsigned i = 0; i < shProg->NumProgramResourceList; i++, res++) {
> >> +      if (res->Data == data)
> >> +         return res;
> >> +   }
> >> +   return NULL;
> >> +}
> >> +
> >>   /* Find a program resource with specific index in given interface.
> >>    */
> >>   struct gl_program_resource *
> >> @@ -1065,8 +1079,18 @@ get_buffer_property(struct
> gl_shader_program
> >> *shProg,
> >>            *val = RESOURCE_ATC(res)->NumUniforms;
> >>            return 1;
> >>         case GL_ACTIVE_VARIABLES:
> >> -         for (unsigned i = 0; i < RESOURCE_ATC(res)->NumUniforms; i++)
> >> -            *val++ = RESOURCE_ATC(res)->Uniforms[i];
> >> +         for (unsigned i = 0; i < RESOURCE_ATC(res)->NumUniforms; i++) {
> >> +            /* Active atomic buffer contains index to UniformStorage. Find
> >> +             * out gl_program_resource via data pointer and then calculate
> >> +             * index of that uniform.
> >> +             */
> >> +            unsigned idx = RESOURCE_ATC(res)->Uniforms[i];
> >> +            struct gl_program_resource *uni =
> >> +               program_resource_find_data(shProg,
> >> +                                          &shProg->UniformStorage[idx]);
> >> +            assert(uni);
> >> +            *val++ = _mesa_program_resource_index(shProg, uni);
> >> +         }
> >
> > The parts GL_SHADER_STORAGE_BLOCK looks very similar to the
> GL_UNIFORM_BLOCK above. Should this code duplication, be avoided?
> 
> You mean parts above not related to this patch? You are welcome to try
> :) I had originally different looking version:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2015-March/079310.html
> 
> I don't have strong preference though.
> 
if you'd set an Glenum programInterface to GL_UNIFORM or GL_BUFFER_VARIABLE to send to _mesa_program_resource_find_name, you would not have had to write the same thing twice :)
Then the GL_NUM_ACTIVE_VARIABLES and the GL_ACTIVE_VARIABLES cases are also very similar...
Never mind,  I don't have any strong opinions either...

Reviewed-by: Marta Lofstedt <marta.lofstedt at intel.com>

> 
> >>            return RESOURCE_ATC(res)->NumUniforms;
> >>         }
> >>      }
> >> --
> >> 2.4.3
> >
> 
> 
> // Tapani