mesa: Fix glGetTransformFeedbackVarying().

The current implementation was totally broken -- it was looking in an
unpopulated structure for varyings, and trying to do so using the
current list of varying names, not the list used at link time.

v2: Fix leaking of memory into the program per re-link.

Reviewed-by: Paul Berry <stereotype441@gmail.com>

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 9e8975e..3dd0883 100644 (file)
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1382,7 +1382,8 @@ public:
    bool assign_location(struct gl_context *ctx, struct gl_shader_program *prog,
                         ir_variable *output_var);
    bool store(struct gl_shader_program *prog,
-              struct gl_transform_feedback_info *info, unsigned buffer) const;
+              struct gl_transform_feedback_info *info, unsigned buffer,
+             unsigned varying) const;
 
 
    /**
@@ -1413,7 +1414,7 @@ public:
 private:
    /**
     * The name that was supplied to glTransformFeedbackVaryings.  Used for
-    * error reporting.
+    * error reporting and glGetTransformFeedbackVarying().
     */
    const char *orig_name;
 
@@ -1449,6 +1450,9 @@ private:
     * if this variable is not a matrix.
     */
    unsigned matrix_columns;
+
+   /** Type of the varying returned by glGetTransformFeedbackVarying() */
+   GLenum type;
 };
 
 
@@ -1520,8 +1524,9 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
       /* Array variable */
       if (!this->is_array) {
          linker_error(prog, "Transform feedback varying %s found, "
-                      "but it's not an array ([] not expected).",
-                      this->orig_name);
+                      "but array dereference required for varying %s[%d]).",
+                      this->orig_name,
+                     output_var->name, output_var->type->length);
          return false;
       }
       /* Check array bounds. */
@@ -1538,6 +1543,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
       this->location = output_var->location + this->array_index * matrix_cols;
       this->vector_elements = output_var->type->fields.array->vector_elements;
       this->matrix_columns = matrix_cols;
+      this->type = GL_NONE;
    } else {
       /* Regular variable (scalar, vector, or matrix) */
       if (this->is_array) {
@@ -1549,6 +1555,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
       this->location = output_var->location;
       this->vector_elements = output_var->type->vector_elements;
       this->matrix_columns = output_var->type->matrix_columns;
+      this->type = output_var->type->gl_type;
    }
    /* From GL_EXT_transform_feedback:
     *   A program will fail to link if:
@@ -1580,7 +1587,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
 bool
 tfeedback_decl::store(struct gl_shader_program *prog,
                       struct gl_transform_feedback_info *info,
-                      unsigned buffer) const
+                      unsigned buffer, unsigned varying) const
 {
    if (!this->is_assigned()) {
       /* From GL_EXT_transform_feedback:
@@ -1602,6 +1609,16 @@ tfeedback_decl::store(struct gl_shader_program *prog,
       ++info->NumOutputs;
       info->BufferStride[buffer] += this->vector_elements;
    }
+
+   info->Varyings[varying].Name = ralloc_strdup(prog, this->orig_name);
+   info->Varyings[varying].Type = this->type;
+   /* Since we require that transform feedback varyings dereference
+    * arrays, there's always only one element of the GL datatype above
+    * present in this varying.
+    */
+   info->Varyings[varying].Size = 1;
+   info->NumVarying++;
+
    return true;
 }
 
@@ -1871,10 +1888,21 @@ store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog,
           sizeof(prog->LinkedTransformFeedback));
    prog->LinkedTransformFeedback.NumBuffers =
       separate_attribs_mode ? num_tfeedback_decls : 1;
+
+   ralloc_free(prog->LinkedTransformFeedback.Varyings);
+
+   memset(&prog->LinkedTransformFeedback, 0,
+          sizeof(prog->LinkedTransformFeedback));
+
+   prog->LinkedTransformFeedback.Varyings =
+      rzalloc_array(prog->LinkedTransformFeedback.Varyings,
+                   struct gl_transform_feedback_varying_info,
+                   num_tfeedback_decls);
+
    for (unsigned i = 0; i < num_tfeedback_decls; ++i) {
       unsigned buffer = separate_attribs_mode ? i : 0;
       if (!tfeedback_decls[i].store(prog, &prog->LinkedTransformFeedback,
-                                    buffer))
+                                    buffer, i))
          return false;
       total_tfeedback_components += tfeedback_decls[i].num_components();
    }

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 35458e3..50d7e34 100644 (file)
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1817,6 +1817,12 @@ struct prog_instruction;
 struct gl_program_parameter_list;
 struct gl_uniform_list;
 
+struct gl_transform_feedback_varying_info {
+   char *Name;
+   GLenum Type;
+   GLint Size;
+};
+
 /** Post-link transform feedback info. */
 struct gl_transform_feedback_info {
    unsigned NumOutputs;
@@ -1835,6 +1841,13 @@ struct gl_transform_feedback_info {
       unsigned DstOffset;
    } Outputs[MAX_PROGRAM_OUTPUTS];
 
+   /** Transform feedback varyings used for the linking of this shader program.
+    *
+    * Use for glGetTransformFeedbackVarying().
+    */
+   struct gl_transform_feedback_varying_info *Varyings;
+   GLint NumVarying;
+
    /**
     * Total number of components stored in each buffer.  This may be used by
     * hardware back-ends to determine the correct stride when interleaving
@@ -2227,7 +2240,13 @@ struct gl_shader_program
     */
    struct string_to_uint_map *FragDataBindings;
 
-   /** Transform feedback varyings */
+   /**
+    * Transform feedback varyings last specified by
+    * glTransformFeedbackVaryings().
+    *
+    * For the current set of transform feeedback varyings used for transform
+    * feedback output, see LinkedTransformFeedback.
+    */
    struct {
       GLenum BufferMode;
       GLuint NumVarying;

diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c
index 02681c6..c2114c2 100644 (file)
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -694,8 +694,7 @@ _mesa_GetTransformFeedbackVarying(GLuint program, GLuint index,
                                   GLsizei *size, GLenum *type, GLchar *name)
 {
    const struct gl_shader_program *shProg;
-   const GLchar *varyingName;
-   GLint v;
+   const struct gl_transform_feedback_info *linked_xfb_info;
    GET_CURRENT_CONTEXT(ctx);
 
    shProg = _mesa_lookup_shader_program(ctx, program);
@@ -705,36 +704,22 @@ _mesa_GetTransformFeedbackVarying(GLuint program, GLuint index,
       return;
    }
 
-   if (index >= shProg->TransformFeedback.NumVarying) {
+   linked_xfb_info = &shProg->LinkedTransformFeedback;
+   if (index >= linked_xfb_info->NumVarying) {
       _mesa_error(ctx, GL_INVALID_VALUE,
                   "glGetTransformFeedbackVaryings(index=%u)", index);
       return;
    }
 
-   varyingName = shProg->TransformFeedback.VaryingNames[index];
+   /* return the varying's name and length */
+   _mesa_copy_string(name, bufSize, length,
+                    linked_xfb_info->Varyings[index].Name);
 
-   v = _mesa_lookup_parameter_index(shProg->Varying, -1, varyingName);
-   if (v >= 0) {
-      struct gl_program_parameter *param = &shProg->Varying->Parameters[v];
-
-      /* return the varying's name and length */
-      _mesa_copy_string(name, bufSize, length, varyingName);
-
-      /* return the datatype and value's size (in datatype units) */
-      if (type)
-         *type = param->DataType;
-      if (size)
-         *size = param->Size;
-   }
-   else {
-      name[0] = 0;
-      if (length)
-         *length = 0;
-      if (type)
-         *type = 0;
-      if (size)
-         *size = 0;
-   }
+   /* return the datatype and value's size (in datatype units) */
+   if (type)
+      *type = linked_xfb_info->Varyings[index].Type;
+   if (size)
+      *size = linked_xfb_info->Varyings[index].Size;
 }