[Mesa-dev] [PATCH v2 3/6] glsl: add support for GL_OES_geometry_shader
Lofstedt, Marta
marta.lofstedt at intel.com
Tue Dec 1 06:40:25 PST 2015
> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Ilia Mirkin
> Sent: Friday, November 27, 2015 8:16 PM
> To: Emil Velikov
> Cc: mesa-dev at 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.velikov at gmail.com>
> wrote:
> > On 27 November 2015 at 19:00, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> >> On Fri, Nov 27, 2015 at 1:55 PM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
> >>> On 27 November 2015 at 18:36, Ilia Mirkin <imirkin at alum.mit.edu>
> wrote:
> >>>> 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?
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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list