glfw: Error query

@IntellectualKitty Not too late at all!

It’s not my intent to replace the error callback, even in the long term. They’re both useful in their own ways and the callback provides some benefits that polling just can’t.

I object to locking and heap allocation on errors not because of the performance impact, which as you say is negligible, but because it introduces complexity and the possibility of failure into the part of the code that should be the most simple and robust.

So 4b80443199d6e11f6c72ef78e6f9d41bff1b016c (see the error-description branch) is something like what I had in mind for option 4, with a bit of option 6 for the interface. It’s the simplest thing I could think of without having a portable compare-and-swap.

Error checking wrapper macros are very handy, but I don’t know that GLFW should provide its own, though it could show an example of one in the documentation. I suspect everyone will want to write their own that works the way they want and integrates with their specific systems. Providing a fully generic one doesn’t seem like it’d save the user a lot of time, as it would likely require customization macros to plug in their magic function.

Yup, the contortions projects like go-glfw have to go through to get local access to the error code is one reason for this feature.

However, even C code needs that sometimes. A complex application may want to change or downgrade its requirements if glfwCreateWindow fails with GLFW_VERSION_UNAVAILABLE, GLFW_FORMAT_UNAVAILABLE or even GLFW_API_UNAVAILABLE. A caller of glfwGetClipboardString may want to tell GLFW_FORMAT_UNAVAILABLE apart from GLFW_PLATFORM_ERROR. This was already mentioned above.

I originally imagined the error callback as an analog to GL_KHR_debug, helping the application developer figure out what they’re doing wrong, but some errors are pretty useful to have access to locally in release code without having to set up a callback and a TLS slot oneself.

It may also have a positive pedagogical effect. I’ve started seeing people exiting from inside the error callback on the assumption that errors are fatal.

Any objections to the current API? Is it time to close this issue?

I hope it’s not too late to comment.

First of all, this proposal addresses an issue that I had with GLFW since I first started using it since I was worried about the difficulty of associating the independent error callback with the GLFW function call that caused it. That said, I’ve grown quite attached to the error callback since it means that I don’t have to check for errors at every single line of GLFW code, and it feels a lot freer and easier to program that way. So, even though I didn’t like the error callback at first, I now feel like it’s one of the many features that makes GLFW so appealing. Lastly, I don’t recall ever having an error with GLFW anyway, so I have no idea how difficult it would be in practice to track an error down using the callback, but since GLFW is far, far easier to use than OpenGL itself I doubt that it would really be very hard.

Second, I think it’s extremely important to report an error description as well as an error code. Getting the error message “Context profiles are only defined for OpenGL version 3.2 and above” is far more helpful than simply reporting GLFW_INVALID_VALUE. Again, this is one of the really nice features of GLFW’s existing error callback system. If you’re trying to track down a problem during development, it’s really helpful to have good diagnostic messages. If you’re trying to track down a problem in an actual release, say platform-specific issues, it’s really helpful to have good diagnostic messages.

I am definitely against option 4 because of the “locking and heap allocation on errors” requirements.

Third, I’m not sure that locking and heap allocation on error is significant enough of a concern to affect a critical design decision like this. The majority of errors are going to occur during development rather than in release, and errors should generally occur infrequently anyway unlike operations in a high performance section of code. Another way of saying this is that errors should be a special case issue, and if you have to be concerned about reporting so many errors that performance and resource utilization considerations become significant then you’ve probably done something extremely stupid. Not reporting critical details about an error condition because of the performance implications is a bit like jumping out of an airplane and not taking a parachute because the extra weight will make you fall faster. So, in practical terms, I think the cost of locking and heap allocation should be irrelevant compared to the value of the diagnostic information you receive.

Fourth, the proposed error query mechanism makes GLFW look more like OpenGL itself. An error query mechanism does, of course, make it easier to track down where an error is occurring, and that’s a valuable feature in any library. This is extremely important in OpenGL which often tends to be difficult and picky, whereas GLFW is very simple and easy to use. That said, with GL_DEBUG_OUTPUT, OpenGL itself is now providing a feature-rich callback system that reports not only errors but deprecations and performance hints as well.

So, I hope that the error query system won’t replace the error callback system, particularly in cases where multiple errors may be produced by a single function call. It seems unlikely that novice users would generally think to query for more than one error – the standard that we’re mostly all used to is reporting the first error encountered – and looping to check for multiple errors may be intimidating. What makes GLFW so nice – and presumably so popular (most of the OpenGL tutorials I’ve come across use GLFW) – is that it is so easy to use and to learn, and looping to check for multiple errors (and even having to check for errors manually instead of using the error callback) may jeopardize that simplicity and user-friendliness.

Fifth, it may be helpful to add a GLFW_CHECK_ERROR macro to support the error query mechanism. I use this type of error support system for OpenGL calls, for example:

GL_CHECK_ERROR(glBindBuffer(GL_ARRAY_BUFFER,m_uiVertexBufferID));

The GL_CHECK_ERROR macro is defined using a mechanism similar to the one used to define the standard C assert, with the addition of including the OpenGL function call:

#if defined (DEBUG)
    #define GL_CHECK_ERROR(opengl_function_call) \
        opengl_function_call; \
        glCheckError(#opengl_function_call,__PRETTY_FUNCTION__,__LINE__, \
            __FILE__)
#else
    #define GL_CHECK_ERROR(opengl_function_call) opengl_function_call
#endif

The actual error reporting code uses spdlog but stderr could of course be used instead:

void glCheckError(
    const char* acOpenGLFunctionCall,
    const char* acFunctionName,
    const sint32_t siLineNumber,
    const char* acFileName) noexcept(true)
{
    // Check for OpenGL error codes, and notify the user of each error that has
    // occurred.
    while (true)
    {
        // Check the next OpenGL error code.  Break out of the loop if there is
        // no error.
        const uint32_t uiErrorCode = glGetError();
        if (GL_NO_ERROR == uiErrorCode)
        {
            break;
        }

        // Convert the OpenGL numerical error code to a human-readable name, if
        // available.
        const char* acErrorName = nullptr;
        switch (uiErrorCode)
        {
        case GL_NO_ERROR:
            acErrorName = "GL_NO_ERROR";
            break;
        case GL_INVALID_ENUM:
            acErrorName = "GL_INVALID_ENUM";
            break;
        case GL_INVALID_VALUE:
            acErrorName = "GL_INVALID_VALUE";
            break;
        case GL_INVALID_OPERATION:
            acErrorName = "GL_INVALID_OPERATION";
            break;
        case GL_INVALID_FRAMEBUFFER_OPERATION:
            acErrorName = "GL_INVALID_FRAMEBUFFER_OPERATION";
            break;
        case GL_OUT_OF_MEMORY:
            acErrorName = "GL_OUT_OF_MEMORY";
            break;
#if defined (GL_STACK_UNDERFLOW)
        case GL_STACK_UNDERFLOW:
            acErrorName = "GL_STACK_UNDERFLOW";
            break;
#endif
#if defined (GL_STACK_OVERFLOW)
        case GL_STACK_OVERFLOW:
            acErrorName = "GL_STACK_OVERFLOW";
            break;
#endif
#if defined (GL_TABLE_TOO_LARGE)
        case GL_TABLE_TOO_LARGE:
            acErrorName = "GL_TABLE_TOO_LARGE";
            break;
#endif
        }

        // Write the OpenGL error name (or code), the OpenGL library function
        // call, the function name, file name, and line number.
        std::shared_ptr<spdlog::logger> spkDefaultLogger =
            spdlog::get("default");
        assert(spkDefaultLogger);
        if (acErrorName)
        {
            spkDefaultLogger->error(
                "OpenGL ERROR: glGetError() returned %s for %s, function "
                    "%s, file %s, line %d.\n",
                acErrorName,
                acOpenGLFunctionCall,
                acFunctionName,
                acFileName,
                siLineNumber);
        }
        else
        {
            spkDefaultLogger->error(
                "OpenGL ERROR: glGetError() returned error code 0x%04Xs "
                    "for %s, function %s, file %s, line %d.\n",
                uiErrorCode,
                acOpenGLFunctionCall,
                acFunctionName,
                acFileName,
                siLineNumber);
        }
    }
}

Using this system to check for and report OpenGL errors makes error handling extremely easy and transparent, and it is silently disabled in release builds (which may not be appropriate for GLFW).

I use (and help maintain) the Go bindings for GLFW.

In idiomatic Go, functions that can fail typically return multiple values, the result and an error variable (reference). That way, the caller can immediately check if the function call failed, and handle the error appropriately. It typically looks like this:

resp, err := http.Get("https://example.com/")
if err != nil {
	// handle error
}
// make use of the HTTP response

When making the Go bindings, we wanted to stay as true as possible to the GLFW C API, but we decided that it will be worth it to make the bindings have idiomatic signatures that return errors right away, so that the users of the Go library have the best experience (see https://github.com/go-gl/glfw/issues/8, https://github.com/go-gl/glfw/issues/27, https://github.com/go-gl/glfw/issues/56).

For example:

err := glfw.Init()
if err != nil {
	log.Fatalln("failed to initialize glfw:", err)
}

w, err := glfw.CreateWindow(...)
if err != nil {
	log.Fatalln("failed to create window:", err)
}

s, err := w.GetClipboardString()
if err != nil {
	return 0, err
}
// use s...

So, for our use case, the simplest API that allows fetching the error (if any) from last call would be sufficient. We don’t need the ability to track last n errors, since our bindings can recover and return the error details from each failed GLFW call, immediately as it happens. The user doesn’t have to worry about that. If the user doesn’t care about some error value, they can ignore it.

Also relevant here is #361, because we ideally want to be able to differentiate between errors caused by invalid API usage, a situation where it doesn’t make sense for the Go bindings to return an error, since the programmer should simply fix the invalid API usage, instead of trying to handle the error as it’s returned.

On the other hand, errors that happen because of system issues, such as “unable to create window” or “unable to fetch clipboard contents because insufficient permissions”, those are errors that the programmer should handle.

The Go bindings currently use glfwSetErrorCallback to perform all of the above, so this is not critical, but I wanted to talk about a use case I’m aware of and our needs/wants. I hope this is useful.

Does anyone have any thoughts about prefacing an error message with the function name in which it was generated? This may be more helpful/appropriate for the error callback, but it still may make diagnosing the source of problems a little easier.

Currently GLFW_INVALID_ENUM errors are reported using %i but 0x%08X matches the #define style used in GLFW. Also, if there isn’t already a function to convert a GLFW enum to a string, would anyone else think this is helpful?

However, I’d also be very interested to hear from those of you who opted for the simpler variants. How do you feel about paying the complexity cost, and/or about this specific implementation?

I’ve only looked at this briefly at a high level. No big concerns from me and my Go perspective.

I think this is a case where it’s reasonable to prioritize more helpful and friendly error messages over some (minor?) performance. If performance turns out to be a bottleneck, after benchmarking and profiling, I’m sure that it’d be possible to address it later. So IMO it’s better to err more on side of helpful/friendly first.

If glfwGetError returns an error description along with the error code, go-gl/glfw can expose/pass through the error description in its error value.

A question: what benefit does this function add to glfw? I honestly can’t see myself using it over the existing error callback API (which I like more than what most other libraries provide).

Assuming such a function is to be added: are there any realistic cases where GLFW will generate multiple errors in response to a single API call? If not, option 1 (where you can query just the latest error) might be sufficient. If errors pile up and some get lost, it’s probably the client code’s fault for not polling often enough.

Allowing the user to set the error state feels like a design misstep to me. If the client code wants to generate its own errors, it can surely do so without needing to go through glfw. I would have to see some example code of how this feature could be put to good use.

I would be in favour of glfwGetError simply returning the latest error with no side-effects, and an additional function like glfwClearError to actually clear it.

I am definitely against option 4 because of the “locking and heap allocation on errors” requirements.