[Mesa-dev] [PATCH 3/4] glsl: add support for GL_OES_geometry_shader

[Ilia Mirkin]

On Thu, Jan 21, 2016 at 10:17 AM, Marta Lofstedt
<marta.lofstedt at linux.intel.com> 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>
> Reviewed-by: Ian Romanick <ian.d.romanick at 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 221aab0..ccc04c0 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);
>           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) {
>           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);
> +      }

So.... this is *really* pedantic, and I think you can disregard my
comment. However, I'd like to say it for posterity anyways:

ARB_shader_image_load_store defines a bunch of interactions, and none
of them talk about removing gl_MaxGeometryImageUniforms when you don't
have GL 3.2. I suspect similar is true of atomic counters but I didn't
check explicitly.

If you want to be super-accurate, you'd make the formerly "if
(!state->es_shader)" blocks read something like

if (!state->es_shader || state->has_geometry_shader())

I also vaguely recall Ian talking about this. I forget what he said.
If he said anything that conflicts with what I said, trust him, not
me.

Either way, this is

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>