From 9d36c96d6ec9f2c05c8e0b9ef18c5462cddee8c1 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 2 Jan 2012 17:08:13 -0800 Subject: [PATCH] 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 --- src/glsl/linker.cpp | 40 +++++++++++++++++++++++++++++++++------ src/mesa/main/mtypes.h | 21 +++++++++++++++++++- src/mesa/main/transformfeedback.c | 37 +++++++++++------------------------- 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 9e8975e887f..3dd088324ab 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); + 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 35458e396ad..50d7e341f56 100644 --- 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 02681c61583..c2114c22766 100644 --- 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; } -- 2.11.0