View Full Version : glUseProgram/glValidateProgram raise error "invalid value"

01-22-2015, 10:02 AM

I'm using C++ and have made classes which each wrap shaders, programs and buffers etc.
I have managed to narrow down my problem where when either glUseProgram(id) or glValidateProgram(id) is called first and afterwards called glGetError(), the error returned is GL_INVALID_VALUE and this happens everytime when either is called (I get the error message string from gluGetErrorString(target) which returns "invalid value").

glValidateProgram(id) succeeds when queried with glGetProgramiv(id, statusOf, &status) but GL_INVALID_VALUE is still raised and I think its true since all calls to glUniform* calls cause GL_INVALID_OPERATION ("invalid operation" from gluGetErrorString) error.

I have queried the locations of the uniform variables from the program object successfully so the id value cannot be completely broken.

I have one another shader program which is hard coded but it causes no errors and has no effect since I have removed it completly during my tests to ensure it had no effect.

I am really puzzled by this problem and I am hoping for quick solution since this project of mine is my thesis and OpenGL is big part of it.

OS: Ubuntu 12.04
GPU Driver: Nvidia 331.113
GPU: Asus 560 Ti
OpenGL version: 4.4.0 (I am using 4.3 and above features)
Editor (if it matters): Qt Creator
Window contexts are created from GLFW 3.0.

Sorry for not posting any code but I think its just a bit too big to copy paste it into forum post but will do if necessary. (Should use my schools servers to put it there in display even if its incomplete).

01-22-2015, 04:02 PM
Ok I have narrowed it down even further.

this->sPrograms.push_back(ShaderProgram(shaders, pinfos->at(i)->name));
sPrograms is vector.

Everything works fine in the ShaderProgram Constructor but after that all uniform queries used by the ID created in in the constructor results in failure. I have checked uniform locations afterwards found within the program and they result in -1. I have tested with glIsProgram and that results in 0 outside the constructor. Yet the glValidateProgram results success.

If it helps I have used GLEW instead of typing the functions I use.

Alfonse Reinheart
01-22-2015, 05:44 PM
There's no possible way to diagnose this with any certainty without knowing what's going on in ShaderProgram. That being said...

I find it disconcerting to see you using a std::vector to store your ShaderProgram objects in. Not so much because it's impossible, but mainly because it's very dangerous if you don't know what you're doing. And impossible to use ShaderProgram like this (as a value-type) if you're not using C++11.

Let's take your ShaderProgram. I can't see what's in it, but I can make some guesses. You pass it "shaders"; I have no clue what that is, but I'm going to guess that it's some form of string array. Therefore, this ShaderProgram constructor creates not only OpenGL shader objects, but also OpenGL program objects. Therefore, your ShaderProgram type internally stores a program object.

Now, if you're following good C++ practice, your ShaderProgram has a destructor that will call glDestroyProgram on that object.

So... what exactly happens in your copy constructor? Oh, you may not be aware, but your code creates a ShaderProgram on the stack, and then passes that to std::vector::push_back. In C++ pre-11, that means that std::vector::push_back will call your copy constructor to construct the internal ShaderProgram stored in the std::vector.

I'm going to take another guess: you didn't write a copy constructor. Well, that means the compiler will supply one for you. And that copy constructor will basically do a memcpy on the contents of ShaderProgram. Which means that, by the end of the call to std::vector::push_back, there will be two versions of ShaderProgram that store the exact same program object.

Now, you created a ShaderProgram on the stack as a temporary, then passed it to std::vector::push_back. It got copied, thus resulting in two ShaderProgram objects that refer to the same program object. And now, since you created a ShaderProgram temporary, after your call to std::vector::push_back, that temporary will be destroyed. Which will call glDeleteProgram on that program object.

And now it isn't a program object anymore.

Even if you wrote a copy constructor, it won't work. That's because it can't work; there's no way to copy a program object. Even if you stored the original shader strings and recompiled/linked it in the copy constructor (and this would be exceptionally slow anyway), it wouldn't work. Unless you assigned all your resource locations, attributes, fragment outputs, and uniform locations within your shader text, the copied program won't necessarily be identical. And even if you did, you'd still have to go through and copy each of the uniform values and other state from one program to the other.

The correct way to do this is to use proper C++ programming. Because ShaderProgram stores and manages an object which cannot be copied, ShaderProgram itself cannot be copied. Therefore, you need to take steps to ensure that any code which attempts to do so fails to compile. You could derive from boost::noncopyable if that's available to you. If not, you can do what boost::noncopyable does for you easily enough: declare the copy constructor (and copy-assignment operator), but don't define them anywhere. Therefore, if some code tries to copy the object, you'll get a linker error.

Now, you'll quickly realize that you can't use a non-copyable ShaderProgram in a std::vector, in pre-11 C++. Any attempt to insert a ShaderProgram into a std::vector requires the use of a copy constructor. And anything that grows the std::vector may need to allocate new memory and copy the old entries into the new memory. Which requires using the copy constructor. So your only option is to store pointers to ShaderProgram objects, which means you need to manage their lifetimes, which is kind of a pain.

I mentioned pre-11 C++ because C++11 and above make it really easy to fix all of this. First, C++11 makes it easy to declare ShaderProgram to be non-copyable by using the "= delete" syntax. More than that however, C++11 has the concept of object movement. You can give ShaderProgram move constructor/assignment functions, which allow you to effectively steal the program object from the moved-from ShaderProgram. And std::vector in C++11 does all of its work via movement whenever that's available, so a move-only ShaderProgram can be stored by value in std::vector.

Plus, you can use emplacement insertion in C++11, so you just do this:

this->sPrograms.emplace_back(shaders, pinfos->at(i)->name);

It will internally construct the ShaderProgram in place, so your move constructor won't even be called.

Feel free to look up the details of this online; it's got nothing to do with OpenGL.

01-23-2015, 01:50 AM
You were absolutely spot on. Didn't even think of it. Should have put up a message when deletion happens since ShaderProgram destructor does call glDeleteProgram(). Quick change to new allocation and things are on track. .emplace_back() for some reason didn't work. It still probably called the destructor. (PS: shaders are Shader objects which have the fully compiled Shader Object reference)

I don't think that I stored the program objects originally in the stack because the sPrograms is part of ShaderProgramManager which is allocated to heap via new. Unless vector overrides this because its container and not just simple variable.

Anyways thanks a lot even when it was my C++ knowledge which caused the error and not OpenGL. Go grab a cookie.

Alfonse Reinheart
01-23-2015, 06:59 AM
.emplace_back() for some reason didn't work. It still probably called the destructor.

More likely your std::vector needed to reallocate memory, so it copied all of the existing entries. The only other reason ShaderProgram's destructor should be called by std::vector::emplace_back is if you're using the worst standard library implementation ever.