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

[Ilia Mirkin]

On Fri, Nov 27, 2015 at 1:31 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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.

Nope, those helpers would be of no help here. An entirely separate set
of helpers could be made, of course, but that's wholly unrelated to
this change. And it has absolutely nothing with the comment I made,
which was why checking for 3.10 here but 3.20 above, and not checking
for the OES_geometry_shader enable.

  -ilia