[Mesa-dev] [PATCH v5 09/70] glsl: return error if unsized arrays are found in OpenGL ES

[Tapani Pälli]

On 09/15/2015 01:23 PM, Samuel Iglesias Gonsálvez wrote:
>
>
> On 15/09/15 12:14, Samuel Iglesias Gonsálvez wrote:
>>
>>
>> On 15/09/15 12:01, Tapani Pälli wrote:
>>>
>>>
>>> On 09/10/2015 04:35 PM, Iago Toral Quiroga wrote:
>>>> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>>>
>>>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>>> ---
>>>>    src/glsl/ast_to_hir.cpp | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>>> index 72c6459..b67ae70 100644
>>>> --- a/src/glsl/ast_to_hir.cpp
>>>> +++ b/src/glsl/ast_to_hir.cpp
>>>> @@ -6252,6 +6252,22 @@ ast_interface_block::hir(exec_list *instructions,
>>>>          else if (state->stage == MESA_SHADER_TESS_CTRL && var_mode ==
>>>> ir_var_shader_out)
>>>>             handle_tess_ctrl_shader_output_decl(state, loc, var);
>>>>
>>>> +      for (unsigned i = 0; i < num_variables; i++) {
>>>
>>> IMO this loop should run to 'num_variables - 1' since last item should
>>> be allowed to be unsized array.
>>>
>>> With this fixed,
>>> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
>>>
>>
>> Right. Thanks!
>>
>
> Mmm, actually this is right. Next patch is the one that allows unsized
> arrays only for the last member of a shader storage buffer object.
>
> This patch is checking if unsized arrays are found in OpenGL ES and
> return an error, before adding the proper support for them in next patch.
>
> What solution do you prefer?
>
> 1) Keep the patch as it is
> 2) Use num_variables - 1, but we are allowing any unsized array if it is
> defined in the last member of any block.
> 3) Modify the error message saying that unsized arrays are not allowed
> at this moment, so there is no confusion with it.

Huh sorry for confusion, I missed the part where this was ripped out as 
a separate patch. Yes, the next patch does the right thing. I choose 
solution 1.


> Sam
>
>> Sam
>>
>>>
>>>> +         /* From GLSL ES 3.10 spec, section 4.1.9 "Arrays":
>>>> +          *
>>>> +          * "If an array is declared as the last member of a shader
>>>> storage
>>>> +          * block and the size is not specified at compile-time, it is
>>>> +          * sized at run-time. In all other cases, arrays are sized only
>>>> +          * at compile-time."
>>>> +          */
>>>> +         if (state->es_shader && fields[i].type->is_unsized_array()) {
>>>> +             _mesa_glsl_error(&loc, state, "unsized array `%s'
>>>> definition: "
>>>> +                              "only last member of a shader storage
>>>> block "
>>>> +                              "can be defined as unsized array",
>>>> +                              fields[i].name);
>>>> +         }
>>>> +      }
>>>> +
>>>>          if (ir_variable *earlier =
>>>>              state->symbols->get_variable(this->instance_name)) {
>>>>             if (!redeclaring_per_vertex) {
>>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>