[PATCH] retrace: use KHR_debug to emit call no's

[Rob Clark]

On Fri, Jan 22, 2016 at 6:35 PM, José Fonseca <jose.r.fonseca at gmail.com> wrote:
> Rob,
>
> Change looks good in principled, but it required a bit more tweaking.
>
> See https://github.com/apitrace/apitrace/commits/call-no-markers  . Give it
> a go and let me know if it still works for you.

oh, sorry for the delayed response.. I overlooked your reply..

thanks for the cleanups and pushing this

> Note in particular:
> - you need to pass --marker command line option to glretrace
> - Mesa refused markers between glBegin/End.  Could you check if this is a
> bug in Mesa or the right behavior?

hmm, I think there are supposed to be some restrictions between glBegin/glEnd..

https://www.opengl.org/sdk/docs/man2/xhtml/glBegin.xml

says: "GL_INVALID_OPERATION is generated if a command other than
glVertex, ... is executed between the execution of glBegin and the
corresponding execution glEnd."..  I'm not really a spec wiz, but I
assume that still applies as other extensions add entry points.

BR,
-R

>
> Jose
>
>
> On Wed, Jan 20, 2016 at 9:52 PM, Rob Clark <robdclark at gmail.com> wrote:
>>
>> If the extension is present, use it to log call numbers for draw
>> commands, using GL_DEBUG_TYPE_MARKER.  If supported by the driver,
>> this embeds the draw call no's in the commandstream.
>>
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  retrace/glretrace.hpp      |  2 ++
>>  retrace/glretrace_main.cpp | 18 ++++++++++++++++++
>>  retrace/retrace.hpp        |  2 ++
>>  retrace/retrace_main.cpp   |  4 ++++
>>  4 files changed, 26 insertions(+)
>>
>> diff --git a/retrace/glretrace.hpp b/retrace/glretrace.hpp
>> index fc36c52..82ed920 100644
>> --- a/retrace/glretrace.hpp
>> +++ b/retrace/glretrace.hpp
>> @@ -57,6 +57,8 @@ struct Context {
>>
>>      bool used;
>>
>> +    bool KHR_debug;
>> +
>>      inline glprofile::Profile
>>      profile(void) const {
>>          return wsContext->profile;
>> diff --git a/retrace/glretrace_main.cpp b/retrace/glretrace_main.cpp
>> index c3783cc..0135662 100755
>> --- a/retrace/glretrace_main.cpp
>> +++ b/retrace/glretrace_main.cpp
>> @@ -399,6 +399,7 @@ initContext() {
>>      supportsElapsed     =
>> currentContext->hasExtension("GL_EXT_timer_query") || supportsTimestamp;
>>      supportsOcclusion   =
>> currentProfile.versionGreaterOrEqual(glprofile::API_GL, 1, 5);
>>      supportsARBShaderObjects =
>> currentContext->hasExtension("GL_ARB_shader_objects");
>> +    currentContext->KHR_debug =
>> currentContext->hasExtension("GL_KHR_debug");
>>
>>  #ifdef __APPLE__
>>      // GL_TIMESTAMP doesn't work on Apple.  GL_TIME_ELAPSED still does
>> however.
>> @@ -766,6 +767,23 @@ public:
>>      dumpState(StateWriter &writer) {
>>          glstate::dumpCurrentContext(writer);
>>      }
>> +
>> +    void
>> +    log(trace::Call &call) {
>> +        glretrace::Context *currentContext =
>> glretrace::getCurrentContext();
>> +        if (currentContext && currentContext->KHR_debug) {
>> +            char *str;
>> +            int len = asprintf(&str, "%d:%s", call.no, call.name());
>> +            if (len > 0) {
>> +                glDebugMessageInsert(GL_DEBUG_SOURCE_THIRD_PARTY,
>> +                                     GL_DEBUG_TYPE_MARKER, 0,
>> +                                     GL_DEBUG_SEVERITY_NOTIFICATION,
>> +                                     len, str);
>> +                free(str);
>> +            }
>> +        }
>> +    }
>> +
>>  };
>>
>>  static GLDumper glDumper;
>> diff --git a/retrace/retrace.hpp b/retrace/retrace.hpp
>> index 082879f..66024c5 100644
>> --- a/retrace/retrace.hpp
>> +++ b/retrace/retrace.hpp
>> @@ -226,6 +226,8 @@ public:
>>
>>      virtual void
>>      dumpState(StateWriter &) = 0;
>> +
>> +    virtual void log(trace::Call &call) {}
>>  };
>>
>>
>> diff --git a/retrace/retrace_main.cpp b/retrace/retrace_main.cpp
>> index bc00ffe..6344880 100644
>> --- a/retrace/retrace_main.cpp
>> +++ b/retrace/retrace_main.cpp
>> @@ -238,6 +238,10 @@ retraceCall(trace::Call *call) {
>>          }
>>      }
>>
>> +    if (call->flags & trace::CALL_FLAG_RENDER) {
>> +       dumper->log(*call);
>> +    }
>> +
>>      retracer.retrace(*call);
>>
>>      if (doSnapshot) {
>> --
>> 2.5.0
>>
>