OpenGL Shader Loader in C++

Question

I made my own custom OpenGL shader loader for C++. Please tell me is I'm doing anything incorrectly or anything to improve on. I've tested this and it works perfectly fine.

main.cc

std::vector<ShaderInfo> shader_info {
    {GL_VERTEX_SHADER, "shader.vert"},
    {GL_FRAGMENT_SHADER, "shader.frag"}
};
GLuint shader_program {LoadShaders(shader_info)};
glUseProgram(shader_program);

shader-loader.hh

struct ShaderInfo {
    GLenum type;
    std::string file;
};

GLuint LoadShaders(const std::vector<ShaderInfo>&);

shader-loader.cc

GLuint LoadShaders(const std::vector<ShaderInfo>& shader_info) {
    GLuint program {glCreateProgram()};
    std::vector<GLuint> detach;

    for (auto info : shader_info) {
        std::ifstream file {info.file, std::ios_base::in};
        file.seekg(0, std::ios_base::end);
        std::string source(file.tellg(), '\0');

        file.seekg(0, std::ios_base::beg);
        file.read(&source[0], source.size());
        file.close();

        GLuint shader {glCreateShader(info.type)};
        GLchar* source_ptr {&source[0]};
        glShaderSource(shader, 1, &source_ptr, nullptr);

        glCompileShader(shader);
        glAttachShader(program, shader);
        glDeleteShader(shader);
        detach.push_back(shader);

        GLint status;
        glGetShaderiv(shader, GL_COMPILE_STATUS, &status);

        if (status == GL_FALSE) {
            GLint length;
            glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &length);

            std::string log(length, '\0');
            glGetShaderInfoLog(shader, length, nullptr, &log[0]);

            glDeleteProgram(program);
            throw std::runtime_error(log);
        }
    }

    glLinkProgram(program);
    for (auto shader : detach)
        glDetachShader(program, shader);

    GLint status;
    glGetProgramiv(program, GL_LINK_STATUS, &status);

    if (status == GL_FALSE) {
        GLint length;
        glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length);

        std::string log(length, '\0');
        glGetProgramInfoLog(program, length, nullptr, &log[0]);

        glDeleteProgram(program);
        throw std::runtime_error(log);
    }

    return program;
}
`````

Answer 1

• for (auto info : shader_info) -> for (auto const& info : shader_info) to avoid a string copy.

• Reading the source file into a string should probably be a separate function.

• We should throw an error if the file fails to open. (tellg will return -1 in that case).

• I find it's often helpful to output more than one shader object log before stopping compilation (for example, if we have a minor error in the vertex shader, and another one in the fragment shader, we can see and fix both at once, rather than only having the vertex shader error in the log).

e.g.:

GLuint LoadShaders(const std::vector<ShaderInfo>& shader_info) {
    // ...
    bool compiled = true; // flag to indicate overall compile status

    for (auto const& info : shader_info) {
        // ... read file and compile shader

        GLint status;
        glGetShaderiv(shader, GL_COMPILE_STATUS, &status);

        if (status == GL_FALSE) {
            // ... get log string
            
            // log the error (if you have an existing logging system, or print to std::cerr)
            LogError("glCompileShader() failed for shader object: " + info.file);
            LogError(log);
            
            compiled = false; // set the overall flag to false
        }
    }
    
    // and now we throw!
    if (!compiled) {
        glDeleteProgram(program);
        throw std::runtime_error("Shader compilation failed!");
    }

    // ... link program etc. as before
}