[Mesa-dev] [WIP 08/13] glsl/linker: GL_ARB_explicit_uniform_location support

[Tapani Pälli]

On 04/05/2014 01:18 AM, Ian Romanick wrote:
> On 03/27/2014 11:45 PM, Tapani Pälli wrote:
>> Patch refactors the existing uniform processing so explicit locations
>> are taken in to account during variable processing. These locations
>> are temporarily stored in gl_uniform_storage before actual locations
>> are set.
8<
>> +
>> +   /* Final check for size of the remap table. Amount of active
>> +    * uniforms vs maximum explicit location.
>> +    */
>> +   const unsigned remap_table_size = MAX2(remap_entries, max_exp_loc + 1);
>> +
>> +   prog->UniformRemapTable =
>> +      rzalloc_array(prog, gl_uniform_storage *, remap_table_size);
> Why not just allocation max_exp_loc + 1 + remap_entries?  See my comment
> below about not being allowed to fail.  At the end we can rerealloc to
> the actual size... or we could just keep reralloc'ing in the loop like
> we did before.

OK, I thought there could be 'out of resources' condition because the
maximum number of available locations is defined. I agree that this is
very academical though and can change this. GL spec mentions only the
case of failure if  "More active uniform or active sampler variables are
used in program than allowed (see sections 7.6, 7.10, and 11.3.3)." but
it does not seem to mention what should be the error message when this
happens .. out of memory?


>> +
>> +   prog->NumUniformRemapTable = remap_table_size;
>> +
>> +   /* Allocate a fake uniform storage which can be used to
>> +    * distinguish if a remap table entry is one of the removed ones.
>> +    */
>> +   if (prog->NumReservedLocations) {
>> +      prog->RemovedUniformStorage =
>> +         rzalloc(prog->ReservedUniformLocations, gl_uniform_storage);
>> +   }
>> +
>> +   /* Build the uniform remap table that is used to set/get uniform locations.
>> +    * First reserve all the explicit locations of the inactive uniforms.
>> +    */
>> +   for (unsigned i = 0; i < prog->NumReservedLocations; i += 2) {
>> +      /* Fill RemovedUniformStorage for this uniform. */
>> +      prog->RemovedUniformStorage->remap_location =
>> +         prog->ReservedUniformLocations[i];
>> +      prog->RemovedUniformStorage->array_elements =
>> +         prog->ReservedUniformLocations[i + 1];
>> +
>> +         if (!reserve_explicit_locations(prog, prog->RemovedUniformStorage))
>> +            return false;
>> +   }
>> +
>> +   /* Reserve all the explicit locations of the active uniforms. */
>>     for (unsigned i = 0; i < num_user_uniforms; i++) {
>> +      if (uniforms[i].remap_location != -1)
>> +         if (!reserve_explicit_locations(prog, &uniforms[i]))
>> +            return false;
>> +   }
>> +
>> +   /* Reserve locations for rest of the uniforms. */
>> +   for (unsigned i = 0; i < num_user_uniforms; i++) {
>> +
>> +      /* Explicit ones have been set already. */
>> +      if (uniforms[i].remap_location != -1)
>> +         continue;
>>  
>> -      /* how many new entries for this uniform? */
>> +      /* How many new entries for this uniform? */
>>        const unsigned entries = MAX2(1, uniforms[i].array_elements);
>>  
>> -      /* resize remap table to fit new entries */
>> -      prog->UniformRemapTable =
>> -         reralloc(prog,
>> -                  prog->UniformRemapTable,
>> -                  gl_uniform_storage *,
>> -                  prog->NumUniformRemapTable + entries);
>> +      /* Seek the uniform remap table for a available slot. */
>> +      int pos = search_uniform_remap_slot(prog, &uniforms[i]);
>>  
>> -      /* set pointers for this uniform */
>> -      for (unsigned j = 0; j < entries; j++)
>> -         prog->UniformRemapTable[prog->NumUniformRemapTable+j] = &uniforms[i];
>> +      /* We cannot find big enough continuous slot for the uniform, this
>> +       * can happen due to fragmentation in the available locations caused
>> +       * by explicit locations.
>> +       */
>> +      if (pos == -1) {
>> +         linker_error(prog, "cannot allocate space "
>> +                      "for uniform %s ", uniforms[i].name);
>> +         return false;
>> +      }
> There's nothing in the spec that allows us to fail in this situation.

OK, I'll remove this case. Because there is a maximum limitation to
locations, I can write a Piglit test for this to happen though :)

>>  
>> -      /* set the base location in remap table for the uniform */
>> -      uniforms[i].remap_location = prog->NumUniformRemapTable;
>> +      /* Set pointers for the uniform. */
>> +      for (unsigned j = pos; j < pos + entries; j++) {
>> +         assert(prog->UniformRemapTable[j] == NULL);
>> +         prog->UniformRemapTable[j] = &uniforms[i];
>> +      }
>>  
>> -      prog->NumUniformRemapTable += entries;
>> +      /* Set the base location in remap table for the uniform. */
>> +      uniforms[i].remap_location = pos;
>>     }
>>  
>>  #ifndef NDEBUG
>>