[Mesa-dev] [PATCH 3/3] glsl: Generate smaller values for uniform locations

[Ian Romanick]

On 06/10/2013 11:21 AM, Brian Paul wrote:
> On 06/10/2013 11:52 AM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Previously we would generate uniform locations as (slot << 16) +
>> array_index.  We do this two handle applications that assume the
>> location of a[2] will be +1 from the location of a[1].  This resulted in
>> every uniform location being at least 0x10000.  We've now encountered
>> two applications that assume uniform values will be smaller.
>>
>> NEITHER BEHAVIOR IS GUARNATEED
>
> GUARANTEED

Oops.

>   OR IMPLIED BY ANY VERSION OF OpenGL.
>>
>> Our workaround for one sort of buggy application breaks a different sort
>> of buggy application.  Irony anyone?
>>
>> Other implementations happen to have both these behaviors (sequential
>> array elements and small values), so let's just match their behavior and
>> let buggy apps break on other implementations.  /Sigh/
>>
>> NOTE: This is a candidate for stable release branches.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: Chad Versace <chad.versace at linux.intel.com>
>> ---
>>   src/glsl/link_uniforms.cpp | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
>> index 84680be..6f67c70 100644
>> --- a/src/glsl/link_uniforms.cpp
>> +++ b/src/glsl/link_uniforms.cpp
>> @@ -739,7 +739,19 @@ link_assign_uniform_locations(struct
>> gl_shader_program *prog)
>>                sizeof(prog->_LinkedShaders[i]->SamplerTargets));
>>      }
>>
>> -   prog->UniformLocationBaseScale = (1U<<16);
>> +   /* Determine the size of the largest uniform array queryable via
>> +    * glGetUniformLocation.  Using this as the location scale
>> guarantees that
>> +    * there is enough "room" for the array index to be stored in the
>> low order
>> +    * part of the uniform location.  It also makes the locations be more
>> +    * tightly packed.
>> +    */
>> +   unsigned max_array_size = 1;
>> +   for (unsigned i = 0; i < num_user_uniforms; i++) {
>> +      if (uniforms[i].array_elements > max_array_size)
>> +     max_array_size = uniforms[i].array_elements;
>
> Tab indent

Dang it!  I thought I caught all of those.  I'll double check the whole 
series before I push.

>> +   }
>> +
>> +   prog->UniformLocationBaseScale = max_array_size;
>>
>>   #ifndef NDEBUG
>>      for (unsigned i = 0; i < num_user_uniforms; i++) {
>>
>
> Series looks good to me.
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>