Shader loading code from files is... unfortunate.
NicolBolas opened this issue · 4 comments
The code used for loading shader strings from a file is:
if(VertexShaderStream.is_open()){
std::string Line = "";
while(getline(VertexShaderStream, Line))
VertexShaderCode += "\n" + Line;
VertexShaderStream.close();
}
First, the "\n" should certainly not be added before Line. That shifts all of the lines down by 1, which is going to be a problem when it comes to debugging shaders. All of the line numbers reported by the compiler will be off by 1.
Second, line-by-line reading of files like this is unfortunate. There are pretty easy ways to slurp an entire file into memory. For example:
std::stringstream sstr;
sstr << FragmentShaderStream.rdbuf();
std::string FragmentShaderCode = sstr.str();
If the extra copy here is disconcerting to you, there are other ways. The point being, there are lots of options that don't involve doing this read a line and concatenate it method.
Now you might think that inefficiency in something like this is irrelevant. And generally speaking it is. But the problem isn't the inefficiency; the problem is that your code is used as an example for new programmers.
opengl-tutorials.org is one of the premier OpenGL tutorial sites on the Internet. Thousands of new programmers learn from this code. They assume that the quality of the code is good, and they will frequently copy-and-paste from it into their application. So when they see a tutorial like yours engaging in this practice, they will naturally assume that this is the right way to read all of the text in a file.
So if they ever need to read a file, they'll copy your code. Which is not good code.
I've seen so many new programmers who've learned from OpenGL using copies of your code. Or code just like it. Who believe that reading an entire file is best done by reading it line-by-line.
I'm hoping that you can help stop the proliferation of such code.
The main concern will be to keep the good readable for someone with no c++ background and the op << does make thing harder to grasp for new developers.
The thing is, using C++ ought to mean using C++. I can understand not wanting to go into the deep magic of template metaprogramming and so forth. But even "Hello, World!" for C++ uses <<. I don't think it's unreasonable to expect people to know what << does. And failing that, a simple comment that says, //reads the whole file would be sufficient.
Personally, I've always been partial to the "seek to the back, get the size, seek to the front, and read the thing" method of reading an entire file. Despite the flaws, it has the virtue of common usage and ease of readability (don't forget to open it in binary reading mode).