[Mesa-dev] [PATCH 1/2] mesa: Fix glGetTransformFeedbackVarying().
Ian Romanick
idr at freedesktop.org
Tue Jan 3 19:19:16 PST 2012
On 01/03/2012 06:52 PM, Eric Anholt wrote:
> 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);
Judging from the ralloc_free in the next hunk, is prog the right
context? It seems like info->Varyings is better.
> + 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;
> }
>
> @@ -1865,13 +1882,21 @@ store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog,
> tfeedback_decl *tfeedback_decls)
> {
> unsigned total_tfeedback_components = 0;
> +
> + ralloc_free(prog->LinkedTransformFeedback.Varyings);
> +
> memset(&prog->LinkedTransformFeedback, 0,
> sizeof(prog->LinkedTransformFeedback));
> +
> + prog->LinkedTransformFeedback.Varyings =
> + rzalloc_array(prog, struct gl_transform_feedback_varying_info,
> + num_tfeedback_decls);
> +
> for (unsigned i = 0; i< num_tfeedback_decls; ++i) {
> unsigned buffer =
> prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS ? 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 107371e..9f0ef69 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;
> @@ -1830,6 +1836,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
> @@ -2222,7 +2235,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 be0d0ff..16ac5dd 100644
> --- a/src/mesa/main/transformfeedback.c
> +++ b/src/mesa/main/transformfeedback.c
> @@ -662,8 +662,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);
> @@ -673,36 +672,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;
> }
>
>
More information about the mesa-dev
mailing list