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;
}
`````

• Sᴀᴍ Onᴇᴌᴀ: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see What should I do when someone answers my question? as well as what you may and may not do after receiving answers.

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
}

• Desmond Rhodes: How can I combine the logs from 2 shaders compilation?
• user673679: Edited to show what I mean. It depends how you're logging things. It's really just moving the throw a bit later.