[Mesa-dev] [PATCH v2] mesa: fix ARRAY_SIZE query for GetProgramResourceiv

[Tapani Pälli]

On 09/04/2015 01:39 PM, Martin Peres wrote:
> On 03/08/15 10:04, Tapani Pälli wrote:
>> Fixes rest of the failures with
>>     ES31-CTS.program_interface_query.no-locations
>>
>> v2: make additional check only for GS inputs
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>   src/mesa/main/shader_query.cpp | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/shader_query.cpp
>> b/src/mesa/main/shader_query.cpp
>> index b49fd38..c1a7660 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -475,8 +475,15 @@ _mesa_program_resource_array_size(struct
>> gl_program_resource *res)
>>         return RESOURCE_XFB(res)->Size > 1 ?
>>                RESOURCE_XFB(res)->Size : 0;
>>      case GL_PROGRAM_INPUT:
>> +   /* Special case with SSO where we have gl_Position as GS program
>> +    * input (no VS available).
>> +    */
>
> It would be good to get a quote from the opengl spec on this.

Urgh, now when getting back to this I've realized this is wrong. I made 
this change to pass resource-query Piglit test but I think either the 
test is wrong *or* maybe we should not expose gl_Position as a GS 
program input .. not sure, this is kind of a weird case with SSO. Will 
try to understand better how this should be handled.


>
>> +   if (res->StageReferences & (1 << MESA_SHADER_GEOMETRY) &&
>> +       is_active_attrib(RESOURCE_VAR(res)))
>> +      return 0;
>> +   /* fallthrough */
>>      case GL_PROGRAM_OUTPUT:
>> -      return RESOURCE_VAR(res)->data.max_array_access;
>> +      return RESOURCE_VAR(res)->type->length;
>
> This change looks good. Why did you use the max_array_acces before? I do
> not get it...

Neither do I :/ Length should be used everywhere.

>>      case GL_UNIFORM:
>>      case GL_VERTEX_SUBROUTINE_UNIFORM:
>>      case GL_GEOMETRY_SUBROUTINE_UNIFORM:
>> @@ -991,7 +998,7 @@ _mesa_program_resource_prop(struct
>> gl_shader_program *shProg,
>>               return 1;
>>         case GL_PROGRAM_INPUT:
>>         case GL_PROGRAM_OUTPUT:
>> -         *val = MAX2(RESOURCE_VAR(res)->type->length, 1);
>> +         *val = MAX2(_mesa_program_resource_array_size(res), 1);
> This makes sense too.
>
> With the quote added (if possible), and a piglit test for regression:
>
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>

Thanks a lot for getting back to this!

// Tapani