Replace SOKOL_LOG with runtime callbacks.

[Manuzor]

Removes SOKOL_LOG from sokol_fontstash.h and sokol_args.h entirely because it's not used there.

Please note that merging this as-is will break the binding repos for windows because they do #define SOKOL_LOG(msg) OutputDebugStringA(msg). Here it is for sokol-zig:

https://github.com/floooh/sokol-zig/blob/2bd3b4f44ae137858dbfd5038db40c40563aa86d/src/sokol/c/sokol_defines.h#L5

This should either be removed, or replaced with something like this:

#define SG_LOG_DEFAULT(msg) { OutputDebugStringA("sg: "); OutputDebugStringA(msg); OutputDebugStringA("\n"); }
#define SAPP_LOG_DEFAULT(msg) { OutputDebugStringA("sapp: "); OutputDebugStringA(msg); OutputDebugStringA("\n"); }
#define SAUDIO_LOG_DEFAULT(msg) { OutputDebugStringA("saudio: "); OutputDebugStringA(msg); OutputDebugStringA("\n"); }
#define SGL_LOG_DEFAULT(msg) { OutputDebugStringA("sgl: "); OutputDebugStringA(msg); OutputDebugStringA("\n"); }
#define SDTX_LOG_DEFAULT(msg) { OutputDebugStringA("sdtx: "); OutputDebugStringA(msg); OutputDebugStringA("\n"); }
#define SFETCH_LOG_DEFAULT(msg) { OutputDebugStringA("sfetch: "); OutputDebugStringA(msg); OutputDebugStringA("\n"); }

Closes #720

[floooh]

I only had a quick glance, but looking good. I'll try to take care of smaller PRs again in a couple of days.

Incidentally I started thinking about a different unified error/warning mechanism for the sokol headers, I'll try this out in sokol_spine.h first, for other headers it would be a bit more work (because a lot of error checking code that currently aborts or results in 'undefined behaviour' when asserts are compiled out needs to be rewritten).

Basic idea being that the sokol headers should (almost) never abort/panic on errors, but record any warnings and errors in an internal buffer and then try to carry on (essentially skipping any API calls once a 'fatal' error was recorded). If the API user is interested in error information they would drain the error buffer at at their own place and frequency (e.g. it's not required to call a 'get error' function after each API call that could go wrong). An error would at least contain an error code and a line number, and in debug mode also a human-readable error message, function name and file name (so that it's possible to output 'clickable' error messages).

But as I said, that's a bigger topic to get right across all headers, so for the time being it makes more sense to merge your PR.

[floooh]

Just a little followup to my previous comment, and after a little walk to think about error handling: I think instead of an internal error buffer I will just call a more 'structured' logging callback which instead of just a string message gets the following information:

• a 'tag' string which identifies the header (e.g. the current prefixes, sg, sapp, sgl etc...
• a 'level' (panic, error, warning - where panic is so serious that the error callback should terminate the program because the code can't continue in any way - this should absolutely be the last resort though)
• a unique error code
• the line number (LINE) where the error occured (or rather the error logging function was called)
• an optional human readable error message (debug mode only)
• the optional header's filename (FILE) (also debug mode only, to construct 'clickable' error messages)

The actual logging output can then be provided by a separate sokol header which plugs directly into the logging callback.

As I said, I'll tinker with this idea in the new sokol_spine.h header, and until this new system is implemented I'll merge your logging callback PR :)

[Manuzor]

Thanks for having a look! Your ideas sound very good to me. Maybe I should adjust my PR slightly to pass a sg_log_message struct or similar to the log-callback (see code snippet below). This way, if your latest idea sticks, it should be easier for people to port to the new behavior once you're done. What do you think?

typedef struct sg_log_message {
    const char* message;
    // add new fields later if necessary.
} sg_log_message;

...
_SOKOL_PRIVATE void _sg_log(const char* msg) {
    sg_log_message log_message = {
        .message = msg,
    };
    if (_sg.desc.logger.log_cb) {
        _sg.desc.logger.log_cb(&log_message);
    }
}
...

[floooh]

I was actually thinking about passing a struct to the logging callback, but the downside of this is that each header needs to define its own struct, and that makes it impossible (without dangerous casting) to use the same logging callback across all sokol headers. With distinct args all headers can define the same callback function signature (without any header-specific types) and it's possible to use the same callback function across all headers.

It's better if you keep the PR as is, I will try to merge this 'soon-ish', and it will be quite a while before I get around to add the 'new logging' to all headers anyway.

[floooh]

@Manuzor I'll try to get this PR merged some time this week. I've been thinking a bit more about it though, and think we should make it compatible with SOKOL_LOG somehow. Not sure what the 'least intrusive' way is in your PR though, any ideas?

Would it make sense to just replace the new *_LOG_DEFAULT with the old SOKOL_LOG?

Also, concerning Windows, since this special case already exists for Android, it probably makes sense to add another one for Windows:

        #if defined(__ANDROID__)
            #include <android/log.h>
            #define SAUDIO_LOG_DEFAULT(s) __android_log_write(ANDROID_LOG_INFO, "SOKOL_AUDIO", s)
        #else
            #include <stdio.h>
            #define SAUDIO_LOG_DEFAULT(s) puts(s)
        #endif

...e.g. something like this:

        #if defined(__ANDROID__)
            #include <android/log.h>
            #define SAUDIO_LOG_DEFAULT(s) __android_log_write(ANDROID_LOG_INFO, "SOKOL_AUDIO", s)
        #elif defined(_WIN32)
            #define SAUDIO_LOG_DEFAULT(s) OutputDebugStringA(msg "\n");
        #else
            #include <stdio.h>
            #define SAUDIO_LOG_DEFAULT(s) puts(s)
        #endif

Thoughts? Also if you don't have the time to put those changes into the PR, I can do this during the merge.

[Manuzor]

Yeah, I think you can just replace *_LOG_DEFAULT with the old SOKOL_LOG and it should work. So if people define SOKOL_LOG as empty, and don't provide a callback, no logging will happen. Not sure if you still want to provide a compile-time switch to turn all logging into complete no-ops. Might not be worth it at this point.

The Windows case is slightly tricky because it requires Windows.h to be included or a proper forward decl of OutputDebugStringA. But I think that would be really helpful. Although it would make sense to also log to stdout on Windows. Ideally we would log to stdout for console applications and use OutputDebugStringA for Windows applications. Not sure if there's an easy way to determine that.

And about the implementation you suggested: I think you'll have to call OutputDebugStringA twice (once for the newline) since the passed message is not (always) a string literal.

I'm not on a PC right now because I'm a bit sick so I won't be able to work on any of this properly until at least the weekend I expect. Sorry!

[floooh]

And about the implementation you suggested: I think you'll have to call OutputDebugStringA twice (once for the newline) since the passed message is not (always) a string literal.

Ah right, my bad :)

I'll work on this PR first: #707 and then I can look into the required SOKOL_LOG changes, no worries.

[floooh]

Ok, I'll start working on this now.

[floooh]

I have simplified the macro setup a bit:

• replace the *_DEFAULT_LOG() macro with SOKOL_LOG() (to make the macros backward compatible)
• drop the 'internal' NO_LOG macros, instead check SOKOL_DEBUG directly

I decided against adding a Windows OutputDebugStringA() ifdef for now... I'm currently thinking that with the next change to replace the simple string-pointer logging callback with a more 'structured' callback, I will also add an easy to plug sokol_log.h header which handles logging for Android and Windows, while the minimal 'fallback logging' will only use puts.

I'll do a bit more testing and then merge to master later today or tomorrow.

PS: 1eebe4c

[floooh]

Ok merged, thanks for the PR!