glsl: Size TCS->TES unsized arrays to gl_MaxPatchVertices for queries.
authorKenneth Graunke <kenneth@whitecape.org>
Tue, 25 Oct 2016 10:38:54 +0000 (03:38 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Thu, 27 Oct 2016 07:56:51 +0000 (00:56 -0700)
SSO validation and other program interface queries want to see that
unsized (non-patch) TCS output/TES input arrays are implicitly sized
to gl_MaxPatchVertices.

By the time we create the program resource lists, we've sized the arrays
to their actual size.  (We try to create TCS output arrays to match the
output patch size right away, and at this point, we should have shrunk
TES input arrays.)  One option would be to keep them sized to
gl_MaxPatchVertices, and defer shrinking them.  But that's a big change,
and I don't think it's a good idea.

Instead, this patch introduces a new ir_variable flag which indicates
the variable is implicitly to gl_MaxPatchVertices.  Then, the linker
munges the types when creating the resource list, ignoring the size
in the IR's types.  Basically, lie about it for resource queries.
It's ugly, but I think it ought to work.

We probably could use var->data.implicit_sized_array for this, but
I opted for a separate bit to try and avoid convoluting the existing
SSBO handling.  They're similar in concept, but share none of the
same code...

Fixes:
ES31-CTS.core.tessellation_shader.single.xfb_captures_data_from_correct_stage
and the ES32-CTS and ESEXT-CTS variants.

v2: Add a comment (requested by Timothy, written by me).

Cc: mesa-stable@lists.freedesktop.org
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>
Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
src/compiler/glsl/ast_to_hir.cpp
src/compiler/glsl/ir.h
src/compiler/glsl/linker.cpp
src/compiler/glsl/lower_named_interface_blocks.cpp

index adedcbbdf3ccc8920b2796136af1e016f82d4365..a9a1ba3c48cdec20ec5ba0f9470f38484503bc13 100644 (file)
@@ -4330,6 +4330,8 @@ handle_tess_ctrl_shader_output_decl(struct _mesa_glsl_parse_state *state,
    if (var->data.patch)
       return;
 
+   var->data.tess_varying_implicit_sized_array = var->type->is_unsized_array();
+
    validate_layout_qualifier_vertex_count(state, loc, var, num_vertices,
                                           &state->tcs_output_size,
                                           "tessellation control shader output");
@@ -4366,6 +4368,7 @@ handle_tess_shader_input_decl(struct _mesa_glsl_parse_state *state,
    if (var->type->is_unsized_array()) {
       var->type = glsl_type::get_array_instance(var->type->fields.array,
             state->Const.MaxPatchVertices);
+      var->data.tess_varying_implicit_sized_array = true;
    } else if (var->type->length != state->Const.MaxPatchVertices) {
       _mesa_glsl_error(&loc, state,
                        "per-vertex tessellation shader input arrays must be "
index 3d28dd5731ff8d7cd245dfcc24e59dc1b3c49abd..f07e3b20910d4f51c99e5ca80aca38a760142dfd 100644 (file)
@@ -832,6 +832,12 @@ public:
 
       unsigned implicit_sized_array:1;
 
+      /**
+       * Is this a non-patch TCS output / TES input array that was implicitly
+       * sized to gl_MaxPatchVertices?
+       */
+      unsigned tess_varying_implicit_sized_array:1;
+
       /**
        * Whether this is a fragment shader output implicitly initialized with
        * the previous contents of the specified render target at the
index 116e17b500487e9331a149dd6fc6b8030c6da25f..215fcc409f7dc19a0386cee5e4570e91a834737c 100644 (file)
@@ -3584,6 +3584,7 @@ static gl_shader_variable *
 create_shader_variable(struct gl_shader_program *shProg,
                        const ir_variable *in,
                        const char *name, const glsl_type *type,
+                       const glsl_type *interface_type,
                        bool use_implicit_location, int location,
                        const glsl_type *outermost_struct_type)
 {
@@ -3641,7 +3642,7 @@ create_shader_variable(struct gl_shader_program *shProg,
 
    out->type = type;
    out->outermost_struct_type = outermost_struct_type;
-   out->interface_type = in->get_interface_type();
+   out->interface_type = interface_type;
    out->component = in->data.location_frac;
    out->index = in->data.index;
    out->patch = in->data.patch;
@@ -3653,6 +3654,17 @@ create_shader_variable(struct gl_shader_program *shProg,
    return out;
 }
 
+static const glsl_type *
+resize_to_max_patch_vertices(const struct gl_context *ctx,
+                             const glsl_type *type)
+{
+   if (!type)
+      return NULL;
+
+   return glsl_type::get_array_instance(type->fields.array,
+                                        ctx->Const.MaxPatchVertices);
+}
+
 static bool
 add_shader_variable(const struct gl_context *ctx,
                     struct gl_shader_program *shProg,
@@ -3699,6 +3711,29 @@ add_shader_variable(const struct gl_context *ctx,
    }
 
    default: {
+      const glsl_type *interface_type = var->get_interface_type();
+
+      /* Unsized (non-patch) TCS output/TES input arrays are implicitly
+       * sized to gl_MaxPatchVertices.  Internally, we shrink them to a
+       * smaller size.
+       *
+       * This can cause trouble with SSO programs.  Since the TCS declares
+       * the number of output vertices, we can always shrink TCS output
+       * arrays.  However, the TES might not be linked with a TCS, in
+       * which case it won't know the size of the patch.  In other words,
+       * the TCS and TES may disagree on the (smaller) array sizes.  This
+       * can result in the resource names differing across stages, causing
+       * SSO validation failures and other cascading issues.
+       *
+       * Expanding the array size to the full gl_MaxPatchVertices fixes
+       * these issues.  It's also what program interface queries expect,
+       * as that is the official size of the array.
+       */
+      if (var->data.tess_varying_implicit_sized_array) {
+         type = resize_to_max_patch_vertices(ctx, type);
+         interface_type = resize_to_max_patch_vertices(ctx, interface_type);
+      }
+
       /* Issue #16 of the ARB_program_interface_query spec says:
        *
        * "* If a variable is a member of an interface block without an
@@ -3711,8 +3746,7 @@ add_shader_variable(const struct gl_context *ctx,
        */
       const char *prefixed_name = (var->data.from_named_ifc_block &&
                                    !is_gl_identifier(var->name))
-         ? ralloc_asprintf(shProg, "%s.%s", var->get_interface_type()->name,
-                           name)
+         ? ralloc_asprintf(shProg, "%s.%s", interface_type->name, name)
          : name;
 
       /* The ARB_program_interface_query spec says:
@@ -3723,6 +3757,7 @@ add_shader_variable(const struct gl_context *ctx,
        */
       gl_shader_variable *sha_v =
          create_shader_variable(shProg, var, prefixed_name, type,
+                                interface_type,
                                 use_implicit_location, location,
                                 outermost_struct_type);
       if (!sha_v)
index a00e60dd7711c0dbc9937f70cf8f03b65ec17f7e..53ef638c9c42e5aff582a1e8289100e1fa4e70fe 100644 (file)
@@ -193,6 +193,8 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions)
             new_var->data.patch = iface_t->fields.structure[i].patch;
             new_var->data.stream = var->data.stream;
             new_var->data.how_declared = var->data.how_declared;
+            new_var->data.tess_varying_implicit_sized_array =
+               var->data.tess_varying_implicit_sized_array;
             new_var->data.from_named_ifc_block = 1;
 
             new_var->init_interface_type(var->type);