[Mesa-dev] [PATCH 01/18] glsl: Add parsing support for multi-stream output in geometry shaders.

[Chris Forbes]

On Wed, Jun 11, 2014 at 7:49 PM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> @@ -509,6 +509,8 @@ struct ast_type_qualifier {
>           /** \name Layout qualifiers for GL_ARB_gpu_shader5 */
>           /** \{ */
>           unsigned invocations:1;
> +         unsigned streamId:1; /* Has streamId value assigned  */
> +         unsigned explicit_streamId:1; /* streamId value assigned explicitely by shader code */

s/explicitely/explicitly/

I'd call these stream / explicit_stream (or if you must, stream_id /
explicit_stream_id). None of the other members here are using camel
case, and explicit_streamId is a crazy mix.

>           /** \} */
>        }
>        /** \brief Set of flags, accessed by name. */
> @@ -542,6 +544,9 @@ struct ast_type_qualifier {
>     /** Maximum output vertices in GLSL 1.50 geometry shaders. */
>     int max_vertices;
>
> +   /** Stream ID in GLSL 1.50 geometry shaders. */
> +   unsigned streamId;
> +
>     /** Input or output primitive type in GLSL 1.50 geometry shaders */
>     GLenum prim_type;
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 140bb74..ab0f50f 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2426,6 +2426,11 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
>     if (qual->flags.q.sample)
>        var->data.sample = 1;
>
> +   if (state->stage == MESA_SHADER_GEOMETRY &&
> +       qual->flags.q.out && qual->flags.q.streamId) {
> +      var->data.streamId = qual->streamId;
> +   }
> +
>     if (qual->flags.q.attribute && state->stage != MESA_SHADER_VERTEX) {
>        var->type = glsl_type::error_type;
>        _mesa_glsl_error(loc, state,
> @@ -5455,6 +5460,7 @@ ast_interface_block::hir(exec_list *instructions,
>           var->data.centroid = fields[i].centroid;
>           var->data.sample = fields[i].sample;
>           var->init_interface_type(block_type);
> +         var->data.streamId = this->layout.streamId;
>
>           if (redeclaring_per_vertex) {
>              ir_variable *earlier =
> diff --git a/src/glsl/ast_type.cpp b/src/glsl/ast_type.cpp
> index 0ee2c49..749f161 100644
> --- a/src/glsl/ast_type.cpp
> +++ b/src/glsl/ast_type.cpp
> @@ -125,9 +125,13 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
>     /* Uniform block layout qualifiers get to overwrite each
>      * other (rightmost having priority), while all other
>      * qualifiers currently don't allow duplicates.
> +    *
> +    * Geometry shaders can have several layout qualifiers
> +    * assigning different stream ID values.
>      */
>
> -   if ((this->flags.i & q.flags.i & ~(ubo_mat_mask.flags.i |
> +   if ((state->stage != MESA_SHADER_GEOMETRY) &&
> +       (this->flags.i & q.flags.i & ~(ubo_mat_mask.flags.i |
>                                       ubo_layout_mask.flags.i |
>                                        ubo_binding_mask.flags.i)) != 0) {
>        _mesa_glsl_error(loc, state,
> @@ -154,6 +158,19 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
>        this->max_vertices = q.max_vertices;
>     }
>
> +   if (state->stage == MESA_SHADER_GEOMETRY &&
> +       state->ARB_gpu_shader5_enable &&
> +       !this->flags.q.explicit_streamId) {
> +      if (q.flags.q.streamId) {
> +         this->flags.q.streamId = 1;
> +         this->streamId = q.streamId;
> +      } else if(this->flags.q.streamId == 0) {

!this->flags.q.streamId, for consistency with the explicit_streamId check above.

> +         /* Assign default global streamId value */
> +         this->flags.q.streamId = 1;
> +         this->streamId = state->out_qualifier->streamId;
> +      }
> +   }
> +
>     if ((q.flags.i & ubo_mat_mask.flags.i) != 0)
>        this->flags.i &= ~ubo_mat_mask.flags.i;
>     if ((q.flags.i & ubo_layout_mask.flags.i) != 0)
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index eddab05..caebfe7 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1395,6 +1395,21 @@ layout_qualifier_id:
>           }
>        }
>
> +      if (state->ARB_gpu_shader5_enable && state->stage == MESA_SHADER_GEOMETRY) {

This check (and others like it) should really also pass if
state->is_version(400, 0) as well, so we
don't have to come back and change them all again.

> +         if (match_layout_qualifier("stream", $1, state) == 0) {
> +            $$.flags.q.streamId = 1;
> +
> +            if (($3 < 0) || ($3 >= MAX_VERTEX_STREAMS)) {

This should really be checking against the real limit... but I see
there's a precedent for this in the
equivalent MAX_GEOMETRY_SHADER_INVOCATIONS case, which is much more
likely to vary
across hardware...

> +               _mesa_glsl_error(& @3, state,
> +                                "invalid stream %d specified", $3);
> +               YYERROR;
> +            } else {
> +               $$.flags.q.explicit_streamId = 1;
> +               $$.streamId = $3;
> +            }
> +         }
> +      }
> +

> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index b4e52d3..8cc58af 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -705,6 +705,11 @@ public:
>         */
>        int location;
>
> +      /*
> +       * Vertex stream output identifier.
> +       */
> +      unsigned streamId;

Same comment as in the ast about naming.

> +
>        /**
>         * output index for dual source blending.
>         */
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev