[Mesa-dev] [PATCH 11/34] glsl: support compilation of geometry shaders

Ian Romanick idr at freedesktop.org
Wed Jul 31 17:16:55 PDT 2013


On 07/30/2013 03:16 PM, Kenneth Graunke wrote:
> On 07/28/2013 11:03 PM, Paul Berry wrote:
>> From: Bryan Cain <bryancain3 at gmail.com>
>>
>> This commit adds all of the parsing and semantics for GLSL 150 style
>> geometry shaders.
>>
>> v2 (Paul Berry <stereotype441 at gmail.com>): Adjust i965's
>> brw_link_shader() to pass the new is_geometry_shader argument to
>> do_set_program_inouts.  Add a few missing calls to
>> get_pipeline_stage().  Fix some signed/unsigned comparison warnings.
>> Fix handling of NULL consumer in assign_varying_locations().
>>
>> v3 (Bryan Cain <bryancain3 at gmail.com>): fix indexing order of 2D
>> arrays.  Also, allow interpolation qualifiers in geometry shaders.
>>
>> v4 (Paul Berry <stereotype441 at gmail.com>): Eliminate
>> get_pipeline_stage()--it is no longer needed thanks to 030ca23 (mesa:
>> renumber shader indices according to their placement in pipeline).
>> Remove 2D stuff.  Move vertices_per_prim() to ir.h, so that it will be
>> accessible from outside the linker.  Remove
>> inject_num_vertices_visitor.  Move use of geom_array_resize_visitor
>> prior to use of array_resizing_visitor.  Rework for GLSL 1.50.  Use a
>> single shader_type argument for do_set_program_inouts()
>> ---
>>   src/glsl/ast_to_hir.cpp                    |  31 +++++--
>>   src/glsl/ir.cpp                            |  21 +++++
>>   src/glsl/ir.h                              |   5 +-
>>   src/glsl/ir_set_program_inouts.cpp         |  86 +++++++++++++-----
>>   src/glsl/linker.cpp                        | 135
>> +++++++++++++++++++++++++++--
>>   src/mesa/drivers/dri/i965/brw_shader.cpp   |   3 +-
>>   src/mesa/main/mtypes.h                     |   4 +-
>>   src/mesa/program/ir_to_mesa.cpp            |   2 +-
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   2 +-
>>   9 files changed, 253 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index 2569cde..431a13d 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -2056,11 +2056,11 @@ apply_type_qualifier_to_variable(const struct
>> ast_type_qualifier *qual,
>>         var->interpolation = INTERP_QUALIFIER_NONE;
>>
>>      if (var->interpolation != INTERP_QUALIFIER_NONE &&
>> -       !(state->target == vertex_shader && var->mode ==
>> ir_var_shader_out) &&
>> -       !(state->target == fragment_shader && var->mode ==
>> ir_var_shader_in)) {
>> +       ((state->target == vertex_shader && var->mode ==
>> ir_var_shader_in) ||
>> +        (state->target == fragment_shader && var->mode ==
>> ir_var_shader_out))) {
>>         _mesa_glsl_error(loc, state,
>> -               "interpolation qualifier `%s' can only be applied to "
>> -               "vertex shader outputs and fragment shader inputs",
>> +               "interpolation qualifier `%s' cannot be applied to "
>> +               "vertex shader inputs or fragment shader outputs",
>>                  var->interpolation_string());
>>      }
>>
>> @@ -2662,6 +2662,27 @@ ast_declarator_list::hir(exec_list *instructions,
>>
>>         var = new(ctx) ir_variable(var_type, decl->identifier,
>> ir_var_auto);
>>
>> +      /* The 'varying in' and 'varying out' qualifiers can only be
>> used with
>> +       * ARB_geometry_shader4 and EXT_geometry_shader4, which we
>> don't support
>> +       * yet.
>> +       */
>> +      if (this->type->qualifier.flags.q.varying) {
>> +     if (this->type->qualifier.flags.q.in) {
>> +        _mesa_glsl_error(& loc, state,
>> +                 "`varying in' qualifier in declaration of "
>> +                 "`%s' only valid for geometry shaders using "
>> +                 "ARB_geometry_shader4 or EXT_geometry_shader4",
>> +                 decl->identifier);
>> +     }
>> +     else if (this->type->qualifier.flags.q.out) {
>
> Style-nit:
>
> } else if (this->type->qualifier.flagsq.out) {
>
> (Mesa historically has used the two-line style in your patch, but has
> largely moved away from that.  In particular, the compiler has always
> used "} else {".)
>
> Might be nice to kill the tabs too, but not a big deal either way.
>
>> +        _mesa_glsl_error(& loc, state,
>> +                 "`varying out' qualifier in declaration of "
>> +                 "`%s' only valid for geometry shaders using "
>> +                 "ARB_geometry_shader4 or EXT_geometry_shader4",
>> +                 decl->identifier);
>> +     }
>> +      }
>> +
>>         /* From page 22 (page 28 of the PDF) of the GLSL 1.10
>> specification;
>>          *
>>          *     "Global variables can only use the qualifiers const,
>> @@ -2906,7 +2927,7 @@ ast_declarator_list::hir(exec_list *instructions,
>>               }
>>               break;
>>            default:
>> -            assert(0);
>> +            break;
>>            }
>>         }
>>
>> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
>> index dad58de..1a3983e 100644
>> --- a/src/glsl/ir.cpp
>> +++ b/src/glsl/ir.cpp
>> @@ -1778,3 +1778,24 @@ ir_rvalue::as_rvalue_to_saturate()
>>
>>      return NULL;
>>   }
>> +
>> +
>> +unsigned
>> +vertices_per_prim(GLenum prim)
>> +{
>> +   switch (prim) {
>> +   case GL_POINTS:
>> +      return 1;
>> +   case GL_LINES:
>> +      return 2;
>> +   case GL_TRIANGLES:
>> +      return 3;
>> +   case GL_LINES_ADJACENCY_ARB:
>> +      return 4;
>> +   case GL_TRIANGLES_ADJACENCY_ARB:
>> +      return 6;
>> +   default:
>> +      assert(!"Bad primitive");
>> +      return 3;
>> +   }
>> +}
>> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
>> index ae79a39..af9d77e 100644
>> --- a/src/glsl/ir.h
>> +++ b/src/glsl/ir.h
>> @@ -2112,7 +2112,7 @@ ir_has_call(ir_instruction *ir);
>>
>>   extern void
>>   do_set_program_inouts(exec_list *instructions, struct gl_program *prog,
>> -                      bool is_fragment_shader);
>> +                      GLenum shader_type);
>
> This patch is getting pretty huge.  It might be nice to do the
> s/is_fragment_shader/shader_type/ in a separate patch, since that's an
> obvious hunk.
>
>>
>>   extern char *
>>   prototype_string(const glsl_type *return_type, const char *name,
>> @@ -2128,4 +2128,7 @@ extern void _mesa_print_ir(struct exec_list
>> *instructions,
>>   } /* extern "C" */
>>   #endif
>>
>> +unsigned
>> +vertices_per_prim(GLenum prim);
>> +
>>   #endif /* IR_H */
>> diff --git a/src/glsl/ir_set_program_inouts.cpp
>> b/src/glsl/ir_set_program_inouts.cpp
>> index 91a8b45..a578e2a 100644
>> --- a/src/glsl/ir_set_program_inouts.cpp
>> +++ b/src/glsl/ir_set_program_inouts.cpp
>> @@ -44,11 +44,10 @@
>>
>>   class ir_set_program_inouts_visitor : public ir_hierarchical_visitor {
>>   public:
>> -   ir_set_program_inouts_visitor(struct gl_program *prog,
>> -                                 bool is_fragment_shader)
>> +   ir_set_program_inouts_visitor(struct gl_program *prog, GLenum
>> shader_type)
>>      {
>>         this->prog = prog;
>> -      this->is_fragment_shader = is_fragment_shader;
>> +      this->shader_type = shader_type;
>>      }
>>      ~ir_set_program_inouts_visitor()
>>      {
>> @@ -61,7 +60,7 @@ public:
>>      virtual ir_visitor_status visit(ir_dereference_variable *);
>>
>>      struct gl_program *prog;
>> -   bool is_fragment_shader;
>> +   GLenum shader_type;
>>   };
>>
>>   static inline bool
>> @@ -112,14 +111,24 @@
>> ir_set_program_inouts_visitor::visit(ir_dereference_variable *ir)
>>         return visit_continue;
>>
>>      if (ir->type->is_array()) {
>> -      mark(this->prog, ir->var, 0,
>> -       ir->type->length * ir->type->fields.array->matrix_columns,
>> -           this->is_fragment_shader);
>> +      int matrix_columns = ir->type->fields.array->matrix_columns;
>> +      int length = ir->type->length;
>
> I was wondering if this was left over from the ARB_gs4 stuff. Apparently
> it's for lowered arrays of instance blocks?  Maybe a comment would be in
> order.
>
>> +      if (this->shader_type == GL_GEOMETRY_SHADER &&
>> +          ir->var->mode == ir_var_shader_in) {
>> +         if (ir->type->element_type()->is_array()) {
>> +            const glsl_type *inner_array_type = ir->type->fields.array;
>> +            matrix_columns =
>> inner_array_type->fields.array->matrix_columns;
>> +            length = inner_array_type->length;
>> +         } else {
>> +            length = 1;
>> +         }
>> +      }
>> +      mark(this->prog, ir->var, 0, length * matrix_columns,
>> +           this->shader_type == GL_FRAGMENT_SHADER);
>>      } else {
>>         mark(this->prog, ir->var, 0, ir->type->matrix_columns,
>> -           this->is_fragment_shader);
>> +           this->shader_type == GL_FRAGMENT_SHADER);
>>      }
>> -
>>      return visit_continue;
>>   }
>>
>> @@ -129,7 +138,40 @@
>> ir_set_program_inouts_visitor::visit_enter(ir_dereference_array *ir)
>>      ir_dereference_variable *deref_var;
>>      ir_constant *index = ir->array_index->as_constant();
>>      deref_var = ir->array->as_dereference_variable();
>> -   ir_variable *var = deref_var ? deref_var->var : NULL;
>> +   ir_variable *var;
>> +   bool is_vert_array = false, is_2D_array = false;
>> +
>> +   /* Check whether this dereference is of a GS input array.  These
>> are special
>> +    * because the array index refers to the index of an input vertex
>> instead of
>> +    * the attribute index.  The exceptions to this exception are 2D
>> arrays
>> +    * such as gl_TexCoordIn.  For these, there is a nested
>> dereference_array,
>> +    * where the inner index specifies the vertex and the outer index
>> specifies
>> +    * the attribute.  To complicate things further, matrix columns
>> are also
>> +    * accessed with dereference_array.  So we have to correctly
>> handle 1D
>> +    * arrays of non-matrices, 1D arrays of matrices, 2D arrays of
>> non-matrices,
>> +    * and 2D arrays of matrices.
>> +    */
>> +   if (this->shader_type == GL_GEOMETRY_SHADER) {
>> +      if (!deref_var) {
>> +         /* Either an outer (attribute) dereference of a 2D array or
>> a column
>> +          * dereference of an array of matrices. */
>
> Style-nit: */ goes on a separate line.
>
>> +         ir_dereference_array *inner_deref =
>> ir->array->as_dereference_array();
>> +         assert(inner_deref);
>> +         deref_var = inner_deref->array->as_dereference_variable();
>> +         is_2D_array = true;
>> +      }
>> +
>> +      if (deref_var && deref_var->var->mode == ir_var_shader_in) {
>> +         if (ir->type->is_array())
>> +            /* Inner (vertex) dereference of a 2D array */
>> +            return visit_continue;
>> +         else
>> +            /* Dereference of a 1D (vertex) array */
>> +            is_vert_array = true;
>> +      }
>> +   }
>
> I'm not terribly comfortable with this code, but I'm not sure what to
> suggest instead.  One concern I have is varying structs.
>
> As I understand it, if a VS outputs a struct, the corresponding GS input
> would be an array of structs.  Wouldn't those be accessed via (array_ref
> (record_ref ...) ...)?
>
> The code above seems to assume that if it's not
>    (array_ref (var_ref ...))
> then it *must* be
>    (array_ref (array_ref ...) ...)
> which seems wrong, or at least dubious.
>
> The "this must be a matrix" stuff concerns me too.
>
>> +
>> +   var = deref_var ? deref_var->var : NULL;
>>
>>      /* Check that we're dereferencing a shader in or out */
>>      if (!var || !is_shader_inout(var))
>> @@ -137,14 +179,17 @@
>> ir_set_program_inouts_visitor::visit_enter(ir_dereference_array *ir)
>>
>>      if (index) {
>>         int width = 1;
>> +      const glsl_type *type = is_vert_array ?
>> +                              deref_var->type->fields.array :
>> deref_var->type;
>> +      int offset = is_vert_array && !is_2D_array ? 0 :
>> index->value.i[0];
>>
>> -      if (deref_var->type->is_array() &&
>> -      deref_var->type->fields.array->is_matrix()) {
>> -     width = deref_var->type->fields.array->matrix_columns;
>> +      if (type->is_array() &&
>> +      type->fields.array->is_matrix()) {
>> +     width = type->fields.array->matrix_columns;
>>         }
>>
>> -      mark(this->prog, var, index->value.i[0] * width, width,
>> -           this->is_fragment_shader);
>> +      mark(this->prog, var, offset * width, width,
>> +           this->shader_type == GL_FRAGMENT_SHADER);
>>         return visit_continue_with_parent;
>>      }
>>
>> @@ -164,7 +209,8 @@
>> ir_set_program_inouts_visitor::visit_enter(ir_function_signature *ir)
>>   ir_visitor_status
>>   ir_set_program_inouts_visitor::visit_enter(ir_expression *ir)
>>   {
>> -   if (is_fragment_shader && ir->operation == ir_unop_dFdy) {
>> +   if (this->shader_type == GL_FRAGMENT_SHADER &&
>> +       ir->operation == ir_unop_dFdy) {
>>         gl_fragment_program *fprog = (gl_fragment_program *) prog;
>>         fprog->UsesDFdy = true;
>>      }
>> @@ -175,7 +221,7 @@ ir_visitor_status
>>   ir_set_program_inouts_visitor::visit_enter(ir_discard *)
>>   {
>>      /* discards are only allowed in fragment shaders. */
>> -   assert(is_fragment_shader);
>> +   assert(this->shader_type == GL_FRAGMENT_SHADER);
>>
>>      gl_fragment_program *fprog = (gl_fragment_program *) prog;
>>      fprog->UsesKill = true;
>> @@ -185,14 +231,14 @@
>> ir_set_program_inouts_visitor::visit_enter(ir_discard *)
>>
>>   void
>>   do_set_program_inouts(exec_list *instructions, struct gl_program *prog,
>> -                      bool is_fragment_shader)
>> +                      GLenum shader_type)
>>   {
>> -   ir_set_program_inouts_visitor v(prog, is_fragment_shader);
>> +   ir_set_program_inouts_visitor v(prog, shader_type);
>>
>>      prog->InputsRead = 0;
>>      prog->OutputsWritten = 0;
>>      prog->SystemValuesRead = 0;
>> -   if (is_fragment_shader) {
>> +   if (shader_type == GL_FRAGMENT_SHADER) {
>>         gl_fragment_program *fprog = (gl_fragment_program *) prog;
>>         memset(fprog->InterpQualifier, 0,
>> sizeof(fprog->InterpQualifier));
>>         fprog->IsCentroid = 0;
>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>> index 7192567..70d9cf4 100644
>> --- a/src/glsl/linker.cpp
>> +++ b/src/glsl/linker.cpp
>> @@ -73,11 +73,14 @@
>>   #include "linker.h"
>>   #include "link_varyings.h"
>>   #include "ir_optimization.h"
>> +#include "ir_rvalue_visitor.h"
>>
>>   extern "C" {
>>   #include "main/shaderobj.h"
>>   }
>>
>> +void linker_error(gl_shader_program *, const char *, ...);
>> +
>>   /**
>>    * Visitor that determines whether or not a variable is ever written.
>>    */
>> @@ -174,6 +177,77 @@ private:
>>   };
>>
>>
>> +class geom_array_resize_visitor : public ir_hierarchical_visitor {
>
> This looks like it might be the code to cross-validate
>
> layout(triangles)
> in vec4 foo[3];
> in vec4 foo[2];
>
> and such.  But only in part?  It might be nice to have that in a
> separate patch as well...it's complicated enough to not be lumped in
> with basic plumbing.
>
>> +public:
>> +   unsigned num_vertices;
>> +   gl_shader_program *prog;
>> +
>> +   geom_array_resize_visitor(unsigned num_vertices, gl_shader_program
>> *prog)
>> +   {
>> +      this->num_vertices = num_vertices;
>> +      this->prog = prog;
>> +   }
>> +
>> +   virtual ~geom_array_resize_visitor()
>> +   {
>> +      /* empty */
>> +   }
>> +
>> +   virtual ir_visitor_status visit(ir_variable *var)
>> +   {
>> +      if (!var->type->is_array() || var->mode != ir_var_shader_in)
>> +         return visit_continue;
>> +
>> +      unsigned size = var->type->length;
>> +
>> +      /* Generate a link error if the shader has declared this array
>> with an
>> +       * incorrect size.
>> +       */
>> +      if (size && size != this->num_vertices) {
>> +         linker_error(this->prog, "size of array %s declared as %u, "
>> +                      "but number of input vertices is %u\n",
>> +                      var->name, size, this->num_vertices);
>> +         return visit_continue;
>> +      }
>> +
>> +      /* Generate a link error if the shader attempts to access an input
>> +       * array using an index too large for its actual size assigned
>> at link
>> +       * time.
>> +       */
>> +      if (var->max_array_access >= this->num_vertices) {
>> +         linker_error(this->prog, "geometry shader accesses element
>> %i of "
>> +                      "%s, but only %i input vertices\n",
>> +                      var->max_array_access, var->name,
>> this->num_vertices);
>> +         return visit_continue;
>> +      }
>> +
>> +      var->type =
>> glsl_type::get_array_instance(var->type->element_type(),
>> +                                                this->num_vertices);
>> +      var->max_array_access = this->num_vertices - 1;
>> +
>> +      return visit_continue;
>> +   }
>> +
>> +   /* Dereferences of input variables need to be updated so that
>> their type
>> +    * matches the newly assigned type of the variable they are
>> accessing. */
>> +   virtual ir_visitor_status visit(ir_dereference_variable *ir)
>> +   {
>> +      ir->type = ir->var->type;
>> +      return visit_continue;
>> +   }
>> +
>> +   /* Dereferences of 2D input arrays need to be updated so that
>> their type
>> +    * matches the newly assigned type of the array they are
>> accessing. */
>> +   virtual ir_visitor_status visit_leave(ir_dereference_array *ir)
>> +   {
>> +      const glsl_type *const vt = ir->array->type;
>> +      if (vt->is_array())
>> +         ir->type = vt->element_type();
>> +      return visit_continue;
>> +   }
>> +};
>> +
>> +
>>   void
>>   linker_error(gl_shader_program *prog, const char *fmt, ...)
>>   {
>> @@ -437,6 +511,24 @@ validate_fragment_shader_executable(struct
>> gl_shader_program *prog,
>>      }
>>   }
>>
>> +/**
>> + * Verify that a geometry shader executable meets all semantic
>> requirements
>> + *
>> + * Also sets prog->Geom.VerticesIn as a side effect.
>> + *
>> + * \param shader Geometry shader executable to be verified
>> + */
>> +void
>> +validate_geometry_shader_executable(struct gl_shader_program *prog,
>> +                    struct gl_shader *shader)
>> +{
>> +   if (shader == NULL)
>> +      return;
>> +
>> +   unsigned num_vertices = vertices_per_prim(prog->Geom.InputType);
>> +   prog->Geom.VerticesIn = num_vertices;
>> +}
>> +
>>
>>   /**
>>    * Generate a string describing the mode of a variable
>> @@ -1091,6 +1183,16 @@ link_intrastage_shaders(void *mem_ctx,
>>      if (linked)
>>         validate_ir_tree(linked->ir);
>>
>> +   /* Set the size of geometry shader input arrays */
>> +   if (linked->Type == GL_GEOMETRY_SHADER) {
>> +      unsigned num_vertices = vertices_per_prim(prog->Geom.InputType);
>> +      geom_array_resize_visitor input_resize_visitor(num_vertices,
>> prog);
>> +      foreach_iter(exec_list_iterator, iter, *linked->ir) {
>> +         ir_instruction *ir = (ir_instruction *)iter.get();
>> +         ir->accept(&input_resize_visitor);
>> +      }
>> +   }
>> +
>>      /* Make a pass over all variable declarations to ensure that
>> arrays with
>>       * unspecified sizes have a size specified.  The size is inferred
>> from the
>>       * max_array_access field.
>> @@ -1648,10 +1750,13 @@ link_shaders(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>>      unsigned num_vert_shaders = 0;
>>      struct gl_shader **frag_shader_list;
>>      unsigned num_frag_shaders = 0;
>> +   struct gl_shader **geom_shader_list;
>> +   unsigned num_geom_shaders = 0;
>>
>>      vert_shader_list = (struct gl_shader **)
>> -      calloc(2 * prog->NumShaders, sizeof(struct gl_shader *));
>> +      calloc(3 * prog->NumShaders, sizeof(struct gl_shader *));
>>      frag_shader_list =  &vert_shader_list[prog->NumShaders];
>> +   geom_shader_list =  &vert_shader_list[prog->NumShaders * 2];
>
> This code is getting ugly.  It looks like the idea is to allocate two
> (now three) separate arrays, but somebody decided to be "clever" and
> batch them into a single calloc/free big enough to hold all of them.

Yeah, that was me.  Since way back, I hate dipping into malloc/free for 
a bunch of small, short-lived allocations.  What I really want to use is 
alloca, but alas.

> I would much prefer to see:
>
>     struct gl_shader **vert_shader_list =
>        calloc(prog->NumShaders, sizeof(struct gl_shader *));
>     struct gl_shader **frag_shader_list =
>        calloc(prog->NumShaders, sizeof(struct gl_shader *));
>     struct gl_shader **geom_shader_list =
>        calloc(prog->NumShaders, sizeof(struct gl_shader *));
>
> and then later:
>
>     free(vert_shader_list);
>     free(frag_shader_list);
>     free(geom_shader_list);
>
> Linking is complicated enough without accidental complexity.
>
>>      unsigned min_version = UINT_MAX;
>>      unsigned max_version = 0;
>> @@ -1677,8 +1782,8 @@ link_shaders(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>>        num_frag_shaders++;
>>        break;
>>         case GL_GEOMETRY_SHADER:
>> -     /* FINISHME: Support geometry shaders. */
>> -     assert(prog->Shaders[i]->Type != GL_GEOMETRY_SHADER);
>> +     geom_shader_list[num_geom_shaders] = prog->Shaders[i];
>> +     num_geom_shaders++;
>>        break;
>>         }
>>      }
>> @@ -1740,6 +1845,22 @@ link_shaders(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>>                    sh);
>>      }
>>
>> +   if (num_geom_shaders > 0) {
>> +      gl_shader *const sh =
>> +     link_intrastage_shaders(mem_ctx, ctx, prog, geom_shader_list,
>> +                 num_geom_shaders);
>> +
>> +      if (!prog->LinkStatus)
>> +     goto done;
>> +
>> +      validate_geometry_shader_executable(prog, sh);
>> +      if (!prog->LinkStatus)
>> +     goto done;
>> +
>> +      _mesa_reference_shader(ctx,
>> &prog->_LinkedShaders[MESA_SHADER_GEOMETRY],
>> +                 sh);
>> +   }
>> +
>>      /* Here begins the inter-stage linking phase.  Some initial
>> validation is
>>       * performed, then locations are assigned for uniforms,
>> attributes, and
>>       * varyings.
>> @@ -1826,7 +1947,11 @@ link_shaders(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>>               prog->_LinkedShaders[MESA_SHADER_VERTEX],
>>               VERT_ATTRIB_GENERIC0, VARYING_SLOT_VAR0);
>>      }
>> -   /* FINISHME: Geometry shaders not implemented yet */
>> +   if (prog->_LinkedShaders[MESA_SHADER_GEOMETRY] != NULL) {
>> +      link_invalidate_variable_locations(
>> +            prog->_LinkedShaders[MESA_SHADER_GEOMETRY],
>> +            VARYING_SLOT_VAR0, VARYING_SLOT_VAR0);
>> +   }
>>      if (prog->_LinkedShaders[MESA_SHADER_FRAGMENT] != NULL) {
>>         link_invalidate_variable_locations(
>>               prog->_LinkedShaders[MESA_SHADER_FRAGMENT],
>> @@ -1860,7 +1985,7 @@ link_shaders(struct gl_context *ctx, struct
>> gl_shader_program *prog)
>>          *     non-zero, but the program object has no vertex or geometry
>>          *     shader;
>>          */
>> -      if (first >= MESA_SHADER_FRAGMENT) {
>> +      if (first == MESA_SHADER_FRAGMENT) {
>
> This hunk is unnecessary (but harmless).
>
>>            linker_error(prog, "Transform feedback varyings specified,
>> but "
>>                         "no vertex or geometry shader is present.");
>>            goto done;
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> index 3322e80..418ea9b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> @@ -236,8 +236,7 @@ brw_link_shader(struct gl_context *ctx, struct
>> gl_shader_program *shProg)
>>         reparent_ir(shader->ir, shader->ir);
>>         ralloc_free(mem_ctx);
>>
>> -      do_set_program_inouts(shader->ir, prog,
>> -                shader->base.Type == GL_FRAGMENT_SHADER);
>> +      do_set_program_inouts(shader->ir, prog, shader->base.Type);
>>
>>         prog->SamplersUsed = shader->base.active_samplers;
>>         _mesa_update_shader_textures_used(shProg, prog);
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index efa2d39..2725eef 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -1851,7 +1851,7 @@ struct gl_program
>>      GLuint Id;
>>      GLubyte *String;  /**< Null-terminated program text */
>>      GLint RefCount;
>> -   GLenum Target;    /**< GL_VERTEX/FRAGMENT_PROGRAM_ARB */
>> +   GLenum Target;    /**< GL_VERTEX/FRAGMENT_PROGRAM_ARB,
>> GL_GEOMETRY_PROGRAM_NV */
>>      GLenum Format;    /**< String encoding format */
>>
>>      struct prog_instruction *Instructions;
>> @@ -1918,6 +1918,7 @@ struct gl_geometry_program
>>   {
>>      struct gl_program Base;   /**< base class */
>>
>> +   GLint VerticesIn;
>>      GLint VerticesOut;
>>      GLenum InputType;  /**< GL_POINTS, GL_LINES, GL_LINES_ADJACENCY_ARB,
>>                              GL_TRIANGLES, or
>> GL_TRIANGLES_ADJACENCY_ARB */
>> @@ -2320,6 +2321,7 @@ struct gl_shader_program
>>
>>      /** Geometry shader state - copied into gl_geometry_program at
>> link time */
>>      struct {
>> +      GLint VerticesIn;
>>         GLint VerticesOut;
>>         GLenum InputType;  /**< GL_POINTS, GL_LINES,
>> GL_LINES_ADJACENCY_ARB,
>>                                 GL_TRIANGLES, or
>> GL_TRIANGLES_ADJACENCY_ARB */
>> diff --git a/src/mesa/program/ir_to_mesa.cpp
>> b/src/mesa/program/ir_to_mesa.cpp
>> index e526582..914aca4 100644
>> --- a/src/mesa/program/ir_to_mesa.cpp
>> +++ b/src/mesa/program/ir_to_mesa.cpp
>> @@ -2975,7 +2975,7 @@ get_mesa_program(struct gl_context *ctx,
>>       */
>>      mesa_instructions = NULL;
>>
>> -   do_set_program_inouts(shader->ir, prog, shader->Type ==
>> GL_FRAGMENT_SHADER);
>> +   do_set_program_inouts(shader->ir, prog, shader->Type);
>>
>>      prog->SamplersUsed = shader->active_samplers;
>>      prog->ShadowSamplers = shader->shadow_samplers;
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 77623f9..52e44ad 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -5128,7 +5128,7 @@ get_mesa_program(struct gl_context *ctx,
>>      prog->Instructions = NULL;
>>      prog->NumInstructions = 0;
>>
>> -   do_set_program_inouts(shader->ir, prog, shader->Type ==
>> GL_FRAGMENT_SHADER);
>> +   do_set_program_inouts(shader->ir, prog, shader->Type);
>>      count_resources(v, prog);
>>
>>      _mesa_reference_program(ctx, &shader->Program, prog);
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



More information about the mesa-dev mailing list