[Mesa-dev] [PATCH 19/19] glsl: Allow arrays of arrays in GLSL ES 3.10 and GLSL 4.30

[Ilia Mirkin]

On Sat, Jun 20, 2015 at 11:38 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> On Sat, 2015-06-20 at 19:35 -0400, Ilia Mirkin wrote:
>> On Sat, Jun 20, 2015 at 8:33 AM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
>> > ---
>> >  src/glsl/ast_to_hir.cpp       | 13 ++++++++-----
>> >  src/glsl/glsl_parser.yy       | 22 ++++++++++++++--------
>> >  src/glsl/glsl_parser_extras.h |  5 +++++
>> >  3 files changed, 27 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> > index 6d2dc2e..81b2765 100644
>> > --- a/src/glsl/ast_to_hir.cpp
>> > +++ b/src/glsl/ast_to_hir.cpp
>> > @@ -1939,12 +1939,15 @@ process_array_type(YYLTYPE *loc, const glsl_type *base,
>> >            *
>> >            * "Only one-dimensional arrays may be declared."
>> >            */
>> > -         if (!state->ARB_arrays_of_arrays_enable) {
>> > +         if (!state->has_arrays_of_arrays()) {
>> > +            const char *const requirement = state->es_shader
>> > +               ? "GLSL ES 310"
>> > +               : "GL_ARB_arrays_of_array or GLSL 430";
>>
>> I think everywhere I've seen it's GLSL ES 3.10 and GLSL 4.30. Also, it
>> should be arrays_of_array*s*
>
> Thanks for pointing out the missing 's' I'll fix that, however it seems
> more common to use GLSL ES 310 and GLSL 430 see glsl_parser_extras.h.
>
> I could only find one instance of "GLSL 4.30" in glsl_parser.yy I
> searched from 3.30-4.50

$ git grep -P 'GLSL (ES )?\d{3}'

reveals just a handful of instances:

src/gallium/drivers/freedreno/freedreno_screen.c:
{"glsl120",   FD_DBG_GLSL120,"Temporary flag to force GLSL 120 (rather
than 130) on a3xx+"},
src/glsl/glsl_parser_extras.h:         const char *const requirement =
"GL_ARB_gpu_shader5 extension or GLSL 400";
src/glsl/glsl_parser_extras.h:            ? "GLSL ES 300"
src/glsl/glsl_parser_extras.h:            :
"GL_ARB_explicit_attrib_location extension or GLSL 330";
src/glsl/glsl_parser_extras.h:            ?
"GL_EXT_separate_shader_objects extension or GLSL ES 310"
src/glsl/glsl_parser_extras.h:            :
"GL_ARB_separate_shader_objects extension or GLSL 420";
src/glsl/glsl_parser_extras.h:            ? "GLSL ES 310"
src/glsl/glsl_parser_extras.h:
"GL_ARB_explicit_attrib_location or GLSL 330.";

Which IMHO should be fixed. Compare that to

$ git grep -P 'GLSL (ES )?\d{1}\.\d{2}' src | wc -l
286

Most of which come from comments... I guess there are just a few that
end up in user strings:

src/glsl/ast_array_index.cpp:                             "expressions
will be forbidden in GLSL 1.30 "
src/glsl/ast_array_index.cpp:                        "expressions is
forbidden in GLSL 1.30 and "
src/glsl/ast_to_hir.cpp:                          "function `%s' in
GLSL ES 3.00", name);
src/glsl/ast_to_hir.cpp:                          "not allowed in GLSL
ES 1.00");
src/glsl/glsl_parser.yy:                            "(GLSL ES 1.00 or
GLSL 1.20 required)",
src/glsl/glsl_parser.yy:                                "%s qualifier
requires GLSL 4.30 or "
src/glsl/glsl_parser_extras.cpp:                          "GLSL 1.00
ES should be selected using "

The version strings reported also have the . in them:

const char *
glsl_compute_version_string(void *mem_ctx, bool is_es, unsigned version)
{
   return ralloc_asprintf(mem_ctx, "GLSL%s %d.%02d", is_es ? " ES" : "",
                          version / 100, version % 100);
}

IMHO it'd be better to standardize on one thing.

  -ilia