[Mesa-dev] [PATCH 2/2] mesa: check if resource backed by buffer in property queries

[Tapani Pälli]

Thanks Eduardo for taking a look at this. Today with a refreshed mind I 
noticed that I can fix this with smaller changes since it seems it's 
actually only the matrix_stride query that is broken for atomic counter 
buffers.

I'll send a new patch and will cc you.

On 10/30/2015 05:35 PM, Eduardo Lima Mitev wrote:
> Patch is,
>
> Reviewed-by: Eduardo Lima Mitev <elima at igalia.com>
>
> On 10/30/2015 01:30 PM, Tapani Pälli wrote:
>> Patch fixes broken behaviour of queries for following properties, from
>> ARB_program_interface_query specification:
>>
>> GL_OFFSET:
>>
>>     "For active variables not backed by a buffer object, an offset of
>>     -1 is written to <params>."
>>
>> GL_ARRAY_STRIDE:
>>
>>     "For active variables not declared as an array of basic types, zero
>>     is written to <params>.  For active variables not backed by a buffer
>>     object, -1 is written to <params>, regardless of the variable type."
>>
>> GL_MATRIX_STRIDE:
>>
>>     "For active variables not declared as a matrix or array of matrices,
>>     zero is written to <params>.  For active variables not backed by a
>>     buffer object, -1 is written to <params>, regardless of the variable
>>     type."
>>
>> These queries may come from GetActiveUniformsiv or GetProgramResourceiv.
>> Patch implements little helper to do 'backed by buffer' check and returns
>> appropriate values.
>>
>> Fixes following CTS test:
>>     ES31-CTS.shader_atomic_counters.basic-program-query
>>
>> No Piglit regressions.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/shader_query.cpp | 40 +++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index dd51bba..b214691 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -1105,6 +1105,26 @@ invalid_operation:
>>      return 0;
>>   }
>>
>> +/**
>> + * Helper that returns if given resource is backed by a buffer object,
>> + * either UBO, SSBO or atomic counter buffer.
>> + */
>> +static bool
>> +backed_by_buffer_object(struct gl_program_resource *res)
>> +{
>> +   switch (res->Type) {
>> +   case GL_BUFFER_VARIABLE:
>> +      return true;
>> +   case GL_UNIFORM:
>> +      if (RESOURCE_UNI(res)->block_index != -1 ||
>> +          RESOURCE_UNI(res)->atomic_buffer_index != -1 ||
>> +          RESOURCE_UNI(res)->is_shader_storage)
>> +         return true;
>> +   default:
>> +      return false;
>> +   }
>> +}
>> +
>>   unsigned
>>   _mesa_program_resource_prop(struct gl_shader_program *shProg,
>>                               struct gl_program_resource *res, GLuint index,
>> @@ -1171,20 +1191,30 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
>>         }
>>      case GL_OFFSET:
>>         VALIDATE_TYPE_2(GL_UNIFORM, GL_BUFFER_VARIABLE);
>> -      *val = RESOURCE_UNI(res)->offset;
>> +      *val = backed_by_buffer_object(res) ? RESOURCE_UNI(res)->offset : -1;
>>         return 1;
>>      case GL_BLOCK_INDEX:
>>         VALIDATE_TYPE_2(GL_UNIFORM, GL_BUFFER_VARIABLE);
>>         *val = RESOURCE_UNI(res)->block_index;
>>         return 1;
>> -   case GL_ARRAY_STRIDE:
>> +   case GL_ARRAY_STRIDE: {
>>         VALIDATE_TYPE_2(GL_UNIFORM, GL_BUFFER_VARIABLE);
>> -      *val = RESOURCE_UNI(res)->array_stride;
>> +      bool backed = backed_by_buffer_object(res);
>> +      if (backed && RESOURCE_UNI(res)->array_elements == 0)
>> +         *val = 0;
>> +      else
>> +         *val = backed ? RESOURCE_UNI(res)->array_stride : -1;
>>         return 1;
>> -   case GL_MATRIX_STRIDE:
>> +   }
>> +   case GL_MATRIX_STRIDE: {
>>         VALIDATE_TYPE_2(GL_UNIFORM, GL_BUFFER_VARIABLE);
>> -      *val = RESOURCE_UNI(res)->matrix_stride;
>> +         bool backed = backed_by_buffer_object(res);
>> +         if (backed && !RESOURCE_UNI(res)->type->is_matrix())
>> +            *val = 0;
>> +         else
>> +            *val = backed ? RESOURCE_UNI(res)->matrix_stride : -1;
>>         return 1;
>> +   }
>>      case GL_IS_ROW_MAJOR:
>>         VALIDATE_TYPE_2(GL_UNIFORM, GL_BUFFER_VARIABLE);
>>         *val = RESOURCE_UNI(res)->row_major;
>>
>