[Mesa-dev] [PATCH v3] glsl: add support for GL_OES_geometry_shader

[Ian Romanick]

On 12/01/2015 06:49 AM, Marta Lofstedt wrote:
> From: Marta Lofstedt <marta.lofstedt at intel.com>
> 
> This adds glsl support of GL_OES_geometry_shader for
> OpenGL ES 3.1.
> 
> Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
> ---
>  src/glsl/builtin_variables.cpp  | 25 +++++++++++++------------
>  src/glsl/glsl_parser.yy         |  4 ++--
>  src/glsl/glsl_parser_extras.cpp |  1 +
>  src/glsl/glsl_parser_extras.h   |  7 +++++++
>  4 files changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
> index e8eab80..6f87600 100644
> --- a/src/glsl/builtin_variables.cpp
> +++ b/src/glsl/builtin_variables.cpp
> @@ -667,7 +667,7 @@ builtin_variable_generator::generate_constants()
>        add_const("gl_MaxVaryingComponents", state->ctx->Const.MaxVarying * 4);
>     }
>  
> -   if (state->is_version(150, 0)) {
> +   if (state->has_geometry_shader()) {
>        add_const("gl_MaxVertexOutputComponents",
>                  state->Const.MaxVertexOutputComponents);
>        add_const("gl_MaxGeometryInputComponents",
> @@ -730,12 +730,11 @@ builtin_variable_generator::generate_constants()
>        add_const("gl_MaxAtomicCounterBindings",
>                  state->Const.MaxAtomicBufferBindings);
>  
> -      /* When Mesa adds support for GL_OES_geometry_shader and
> -       * GL_OES_tessellation_shader, this will need to change.
> -       */
> -      if (!state->es_shader) {
> +      if (state->has_geometry_shader()) {
>           add_const("gl_MaxGeometryAtomicCounters",
>                     state->Const.MaxGeometryAtomicCounters);
> +      }
> +      if (!state->es_shader) {
>           add_const("gl_MaxTessControlAtomicCounters",
>                     state->Const.MaxTessControlAtomicCounters);

I think you got a little over excited here.  I don't see MaxTess*Atomic
anywhere in any GLES extension... not even in
GL_EXT_tessellation_shader, which seems like a bug in that extension spec.

https://www.khronos.org/bugzilla/show_bug.cgi?id=1427

>           add_const("gl_MaxTessEvaluationAtomicCounters",
> @@ -753,12 +752,11 @@ builtin_variable_generator::generate_constants()
>        add_const("gl_MaxAtomicCounterBufferSize",
>                  state->Const.MaxAtomicCounterBufferSize);
>  
> -      /* When Mesa adds support for GL_OES_geometry_shader and
> -       * GL_OES_tessellation_shader, this will need to change.
> -       */
> -      if (!state->es_shader) {
> +      if (state->has_geometry_shader()) {
>           add_const("gl_MaxGeometryAtomicCounterBuffers",
>                     state->Const.MaxGeometryAtomicCounterBuffers);
> +      }
> +      if(!state->es_shader) {

Here too.

With these two things reverted, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>           add_const("gl_MaxTessControlAtomicCounterBuffers",
>                     state->Const.MaxTessControlAtomicCounterBuffers);
>           add_const("gl_MaxTessEvaluationAtomicCounterBuffers",
> @@ -814,13 +812,16 @@ builtin_variable_generator::generate_constants()
>        add_const("gl_MaxCombinedImageUniforms",
>                  state->Const.MaxCombinedImageUniforms);
>  
> +      if (state->has_geometry_shader()) {
> +         add_const("gl_MaxGeometryImageUniforms",
> +                   state->Const.MaxGeometryImageUniforms);
> +      }
> +
>        if (!state->es_shader) {
>           add_const("gl_MaxCombinedImageUnitsAndFragmentOutputs",
>                     state->Const.MaxCombinedShaderOutputResources);
>           add_const("gl_MaxImageSamples",
>                     state->Const.MaxImageSamples);
> -         add_const("gl_MaxGeometryImageUniforms",
> -                   state->Const.MaxGeometryImageUniforms);
>        }
>  
>        if (state->is_version(450, 310)) {
> @@ -1057,7 +1058,7 @@ builtin_variable_generator::generate_fs_special_vars()
>     if (state->is_version(120, 100))
>        add_input(VARYING_SLOT_PNTC, vec2_t, "gl_PointCoord");
>  
> -   if (state->is_version(150, 0)) {
> +   if (state->has_geometry_shader()) {
>        var = add_input(VARYING_SLOT_PRIMITIVE_ID, int_t, "gl_PrimitiveID");
>        var->data.interpolation = INTERP_QUALIFIER_FLAT;
>     }
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 5a8f980..7a875fa 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -1262,7 +1262,7 @@ layout_qualifier_id:
>              }
>           }
>  
> -         if ($$.flags.i && !state->is_version(150, 0)) {
> +         if ($$.flags.i && !state->has_geometry_shader()) {
>              _mesa_glsl_error(& @1, state, "#version 150 layout "
>                               "qualifier `%s' used", $1);
>           }
> @@ -1499,7 +1499,7 @@ layout_qualifier_id:
>        if (match_layout_qualifier("max_vertices", $1, state) == 0) {
>           $$.flags.q.max_vertices = 1;
>           $$.max_vertices = new(ctx) ast_layout_expression(@1, $3);
> -         if (!state->is_version(150, 0)) {
> +         if (!state->has_geometry_shader()) {
>              _mesa_glsl_error(& @3, state,
>                               "#version 150 max_vertices qualifier "
>                               "specified", $3);
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index 29cf0c6..cd0714e 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -635,6 +635,7 @@ static const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = {
>     /* OES extensions go here, sorted alphabetically.
>      */
>     EXT(OES_EGL_image_external,         false, true,      OES_EGL_image_external),
> +   EXT(OES_geometry_shader,            false, true,      OES_geometry_shader),
>     EXT(OES_standard_derivatives,       false, true,      OES_standard_derivatives),
>     EXT(OES_texture_3D,                 false, true,      dummy_true),
>     EXT(OES_texture_storage_multisample_2d_array, false, true, ARB_texture_multisample),
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 17ff0b5..b47c540 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -260,6 +260,11 @@ struct _mesa_glsl_parse_state {
>        return ARB_compute_shader_enable || is_version(430, 310);
>     }
>  
> +   bool has_geometry_shader() const
> +   {
> +      return OES_geometry_shader_enable || is_version(150, 320);
> +   }
> +
>     void process_version_directive(YYLTYPE *locp, int version,
>                                    const char *ident);
>  
> @@ -579,6 +584,8 @@ struct _mesa_glsl_parse_state {
>      */
>     bool OES_EGL_image_external_enable;
>     bool OES_EGL_image_external_warn;
> +   bool OES_geometry_shader_enable;
> +   bool OES_geometry_shader_warn;
>     bool OES_standard_derivatives_enable;
>     bool OES_standard_derivatives_warn;
>     bool OES_texture_3D_enable;
>