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

[Tapani Pälli]

On 11/01/2015 01:20 AM, Timothy Arceri wrote:
> 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

OK, I did not remember how it goes I thought to check this only later :) 
Then this patch seems correct after all.

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