[Mesa-dev] [PATCH v2] mesa: remove ARB_geometry_shader4
Ian Romanick
idr at freedesktop.org
Mon Nov 30 11:21:08 PST 2015
On 11/29/2015 03:20 PM, Kenneth Graunke wrote:
> 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.
I had forgotten about GL_ARB_separate_shader_objects. It's annoying
that the glProgramParameteri man page doesn't mention it. We *do*
support that extension in compatibility too:
EXT(ARB_separate_shader_objects , dummy_true , GLL, GLC, x , x , 2010)
> #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?
The usual way to tell whether or not something can go in a display list
is whether or not it's GLX protocol is a render op or a single op.
Before display lists were deprecated, all render ops could go in a
display list. In GL_ARB_geometry_shader4, glProgramParameteriARB is a
render op:
ProgramParameteriARB
2 16 rendering command length
2 266 rendering command opcode
4 CARD32 program
4 ENUM pname
4 INT32 value
BUT... GL_ARB_geometry_shader4 saves the day:
Add the command ProgramParameteriARB to the list of commands that are not
compiled into a display list, but executed immediately, under "Program and
Shader Objects", p. 241
Based on that, I'd like to see a patch for 11.1 and 11.0 that removes
glProgramParameteri display list support.
> --Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151130/276e75fe/attachment.sig>
More information about the mesa-dev
mailing list