[Mesa-dev] [PATCH v2 1/4] mesa: add GREMEDY_string_marker

[Jose Fonseca]

On 20/01/16 19:48, Rob Clark wrote:
> On Wed, Jan 20, 2016 at 2:44 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Wed, Jan 20, 2016 at 2:32 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> From: Rob Clark <robclark at freedesktop.org>
>>>
>>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>>> ---
>>>   src/mapi/glapi/gen/GREMEDY_string_marker.xml | 18 ++++++++++++++++++
>>>   src/mapi/glapi/gen/Makefile.am               |  1 +
>>>   src/mapi/glapi/gen/gl_API.xml                |  2 ++
>>>   src/mesa/main/dd.h                           |  6 ++++++
>>>   src/mesa/main/errors.c                       | 13 +++++++++++++
>>>   src/mesa/main/errors.h                       |  3 +++
>>>   src/mesa/main/extensions_table.h             |  2 ++
>>>   src/mesa/main/mtypes.h                       |  1 +
>>>   src/mesa/main/tests/dispatch_sanity.cpp      |  3 +++
>>>   9 files changed, 49 insertions(+)
>>>   create mode 100644 src/mapi/glapi/gen/GREMEDY_string_marker.xml
>>>
>>> diff --git a/src/mapi/glapi/gen/GREMEDY_string_marker.xml b/src/mapi/glapi/gen/GREMEDY_string_marker.xml
>>> new file mode 100644
>>> index 0000000..ffa3eac
>>> --- /dev/null
>>> +++ b/src/mapi/glapi/gen/GREMEDY_string_marker.xml
>>> @@ -0,0 +1,18 @@
>>> +<?xml version="1.0"?>
>>> +<!DOCTYPE OpenGLAPI SYSTEM "gl_API.dtd">
>>> +
>>> +<!-- Note: no GLX protocol info yet. -->
>>> +
>>> +
>>> +<OpenGLAPI>
>>> +
>>> +<category name="GL_GREMEDY_string_marker" number="311">
>>> +
>>> +    <function name="StringMarkerGREMEDY">
>>> +        <param name="len" type="GLsizei"/>
>>> +        <param name="string" type="const GLvoid *"/>
>>> +    </function>
>>> +
>>> +</category>
>>> +
>>> +</OpenGLAPI>
>>> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
>>> index 900b61a..a40de49 100644
>>> --- a/src/mapi/glapi/gen/Makefile.am
>>> +++ b/src/mapi/glapi/gen/Makefile.am
>>> @@ -188,6 +188,7 @@ API_XML = \
>>>          EXT_texture_array.xml \
>>>          EXT_texture_integer.xml \
>>>          EXT_transform_feedback.xml \
>>> +       GREMEDY_string_marker.xml \
>>>          INTEL_performance_query.xml \
>>>          KHR_debug.xml \
>>>          KHR_context_flush_control.xml \
>>> diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml
>>> index 593ace4..d7ab3bf 100644
>>> --- a/src/mapi/glapi/gen/gl_API.xml
>>> +++ b/src/mapi/glapi/gen/gl_API.xml
>>> @@ -12620,6 +12620,8 @@
>>>
>>>   <xi:include href="EXT_framebuffer_object.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
>>>
>>> +<xi:include href="GREMEDY_string_marker.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
>>> +
>>>   <xi:include href="EXT_packed_depth_stencil.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
>>>
>>>   <xi:include href="EXT_provoking_vertex.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
>>> diff --git a/src/mesa/main/dd.h b/src/mesa/main/dd.h
>>> index 70ed563..d4378e5 100644
>>> --- a/src/mesa/main/dd.h
>>> +++ b/src/mesa/main/dd.h
>>> @@ -762,6 +762,12 @@ struct dd_function_table {
>>>      void (*UseProgram)(struct gl_context *ctx, struct gl_shader_program *shProg);
>>>      /*@}*/
>>>
>>> +   /**
>>> +    * \name GREMEDY debug/marker functions
>>> +    */
>>> +   /*@{*/
>>> +   void (*EmitStringMarker)(struct gl_context *ctx, const GLchar *string, GLsizei len);
>>> +   /*@}*/
>>>
>>>      /**
>>>       * \name Support for multiple T&L engines
>>> diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
>>> index 9e66109..fc3c6d0 100644
>>> --- a/src/mesa/main/errors.c
>>> +++ b/src/mesa/main/errors.c
>>> @@ -1276,6 +1276,19 @@ _mesa_free_errors_data(struct gl_context *ctx)
>>>      mtx_destroy(&ctx->DebugMutex);
>>>   }
>>>
>>> +void GLAPIENTRY
>>> +_mesa_StringMarkerGREMEDY(GLsizei len, const GLvoid * string)
>>> +{
>>> +   GET_CURRENT_CONTEXT(ctx);
>>> +   if (ctx->Driver.EmitStringMarker) {
>>
>> Traditionally this is done more like
>>
>> if (!ctx->Const.GREMEDY_bla) {
>>    _mesa_error(go away);
>>    return;
>> }
>>
>> assume that EmitStringMarker is set, otherwise it's the driver's fault
>> for enabling this ext and not providing a driver callback.
>>
>> But as discussed on IRC, I don't think we should have the GREMEDY
>> stuff at all in the first place. Would be interested in knowing what
>> others have to say.
>>
>
> fwiw, the gremedy part is left over from early incarnation of this,
> before I discovered the khr_debug part.  But figured since I needed
> the EmitStringMarker driver entrypoint anyways, it was trivial to keep
> the extension around (hidden by default).  Maybe some old games/etc
> already have support for it.

https://www.opengl.org/registry/specs/GREMEDY/string_marker.txt states:

   This extension is really only useful for these debuggers and
   profilers, and not for actual drivers. In fact, it is not expected
   that any standard driver would ever implement this extension.

I wouldn't be surprised if enabling this extension triggers application 
anti-debugging mechanisms.

The current implementation only enables it if ST_DEBUG=gremedy, and we 
should keep it off by default like that.

Jose