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

[Emil Velikov]

On 27 November 2015 at 18:02, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Fri, Nov 27, 2015 at 9:31 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>
>> ---
>>  src/glsl/builtin_variables.cpp  | 17 +++++------------
>>  src/glsl/glsl_parser.yy         |  4 ++--
>>  src/glsl/glsl_parser_extras.cpp |  1 +
>>  src/glsl/glsl_parser_extras.h   |  2 ++
>>  4 files changed, 10 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
>> index e8eab80..6a53789 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->is_version(150, 320) || state->OES_geometry_shader_enable) {
>>        add_const("gl_MaxVertexOutputComponents",
>>                  state->Const.MaxVertexOutputComponents);
>>        add_const("gl_MaxGeometryInputComponents",
>> @@ -729,11 +729,7 @@ builtin_variable_generator::generate_constants()
>>                  state->Const.MaxCombinedAtomicCounters);
>>        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->es_shader || state->OES_geometry_shader_enable) {
>>           add_const("gl_MaxGeometryAtomicCounters",
>>                     state->Const.MaxGeometryAtomicCounters);
>>           add_const("gl_MaxTessControlAtomicCounters",
>
> Do you really want to be adding this in for OES_geometry_shader?
>
>> @@ -753,10 +749,7 @@ 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->es_shader || state->OES_geometry_shader_enable) {
>>           add_const("gl_MaxGeometryAtomicCounterBuffers",
>>                     state->Const.MaxGeometryAtomicCounterBuffers);
>>           add_const("gl_MaxTessControlAtomicCounterBuffers",
>> @@ -814,7 +807,7 @@ builtin_variable_generator::generate_constants()
>>        add_const("gl_MaxCombinedImageUniforms",
>>                  state->Const.MaxCombinedImageUniforms);
>>
>> -      if (!state->es_shader) {
>> +      if (!state->es_shader || state->OES_geometry_shader_enable) {
>>           add_const("gl_MaxCombinedImageUnitsAndFragmentOutputs",
>>                     state->Const.MaxCombinedShaderOutputResources);
>>           add_const("gl_MaxImageSamples",
>> @@ -1057,7 +1050,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->is_version(150, 320) || state->OES_geometry_shader_enable) {
>>        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..fae6d0b 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->is_version(150, 320) && !state->OES_geometry_shader_enable) {
>>              _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->is_version(150, 310)) {
>
> Why is this one different? Shouldn't this also be the same as the above check?
>
Depends on how much the OES_geometry_shader text differs from the one
in GLES 3.2. If they are the same - one can fix the
_mesa_glsl_supported_extensions[] handling to use the existing
_mesa_has_#extension_name() helpers from Nanley and drop the GLES
version checks above.

This sounds like the better long term solution, imho, although others
might disagree.

If they differ ... we might want to pull the same thing as the ARB
version - never expose the standalone extension but have the
functionality for respective core version.

Cheers,
Emil