Color passed to shaders doesn't work

I am pretty new to OpenGL and have a simple program that I am messing with at the moment.

The issue that I am running into is when I pass an array to represent the color of points, the color just ends up being black. It works fine if I just explicitly define a color in the fragment shader, and if I switch the indices of the data in the VAO, the triangle is moved to the points that are the colors.

I tried using API trace and got the following:

6872: message: major api error 1282: GL_INVALID_OPERATION error generated. <program> is not a program object, or <shader> is not a shader object.
6872 @0 glAttachShader(program = 1, shader = 1)
6872: warning: glGetError(glAttachShader) = GL_INVALID_OPERATION
6876: message: major api error 1282: GL_INVALID_OPERATION error generated. Handle does not refer to the expected type of object (GL_SHADER_OBJECT_ARB).
6876 @0 glDeleteShader(shader = 1)
6876: warning: glGetError(glDeleteShader) = GL_INVALID_OPERATION
6878: message: major api error 1282: GL_INVALID_OPERATION error generated. Handle does not refer to the expected type of object (GL_SHADER_OBJECT_ARB).
6878 @0 glDeleteShader(shader = 1)
6878: warning: glGetError(glDeleteShader) = GL_INVALID_OPERATION
Rendered 507 frames in 8.52598 secs, average of 59.4653 fps

All of my code is here, but it is heavily in progress. The bit that is likely causing some type of issue is the drawing segment below:

    // TODO: Use this code once per mesh
    // Make a VBO to hold points
    GLuint vbo = 0;
    glGenBuffers(1, &vbo);
    glBindBuffer(GL_ARRAY_BUFFER, vbo);
    glBufferData(GL_ARRAY_BUFFER, points.size() * sizeof(GLfloat), &points[0], drawType);

    // VBO TO hold colors
    GLuint colorVbo = 0;
    glGenBuffers(1, &colorVbo);
    glBindBuffer(GL_ARRAY_BUFFER, colorVbo);

    float colors[] = {
            0.5, 0.0, 0.0,
            0.0, 0.5, 0.0,
            0.0, 0.0, 0.5
    };

    glBufferData(GL_ARRAY_BUFFER, 9 * sizeof(float), colors, GL_STATIC_DRAW);

    // Make a VAO for the points VBO
    GLuint vao = 0;
    glGenVertexArrays(1, &vao);
    glBindVertexArray(vao);

    // Points
    glBindBuffer(GL_ARRAY_BUFFER, vbo);
    glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 0, NULL);

    // Colors
    glBindBuffer(GL_ARRAY_BUFFER, colorVbo);
    glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, 0, NULL);

    GLint colorpos = glGetAttribLocation(Program::getCurrentProgram(), "vertColor");

    glEnableVertexAttribArray(0);   // Points
    glEnableVertexAttribArray(1);   // Colors

    glLinkProgram(Program::getCurrentProgram());

    // Draw Mesh
    glDrawArrays(drawShape, 0, points.size()/2);
}

Some of the things that you may need to know is that points is a Vector<float>, Program::getCurrentProgram() is a static function that returns the program currently being used, and drawShape in this case is GL_TRIANGLES.

Vertex Shader:

#version 400 core

layout(location=0) in vec3 vert;
layout(location=1) in vec3 vertColor;

out vec3 color;

void main()
{
    color = vertColor;
    gl_Position = vec4(vert, 1);
}

Fragment Shader:

#version 400 core

in vec3 color;

out vec4 finalColor;

void main()
{
    finalColor = vec4(color, 1.0);
}

I apologize if this is an issue that takes a bit of looking. I have looked around for the past day or so and tried several different things, all of which didn't work. If any other information is needed so someone doesn't have to sort through all of my code, please let me know.

What I get from apitrace seems to indicate that I may be doing something wrong with the shader IDs, although that error still occurs if I code the color into the fragment shader and the color then works.

I'm using GLFW for creating my OpenGL contexts. I have set an error callback and I'm not getting anything from that and I was under the impression that it was also supposed to pass through OpenGL errors, although I didn't see anything on their FAQ saying that explicitly.

I'm also checking errors while compiling shaders and linking the program, and nothing is happening there either.

Also, I was wondering if using C++ could risk losing OpenGL states once an object goes out of scope and delete is called.

Edit: Here are a couple things that were mentioned that I did not show:

Shader Initialization:

// Create OpenGL Shader
shaderID = glCreateShader(shaderType);

glShaderSource(shaderID, 1, &cShaderString, NULL);

// Compile
glCompileShader(shaderID);

Shader Destructor just calls glDeleteShader(shaderID);

This is pretty standard. This is in the constructor for a shader class and shaderID is a member variable of a Shader. shaderType is a GLenum which in this case is either GL_VERTEX_SHADER or GL_FRAGMENT_SHADER.

Program Initialization: // Create Program programID = glCreateProgram();

// Iterate through Shaders
for(std::vector<Shader>::iterator shader = shaders.begin(); shader != shaders.end(); shader++)
{
    // Attach Shader
    glAttachShader(programID, shader->getShaderID());
}

// Link program
glLinkProgram(programID);

Program Destructor:

// Iterate through attached shaders
for(std::vector<Shader>::iterator shader = shaders.begin(); shader != shaders.end(); shader++)
{
    // Detach Shader
    glDetachShader(programID, shader->getShaderID());
}

// Delete program
glDeleteProgram(programID);

Once again, this seems pretty standard and is in the constructor for a Program class.

Edit 2: After messing with a bit of code, I have found a rather curious thing. If I move the code that creates and binds buffers and draws the triangle to the constructor of what I'm using as a class to represent a triangle, I get the location as the color rather than the position. This is the same thing that happens when I switch the indices on the data (position to 1 and color to 0).

Edit 3: Actually, after looking a bit more, it seems to be drawing a completely different triangle than the colors. Making a vao member for the Triangle class and binding it on draw seemed to fix this issue.

Edit 4: It seems that glValidateProgram() does yield a successful result, which makes me doubt having issues with its shaders.

Also, I was wondering if using C++ could risk losing OpenGL states once an object goes out of scope and delete is called.

Yes, there is. And I believe that is exactly what is happening here. This doesn't mean that you can't wrap OpenGL objects in C++ classes, but you have to be very careful.

The problem you run into here is that you delete the wrapped OpenGL object in the destructor of your C++ objects. So anytime one of these objects goes out of scope, or gets destroyed for any other reason, it takes along the corresponding OpenGL object.

This causes particularly ugly problems if you copy the objects. Let's look an artificial example:

Shader a(...);
{
    Shader b = a;
}

After this, the OpenGL object that you created in the constructor of a will have been deleted. When creating b as a copy of a, the default copy constructor will copy the shader id stored in a to b. Then, when b goes out of scope, its destructor deletes the shader id. It is still stored in a, but now invalid.

Of course you wouldn't write code exactly like the above. But you can have very similar scenarios if you pass objects to functions/methods by value, which is partly happening in your code.

Even more dangerously, because harder to recognize, is what happens if you store these objects in containers like a std::vector. A vector reallocates its memory when adding a new element would make it exceed its current capacity. During reallocation, it creates copies of all existing elements in the vector (which invokes the copy constructor), and then destroys the original elements. Which is again very similar to the example above, where you end up with objects referencing deleted OpenGL objects.

This is exactly what happens in your case. You push Shader objects into a vector, and the OpenGL ids of objects that are already in the vector are deleted as part of the vector being reallocated when you push in more elements.

The core problem is that these C++ objects can't safely be copied. The default copy behavior (memberwise assignment) does not work. Generally you need to implement copy constructors and assignment operators in C++ for classes where the default implementations are not sufficient. But there's really no good way to do this if you have ids of OpenGL objects as members, unless you implement more elaborate schemes that could involve sharing sub-objects, reference counting, etc.

One thing I would always recommend is to have private copy constructors and assignment operators for classes that can't be properly copied. This will make sure that you get compile time errors if you accidentally copy objects, instead of mysterious runtime behavior.

Then, to store the objects in containers, the easiest approach is to store pointers to the objects in the container, instead of the objects themselves. You can use smart pointers (e.g. smart_ptr) to simplify memory management while doing that.