[Mesa-dev] [PATCH] mesa: remove ARB_geometry_shader4

Lofstedt, Marta marta.lofstedt at intel.com
Tue Nov 24 05:56:08 PST 2015


> -----Original Message-----
> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> Behalf Of Kenneth Graunke
> Sent: Tuesday, November 24, 2015 12:12 PM
> To: mesa-dev at lists.freedesktop.org; Marta Lofstedt
> Subject: Re: [Mesa-dev] [PATCH] mesa: remove ARB_geometry_shader4
> 
> On Tuesday, November 24, 2015 10:20:11 AM Marta Lofstedt wrote:
> > From: Marta Lofstedt <marta.lofstedt at intel.com>
> 
> Hi Marta!
> 
> Thanks for doing this!
> 
> > Since all drivers support OpenGL 3.2 geometry shaders, there is no
> > reason to keep the ARB_geometry_shader4.
> 
> We do have drivers that don't support OpenGL 3.2 (such as Ironlake).
> I might instead say something like:
> 
> No drivers currently implement ARB_geometry_shader4, nor are there any
> plans to implement it.  We only support the version of geometry shaders that
> was incorporated into OpenGL 3.2 / GLSL 1.50.

Thanks Kenneth,

I will fix V2 tomorrow anyways, since I forgot to remove some enum_strings.cpp stuff.

> 
> >
> > Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
> > ---
> >  src/mapi/glapi/gen/ARB_geometry_shader4.xml | 57 -------------------------
> ----
> >  src/mapi/glapi/gen/Makefile.am              |  1 -
> >  src/mapi/glapi/gen/gl_API.xml               |  2 -
> >  src/mesa/main/api_validate.c                |  2 +-
> >  src/mesa/main/config.h                      |  2 +-
> >  src/mesa/main/context.h                     |  3 +-
> >  src/mesa/main/dlist.c                       |  6 +--
> >  src/mesa/main/get.c                         |  7 ----
> >  src/mesa/main/get_hash_params.py            | 12 ++----
> >  src/mesa/main/mtypes.h                      |  3 +-
> >  10 files changed, 11 insertions(+), 84 deletions(-)  delete mode
> > 100644 src/mapi/glapi/gen/ARB_geometry_shader4.xml
> >
> > diff --git a/src/mapi/glapi/gen/ARB_geometry_shader4.xml
> > b/src/mapi/glapi/gen/ARB_geometry_shader4.xml
> > deleted file mode 100644
> > index 280e7a0..0000000
> > --- a/src/mapi/glapi/gen/ARB_geometry_shader4.xml
> > +++ /dev/null
> > @@ -1,57 +0,0 @@
> > -<?xml version="1.0"?>
> > -<!DOCTYPE OpenGLAPI SYSTEM "gl_API.dtd">
> > -
> > -<!-- Note: no GLX protocol info yet. -->
> > -
> > -<OpenGLAPI>
> > -
> > -
> > -<category name="GL_ARB_geometry_shader4" number="47">
> > -    <enum name="GEOMETRY_SHADER_ARB"              value="0x8DD9"/>
> > -    <enum name="GEOMETRY_VERTICES_OUT_ARB"
> value="0x8DDA"/>
> > -    <enum name="GEOMETRY_INPUT_TYPE_ARB"          value="0x8DDB"/>
> > -    <enum name="GEOMETRY_OUTPUT_TYPE_ARB"         value="0x8DDC"/>
> > -    <enum name="MAX_GEOMETRY_TEXTURE_IMAGE_UNITS_ARB"
> value="0x8C29"/>
> > -    <enum name="MAX_GEOMETRY_VARYING_COMPONENTS_ARB"
> value="0x8DDD"/>
> > -    <enum name="MAX_VERTEX_VARYING_COMPONENTS_ARB"
> value="0x8DDE"/>
> > -    <enum name="MAX_VARYING_COMPONENTS"
> value="0x8B4B"/>
> > -    <enum name="MAX_GEOMETRY_UNIFORM_COMPONENTS_ARB"
> value="0x8DDF"/>
> > -    <enum name="MAX_GEOMETRY_OUTPUT_VERTICES_ARB"
> value="0x8DE0"/>
> > -    <enum
> name="MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS_ARB"
> value="0x8DE1"/>
> > -    <enum name="LINES_ADJACENCY_ARB"          value="0xA"/>
> > -    <enum name="LINE_STRIP_ADJACENCY_ARB"     value="0xB"/>
> > -    <enum name="TRIANGLES_ADJACENCY_ARB"      value="0xC"/>
> > -    <enum name="TRIANGLE_STRIP_ADJACENCY_ARB" value="0xD"/>
> > -    <enum name="FRAMEBUFFER_INCOMPLETE_LAYER_TARGETS_ARB"
> value="0x8DA8"/>
> > -    <enum name="FRAMEBUFFER_INCOMPLETE_LAYER_COUNT_ARB"
> value="0x8DA9"/>
> > -    <enum name="FRAMEBUFFER_ATTACHMENT_LAYERED_ARB"
> value="0x8DA7"/>
> > -    <enum name="FRAMEBUFFER_ATTACHMENT_TEXTURE_LAYER"
> value="0x8CD4"/>
> > -    <enum name="PROGRAM_POINT_SIZE_ARB" value="0x8642"/>
> > -    <function name="ProgramParameteriARB"  alias="ProgramParameteri">
> > -        <param name="program" type="GLuint"/>
> > -        <param name="pname" type="GLenum"/>
> > -        <param name="value" type="GLint"/>
> > -    </function>
> > -    <function name="FramebufferTextureARB"
> alias="FramebufferTexture">
> > -        <param name="target" type="GLenum"/>
> > -        <param name="attachment" type="GLenum"/>
> > -        <param name="texture" type="GLuint"/>
> > -        <param name="level" type="GLint"/>
> > -    </function>
> > -    <function name="FramebufferTextureLayerARB"
> alias="FramebufferTextureLayer">
> > -        <param name="target" type="GLenum"/>
> > -        <param name="attachment" type="GLenum"/>
> > -        <param name="texture" type="GLuint"/>
> > -        <param name="level" type="GLint"/>
> > -        <param name="layer" type="GLint"/>
> > -    </function>
> > -    <function name="FramebufferTextureFaceARB" exec="skip">
> > -        <param name="target" type="GLenum"/>
> > -        <param name="attachment" type="GLenum"/>
> > -        <param name="texture" type="GLuint"/>
> > -        <param name="level" type="GLint"/>
> > -        <param name="face" type="GLenum"/>
> > -    </function>
> > -</category>
> > -
> > -</OpenGLAPI>
> > diff --git a/src/mapi/glapi/gen/Makefile.am
> > b/src/mapi/glapi/gen/Makefile.am index a5a26a6..40b0e65 100644
> > --- a/src/mapi/glapi/gen/Makefile.am
> > +++ b/src/mapi/glapi/gen/Makefile.am
> > @@ -133,7 +133,6 @@ API_XML = \
> >  	ARB_ES3_compatibility.xml \
> >  	ARB_framebuffer_no_attachments.xml \
> >  	ARB_framebuffer_object.xml \
> > -	ARB_geometry_shader4.xml \
> >  	ARB_get_program_binary.xml \
> >  	ARB_get_texture_sub_image.xml \
> >  	ARB_gpu_shader_fp64.xml \
> > diff --git a/src/mapi/glapi/gen/gl_API.xml
> > b/src/mapi/glapi/gen/gl_API.xml index ec83cd4..8ff110c 100644
> > --- a/src/mapi/glapi/gen/gl_API.xml
> > +++ b/src/mapi/glapi/gen/gl_API.xml
> > @@ -7975,8 +7975,6 @@
> >
> >  <!-- 46. GL_ARB_framebuffer_sRGB -->
> >
> > -<xi:include href="ARB_geometry_shader4.xml"
> > xmlns:xi="http://www.w3.org/2001/XInclude"/>
> > -
> >  <!-- 48. GL_ARB_half_float_vertex -->
> >
> >  <xi:include href="ARB_instanced_arrays.xml"
> > xmlns:xi="http://www.w3.org/2001/XInclude"/>
> > diff --git a/src/mesa/main/api_validate.c
> > b/src/mesa/main/api_validate.c index a490189..cbfb6b5 100644
> > --- a/src/mesa/main/api_validate.c
> > +++ b/src/mesa/main/api_validate.c
> > @@ -170,7 +170,7 @@ _mesa_valid_prim_mode(struct gl_context *ctx,
> GLenum mode, const char *name)
> >        return GL_FALSE;
> >     }
> >
> > -   /* From the ARB_geometry_shader4 spec:
> > +   /* From the OpenGL 4.5 specification, section 11.3.1:
> >      *
> >      * The error INVALID_OPERATION is generated if Begin, or any command
> that
> >      * implicitly calls Begin, is called when a geometry shader is active and:
> > diff --git a/src/mesa/main/config.h b/src/mesa/main/config.h index
> > f29de5f..2d53e2f 100644
> > --- a/src/mesa/main/config.h
> > +++ b/src/mesa/main/config.h
> > @@ -246,7 +246,7 @@
> >  #define MAX_FEEDBACK_BUFFERS 4
> >  #define MAX_FEEDBACK_ATTRIBS 32
> >
> > -/** For GL_ARB_geometry_shader4 */
> > +/** For geometry shader */
> >  /*@{*/
> >  #define MAX_GEOMETRY_UNIFORM_COMPONENTS              512
> >  #define MAX_GEOMETRY_OUTPUT_VERTICES                 256
> > diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h index
> > 4798b1f..8b64f45 100644
> > --- a/src/mesa/main/context.h
> > +++ b/src/mesa/main/context.h
> > @@ -330,8 +330,7 @@ _mesa_is_gles31(const struct gl_context *ctx)
> > static inline bool  _mesa_has_geometry_shaders(const struct gl_context
> > *ctx)  {
> > -   return _mesa_is_desktop_gl(ctx) &&
> > -      (ctx->Version >= 32 || ctx->Extensions.ARB_geometry_shader4);
> > +   return _mesa_is_desktop_gl(ctx) && ctx->Version >= 32;
> >  }
> >
> >
> > diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c index
> > 2b65b2e..dc3b632 100644
> > --- a/src/mesa/main/dlist.c
> > +++ b/src/mesa/main/dlist.c
> > @@ -457,7 +457,7 @@ typedef enum
> >     OPCODE_SAMPLER_PARAMETERIIV,
> >     OPCODE_SAMPLER_PARAMETERUIV,
> >
> > -   /* GL_ARB_geometry_shader4 */
> > +   /* For geometry shader */
> >     OPCODE_PROGRAM_PARAMETERI,
> >     OPCODE_FRAMEBUFFER_TEXTURE,
> >     OPCODE_FRAMEBUFFER_TEXTURE_FACE,
> > @@ -7554,7 +7554,7 @@ save_SamplerParameterIuiv(GLuint sampler,
> GLenum pname, const GLuint *params)
> >     }
> >  }
> >
> > -/* GL_ARB_geometry_shader4 */
> > +/* OpenGL 3.2 */
> >  static void GLAPIENTRY
> >  save_ProgramParameteri(GLuint program, GLenum pname, GLint value)  {
> > @@ -8841,7 +8841,7 @@ execute_list(struct gl_context *ctx, GLuint list)
> >              }
> >              break;
> >
> > -         /* GL_ARB_geometry_shader4 */
> > +         /* OpenGL 3.2 geometry shader */
> >           case OPCODE_PROGRAM_PARAMETERI:
> >              CALL_ProgramParameteri(ctx->Exec, (n[1].ui, n[2].e, n[3].i));
> >              break;
> 
> This part isn't quite right - this is legacy GL display list code, which doesn't
> exist in OpenGL 3.2 core profile.  So it can't be for GL 3.2 geometry shaders :)
> 
> It looks like glProgramParameteri is used for three things:
> 
> 1. GL_ARB_geometry_shader4 pnames
> 2. GL_PROGRAM_SEPARABLE from GL_ARB_separate_shader_objects 3.
> GL_PROGRAM_BINARY_RETRIEVABLE_HINT from
> GL_ARB_get_program_binary
> 
> #2 is not a concern, as we only support SSO in core profiles, where this code
> doesn't exist.  The SSO spec also mentions a bunch of display list interactions,
> but doesn't mention ProgramParameteri, so I would assume that it isn't
> there.  (It doesn't make much sense...)
> 
> #3 is trickier...we /do/ apparently support GL_ARB_get_program_binary in
> legacy profiles (for some bizarre reason).  But the extension doesn't mention
> anything about display list interactions.  I think it's fair to assume that there
> aren't any, especially since it makes very little sense to set compilation hints
> in a display list :)
> 
> Also...looking at the other opcodes in that area...there's two for
> glFramebufferTexture...which is only available in 3.2, since layered rendering
> doesn't exist before then (without a GS, you can't set LayerID anyway).  We
> also don't support GL_AMD_vertex_shader_layered in legacy profile.  So no
> layered rendering there, either.
> 
> The upshot is that I think you can delete even more code :)
> 
> - Delete save_ProgramParmateri and save_FramebufferTexture
> - Delete OPCODE_PROGRAM_PARAMETERI,
> OPCODE_FRAMEBUFFER_TEXTURE,
>   and OPCODE_FRAMEBUFFER_TEXTURE_FACE, and everything that uses
>   them.  (The FACE variant isn't even hooked up at all...heh!)
> 
> With that extra code removed, this would get a:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> Thanks again for working on GL_OES_geometry_shader, and cleaning this up
> along the way!
> 
> > diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c index
> > 539c411..c6a2e5b 100644
> > --- a/src/mesa/main/get.c
> > +++ b/src/mesa/main/get.c
> > @@ -398,7 +398,6 @@ EXTRA_EXT(EXT_pixel_buffer_object);
> >  EXTRA_EXT(ARB_vertex_program);
> >  EXTRA_EXT2(NV_point_sprite, ARB_point_sprite);
> > EXTRA_EXT2(ARB_vertex_program, ARB_fragment_program);
> > -EXTRA_EXT(ARB_geometry_shader4);
> EXTRA_EXT(ARB_color_buffer_float);
> >  EXTRA_EXT(EXT_framebuffer_sRGB);
> >  EXTRA_EXT(OES_EGL_image_external);
> > @@ -455,12 +454,6 @@ static const int extra_gl32_es3[] = {
> >      EXTRA_END,
> >  };
> >
> > -static const int extra_gl32_ARB_geometry_shader4[] = {
> > -    EXTRA_VERSION_32,
> > -    EXT(ARB_geometry_shader4),
> > -    EXTRA_END
> > -};
> > -
> >  static const int extra_gl40_ARB_sample_shading[] = {
> >     EXTRA_VERSION_40,
> >     EXT(ARB_sample_shading),
> > diff --git a/src/mesa/main/get_hash_params.py
> > b/src/mesa/main/get_hash_params.py
> > index 9b22b91..0c58b30 100644
> > --- a/src/mesa/main/get_hash_params.py
> > +++ b/src/mesa/main/get_hash_params.py
> > @@ -765,14 +765,6 @@ descriptor=[
> >    [ "MAX_TRANSFORM_FEEDBACK_BUFFERS",
> "CONTEXT_INT(Const.MaxTransformFeedbackBuffers),
> extra_ARB_transform_feedback3" ],
> >    [ "MAX_VERTEX_STREAMS", "CONTEXT_INT(Const.MaxVertexStreams),
> > extra_ARB_transform_feedback3_ARB_gpu_shader5" ],
> >
> > -# GL_ARB_geometry_shader4
> > -  [ "MAX_GEOMETRY_TEXTURE_IMAGE_UNITS_ARB",
> >
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxTextureIma
> geUnits)
> > , extra_gl32_ARB_geometry_shader4" ],
> > -  [ "MAX_GEOMETRY_OUTPUT_VERTICES_ARB",
> > "CONTEXT_INT(Const.MaxGeometryOutputVertices),
> > extra_gl32_ARB_geometry_shader4" ],
> > -  [ "MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS_ARB",
> > "CONTEXT_INT(Const.MaxGeometryTotalOutputComponents),
> > extra_gl32_ARB_geometry_shader4" ],
> > -  [ "MAX_GEOMETRY_UNIFORM_COMPONENTS_ARB",
> >
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxUniformCo
> mponents)
> > , extra_gl32_ARB_geometry_shader4" ],
> > -  [ "MAX_GEOMETRY_VARYING_COMPONENTS_ARB",
> >
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxOutputCo
> mponents),
> > extra_ARB_geometry_shader4" ],
> > -  [ "MAX_VERTEX_VARYING_COMPONENTS_ARB",
> >
> "CONTEXT_INT(Const.Program[MESA_SHADER_VERTEX].MaxOutputCompo
> nents),
> > extra_ARB_geometry_shader4" ],
> > -
> >  # GL_ARB_color_buffer_float
> >    [ "RGBA_FLOAT_MODE_ARB", "BUFFER_FIELD(Visual.floatMode,
> > TYPE_BOOLEAN), extra_core_ARB_color_buffer_float_and_new_buffers"
> ],
> >
> > @@ -800,6 +792,10 @@ descriptor=[
> >    [ "CONTEXT_PROFILE_MASK", "CONTEXT_INT(Const.ProfileMask),
> extra_version_32" ],
> >    [ "MAX_GEOMETRY_INPUT_COMPONENTS",
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxInputComp
> onents), extra_version_32" ],
> >    [ "MAX_GEOMETRY_OUTPUT_COMPONENTS",
> >
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxOutputCo
> mponents),
> > extra_version_32" ],
> > +  [ "MAX_GEOMETRY_TEXTURE_IMAGE_UNITS",
> > +
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxTextureIma
> geUnit
> > + s), extra_version_32" ],  [ "MAX_GEOMETRY_OUTPUT_VERTICES",
> > + "CONTEXT_INT(Const.MaxGeometryOutputVertices), extra_version_32"
> ],
> > + [ "MAX_GEOMETRY_TOTAL_OUTPUT_COMPONENTS",
> > + "CONTEXT_INT(Const.MaxGeometryTotalOutputComponents),
> > + extra_version_32" ],  [ "MAX_GEOMETRY_UNIFORM_COMPONENTS",
> > +
> "CONTEXT_INT(Const.Program[MESA_SHADER_GEOMETRY].MaxUniformCo
> mponent
> > + s), extra_version_32" ],
> >
> >  # GL_ARB_robustness
> >    [ "RESET_NOTIFICATION_STRATEGY_ARB",
> > "CONTEXT_ENUM(Const.ResetStrategy), NO_EXTRA" ], diff --git
> > a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index
> > d425571..46de8f8 100644
> > --- a/src/mesa/main/mtypes.h
> > +++ b/src/mesa/main/mtypes.h
> > @@ -3397,7 +3397,7 @@ struct gl_constants
> >      */
> >     GLuint MaxUserAssignableUniformLocations;
> >
> > -   /** GL_ARB_geometry_shader4 */
> > +   /** geometry shader */
> >     GLuint MaxGeometryOutputVertices;
> >     GLuint MaxGeometryTotalOutputComponents;
> >
> > @@ -3682,7 +3682,6 @@ struct gl_extensions
> >     GLboolean ARB_enhanced_layouts;
> >     GLboolean ARB_explicit_attrib_location;
> >     GLboolean ARB_explicit_uniform_location;
> > -   GLboolean ARB_geometry_shader4;
> >     GLboolean ARB_gpu_shader5;
> >     GLboolean ARB_gpu_shader_fp64;
> >     GLboolean ARB_half_float_vertex;
> >


More information about the mesa-dev mailing list