[Mesa-dev] [PATCH v2] mesa: remove ARB_geometry_shader4
Ian Romanick
idr at freedesktop.org
Mon Nov 30 11:24:52 PST 2015
Also... this patch is
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
On 11/30/2015 11:21 AM, Ian Romanick wrote:
> 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
>
>
>
>
> _______________________________________________
> 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