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

[Samuel Iglesias Gonsálvez]

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?

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