[Mesa-dev] [PATCH v2] mesa: remove ARB_geometry_shader4
Kenneth Graunke
kenneth at whitecape.org
Sun Nov 29 15:20:32 PST 2015
On Sunday, November 29, 2015 02:17:06 PM Ian Romanick wrote:
> On 11/25/2015 03:16 AM, Marta Lofstedt wrote:
> > From: Marta Lofstedt <marta.lofstedt at intel.com>
> >
> > 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.
> >
> > 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 | 55 ----------------------------
> > src/mesa/main/get.c | 7 ----
> > src/mesa/main/get_hash_params.py | 12 ++----
> > src/mesa/main/mtypes.h | 3 +-
> > src/mesa/main/tests/enum_strings.cpp | 6 ---
> > 11 files changed, 9 insertions(+), 141 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..6243bdd 100644
> > --- a/src/mapi/glapi/gen/gl_API.xml
> > +++ b/src/mapi/glapi/gen/gl_API.xml
> > @@ -7975,7 +7975,7 @@
> >
> > <!-- 46. GL_ARB_framebuffer_sRGB -->
> >
> > -<xi:include href="ARB_geometry_shader4.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
> > +<!-- 47. GL_ARB_geometry_shader4. There are no intentions to implement this extension -->
> >
> > <!-- 48. GL_ARB_half_float_vertex -->
> >
> > 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..ba2e670 100644
> > --- a/src/mesa/main/dlist.c
> > +++ b/src/mesa/main/dlist.c
> > @@ -457,11 +457,6 @@ typedef enum
> > OPCODE_SAMPLER_PARAMETERIIV,
> > OPCODE_SAMPLER_PARAMETERUIV,
> >
> > - /* GL_ARB_geometry_shader4 */
> > - OPCODE_PROGRAM_PARAMETERI,
> > - OPCODE_FRAMEBUFFER_TEXTURE,
> > - OPCODE_FRAMEBUFFER_TEXTURE_FACE,
> > -
> > /* GL_ARB_sync */
> > OPCODE_WAIT_SYNC,
> >
> > @@ -7554,44 +7549,6 @@ save_SamplerParameterIuiv(GLuint sampler, GLenum pname, const GLuint *params)
> > }
> > }
> >
> > -/* GL_ARB_geometry_shader4 */
> > -static void GLAPIENTRY
> > -save_ProgramParameteri(GLuint program, GLenum pname, GLint value)
> > -{
> > - Node *n;
> > - GET_CURRENT_CONTEXT(ctx);
> > - ASSERT_OUTSIDE_SAVE_BEGIN_END_AND_FLUSH(ctx);
> > - n = alloc_instruction(ctx, OPCODE_PROGRAM_PARAMETERI, 3);
> > - if (n) {
> > - n[1].ui = program;
> > - n[2].e = pname;
> > - n[3].i = value;
> > - }
> > - if (ctx->ExecuteFlag) {
> > - CALL_ProgramParameteri(ctx->Exec, (program, pname, value));
> > - }
> > -}
>
> You can't remove this function. It's still used for
> GL_PROGRAM_BINARY_RETRIEVABLE_HINT. See
> https://www.opengl.org/sdk/docs/man/html/glProgramParameter.xhtml. We
> support GL_ARB_get_program_binary on compatibility profile. We should
> probably have a compatibility profile piglit test for this. :)
I asked Marta to remove this, and explained my reasoning in:
http://lists.freedesktop.org/archives/mesa-dev/2015-November/101269.html
I'll recap here. glProgramParameteri is used for:
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 is not intended to work.
#3 is trickier...as you say, we support GL_ARB_get_program_binary in
legacy profiles. But the extension doesn't mention anything about
display list interactions. Sure, you could make a display list that
sets GL_PROGRAM_BINARY_RETRIEVABLE_HINT, but it sets a program
compilation flag that only takes effect at the next glLinkProgram or
glProgramBinary call...neither of which are allowed in display lists.
Sure, you could run a display list to set it, /then/ call LinkProgram,
but...why?
Do you disagree, Ian? Have I missed some text somewhere, or are we
just interpreting the spec differently?
--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151129/3cf6f0f2/attachment.sig>
More information about the mesa-dev
mailing list