Possibly faster GL timestamp query result check

[slomp]

Addresses #1380

(Co-authored by @sacereda)

[slomp]

Two things to consider:

• Is ~0ull a good default? Should we use 0 instead? Could these timestamps actually happen and cause trouble?
• Should the GetTimestamp helper function be tracy_force_inline?

[wolfpld]

Is ~0ull a good default? Should we use 0 instead? Could these timestamps actually happen and cause trouble?

The actual (non-conforming) Intel implementation I looked at did something like mod 36 bits, so you can get 0, but not ~0.

[slomp]

I suppose if we wanted to be paranoid about it, we would:
a) initialize the timestamp to the ˜0ull sentinel
b) call glGetQueryObjectui64v with GL_QUERY_RESULT_NO_WAIT
c) if the resulting timestamp is the sentinel, fall-through the habitual GL_QUERY_RESULT_AVAILABLE path to be sure

[slomp]

I suppose if we wanted to be paranoid about it, we would: a) initialize the timestamp to the ˜0ull sentinel b) call glGetQueryObjectui64v with GL_QUERY_RESULT_NO_WAIT c) if the resulting timestamp is the sentinel, fall-through the habitual GL_QUERY_RESULT_AVAILABLE path to be sure

On a second thought, this could sort of defeat the purpose of the GL_QUERY_RESULT_NO_WAIT optimization, as it would just short-circuit the case where the timestamp is ready, but would extend the case where it is not. So it's a trade-off between what to expect on average with the pending queries every time Collect() gets called.

[wolfpld]

GL_QUERY_RESULT_NO_WAIT is used unconditionally without a compile-time guard. The symbol only exists when GL headers include GL_ARB_query_buffer_object. The runtime CheckFeature check doesn't help if the symbol is absent at compile time — the file won't compile. Projects that currently compile against TracyOpenGL.hpp without this extension in their headers will break.

The existing code does !defined GL_TIMESTAMP && defined GL_TIMESTAMP_EXT check and "renames" the alternative symbols and defines. How is this handled here for the GLES and/or non-core path?

[sacereda]

I guess in that case you could guard that branch using the original ifdef:

if (0)
    ;
#ifdef GL_QUERY_RESULT_NO_WAIT
else if( m_supportsQueryBufferObject )
{
    ...
}
#endif
else
{
    ...
}

At some point I also considered a compile-time opt-in mechanism as an alternative (TRACY_FAST_COLLECT or similar), but I'm not sure whether you already have something along those lines in the codebase.

[slomp]

According to the registry, there's no preceding EXT variant of GL_QUERY_RESULT_NO_WAIT. It seems to first appear as an AMD extension in AMD_query_buffer_object, and immediately promoted to core in ARB_query_buffer_object).

I suppose we could do something like this:

#ifndef GL_QUERY_RESULT_NO_WAIT
#define GL_QUERY_RESULT_NO_WAIT 0x9194
#endif

I am not well-versed in GLES, but the glGetString and glGetStringi should be there to check for extensions. The "version check" I am not so sure, but even if glGetIntegerv( GL_MAJOR_VERSION, &major ); is not applicable for GLES, CheckFeature would fall-through the common-denominator (glGetString) path.

[slomp]

glGetString is available on GLES 2:
https://docs.gl/es2/glGetString

glGetStringi is available on GLES 3:
https://docs.gl/es3/glGetString

GL_MAJOR_VERSION (glGetIntegerv()) is on GLES 3:
https://docs.gl/es3/glGet

So I suppose we would also need an #ifndef GL_MAJOR_VERSION check to cover all basis.