[Mesa-dev] [PATCH] mesa: fix name returned for XFB varyings

[Tapani Pälli]

On 08/03/2015 02:47 PM, Martin Peres wrote:
>
>
> On 03/08/15 14:35, Tapani Pälli wrote:
>>
>>
>> On 08/03/2015 01:57 PM, Martin Peres wrote:
>>> On 03/08/15 13:50, Tapani Pälli wrote:
>>>>
>>>>
>>>> On 08/03/2015 01:48 PM, Martin Peres wrote:
>>>>> On 03/08/15 11:10, Tapani Pälli wrote:
>>>>>> _mesa_get_program_resource_name has logic to append '[0]' in name
>>>>>> if variable is an array, this should be skipped for XFB varyings
>>>>>> that have array index already appended.
>>>>>>
>>>>>> Fixes:
>>>>>> ES31-CTS.program_interface_query.transform-feedback-types
>>>>>>
>>>>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>>>>> ---
>>>>>>   src/mesa/main/shader_query.cpp | 6 ++++++
>>>>>>   1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/src/mesa/main/shader_query.cpp
>>>>>> b/src/mesa/main/shader_query.cpp
>>>>>> index c1a7660..a50c348 100644
>>>>>> --- a/src/mesa/main/shader_query.cpp
>>>>>> +++ b/src/mesa/main/shader_query.cpp
>>>>>> @@ -726,6 +726,12 @@ _mesa_get_program_resource_name(struct
>>>>>> gl_shader_program *shProg,
>>>>>>      bool add_index = !(((programInterface == GL_PROGRAM_INPUT) &&
>>>>>>                          res->StageReferences & (1 <<
>>>>>> MESA_SHADER_GEOMETRY)));
>>>>>> +   /* Transform feedback varyings have possible array index already
>>>>>> appended
>>>>>> +    * in their names.
>>>>>> +    */
>>>>>
>>>>> Possibly have the array index already appended? So how do you detect
>>>>> that here?
>>>>
>>>> By 'possible array index' I mean that not all the XFB varyings are
>>>> arrays. Maybe better text would be just:
>>>>
>>>> "Transform feedback varyings have array index already appended in
>>>> their names"
>>>>
>>>> If it's not array, then of course it does not have it.
>>>
>>> Yes, this comment makes a lot more sense. But I guess having this
>>> distinction is the problem. How much work would XFB varyings working in
>>> the same as any other resource when it comes to the name?
>>
>> XFB resources point to 'gl_transform_feedback_varying_info' which gets
>> generated with names that come as user input from
>> glTransformFeedbackVaryings function (and should match what comes out
>> from glGetTransformFeedbackVarying). I don't see a straightforward way
>> to change this without quite a bit of changes in the XFB code so I
>> would like to have this distinction, it is a different sort of
>> resource type.
>
> I see. Yeah, makes sense and the workaround is ok.
>
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>

Thanks. Now that I think about this more, the way how transform feedback 
varying name works seems not consistent with the program_interface_query 
spec and it could be we need to use the 'basename' after all (and then 
apply [0] if array). I will take a bit more look in to this. There's 
other places, for example GL_NAME_LENGTH query that would not work ok 
either.

// Tapani