[Mesa-dev] [PATCH V2 6/8] glsl: Allow arrays of arrays as input to vertex shader

[Paul Berry]

On 21 January 2014 18:23, Paul Berry <stereotype441 at gmail.com> wrote:

> On 21 January 2014 04:19, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
>
>> Signed-off-by: Timothy Arceri <t_arceri at yahoo.com.au>
>> ---
>>  src/glsl/ast_to_hir.cpp | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 226d128..62b7ec2 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -3176,6 +3176,10 @@ ast_declarator_list::hir(exec_list *instructions,
>>                if (state->is_version(120, 300))
>>                   break;
>>                /* FALLTHROUGH */
>> +            case GLSL_TYPE_ARRAY:
>> +               if (state->ARB_arrays_of_arrays_enable)
>> +                  break;
>> +               /* FALLTHROUGH */
>>             default:
>>                _mesa_glsl_error(& loc, state,
>>                                 "vertex shader input / attribute cannot
>> have "
>>
>
> I see two problems with this:
>
> 1. Since the case above has a fall-through, this has an unintended side
> effect: if ARB_arrays_of_arrays is enabled, then integer vertex attributes
> will be allowed prior to GLSL 1.20.  Although it's unlikely that anyone
> will ever try to write a GLSL 1.10 shader using ARB_arrays_of_arrays, it
> would be nice to get the logic right.
>
> 2. When the type of the input is an array of arrays, we need to recurse
> through it to make sure it doesn't contain any prohibited types (e.g. an
> array of arrays of structs should still be prohibited).
>
> I think the easiest solution to this would be to replace the line:
>
>         const glsl_type *check_type = var->type->is_array()
>            ? var->type->fields.array : var->type;
>
> with something like this:
>
>             const glsl_type *check_type = var->type;
>             while (check_type->is_array())
>                check_type = check_type->element_type();
>
> Then we can leave the switch statement alone.
>
> With that fixed, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>
>

As an aside, this is just one example of what I suspect are many places
where the GLSL compiler makes the implicit assumption that an array can't
contain an array.  Once we think ARB_arrays_of_arrays support is complete,
it would probably be worth it for someone with a lot of familiarity with
the GLSL front end to grep for is_array() to make sure we haven't missed
anything :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140121/0de29c50/attachment.html>