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

[Samuel Iglesias Gonsálvez]

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.

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