[Mesa-dev] [PATCH v2 2/7] glsl/linker: produce gl_shader_program Geom.Invocations

[Paul Berry]

On 28 January 2014 11:22, Jordan Justen <jordan.l.justen at intel.com> wrote:

> Grab the parsed invocation count, check for consistency
> during linking, and finally save the result in
> gl_shader_program Geom.Invocations.
>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/glsl/glsl_parser_extras.cpp |  2 ++
>  src/glsl/linker.cpp             | 18 ++++++++++++++++++
>  src/mesa/main/mtypes.h          |  2 ++
>  3 files changed, 22 insertions(+)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp
> b/src/glsl/glsl_parser_extras.cpp
> index 87784ed..3e98966 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -1339,6 +1339,8 @@ set_shader_inout_layout(struct gl_shader *shader,
>     if (state->out_qualifier->flags.q.max_vertices)
>        shader->Geom.VerticesOut = state->out_qualifier->max_vertices;
>
> +   shader->Geom.Invocations = state->gs_invocations;
> +
>

Considering my comment on patch 1, I think this will change to:

shader->Geom.Invocations = 0;
if (state->in_qualifier->flags.q.invocations)
   shader->Geom.Invocations = state->in_qualifier->invocations;



>     if (state->gs_input_prim_type_specified) {
>        shader->Geom.InputType = state->gs_input_prim_type;
>     } else {
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 93b4754..800de0b 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1206,6 +1206,7 @@ link_gs_inout_layout_qualifiers(struct
> gl_shader_program *prog,
>                                 unsigned num_shaders)
>  {
>     linked_shader->Geom.VerticesOut = 0;
> +   linked_shader->Geom.Invocations = 0;
>     linked_shader->Geom.InputType = PRIM_UNKNOWN;
>     linked_shader->Geom.OutputType = PRIM_UNKNOWN;
>
> @@ -1259,6 +1260,18 @@ link_gs_inout_layout_qualifiers(struct
> gl_shader_program *prog,
>          }
>          linked_shader->Geom.VerticesOut = shader->Geom.VerticesOut;
>        }
> +
> +      if (shader->Geom.Invocations != 0) {
> +        if (linked_shader->Geom.Invocations != 0 &&
> +            linked_shader->Geom.Invocations != shader->Geom.Invocations) {
> +           linker_error(prog, "geometry shader defined with conflicting "
> +                        "invocation count (%d and %d)\n",
> +                        linked_shader->Geom.Invocations,
> +                        shader->Geom.Invocations);
> +           return;
> +        }
> +        linked_shader->Geom.Invocations = shader->Geom.Invocations;
> +      }
>     }
>
>     /* Just do the intrastage -> interstage propagation right now,
> @@ -1285,6 +1298,11 @@ link_gs_inout_layout_qualifiers(struct
> gl_shader_program *prog,
>        return;
>     }
>     prog->Geom.VerticesOut = linked_shader->Geom.VerticesOut;
> +
> +   if (linked_shader->Geom.Invocations == 0)
> +      linked_shader->Geom.Invocations = 1;
>

I think it would be equivalent if we dropped these two lines and simply
initialized linked_shader->Geom.Invocations to 1 at the top of the
function--is that right?


> +
> +   prog->Geom.Invocations = linked_shader->Geom.Invocations;
>  }
>
>  /**
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 3d42a21..2d90b0a 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2401,6 +2401,7 @@ struct gl_shader
>      */
>     struct {
>        GLint VerticesOut;
> +      GLint Invocations;
>

It would be nice to have a comment above this declaration explaining that
if the shader didn't specify an invocation count, this value will be 1 (so
that back-end implementors know they don't have to coerce 0 to 1).


>        /**
>         * GL_POINTS, GL_LINES, GL_LINES_ADJACENCY, GL_TRIANGLES, or
>         * GL_TRIANGLES_ADJACENCY, or PRIM_UNKNOWN if it's not set in this
> @@ -2599,6 +2600,7 @@ struct gl_shader_program
>     struct {
>        GLint VerticesIn;
>        GLint VerticesOut;
> +      GLint Invocations;
>

Same comment applies here.


>        GLenum InputType;  /**< GL_POINTS, GL_LINES, GL_LINES_ADJACENCY_ARB,
>                                GL_TRIANGLES, or GL_TRIANGLES_ADJACENCY_ARB
> */
>        GLenum OutputType; /**< GL_POINTS, GL_LINE_STRIP or
> GL_TRIANGLE_STRIP */
> --
> 1.8.5.3
>

With those minor issues addressed, this patch is:

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