PDA

View Full Version : Passing vboId into function?



fiodis
05-28-2013, 07:50 AM
I'm trying to create a 4 by 4 square grid in the XZ-plane, with this bit of code:

void Grid::init(GLuint& terVboId, std::vector<float>& offsetVec, unsigned int size){
std::vector<glm::vec3> vertices;
float curX = -(size-1)/2;
float maxX = (size-1)/2;
while (curX <= maxX){
float curZ = -(size-1)/2;
float maxZ = (size-1)/2;
while (curZ <= maxZ){
vertices.push_back(glm::vec3(curX, 0.0f, curZ));
curZ++;
}
curX++;
}
glBindBuffer(GL_ARRAY_BUFFER, terVboId);
glBufferData(GL_ARRAY_BUFFER, sizeof(float)*3*vertices.size(), &vertices[0], GL_STATIC_DRAW);
}
I know offsetVec is unused, I'll be using it later. In my main initOpenGL() function I have something like this:

glGenBuffers(1, &terVboId);
Grid baseGrid;
baseGrid.init(terVboId, gridOffsets, 5);
It compiles fine, but I get an error whilst running it: Vector subscript out of range.
The error goes away if I comment out the glBufferData call, which would normally make me think I've got the size of the buffer wrong, but I've used this same general code before without a problem. The only difference is that before the Grid class had its own VBO, so I didn't need to pass the ID into the Grid::init function. But I've been told many times it's more efficient to store things with the same vertex attributes in the same VBO, so rather than having one VBO per Grid I'm trying to just generate one VBO in the main, initializeOpenGL function, and pass its ID into the Grid::init function to fill it.

I'm guessing I did that wrong. How would you pass a buffer id into some function? I assumed that since it was a GLuint it'd be like any other int, but it seems not.

thokra
05-28-2013, 09:04 AM
The error goes away if I comment out the glBufferData call, which would normally make me think I've got the size of the buffer wrong, but I've used this same general code before without a problem.

This is a purely C++ problem, nothing to do with glBufferData() (or have you seen the GL report error messages at run time on its own without having enabled debug output? ;) ). It's an MSVC++ error message since the runtime checks if the array index (0 in your case) is actually inside the current range of values stored in the vector (i.e. the value returned by size(), not capacity() !). If there is nothing there, i.e. size() == 0, the index 0 cannot be used to index into the vector - vector[0] refers to the first element.



assert(!vertices.empty());


before accessing the vector would have told you the solution already: actually put stuff in there before operating on its contents! :) I assume the code that tries to fill the vector doesn't work correctly, i.e. you never call push_back().

Passing the name of a buffer, i.e. a GLuint, somewhere is fine. However, there is no reason to pass the name as a reference to GLuint. It's a primitive data type, just take it by value:


void Grid::init(GLuint terVboId, std::vector<float>& offsetVec, unsigned int size)

fiodis
05-28-2013, 09:37 AM
I assume the code that tries to fill the vector doesn't work correctly, i.e. you never call push_back().
But I do call push_back() in the snippet I posted. What is more, when I add

std::cout << "\nSize of vertices vector is: " << vertices.size();
the code tells me vertices.size() = 25, as expected. I don't see any problem with the C++, and this is a code snippet I've used to fill vectors successfully in the past. The problem only comes up when I uncomment glBufferData, but I've used &vertices[0] in a glBufferData call before without a problem, so I'm not sure where the issue is. I can only assume it's some deep magic about how OpenGL works that a newbie like me doesn't know. :(

Alfonse Reinheart
05-28-2013, 09:49 AM
I can only assume it's some deep magic about how OpenGL works that a newbie like me doesn't know.

Or you could assume that we know what we're talking about when we tell you that it isn't. If the OpenGL driver crashed, you wouldn't get that error. The error you're getting is generated by your C++ standard library. You can tell by debugging and seeing exactly the line where it appears. It will likely appear in a C++ library header like `vector` or some such.

thokra
05-28-2013, 09:51 AM
Well, what I can say is that glBufferData will not change the contents of the vector - as you can see, the function takes a pointer to const.

Are you absolute sure the vector is filled during the invocation of your function that generates the runtime error?

In any case, I suggest you insert thisafter filling the vector and before glBufferData():


assert(vector.size() == 25);

if you are sure that the size of the vector after the algorithm went through is always supposed to be 25. That's what you call a post condition - and post conditions should be met. (EDIT: As should pre-conditions ... or any essential condition for that matter):

If you got a partly C++11 implementation that implements std::vector::data() you can simply call:


glBufferData(...,..., vector.data(), ...);

data() returns a pointer to the first element and you don't have to pass &v[0] which is simply ugly. :)

fiodis
05-28-2013, 03:37 PM
Using vector.data() instead of &vector[0] fixed the problem, thanks thokra! :)

thokra
05-28-2013, 03:41 PM
If you didn't fix the initial problem, using data() is nothing but dangerous... data() is for convenience. Find the cause of the runtime error or consider it not fixed ...

Alfonse Reinheart
05-28-2013, 04:37 PM
If you didn't fix the initial problem, using data() is nothing but dangerous... data() is for convenience.

No it isn't. If a `vector` is empty, then vector::data is required to return a NULL pointer. However, `v[0]` must return a reference to something, even if the vector is empty. Which is why he gets an index-out-of-bounds error, and if not for that, he'd almost certainly get a GPF.

thokra
05-29-2013, 07:03 AM
No it isn't.

I assume you're referring to my convenience argument, because there is no way you can argue that using a potential null-pointer is not potentially dangerous if unchecked. It's fine with glBufferData(), however. In the OPs use-case it's still far from being the desired result if operator[] still generates a runtime error for n == 0. As for convenience, this is the wording of the official defect description (http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#464) that argued to include data() as a member of std::vector and std::map:


To add slightly more convenience to vector<T> and map<Key,T> we should consider to add [..]vector<T>::data() member (const and non-const version) semantics: if( empty() ) return 0; else return buffer_;

Yes, the return value is defined - it's still purely a convenience function that spares you the necessity to check if the vector actually contains something, instead of blindly using &v[0]. In return you must still check if a nullptr is returned.


v[0] must return a reference to something, even if the vector is empty.

Nope. If the vector is empty the result is undefined for operator[]. Operator[] is not even required to check bounds for performance reasons - unlike at() which will throw an exception.

carsten neumann
05-29-2013, 09:12 AM
Since we are off-topic anyway ;) ;)
I can't find where the standard (going by the draft n3242.pdf (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf)) says that data() returns nullptr for empty vector, is that really guaranteed? $23.3.6.4 only says:



Returns: A pointer such that [data(), data()+size()) is a valid range. For non-empty vector, data() == &front().


The range is empty, so to me any pointer seems like a valid return value in this case - and as a micro-optimization an implementation could save a conditional by always returning the pointer to the start of the allocated memory (which is probably nullptr for a newly created vector, but something else for a vector with size() == 0 and capacity() > 0). In fact, that seems to be what my system libstdc++ (gcc 4.7.2) does.



v[0] must return a reference to something, even if the vector is empty.


I think you guys have a little miscommunication here: exactly because operator[] does not throw, it has to return some value; for an empty vector that pretty much means it must return garbage - it has no way to signal an error.

thokra
05-29-2013, 10:19 AM
I think you guys have a little miscommunication here

Yes, my apologies. I likely mistook Alfonse's statement for "return a valid reference" - which is of course not possible with an empty vector.



I can't find where the standard (going by the draft n3242.pdf (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf)) says that data() returns nullptr for empty vector, is that really guaranteed?

The standard doesn't explicitly say data() is required to return a nullptr. See this SO post (http://stackoverflow.com/questions/7505267/is-it-defined-to-provide-an-empty-range-to-c-standard-algorithms). In summation:


An iterator j is called reachable from an iterator i if and only if there is a finite sequence of applications of the expression ++i that makes i == j.

and


Range [i,j) is valid if and only if j is reachable from i.

Now, if you apply ++i zero times this still holds and you get i==i iff j==i. So, [i, i) is still a valid range. Awesome, isn't it?

Since a pointer is technically a special case of an iterator, 0 can also be used to form a valid, albeit empty, range [0, 0 + size()) where size == 0. Therefore, it's totally cool for std::vector::data() to return 0. Interestingly, it doesn't make sense to state [nullptr, nullptr + size()) because you can't do arithmetic on a nullptr_t. ;)

I hope I got that right.