[Mesa-dev] [PATCH] svga: use the debug callback to report issues to the state tracker

[Jose Fonseca]

On 04/12/15 22:40, Jose Fonseca wrote:
> On 04/12/15 20:04, Ilia Mirkin wrote:
>> On Fri, Dec 4, 2015 at 3:01 PM, Brian Paul <brianp at vmware.com> wrote:
>>> On 12/04/2015 12:46 PM, Ilia Mirkin wrote:
>>>>
>>>> On Fri, Dec 4, 2015 at 2:42 PM, Brian Paul <brianp at vmware.com> wrote:
>>>>>
>>>>> +   if (templ->poly_smooth && svga->debug.callback.debug_message) {
>>>>> +      /* note: we always need a % something in the message string */
>>>>
>>>>
>>>> Why? Did I mess something up?
>>>
>>>
>>> If I write it without the dummy %s I get:
>>>
>>> In file included from
>>> ../../../../src/gallium/auxiliary/util/u_inlines.h:36:0,
>>>                   from svga_pipe_rasterizer.c:29:
>>> svga_pipe_rasterizer.c: In function 'svga_create_rasterizer_state':
>>> ../../../../src/gallium/auxiliary/util/u_debug.h:273:40: error: expected
>>> expression before ')' token
>>>                          fmt, __VA_ARGS__); \
>>>                                          ^
>>> svga_pipe_rasterizer.c:357:7: note: in expansion of macro
>>> 'pipe_debug_message'
>>>         pipe_debug_message(&svga->debug.callback, CONFORMANCE,
>>>         ^
>>>
>>
>> Hmmmm... so that'd be a "yes" to me messing it up :) I'll have a look
>> to see if there's something simple I can do here. Off-hand it feels
>> like it should be #__VA_ARGS__? Been a while since I looked at the
>> specifics of all this.
>>
>>>
>>>>
>>>> BTW, right now this callback is only set for debug contexts. Perhaps
>>>> that was not an ideal decision... just an FYI though.
>>>
>>>
>>> apitrace creates a debug context so that's the main thing for me
>>> right now.
>>> But it might be nice if setting MESA_DEBUG would cause the debug/info
>>> messages to be printed to stderr for those apps that don't use
>>> GL_ARB_debug_output.
>>
>> Yeah, we should probably also throw in Enable/Disable hooks into
>> st/mesa that flip it on/off when flipping GL_DEBUG_OUTPUT (or whatever
>> the enum is).
>>
>> BTW, note that apitrace cuts messages off after 100 of them which is
>> pretty annoying -- I opened an issue at
>> https://github.com/apitrace/apitrace/issues/395 a while back.
>
> Not just "100", but 100 *per ID*.
>
> Which is very different.
>
> Why is it so annoying?  A patch for an option is welcome anyway BTW.
>

I noticed that pipe_debug_message() doesn't take a message ID.

It looks  like the same id is passed for every context:


/**
    * Handles generating a GL_ARB_debug_output message ID generated by 
the GL or
    * GLSL compiler.
    *
    * The GL API has this "ID" mechanism, where the intention is to allow a
    * client to filter in/out messages based on source, type, and ID. 
Of course,
    * building a giant enum list of all debug output messages that Mesa 
might
    * generate is ridiculous, so instead we have our caller pass us a 
pointer to
    * static storage where the ID should get stored.  This ID will be shared
    * across all contexts for that message (which seems like a desirable
    * property, even if it's not expected by the spec), but note that it 
won't be
    * the same between executions if messages aren't generated in the 
same order.
    */


This is far from ideal.

I think at east Mesa should use ID namespaces: each Mesa driver / pipe 
driver should be assigned an unique driver ID (high word), then use low 
word internally.

For the low word, it doesn't need to be an enum per message, but it 
should be more fined grained than one ID per context.

If nothing else, you could use the address or hash of the printf-like 
format string to make an id.


Jose