[Mesa-dev] [PATCH] glsl: initialize samplers mapping with 0

[Kenneth Graunke]

On 04/28/2012 11:20 AM, Vadim Girlin wrote:
> According to GLSL 1.30 specification, initial value for all uniforms
> without initializer is 0.  Some applications rely on this behaviour,
> e.g. google's MapGL doesn't set all sampler uniforms.
>
> (see https://bugs.freedesktop.org/show_bug.cgi?id=49088 )
>
> Signed-off-by: Vadim Girlin<vadimgirlin at gmail.com>
> ---
>
> Tested with r600g only - no regressions.

Awesome find!  I was at a complete loss here. :)

It turns out this is in the 1.20 spec too; it looks like 1.10 doesn't 
say (but that isn't too surprising).  I might add a comment:

/* From the GLSL 1.20 specification, page 24, section 4.3.5 "Uniform":
  * "The link time initial value is either the value of the variable's
  *  initializer, or 0 if no initializer present.  Sampler types cannot
  *  have initializers."
  */

Also, do you really need the memsets in ir_to_mesa and st_glsl_to_tgsi? 
  Everything should go through the linker, so that seems somewhat 
redundant.  I tested with just the link_uniforms change and that was 
enough to fix MapsGL on i965/Sandybridge.

Assuming you drop the other hunks and add a comment, you can add:
Reviewed-and-tested-by: Kenneth Graunke <kenneth at whitecape.org>

Ian, does this look right to you?

>   src/glsl/link_uniforms.cpp                 |    4 +---
>   src/mesa/program/ir_to_mesa.cpp            |    1 +
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |    1 +
>   3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> index 2f1c9f5..cb80caf 100644
> --- a/src/glsl/link_uniforms.cpp
> +++ b/src/glsl/link_uniforms.cpp
> @@ -329,9 +329,7 @@ link_assign_uniform_locations(struct gl_shader_program *prog)
>         prog->UniformHash = new string_to_uint_map;
>      }
>
> -   for (unsigned i = 0; i<  Elements(prog->SamplerUnits); i++) {
> -      prog->SamplerUnits[i] = i;
> -   }
> +   memset(prog->SamplerUnits, 0, sizeof(prog->SamplerUnits));
>
>      /* First pass: Count the uniform resources used by the user-defined
>       * uniforms.  While this happens, each active uniform will have an index
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index 840648e..685c1b9 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -2865,6 +2865,7 @@ get_mesa_program(struct gl_context *ctx,
>      prog = ctx->Driver.NewProgram(ctx, target, shader_program->Name);
>      if (!prog)
>         return NULL;
> +   memset(prog->SamplerUnits, 0, sizeof(prog->SamplerUnits));
>      prog->Parameters = _mesa_new_parameter_list();
>      v.ctx = ctx;
>      v.prog = prog;
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 9e68deb..7c4275a 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -4779,6 +4779,7 @@ get_mesa_program(struct gl_context *ctx,
>      prog = ctx->Driver.NewProgram(ctx, target, shader_program->Name);
>      if (!prog)
>         return NULL;
> +   memset(prog->SamplerUnits, 0, sizeof(prog->SamplerUnits));
>      prog->Parameters = _mesa_new_parameter_list();
>      v->ctx = ctx;
>      v->prog = prog;