Hopson97 / open-builder

Open "Minecraft-like" game with multiplayer support and Lua scripting support for the both client and server
https://github.com/Hopson97/open-builder/projects/3
GNU General Public License v3.0
700 stars 80 forks source link

Shader compilation fails depending on drivers #209

Open cuckydev opened 3 years ago

cuckydev commented 3 years ago

I know this project is more likely than not defunct now, however this is a problem that should be fixed for later projects if possible. Shader compilation can fail depending on your drivers... well, kind of. It's not that the compilation itself is wrong, but the error checking is. shader.cpp L34 GL_INFO_LOG_LENGTH returns the length of the info message including the terminator key. However, on some OpenGL drivers, it will return 0 for an empty message (such as the one used to test this project), while others return 1.

untodesu commented 3 years ago

GL_INFO_LOG_LENGTH must be zero if the string is empty, however glGetProgramInfoLog will put to the buffer at least one character which is null-terminator. here is the proof Does it really return 1?

cuckydev commented 3 years ago

GL_INFO_LOG_LENGTH must be zero if the string is empty, however glGetProgramInfoLog will put to the buffer at least one character which is null-terminator. here is the proof Does it really return 1?

Yes. Again, it's a strange driver issue. It's better to check if it's > 1 instead of non-zero to combat this.

untodesu commented 3 years ago

It's better to check if it's > 1

But why, though? Does it actually fail or something? I really cannot find a reason why this check should exist in the first place... Passing zero to the glGetProgramInfoLog won't do anything harmful: it just won't write anything to the buffer. Situation with the length equals to one is kinda different, it will write a single byte which will be null terminator. Again, I don't see anything bad at this

untodesu commented 3 years ago

The only infolog-related thing I can suggest is to try to discard the shader infolog and go just with the program infolog. It seems, if the shader compilation has failed and we try to link the program with the failed shader, it will merge infologs and put GL_FALSE to the GL_LINK_STATUS. I'm not sure this behaviour should exists, however it makes code shorter:

unsigned int vs = glCreateShader(GL_VERTEX_SHADER);
glShaderSource(vs, 1, &my_vs_src, 0);
glCompileShader(vs);
// Nope, we aren't checking the infolog here.

unsigned int fs = glCreateShader(GL_FRAGMENT_SHADER);
glShaderSource(fs, 1, &my_fs_src, 0);
glCompileShader(fs);
// Nah, we ain't checking it here!

glAttachShader(program, vs);
glAttachShader(program, fs);
glLinkProgram(program);

int result;
glGetProgramiv(program, GL_LINK_STATUS, &result);

// Check for the infolog
if(result == GL_FALSE) {
    int length;
    glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length);
    if(length > 0) {
        std::string infolog(static_cast<size_t>(length), 0);
        glGetProgramInfoLog(program, length, 0, &infolog[0]);
        // UNDONE: Do something with the message.
        // My code usually outputs it to gl_shaders.log and logs an error message that
        // a shader has some errors and you have to check the gl_shaders.log for more details.
    }
}

// This may be replaced with the return statement.
const bool success = result == GL_TRUE;
cuckydev commented 3 years ago

The problem with the code linked in the issue is that it checks if the length is 0, which again, is driver dependent behaviour.

untodesu commented 3 years ago

image I don't think the specification describes this as a driver-dependent value.

Also you wouldn't use the info log for anything but error checking, so when an error happens, there's something in the infolog anyway.

cuckydev commented 3 years ago

You still don't understand. The error condition here if the log length is equal to 0. The issue is that for an empty infolog (no error), it's only 0 on some drivers, but 1 on another.

untodesu commented 3 years ago

You still don't understand. The error condition here if the log length is equal to 0. The issue is that for an empty infolog (no error), it's only 0 on some drivers, but 1 on another.

I honestly don't understand why should we check for length in the first place since if any error happens, infolog contains something.