> -----Original Message----- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > Behalf Of Ilia Mirkin > Sent: Friday, November 27, 2015 8:16 PM > To: Emil Velikov > Cc: mesa-dev@lists.freedesktop.org > Subject: Re: [Mesa-dev] [PATCH v2 3/6] glsl: add support for > GL_OES_geometry_shader > > On Fri, Nov 27, 2015 at 2:12 PM, Emil Velikov <emil.l.veli...@gmail.com> > wrote: > > On 27 November 2015 at 19:00, Ilia Mirkin <imir...@alum.mit.edu> wrote: > >> On Fri, Nov 27, 2015 at 1:55 PM, Emil Velikov <emil.l.veli...@gmail.com> > wrote: > >>> On 27 November 2015 at 18:36, Ilia Mirkin <imir...@alum.mit.edu> > wrote: > >>>> On Fri, Nov 27, 2015 at 1:31 PM, Emil Velikov > <emil.l.veli...@gmail.com> wrote: > >>>>> On 27 November 2015 at 18:02, Ilia Mirkin <imir...@alum.mit.edu> > wrote: > >>>>>> On Fri, Nov 27, 2015 at 9:31 AM, Marta Lofstedt > >>>>>> <marta.lofst...@linux.intel.com> wrote: > >>>>>>> From: Marta Lofstedt <marta.lofst...@intel.com> > >>>>>>> > >>>>>>> This adds glsl support of GL_OES_geometry_shader for OpenGL ES > >>>>>>> 3.1. > >>>>>>> > >>>>>>> Signed-off-by: Marta Lofstedt <marta.lofst...@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?
Yes, it should be the same, thanks for noticing, Ilia. V3 coming up. > >>>>>> > >>>>> 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. > >>>> > >>> Actually things made perfect sense although I could have been too > subtle. > >>> > >>> From a very quick look the functionality provided by the extension > >>> is identical to the one in the core GLES 3.2 spec. Thus one could > >>> just > >> > >> Let's assume that it is. > >> > >>> check for the extension boolean, omitting the ES version check. On > >>> the other hand the extension enable is quite hacky (look at > >>> compatible_with_state()), so instead of adding more error prone code > >>> one can just reuse (ok build on top) of the existing helpers. > >> > >> On the other other hand, those helpers are for gl_context, not for > >> the glsl compiler state (I forget the struct name). They're just for > >> a totally different thing. > >> > > Hmm isn't the compiler state dependent on the API used ? I.e. if the > > API does not support foo, the glsl compiler state should reflect that. > > Getting off-topic here. You might have GL 4.5 support, but if I hand you a > > #version 130 > > shader, then you have to pretend it's GL 3.0-land. Similarly with various > extension enables. The mesa parser state keeps track of all that. > > -ilia > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
