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

[Jose Fonseca]

On 04/12/15 23:20, Ilia Mirkin wrote:
> On Fri, Dec 4, 2015 at 6:14 PM, Jose Fonseca <jfonseca at vmware.com> wrote:
>> 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's a little clever... this is why pipe_debug_message is a macro.
> Every separate callsite gets its own id. It's initialized to 0, and
> passed in as a pointer... the mesa function actually assigns it a new
> id if the passed in id is 0. It's a bit racey and definitely a weird
> API, but... it was easy :)

Ah, I see it now.  It's not reproducible, but at least different message 
sites have different id.

Jose