On 07/10/15 01:19, Timothy Arceri wrote:
> On Tue, 2015-10-06 at 15:07 +0200, Samuel Iglesias Gonsálvez wrote:
>> On 06/10/15 12:59, Timothy Arceri wrote:
>>> On Tue, 2015-10-06 at 10:08 +0200, Samuel Iglesias Gonsalvez wrote:
>>>> From ARB_program_query_interface:
>>>>
>>>> For the property ARRAY_SIZE, a single integer identifying the
>>>> number of
>>>> active array elements of an active variable is written to
>>>> <params>.
>>>> The
>>>> array size returned is in units of the type associated with the
>>>> property
>>>> TYPE. For active variables not corresponding to an array of
>>>> basic
>>>> types,
>>>> the value one is written to <params>. If the variable is a
>>>> shader
>>>> storage block member in an array with no declared size, the
>>>> value
>>>> zero
>>>> is written to <params>.
>>>>
>>>> v2:
>>>>
>>>> - Unsized arrays of arrays have an array size different than zero
>>>>
>>>> Signed-off-by: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
>>>> ---
>>>> src/mesa/main/shader_query.cpp | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/mesa/main/shader_query.cpp
>>>> b/src/mesa/main/shader_query.cpp
>>>> index dfc39f6..17076b8 100644
>>>> --- a/src/mesa/main/shader_query.cpp
>>>> +++ b/src/mesa/main/shader_query.cpp
>>>> @@ -1304,8 +1304,12 @@ _mesa_program_resource_prop(struct
>>>> gl_shader_program *shProg,
>>>> switch (res->Type) {
>>>> case GL_UNIFORM:
>>>> case GL_BUFFER_VARIABLE:
>>>> + if (RESOURCE_UNI(res)->is_shader_storage &&
>>>> + RESOURCE_UNI(res)->is_unsized_array)
>>>> + *val = RESOURCE_UNI(res)->array_elements;
>>>
>>> You don't need RESOURCE_UNI(res)->is_unsized_array here anymore
>>> right?
>>>
>>
>> I still needed it here: this rule only applies to unsized arrays. The
>> rest of variables need the MAX2(...) because active variables not
>> corresponding to an array of basic types should write the value one
>> in
>> '*val'.
>
> Right I forgot about types not being an array. My concern is adding yet
> another field to the uniform storage struct. IMO it already contains
> too many fields, since this is something that hangs around in memory
> for every uniform we should probably make at least some effort to keep
> it as small as possible.
>
> I haven't tested it but I think you could use (RESOURCE_UNI(res)
> ->array_stride > 0) as a test of whether the buffer is an array. So
> here you could have:
>
> if (RESOURCE_UNI(res)->is_shader_storage &&
> RESOURCE_UNI(res)->array_stride > 0)
>
> And in the patch you linked to [0] you would have:
>
> case GL_BUFFER_VARIABLE:
> if (RESOURCE_UNI(res)->array_stride > 0 &&
> RESOURCE_UNI(res)->array_elements == 0)
> return 1;
> else
> return RESOURCE_UNI(res)->array_elements;
>
> You also might want some comments to go with it but what do you think
> about this approach?
>
Yes, I like it. Using array_stride > 0 to detect if the buffer variable
is an array seems to be a good idea. I will prepare a new version of the
patches with that changed.
Thanks,
Sam
>>
>> If I remove it from here (just for testing), hundreds of dEQP GLES31
>> tests start to fail
>> (dEQP-GLES31.functional.ssbo.layout.single_basic_type.*.*).
>>
>> RESOURCE_UNI(res)->is_unsized_array is also needed in "main: consider
>> that unsized arrays have at least one active element" [0].
>>
>> Sam
>>
>> [0] http://lists.freedesktop.org/archives/mesa-dev/2015-October/09603
>> 8.html
>>
>>> RESOURCE_UNI(res)->is_shader_storage is all you need so you can
>>> also
>>> drop the patch that adds is_unsized_array to the uniform storage
>>> struct
>>>
>>> With that change: Reviewed-by: Timothy Arceri <
>>> t_arc...@yahoo.com.au>
>>>
>>>
>>>> + else
>>>> *val = MAX2(RESOURCE_UNI(res)->array_elements, 1);
>>>> - return 1;
>>>> + return 1;
>>>> case GL_PROGRAM_INPUT:
>>>> case GL_PROGRAM_OUTPUT:
>>>> *val = MAX2(_mesa_program_resource_array_size(res), 1);
>>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
