On 3 January 2012 19:19, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div>On 01/03/2012 06:52 PM, Eric Anholt wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. --- src/glsl/linker.cpp | 37 +++++++++++++++++++++++++++++++------ src/mesa/main/mtypes.h | 21 ++++++++++++++++++++- src/mesa/main/transformfeedback.c | 37 +++++++++++-------------------------- 3 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 6587008..bf72a55 100644 --- 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); </blockquote> </div></div> Judging from the ralloc_free in the next hunk, is prog the right context? It seems like info->Varyings is better. </blockquote><div> I was just about to make a similar comment. With that change, these two patches are Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> </div></div>