[Mesa-dev] [PATCH 1/2] mesa: Fix glGetTransformFeedbackVarying().

[Paul Berry]

On 3 January 2012 19:19, Ian Romanick <idr at freedesktop.org> wrote:

> 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.
>

I was just about to make a similar comment.  With that change, these two
patches are

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120103/88a77b87/attachment.html>