Also... this patch is Reviewed-by: Ian Romanick <ian.d.roman...@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.lofst...@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.lofst...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
