rougier / freetype-gl

OpenGL text using one vertex buffer, one texture and FreeType
Other
1.65k stars 266 forks source link

Assertion failed when minimizing window of demo #137

Closed schmittl closed 7 years ago

schmittl commented 7 years ago

I get the following output, if I try to minimize e.g. the ansi demo window:

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
Using GLEW 2.0.0
Assertion failed!

Program: C:\git\freetype-gl\build\demos\ansi.exe
File: C:\git\freetype-gl\mat4.c, Line 115

Expression: right != left

Cause: Almost every demo (except cube and embedded-font) contains the following snippet:

// ---------------------------------------------------------------- reshape ---
void reshape( GLFWwindow* window, int width, int height )
{
    glViewport(0, 0, width, height);
    mat4_set_orthographic( &projection, 0, width, 0, height, -1, 1);
}

The reshape function is passed as a callback to glfwSetFramebufferSizeCallback. When minimizing the window, the framebuffer size changes and the reshape callback is called. The width and height parameters are set to 0. Thus inside the mat4_set_orthographic function in mat4.c the assertion fails, because both right and left values are 0: (Same applies to bottom and top values.)

    assert( right  != left );
    assert( bottom != top  );

This happens with all demos except cube and embedded-font. I am not sure if you are already aware of this issue, but I guess it would be nice if the demos did not crash if you minimize them.

rougier commented 7 years ago

Thanks for the report. And no, I was not aware of the problem. We could use something like max(width, 1) and max(height, 1) when calling mat4_set_orthographic

adrianbroher commented 7 years ago

I prefer to return immediately and don't return equally wrong output when a user inserts invalid data into a mat4 functions.

schmittl commented 7 years ago

Thanks for the reply and the fix. I think a little bug was introduced with the latest commit and now the demos just display a white screen.

In mat4.c there is an extra semicolon here:

void
mat4_set_identity( mat4 *self )
{
    if (!self);              <----- extra semicolon
        return;
rougier commented 7 years ago

Wait, if arguments given are wrong, you'll silently ignore them, and silently propagating them at upper levels. assert statements ensure an early fail where relevant (but in the above case).

adrianbroher commented 7 years ago

assert terminates the application and this is unacceptable behavior for any library. asserts work only in debug builds so they don't 'ensure' anything.

wmamrak commented 7 years ago

What behavior is acceptable by a library/function/algorithm depends on its contract. When you break the contract, what happens at most is undefined behavior. Asserts make developers aware of undefined behaviors without sacrificing release code efficiency. For such a simple functions as these it is obvious that self must be a valid pointer, so examining its validity at each call is a waste of resources. I would stay with the asserts and add appropriate comments about self validity. And later, I would fix all the user code where the asserts fail.