[Mesa-dev] [PATCH] glsl: move uniform calculation to link_uniforms

[Tapani Pälli]

On 01/17/2016 12:32 AM, Matt Turner wrote:
> On Fri, Jan 15, 2016 at 9:40 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Patch moves uniform calculation to happen during link_uniforms, this
>> is possible with help of UniformRemapTable that has all the reserved
>> locations.
>>
>> Location assignment for implicit locations is changed so that we
>> utilize also the 'holes' that explicit uniform location assignment
>> might have left in UniformRemapTable, this makes it possible to fit
>> more uniforms as previously we were lazy here and wasting space.
>>
>> Fixes following CTS tests:
>>     ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max
>>     ES31-CTS.explicit_uniform_location.uniform-loc-mix-with-implicit-max-array
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/glsl/link_uniforms.cpp | 78 ++++++++++++++++++++++++++++++++++++++++------
>>   src/glsl/linker.cpp        | 19 ++++-------
>>   src/glsl/linker.h          |  3 +-
>>   3 files changed, 77 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>> index 33b2d4c..91e973a 100644
>> --- a/src/glsl/link_uniforms.cpp
>> +++ b/src/glsl/link_uniforms.cpp
>> @@ -1057,9 +1057,31 @@ assign_hidden_uniform_slot_id(const char *name, unsigned hidden_id,
>>      uniform_size->map->put(hidden_uniform_start + hidden_id, name);
>>   }
>>
>> +/**
>> + * Search UniformRemapTable for empty block big enough to hold given uniform.
>> + * TODO Optimize this algorithm later if it turns out to be a major bottleneck.
>> + */
>> +static int
>> +seek_empty_block(struct gl_shader_program *prog,
> I might name this "find" instead of "seek"

ok, will change

>> +                 struct gl_uniform_storage *uniform)
>> +{
>> +   const unsigned entries = MAX2(1, uniform->array_elements);
>> +   for (unsigned i = 0, j; i < prog->NumUniformRemapTable; i++)
> This function *needs* braces! Four nesting levels and only two of them
> include braces.
>
> Shouldn't j be declared in the inner for-loop? I don't think j's
> larger scope is needed by this code.

I declared it there to avoid going over 80 chars line width for the 
inner loop but I can divide it to 2 lines, will add braces too, thanks!

>> +      if (prog->UniformRemapTable[i] == NULL) {
>> +         for (j = i; j < entries && j < prog->NumUniformRemapTable; j++)
>> +            if (prog->UniformRemapTable[j] != NULL) {
>> +               i = j;
>> +               break;
>> +            }
>> +         return i;
>> +      }
>> +   return -1;
>> +}