[Mesa-dev] [PATCH 6/7] RFC: nir/xfb_info: arrays of basic types adds just one varying

[Alejandro Piñeiro]

On OpenGL, a array of a simple type adds just one varying. So
gl_transform_feedback_varying_info struct defined at mtypes.h includes
the parameters Type (base_type) and Size (number of elements).

This commit checks this when the recursive add_var_xfb_outputs call
handles arrays, to ensure that just one is addded.

RFC: Until this point, all changes were reasonable, but this change is
(imho) ugly. My idea was introducing as less as possible changes on
the code, specially on its logic/flow. But this commit is almost a
hack. The ideal solution would be to change the focus of the recursive
function, focusing on varyings, and at each varying, recursively add
outputs. But that seems like an overkill for a pass that was
originally intended for consumers only caring about the outputs. So
perhaps ARB_gl_spirv should keep their own gathering pass, with
vayings and outputs, and let this one untouched for those that only
care on outputs.
---
 src/compiler/nir/nir_gather_xfb_info.c | 52 ++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 9 deletions(-)

diff --git a/src/compiler/nir/nir_gather_xfb_info.c b/src/compiler/nir/nir_gather_xfb_info.c
index 948b802a815..cb0e2724cab 100644
--- a/src/compiler/nir/nir_gather_xfb_info.c
+++ b/src/compiler/nir/nir_gather_xfb_info.c
@@ -36,23 +36,59 @@ nir_gather_xfb_info_create(void *mem_ctx, uint16_t output_count, uint16_t varyin
    return xfb;
 }
 
+static bool
+glsl_type_is_leaf(const struct glsl_type *type)
+{
+   if (glsl_type_is_struct(type) ||
+       (glsl_type_is_array(type) &&
+        (glsl_type_is_array(glsl_get_array_element(type)) ||
+         glsl_type_is_struct(glsl_get_array_element(type))))) {
+      return false;
+   } else {
+      return true;
+   }
+}
+
+static void
+add_var_xfb_varying(nir_xfb_info *xfb,
+                    nir_variable *var,
+                    unsigned offset,
+                    const struct glsl_type *type)
+{
+   nir_xfb_varying_info *varying = &xfb->varyings[xfb->varying_count++];
+
+   varying->type = type;
+   varying->buffer = var->data.xfb_buffer;
+   varying->offset = offset;
+   xfb->buffers[var->data.xfb_buffer].varying_count++;
+}
+
 static void
 add_var_xfb_outputs(nir_xfb_info *xfb,
                     nir_variable *var,
                     unsigned *location,
                     unsigned *offset,
-                    const struct glsl_type *type)
+                    const struct glsl_type *type,
+                    bool varying_added)
 {
    if (glsl_type_is_array(type) || glsl_type_is_matrix(type)) {
       unsigned length = glsl_get_length(type);
+      bool local_varying_added = varying_added;
+
       const struct glsl_type *child_type = glsl_get_array_element(type);
+      if (glsl_type_is_leaf(child_type)) {
+
+         add_var_xfb_varying(xfb, var, *offset, type);
+         local_varying_added = true;
+      }
+
       for (unsigned i = 0; i < length; i++)
-         add_var_xfb_outputs(xfb, var, location, offset, child_type);
+         add_var_xfb_outputs(xfb, var, location, offset, child_type, local_varying_added);
    } else if (glsl_type_is_struct(type)) {
       unsigned length = glsl_get_length(type);
       for (unsigned i = 0; i < length; i++) {
          const struct glsl_type *child_type = glsl_get_struct_field(type, i);
-         add_var_xfb_outputs(xfb, var, location, offset, child_type);
+         add_var_xfb_outputs(xfb, var, location, offset, child_type, varying_added);
       }
    } else {
       assert(var->data.xfb_buffer < NIR_MAX_XFB_BUFFERS);
@@ -83,11 +119,9 @@ add_var_xfb_outputs(nir_xfb_info *xfb,
       uint8_t comp_mask = ((1 << comp_slots) - 1) << var->data.location_frac;
       unsigned location_frac = var->data.location_frac;
 
-      nir_xfb_varying_info *varying = &xfb->varyings[xfb->varying_count++];
-      varying->type = type;
-      varying->buffer = var->data.xfb_buffer;
-      varying->offset = *offset;
-      xfb->buffers[var->data.xfb_buffer].varying_count++;
+      if (!varying_added) {
+         add_var_xfb_varying(xfb, var, *offset, type);
+      }
 
       assert(attrib_slots <= 2);
       for (unsigned s = 0; s < attrib_slots; s++) {
@@ -171,7 +205,7 @@ nir_gather_xfb_info(const nir_shader *shader, void *mem_ctx)
 
          unsigned location = var->data.location;
          unsigned offset = var->data.offset;
-         add_var_xfb_outputs(xfb, var, &location, &offset, var->type);
+         add_var_xfb_outputs(xfb, var, &location, &offset, var->type, false);
       }
    }
    assert(xfb->output_count == num_outputs);
-- 
2.14.1