2
\$\begingroup\$

I am trying to learn OpenGL. I have a main.cpp where I create the vertex and fragment shaders as strings, compile and link and it works fine. When I try to load the identical shaders from text files, it no longer links. Having trouble figuring out why. (I am using Linux if that matters)

Here is where I define the shader source:

    /*
    const char* vertex_shader = "#version 410\n"
                    "in vec3 vp;"
                    "void main () {"
                    "  gl_Position = vec4 (vp, 1.0);"
                    "}";

    const char* fragment_shader =  "#version 410\n"
                       "uniform vec4 inputColour;"
                       "out vec4 frag_colour;"
                       "void main () {"
                       "  frag_colour = inputColour;"
                       "}";

    */
    const char* vertex_shader = load_shader("test.vert").c_str();
    const char* fragment_shader = load_shader("test.frag").c_str();

Again, if I change what I comment out, so that the shader source is hardcoded, it works fine. If I try to load from files like I show here, it fails to link. Each individual shader seems to compile fine.

Here is my load_shader() function, it just gets the text from the file and logs some data.

std::string load_shader(std::string file){
    std::ifstream fin(file);
    if(fin.is_open()){
        std::string shader = "";
        std::string line = "";
        while(!fin.eof()){
            std::getline(fin, line);
            shader.append(line +"\n");
        }
        fin.close();
        l.add_string("LOADED SHADER");
        l.add_line(file);
        l.add_line(shader);
        l.add_line("---------------------------------");
        return shader;
    }else{
        l.add_string("FAILED TO LOAD SHADER");
        l.add_line(file);
        return "";
    }
}

Could my load_shader() function be making a bad string somehow?

EDIT: I got my log functions working slightly better. glGetProgramInfo() now returns this:

Vertex shader(s) failed to link, fragment shader(s) failed to link.
Vertex link error: INVALID_OPERATION.
ERROR: error(#97) No program main found
fragment link error: INVALID_OPERATION.
ERROR: error(#97) No program main found

EDIT 2: According to this, it is most likely a bad source string. But it doesn't say why. I'll keep working at it.

EDIT 3: I tried to compare the strings I was getting from each method and noticed something weird. Here is my check code, where I make strings for each input method, then loop through and print the chars to a file.

const char* vertex_shader = "#version 410\n"
                "in vec3 vp;"
                "void main () {"
                "  gl_Position = vec4 (vp, 1.0);"
                "}";

const char* fragment_shader =  "#version 410\n"
                   "uniform vec4 inputColour;"
                   "out vec4 frag_colour;"
                   "void main () {"
                   "  frag_colour = inputColour;"
                   "}";


const char* vertex_shader2 = load_shader("test.vert").c_str();
const char* fragment_shader2 = load_shader("test.frag").c_str();
std::ofstream shader("shaders.txt");
int i = 0;
while(vertex_shader[i] != '\0'){
    shader << vertex_shader[i];
    i++;
}
shader << std::endl;
i = 0;
while(fragment_shader[i] != '\0'){
    shader << fragment_shader[i];
    i++;
}
shader << std::endl;
i = 0;
while(vertex_shader2[i] != '\0'){
    shader << vertex_shader2[i];
    i++;
}
shader << std::endl;
i = 0;
while(fragment_shader2[i] != '\0'){
    shader << fragment_shader2[i];
    i++;
}
shader << std::endl;
shader.close();

And here is the shaders.txt output:

#version 410
in vec3 vp;void main () {  gl_Position = vec4 (vp, 1.0);}
#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}
#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}
#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}

As you can see, I'm getting a copy of the fragment shader text where I should be seeing the vertex shader. To be sure, here are the shader files I'm trying to upload.

test.vert:

#version 410
in vec3 vp;void main () {  gl_Position = vec4 (vp, 1.0);}

test.frag:

#version 410
uniform vec4 inputColour;out vec4 frag_colour;void main () {  frag_colour = inputColour;}

Also, when I changed test.frag a little, that change appeared in the wrong line in the shaders.txt. This shows that the extra copy is coming from the file input instead of the hardcoded version.

EDIT 4: I am apparently getting some sort of string overwrite problem. I tested with std::cout like so:

const char* vertex_shader2 = load_shader("test.vert").c_str();
const char* fragment_shader2 = load_shader("test.frag").c_str();
std::cout << vertex_shader2 << std::endl;
std::cout << fragment_shader2 << std::endl;

In this case, both cout statements give the test.frag string. But this one works fine:

const char* vertex_shader2 = load_shader("test.vert").c_str();
std::cout << vertex_shader2 << std::endl;
const char* fragment_shader2 = load_shader("test.frag").c_str();
std::cout << fragment_shader2 << std::endl;

So the second load_shader() call is overwriting the first call.

\$\endgroup\$
  • \$\begingroup\$ This is not worthy of a full answer, but eof() isn't true until a failed operation has been made at the end of the file. As such, you're doing one append too much, with a string that may not contain what you desire. Consider testing the success of the extraction operation itself, while (std::getline(..)) { .. } \$\endgroup\$ – Lars Viklund Aug 14 '15 at 20:54
8
\$\begingroup\$

You are using pointers to data inside a temporary object:

const char* vertex_shader = load_shader("test.vert").c_str();
const char* fragment_shader = load_shader("test.frag").c_str();

Which means both vertex_shader and fragment_shader are pointing to invalid memory, since the strings returned by load_shader get destroyed at the end of each statement. Accessing that memory results in undefined behaviour.

Try keeping the strings instead:

std::string vertex_shader = load_shader("test.vert");
std::string fragment_shader = load_shader("test.frag");

And then giving their .c_str()s to the shader compiler.

\$\endgroup\$
  • \$\begingroup\$ I guess I assumed the .c_str() function would return the char array and that the original string would no longer be needed. \$\endgroup\$ – user137 Aug 14 '15 at 17:18
  • \$\begingroup\$ @user137: It returns the char array, for sure. The problem is that the std::string object you got that array from, is destroyed after that load_shader (...) call returns. You need to make a copy, not get the address of an array that's about to be destroyed ;) \$\endgroup\$ – Andon M. Coleman Aug 14 '15 at 20:15
2
\$\begingroup\$

The other answers already mention that you're getting a dangling pointer and should return the strings. But reading shaders from a file, really comes down to putting the whole file into a string. Here's one way to do it:

// Read shader file into one string
std::string shaderSource;
if ( !readFileIntoString( shaderFileName, shaderSource ) ) {
    return false;
}

const GLchar *source[] = { shaderSource.c_str() };
// (...) Use the source

And readFileIntoString can be written in different ways. Here you send the name of your shader file and pass a string to read the file into.

bool readFileIntoString( const std::string &fileName, std::string &destination ) {
    std::ifstream in( fileName, std::ios::in | std::ios::binary );
    if ( in ) {
        in.seekg( 0, std::ios::end );
        destination.resize( in.tellg() );
        in.seekg( 0, std::ios::beg );
        in.read( &destination[ 0 ], destination.size() );
        in.close();
        return true;
    }
    return false;
}
\$\endgroup\$
1
\$\begingroup\$
const char* vertex_shader = load_shader("test.vert").c_str();
const char* fragment_shader = load_shader("test.frag").c_str();

will result in dangling pointers (with undefined behavior if you dereference it), after you assign the char* to the variable the temp std::string (and backing array you just got a pointer to) gets deleted.

const std::string vertex_shader = load_shader("test.vert");
const std::string fragment_shader = load_shader("test.frag");

Keep those variables alive as long as you need the strings.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.