[Mesa-dev] [PATCH 2/4] mesa: enable enums for OES_geometry_shader

Lofstedt, Marta marta.lofstedt at intel.com
Tue Jan 26 00:48:01 PST 2016


I am sorry for this.

I am sending up a v5 where I have accounted for that check_extra doesn't evaluate lazily 
FYI I believe that the the EXTRA_EXT_*_TESS tokens may need to be looked over, since they are bound to cause similar trouble when we enable oes_tesselation_shader for GLES 3.1.

I also moved MAX_FRAMEBUFFER_LAYER to [GL, GL_CORE, GLES31] and fixed up the EXTRA_EXT_FB_NO_ATTACH_CS.

And of course I have no longer any piglit regressions.

> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Marek Olšák
> Sent: Sunday, January 24, 2016 3:48 PM
> To: Ilia Mirkin
> Cc: mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH 2/4] mesa: enable enums for
> OES_geometry_shader
> 
> Hi,
> 
> I have decided to revert this patch, because it breaks a bunch GS piglit tests.
> 
> Next time please make sure there are no piglit regressions.
> 
> Thanks,
> Marek
> 
> On Sat, Jan 23, 2016 at 1:41 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> > 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>
> >>
> >> Enable GL_OES_geometry_shader enums for OpenGL ES 3.1.
> >>
> >> V4: EXTRA tokens updated according to comments from Ilia Mirkin.
> >>
> >> Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
> >> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
> >> ---
> >>  src/mesa/main/get.c              | 65
> ++++++++++++++++++++++++++++++++++------
> >>  src/mesa/main/get_hash_params.py | 52
> >> +++++++++++++++++++-------------
> >>  2 files changed, 87 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index
> >> 95cb18c..d8f9e25a 100644
> >> --- a/src/mesa/main/get.c
> >> +++ b/src/mesa/main/get.c
> >> @@ -147,11 +147,14 @@ enum value_extra {
> >>     EXTRA_VALID_CLIP_DISTANCE,
> >>     EXTRA_FLUSH_CURRENT,
> >>     EXTRA_GLSL_130,
> >> -   EXTRA_EXT_UBO_GS4,
> >> -   EXTRA_EXT_ATOMICS_GS4,
> >> -   EXTRA_EXT_SHADER_IMAGE_GS4,
> >> +   EXTRA_EXT_UBO_GS,
> >> +   EXTRA_EXT_ATOMICS_GS,
> >> +   EXTRA_EXT_SHADER_IMAGE_GS,
> >>     EXTRA_EXT_ATOMICS_TESS,
> >>     EXTRA_EXT_SHADER_IMAGE_TESS,
> >> +   EXTRA_EXT_SSBO_GS,
> >> +   EXTRA_EXT_FB_NO_ATTACH_GS,
> >> +   EXTRA_EXT_ES_GS,
> >>  };
> >>
> >>  #define NO_EXTRA NULL
> >> @@ -308,7 +311,7 @@ static const int
> >> extra_ARB_transform_feedback2_api_es3[] = {  };
> >>
> >>  static const int
> extra_ARB_uniform_buffer_object_and_geometry_shader[] = {
> >> -   EXTRA_EXT_UBO_GS4,
> >> +   EXTRA_EXT_UBO_GS,
> >>     EXTRA_END
> >>  };
> >>
> >> @@ -343,12 +346,12 @@ static const int extra_EXT_texture_array_es3[]
> >> = {  };
> >>
> >>  static const int
> extra_ARB_shader_atomic_counters_and_geometry_shader[] = {
> >> -   EXTRA_EXT_ATOMICS_GS4,
> >> +   EXTRA_EXT_ATOMICS_GS,
> >>     EXTRA_END
> >>  };
> >>
> >>  static const int
> extra_ARB_shader_image_load_store_and_geometry_shader[] = {
> >> -   EXTRA_EXT_SHADER_IMAGE_GS4,
> >> +   EXTRA_EXT_SHADER_IMAGE_GS,
> >>     EXTRA_END
> >>  };
> >>
> >> @@ -375,6 +378,29 @@ static const int
> extra_ARB_shader_storage_buffer_object_es31[] = {
> >>     EXTRA_END
> >>  };
> >>
> >> +static const int
> extra_ARB_shader_storage_buffer_object_and_geometry_shader[] = {
> >> +   EXTRA_EXT_SSBO_GS,
> >> +   EXTRA_END
> >> +};
> >> +
> >> +
> >> +static const int
> extra_ARB_framebuffer_no_attachments_and_geometry_shader[] = {
> >> +   EXTRA_EXT_FB_NO_ATTACH_GS,
> >> +   EXTRA_END
> >> +};
> >> +
> >> +static const int extra_ARB_viewport_array_or_oes_geometry_shader[]
> = {
> >> +   EXT(ARB_viewport_array),
> >> +   EXTRA_EXT_ES_GS,
> >> +   EXTRA_END
> >> +};
> >> +
> >> +static const int extra_ARB_gpu_shader5_or_oes_geometry_shader[] = {
> >> +   EXT(ARB_gpu_shader5),
> >> +   EXTRA_EXT_ES_GS,
> >> +   EXTRA_END
> >> +};
> >> +
> >>  EXTRA_EXT(ARB_texture_cube_map);
> >>  EXTRA_EXT(EXT_texture_array);
> >>  EXTRA_EXT(NV_fog_distance);
> >> @@ -424,6 +450,7 @@ EXTRA_EXT(ARB_tessellation_shader);
> >>  EXTRA_EXT(ARB_shader_subroutine);
> >>  EXTRA_EXT(ARB_shader_storage_buffer_object);
> >>  EXTRA_EXT(ARB_indirect_parameters);
> >> +EXTRA_EXT(OES_geometry_shader);
> >>
> >>  static const int
> >>  extra_ARB_color_buffer_float_or_glcore[] = { @@ -455,6 +482,12 @@
> >> static const int extra_gl32_es3[] = {
> >>      EXTRA_END,
> >>  };
> >>
> >> +static const int extra_version_32_OES_geometry_shader[] = {
> >> +    EXTRA_VERSION_32,
> >> +    EXT(OES_geometry_shader),
> >> +    EXTRA_END
> >> +};
> >> +
> >>  static const int extra_gl40_ARB_sample_shading[] = {
> >>     EXTRA_VERSION_40,
> >>     EXT(ARB_sample_shading),
> >> @@ -1154,17 +1187,17 @@ check_extra(struct gl_context *ctx, const char
> *func, const struct value_desc *d
> >>           if (ctx->Const.GLSLVersion >= 130)
> >>              api_found = GL_TRUE;
> >>          break;
> >> -      case EXTRA_EXT_UBO_GS4:
> >> +      case EXTRA_EXT_UBO_GS:
> >>           api_check = GL_TRUE;
> >>           api_found = (ctx->Extensions.ARB_uniform_buffer_object &&
> >>                        _mesa_has_geometry_shaders(ctx));
> >>           break;
> >> -      case EXTRA_EXT_ATOMICS_GS4:
> >> +      case EXTRA_EXT_ATOMICS_GS:
> >>           api_check = GL_TRUE;
> >>           api_found = (ctx->Extensions.ARB_shader_atomic_counters &&
> >>                        _mesa_has_geometry_shaders(ctx));
> >>           break;
> >> -      case EXTRA_EXT_SHADER_IMAGE_GS4:
> >> +      case EXTRA_EXT_SHADER_IMAGE_GS:
> >>           api_check = GL_TRUE;
> >>           api_found = (ctx->Extensions.ARB_shader_image_load_store &&
> >>                        _mesa_has_geometry_shaders(ctx)); @@ -1179,6
> >> +1212,20 @@ check_extra(struct gl_context *ctx, const char *func, const
> struct value_desc *d
> >>           api_found = ctx->Extensions.ARB_shader_image_load_store &&
> >>                       _mesa_has_tessellation(ctx);
> >>           break;
> >> +      case EXTRA_EXT_SSBO_GS:
> >> +         api_check = GL_TRUE;
> >> +         api_found = (ctx->Extensions.ARB_shader_storage_buffer_object
> &&
> >> +                      _mesa_has_geometry_shaders(ctx));
> >> +         break;
> >> +      case EXTRA_EXT_FB_NO_ATTACH_GS:
> >> +         api_check = GL_TRUE;
> >> +         api_found = (ctx->Extensions.ARB_framebuffer_no_attachments
> &&
> >> +                      _mesa_has_geometry_shaders(ctx));
> >> +         break;
> >> +      case EXTRA_EXT_ES_GS:
> >> +         api_check = GL_TRUE;
> >> +         api_found = _mesa_has_OES_geometry_shader(ctx);
> >> +         break;
> >>        case EXTRA_END:
> >>          break;
> >>        default: /* *e is a offset into the extension struct */ diff
> >> --git a/src/mesa/main/get_hash_params.py
> >> b/src/mesa/main/get_hash_params.py
> >> index af7a8f4..6da0f84 100644
> >> --- a/src/mesa/main/get_hash_params.py
> >> +++ b/src/mesa/main/get_hash_params.py
> >> @@ -499,6 +499,37 @@ descriptor=[
> >>  { "apis": ["GL_CORE", "GLES31"], "params": [  # GL_ARB_draw_indirect
> >> / GLES 3.1
> >>    [ "DRAW_INDIRECT_BUFFER_BINDING", "LOC_CUSTOM, TYPE_INT, 0,
> >> extra_ARB_draw_indirect" ],
> >> +
> >> +# GL 3.2 / GL OES_geometry_shader
> >> +  [ "MAX_GEOMETRY_INPUT_COMPONENTS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxInputCom
> ponents)
> >> +, extra_version_32_OES_geometry_shader" ],
> >> +  [ "MAX_GEOMETRY_OUTPUT_COMPONENTS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxOutputCo
> mponents
> >> +), extra_version_32_OES_geometry_shader" ],
> >> +  [ "MAX_GEOMETRY_TEXTURE_IMAGE_UNITS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxTextureI
> mageUnit
> >> +s), extra_version_32_OES_geometry_shader" ],
> >> +  [ "MAX_GEOMETRY_OUTPUT_VERTICES",
> >> +"CONTEXT_INT(Const.MaxGeometryOutputVertices),
> >> +extra_version_32_OES_geometry_shader" ],
> >> +  [ "MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS",
> >> +"CONTEXT_INT(Const.MaxGeometryTotalOutputComponents),
> >> +extra_version_32_OES_geometry_shader" ],
> >> +  [ "MAX_GEOMETRY_UNIFORM_COMPONENTS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxUniformC
> omponent
> >> +s), extra_version_32_OES_geometry_shader" ],
> >> +
> >> +# GL_ARB_shader_image_load_store / geometry shader
> >> +  [ "MAX_GEOMETRY_IMAGE_UNIFORMS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxImageUni
> forms),
> >> +extra_ARB_shader_image_load_store_and_geometry_shader" ],
> >> +
> >> +# GL_ARB_shader_atomic_counters / geometry shader
> >> +  [ "MAX_GEOMETRY_ATOMIC_COUNTER_BUFFERS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicBu
> ffers),
> >> +extra_ARB_shader_atomic_counters_and_geometry_shader " ],
> >> +  [ "MAX_GEOMETRY_ATOMIC_COUNTERS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxAtomicCo
> unters),
> >> +extra_ARB_shader_atomic_counters_and_geometry_shader" ],
> >> +
> >> +# GL_ARB_shader_storage_buffer_object / geometry shader
> >> +  [ "MAX_GEOMETRY_SHADER_STORAGE_BLOCKS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_FRAGMENT].MaxShaderSt
> orageBlo
> >> +cks),
> extra_ARB_shader_storage_buffer_object_and_geometry_shader" ],
> >> +
> >> +# GL_ARB_uniform_buffer_object / geometry shader
> >> +  [ "MAX_GEOMETRY_UNIFORM_BLOCKS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxUniformBl
> ocks),
> >> +extra_ARB_uniform_buffer_object_and_geometry_shader" ],
> >> +  [ "MAX_COMBINED_GEOMETRY_UNIFORM_COMPONENTS",
> >>
> +"CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxCombined
> UniformC
> >> +omponents),
> extra_ARB_uniform_buffer_object_and_geometry_shader" ],
> >> +
> >> +# GL_ARB_framebuffer_no_attachments / geometry shader
> >> +  [ "MAX_FRAMEBUFFER_LAYERS",
> >> +"CONTEXT_INT(Const.MaxFramebufferLayers),
> >> +extra_ARB_framebuffer_no_attachments_and_geometry_shader" ],
> >
> > Blah, this regresses ARB_framebuffer_no_attachments for 2 reasons --
> >
> > (a) This is in a GL_CORE / GLES31 section, but this enum should be
> > available in compat
> > (b) In compat, there is no geometry shader accessible. Really this
> > should be something like ARB_no_fb && (mesa_is_desktop_gl ||
> > mesa_has_OES_geometry)?
> >
> > I'm seeing 'bin/arb_framebuffer_no_attachments-minmax -fbo -auto' fail
> > because of this... did it not regress for you? The test runs in a
> > compat context...
> >
> >   -ilia
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> 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