[Mesa-dev] [PATCH] glsl: Update ES 3.2 shader output restrictions.

[Kenneth Graunke]

On Monday, January 2, 2017 2:04:34 PM PST Iago Toral wrote:
> On Mon, 2017-01-02 at 03:32 -0800, Kenneth Graunke wrote:
> > This disallows fancy varyings in tessellation and geometry shaders,
> > as required by ES 3.2.
> > 
> > Fixes:
> > dEQP-
> > GLES31.functional.tessellation.user_defined_io.negative.per_patch_arr
> > ay_of_structs
> > dEQP-
> > GLES31.functional.tessellation.user_defined_io.negative.per_patch_str
> > ucts_containing_arrays
> > 
> > (Not a candidate for stable branches as it only disallows things
> > which
> > should be working as desktop GL allows them.)
> > 
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/compiler/glsl/ast_to_hir.cpp | 38
> > +++++++++++++++++++++++++++++++-------
> >  1 file changed, 31 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/compiler/glsl/ast_to_hir.cpp
> > b/src/compiler/glsl/ast_to_hir.cpp
> > index 9c633863b19..762f0a88bd3 100644
> > --- a/src/compiler/glsl/ast_to_hir.cpp
> > +++ b/src/compiler/glsl/ast_to_hir.cpp
> > @@ -5039,6 +5039,24 @@ ast_declarator_list::hir(exec_list
> > *instructions,
> >            *     * A matrix
> >            *     * A structure
> >            *     * An array of array
> > +          *
> > +          * ES 3.20 updates this to apply to tessellation and
> > geometry shaders
> > +          * as well.  Because there are per-vertex arrays in the new
> > stages,
> > +          * it strikes the "array of..." rules and replaces them
> > with these:
> > +          *
> > +          *     * For per-vertex-arrayed variables (applies to
> > tessellation
> > +          *       control, tessellation evaluation and geometry
> > shaders):
> > +          *
> > +          *       * Per-vertex-arrayed arrays of arrays
> > +          *       * Per-vertex-arrayed arrays of structures
> > +          *
> > +          *     * For non-per-vertex-arrayed variables:
> > +          *
> > +          *       * An array of arrays
> > +          *       * An array of structures
> > +          *
> > +          * which basically says to unwrap the per-vertex aspect and
> > apply
> > +          * the old rules.
> >            */
> >           if (state->es_shader) {
> >              if (var->type->is_array() &&
> > @@ -5048,17 +5066,23 @@ ast_declarator_list::hir(exec_list
> > *instructions,
> >                                  "cannot have an array of arrays",
> >                                  _mesa_shader_stage_to_string(state-
> > >stage));
> >              }
> > -            if (state->stage == MESA_SHADER_VERTEX) {
> > -               if (var->type->is_array() &&
> > -                   var->type->fields.array->is_record()) {
> > +            if (state->stage <= MESA_SHADER_GEOMETRY) {
> > +               const glsl_type *type = var->type;
> > +
> > +               if (state->stage == MESA_SHADER_TESS_CTRL &&
> > +                   !var->data.patch && var->type->is_array()) {
> > +                  type = var->type->fields.array;
> > +               }
> > +
> > +               if (type->is_array() && type->fields.array-
> > >is_record()) {
> >                    _mesa_glsl_error(&loc, state,
> >                                     "vertex shader output "
> >                                     "cannot have an array of
> > structs");
> 
> Since this code now runs for stages other than vertex I think we want
> to fix the error message to use _mesa_shader_stage_to_string()  instead
> of the hard coded 'vertex' string.
> 
> >                 }
> > -               if (var->type->is_record()) {
> > -                  for (unsigned i = 0; i < var->type->length; i++) {
> > -                     if (var->type->fields.structure[i].type-
> > >is_array() ||
> > -                         var->type->fields.structure[i].type-
> > >is_record())
> > +               if (type->is_record()) {
> > +                  for (unsigned i = 0; i < type->length; i++) {
> > +                     if (type->fields.structure[i].type->is_array()
> > ||
> > +                         type->fields.structure[i].type-
> > >is_record())
> >                          _mesa_glsl_error(&loc, state,
> >                                           "vertex shader output
> > cannot have a "
> >                                           "struct that contains an "
> 
> Same thing here.
> 
> Otherwise, it looks good to me:
> 
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

Whoops.  Thanks :)  Fixed locally.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170102/fbc87cf8/attachment.sig>