[Mesa-dev] [PATCH] mesa/glsl: introduce a remap table for uniform locations

[Tapani Pälli]

On 03/07/2014 09:13 PM, Eric Anholt wrote:
> Tapani Pälli <tapani.palli at intel.com> writes:
>
> I think the way I'd write this comment is "Mapping from GL uniform
> locations returned by glUniformLocation to UniformStorage entries.
> Arrays will have multiple contiguous slots in the UniformRemapTable, all
> pointing to the same UniformStorage entry."
>
> ("Remap" is vague, and I ended up reading the code to figure out what
> this was mapping from.)

Thanks, I'll change the comment and the other comment error you found.

>> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
>> index 8cc5da7..700a333 100644
>> --- a/src/mesa/main/uniform_query.cpp
>> +++ b/src/mesa/main/uniform_query.cpp
>> @@ -246,6 +246,13 @@ validate_uniform_parameters(struct gl_context *ctx,
>>         return false;
>>      }
>>   
>> +   /* Check that the given location is in bounds of uniform remap table. */
>> +   if (location >= (GLint) shProg->NumUniformRemapTable) {
>> +      _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
>> +		  caller, location);
>> +      return false;
>> +   }
>> +
>>      _mesa_uniform_split_location_offset(shProg, location, loc, array_index);
>>   
>>      if (*loc >= shProg->NumUserUniformStorage) {
> Couldn't the new test just replace the *loc >=
> shProg->NumUserUniformStorage test?  We're always going to have a loc
> pointing to a valid value, since you looked loc up from the remaptable.

True, I will remove the old check for *loc as that one will be always 
correct. New check has to be before split function call as otherwise 
assert introduced there will trigger when user given location is off bounds.

// Tapani