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

[Brian Paul]

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

  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


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