[Mesa-dev] [PATCH] glsl: mark explicit uniforms as explicit in other stages too

[Tapani Pälli]

On 01/14/2016 01:49 PM, Timothy Arceri wrote:
> On Thu, 2016-01-14 at 13:02 +0200, Tapani Pälli wrote:
>> If shader declares uniform explicit location in one stage but
>> implicit in
>> another, explicit location should be used. Patch marks implicit
>> uniforms
>> as explicit if they were explicit in another stage. This makes sure
>> that
>> we don't treat them implicit later when assigning locations.
>
>
> I think it would be better to fix the code in cross_validate_globals()
> it seems it is halfway doing this already but it only copies the
> explicit location from later stages into the first uniform it found,
> and not the otherway around

True, I'll try moving the copying there.

> I think you just need to check if the existing uniform is explicit and
> copy the location to the currently matched var if its not explicit and
> it should fix this bug.

Right, do the same as this patch but no additional map required, nice!

>>
>> Fixes following CTS test:
>>     ES31-CTS.explicit_uniform_location.uniform-loc-implicit-in-some
>> -stages3
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/glsl/linker.cpp | 36 +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index 564c471..70e701c 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -3131,11 +3131,33 @@ check_explicit_uniform_locations(struct
>> gl_context *ctx,
>>       */
>>      string_to_uint_map *uniform_map = new string_to_uint_map;
>>
>> -   if (!uniform_map) {
>> +   /* This map is used to validate that uniforms with explicit
>> location are
>> +    * given the same explicit location properly across shader stages
>> which
>> +    * reference the same uniform without explicit location
>> qualifier.
>> +    */
>> +   string_to_uint_map *explicit_map = new string_to_uint_map;
>> +
>> +   if (!uniform_map || !explicit_map) {
>>         linker_error(prog, "Out of memory during linking.\n");
>>         return;
>>      }
>>
>> +   /* A pass to generate hash that contains all uniforms where
>> location
>> +    * has been set explicitly.
>> +    */
>> +   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>> +      struct gl_shader *sh = prog->_LinkedShaders[i];
>> +      if (!sh)
>> +         continue;
>> +      foreach_in_list(ir_instruction, node, sh->ir) {
>> +         ir_variable *var = node->as_variable();
>> +         if (!var || var->data.mode != ir_var_uniform ||
>> +             !var->data.explicit_location)
>> +            continue;
>> +         explicit_map->put(var->data.location, var->name);
>> +      }
>> +   }
>> +
>>      unsigned entries_total = 0;
>>      for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>>         struct gl_shader *sh = prog->_LinkedShaders[i];
>> @@ -3148,6 +3170,17 @@ check_explicit_uniform_locations(struct
>> gl_context *ctx,
>>            if (!var || var->data.mode != ir_var_uniform)
>>               continue;
>>
>> +         /* Check if this uniform with implicit location was marked
>> explicit
>> +          * by any shader stage. If so, mark it explicit in this
>> stage too to
>> +          * make sure later processing does not treat it as implicit
>> one.
>> +          */
>> +         unsigned hash_loc;
>> +         if (!var->data.explicit_location &&
>> +             explicit_map->get(hash_loc, var->name)) {
>> +            var->data.explicit_location = 1;
>> +            var->data.location = hash_loc;
>> +         }
>> +
>>            entries_total += var->type->uniform_locations();
>>
>>            if (var->data.explicit_location) {
>> @@ -3173,6 +3206,7 @@ check_explicit_uniform_locations(struct
>> gl_context *ctx,
>>                      ctx->Const.MaxUserAssignableUniformLocations);
>>      }
>>      delete uniform_map;
>> +   delete explicit_map;
>>   }
>>
>>   static bool