[Mesa-dev] [PATCH v2] glsl: Assign transform feedback varying slots in linker.

Paul Berry stereotype441 at gmail.com
Tue Nov 8 06:08:16 PST 2011


On 8 November 2011 05:15, Marek Olšák <maraeo at gmail.com> wrote:

> On Tue, Nov 8, 2011 at 7:28 AM, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > This patch modifies the GLSL linker to assign additional slots for
> > varying variables used by transform feedback, and record the varying
> > slots used by transform feedback for use by the driver back-end.
> >
> > This required modifying assign_varying_locations() so that it assigns
> > a varying location if either (a) the varying is used by the next stage
> > of the GL pipeline, or (b) the varying is required by transform
> > feedback.  In order to avoid duplicating the code to assign a single
> > varying location, I moved it into its own function,
> > assign_varying_location().
> >
> > In addition, to support transform feedback in the case where there is
> > no fragment shader, it is now possible to call
> > assign_varying_locations() with a consumer of NULL.
> > ---
> > Changes from v1:
> >
> > - Fixed loop bound in tfeedback_decl::store() (was this->vector_elements,
> >  should have been this->matrix_columns).
> >
> > - Fixed the case where transform feedback is in use but there is no
> fragment
> >  shader.
> >
> >  src/glsl/linker.cpp    |  552
> ++++++++++++++++++++++++++++++++++++++++++------
> >  src/mesa/main/mtypes.h |   13 ++
> >  2 files changed, 502 insertions(+), 63 deletions(-)
> >
> > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> > index 915d5bb..5cccd7f 100644
> > --- a/src/glsl/linker.cpp
> > +++ b/src/glsl/linker.cpp
> > @@ -1519,10 +1519,358 @@ demote_shader_inputs_and_outputs(gl_shader *sh,
> enum ir_variable_mode mode)
> >  }
> >
> >
> > +/**
> > + * Data structure tracking information about a transform feedback
> declaration
> > + * during linking.
> > + */
> > +class tfeedback_decl
> > +{
> > +public:
> > +   bool init(struct gl_shader_program *prog, const void *mem_ctx,
> > +             const char *input);
> > +   static bool is_same(const tfeedback_decl &x, const tfeedback_decl
> &y);
> > +   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;
> > +
> > +
> > +   /**
> > +    * True if assign_location() has been called for this object.
> > +    */
> > +   bool is_assigned() const
> > +   {
> > +      return this->location != -1;
> > +   }
> > +
> > +   /**
> > +    * Determine whether this object refers to the variable var.
> > +    */
> > +   bool matches_var(ir_variable *var) const
> > +   {
> > +      return strcmp(var->name, this->var_name) == 0;
> > +   }
> > +
> > +   /**
> > +    * The total number of varying components taken up by this variable.
>  Only
> > +    * valid if is_assigned() is true.
> > +    */
> > +   unsigned num_components() const
> > +   {
> > +      return this->vector_elements * this->matrix_columns;
> > +   }
> > +
> > +private:
> > +   /**
> > +    * The name that was supplied to glTransformFeedbackVaryings.  Used
> for
> > +    * error reporting.
> > +    */
> > +   const char *orig_name;
> > +
> > +   /**
> > +    * The name of the variable, parsed from orig_name.
> > +    */
> > +   char *var_name;
> > +
> > +   /**
> > +    * True if the declaration in orig_name represents an array.
> > +    */
> > +   bool is_array;
> > +
> > +   /**
> > +    * If is_array is true, the array index that was specified in
> orig_name.
> > +    */
> > +   unsigned array_index;
> > +
> > +   /**
> > +    * The vertex shader output location that the linker assigned for
> this
> > +    * variable.  -1 if a location hasn't been assigned yet.
> > +    */
> > +   int location;
> > +
> > +   /**
> > +    * If location != -1, the number of vector elements in this
> variable, or 1
> > +    * if this variable is a scalar.
> > +    */
> > +   unsigned vector_elements;
> > +
> > +   /**
> > +    * If location != -1, the number of matrix columns in this variable,
> or 1
> > +    * if this variable is not a matrix.
> > +    */
> > +   unsigned matrix_columns;
> > +};
> > +
> > +
> > +/**
> > + * Initialize this object based on a string that was passed to
> > + * glTransformFeedbackVaryings.  If there is a parse error, the error is
> > + * reported using linker_error(), and false is returned.
> > + */
> > +bool
> > +tfeedback_decl::init(struct gl_shader_program *prog, const void
> *mem_ctx,
> > +                     const char *input)
> > +{
> > +   /* We don't have to be pedantic about what is a valid GLSL variable
> name,
> > +    * because any variable with an invalid name can't exist in the IR
> anyway.
> > +    */
> > +
> > +   this->location = -1;
> > +   this->orig_name = input;
> > +
> > +   const char *bracket = strrchr(input, '[');
> > +
> > +   if (bracket) {
> > +      this->var_name = ralloc_strndup(mem_ctx, input, bracket - input);
> > +      if (sscanf(bracket, "[%u]", &this->array_index) == 1) {
> > +         this->is_array = true;
> > +         return true;
> > +      }
> > +   } else {
> > +      this->var_name = ralloc_strdup(mem_ctx, input);
> > +      this->is_array = false;
> > +      return true;
> > +   }
> > +
> > +   linker_error(prog, "Cannot parse transform feedback varying %s",
> input);
> > +   return false;
> > +}
> > +
> > +
> > +/**
> > + * Determine whether two tfeedback_decl objects refer to the same
> variable and
> > + * array index (if applicable).
> > + */
> > +bool
> > +tfeedback_decl::is_same(const tfeedback_decl &x, const tfeedback_decl
> &y)
> > +{
> > +   if (strcmp(x.var_name, y.var_name) != 0)
> > +      return false;
> > +   if (x.is_array != y.is_array)
> > +      return false;
> > +   if (x.is_array && x.array_index != y.array_index)
> > +      return false;
> > +   return true;
> > +}
> > +
> > +
> > +/**
> > + * Assign a location for this tfeedback_decl object based on the
> location
> > + * assignment in output_var.
> > + *
> > + * If an error occurs, the error is reported through linker_error() and
> false
> > + * is returned.
> > + */
> > +bool
> > +tfeedback_decl::assign_location(struct gl_context *ctx,
> > +                                struct gl_shader_program *prog,
> > +                                ir_variable *output_var)
> > +{
> > +   if (output_var->type->is_array()) {
> > +      /* 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);
> > +         return false;
> > +      }
> > +      /* Check array bounds. */
> > +      if (this->array_index >=
> > +          (unsigned) output_var->type->array_size()) {
> > +         linker_error(prog, "Transform feedback varying %s has index "
> > +                      "%i, but the array size is %i.",
> > +                      this->orig_name, this->array_index,
> > +                      output_var->type->array_size());
> > +      }
> > +      const unsigned matrix_cols =
> > +         output_var->type->fields.array->matrix_columns;
> > +      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;
> > +   } else {
> > +      /* Regular variable (scalar, vector, or matrix) */
> > +      if (this->is_array) {
> > +         linker_error(prog, "Transform feedback varying %s found, "
> > +                      "but it's an array ([] expected).",
> > +                      this->orig_name);
> > +         return false;
> > +      }
> > +      this->location = output_var->location;
> > +      this->vector_elements = output_var->type->vector_elements;
> > +      this->matrix_columns = output_var->type->matrix_columns;
> > +   }
> > +   /* From GL_EXT_transform_feedback:
> > +    *   A program will fail to link if:
> > +    *
> > +    *   * the total number of components to capture in any varying
> > +    *     variable in <varyings> is greater than the constant
> > +    *     MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS_EXT and the
> > +    *     buffer mode is SEPARATE_ATTRIBS_EXT;
> > +    */
> > +   if (prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS &&
> > +       this->num_components() >
> > +       ctx->Const.MaxTransformFeedbackSeparateComponents) {
> > +      linker_error(prog, "Transform feedback varying %s exceeds "
> > +                   "MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS.",
> > +                   this->orig_name);
> > +      return false;
> > +   }
> > +
> > +   return true;
> > +}
> > +
> > +
> > +/**
> > + * Update gl_transform_feedback_info to reflect this tfeedback_decl.
> > + *
> > + * If an error occurs, the error is reported through linker_error() and
> false
> > + * is returned.
> > + */
> > +bool
> > +tfeedback_decl::store(struct gl_shader_program *prog,
> > +                      struct gl_transform_feedback_info *info,
> > +                      unsigned buffer) const
> > +{
> > +   if (!this->is_assigned()) {
> > +      /* From GL_EXT_transform_feedback:
> > +       *   A program will fail to link if:
> > +       *
> > +       *   * any variable name specified in the <varyings> array is not
> > +       *     declared as an output in the geometry shader (if present)
> or
> > +       *     the vertex shader (if no geometry shader is present);
> > +       */
> > +      linker_error(prog, "Transform feedback varying %s undeclared.",
> > +                   this->orig_name);
> > +      return false;
> > +   }
> > +   for (unsigned v = 0; v < this->matrix_columns; ++v) {
> > +      info->Outputs[info->NumOutputs].OutputRegister = this->location +
> v;
> > +      info->Outputs[info->NumOutputs].NumComponents =
> this->vector_elements;
> > +      info->Outputs[info->NumOutputs].OutputBuffer = buffer;
> > +      ++info->NumOutputs;
> > +   }
> > +   return true;
> > +}
> > +
> > +
> > +/**
> > + * Parse all the transform feedback declarations that were passed to
> > + * glTransformFeedbackVaryings() and store them in tfeedback_decl
> objects.
> > + *
> > + * If an error occurs, the error is reported through linker_error() and
> false
> > + * is returned.
> > + */
> > +static bool
> > +parse_tfeedback_decls(struct gl_shader_program *prog, const void
> *mem_ctx,
> > +                      unsigned num_names, char **varying_names,
> > +                      tfeedback_decl *decls)
> > +{
> > +   for (unsigned i = 0; i < num_names; ++i) {
> > +      if (!decls[i].init(prog, mem_ctx, varying_names[i]))
> > +         return false;
> > +      /* From GL_EXT_transform_feedback:
> > +       *   A program will fail to link if:
> > +       *
> > +       *   * any two entries in the <varyings> array specify the same
> varying
> > +       *     variable;
> > +       *
> > +       * We interpret this to mean "any two entries in the <varyings>
> array
> > +       * specify the same varying variable and array index", since
> transform
> > +       * feedback of arrays would be useless otherwise.
> > +       */
> > +      for (unsigned j = 0; j < i; ++j) {
> > +         if (tfeedback_decl::is_same(decls[i], decls[j])) {
> > +            linker_error(prog, "Transform feedback varying %s specified
> "
> > +                         "more than once.", varying_names[i]);
> > +         }
> > +         return false;
>
> The "return false" statement should be put next to linker_error.
> Otherwise it always fails when the number of TFB varyings is at least
> 2.
>

Whoops, you're right of course.


>
> With that fixed, this patch is:
>
> Reviewed-by: Marek Olšák <maraeo at gmail.com>
> Tested-by: Marek Olšák <maraeo at gmail.com>
>

Thanks for the quick turnaround, Marek.  I'll wait until the end of the day
to push this upstream, in case anyone else has any review comments.

Paul
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111108/d8353870/attachment-0001.htm>


More information about the mesa-dev mailing list