# Refactoring an immediate drawing function into VBO, access violation error

I have a MD2 model loader, I am trying to substitute its immediate drawing function with a Vertex Buffer Object one....

I am getting a really annoying access violation reading error and I can't figure out why, but mostly I'd like an opinion as to whether this looks correct (never used VBOs before).

This is the original function (that compiles ok) which calculates the keyframe and draws at the same time:

glBegin(GL_TRIANGLES);
for(int i = 0; i < numTriangles; i++) {
MD2Triangle* triangle = triangles + i;
for(int j = 0; j < 3; j++) {
MD2Vertex* v1 = frame1->vertices + triangle->vertices[j];
MD2Vertex* v2 = frame2->vertices + triangle->vertices[j];
Vec3f pos = v1->pos * (1 - frac) + v2->pos * frac;
Vec3f normal = v1->normal * (1 - frac) + v2->normal * frac;
if (normal[0] == 0 && normal[1] == 0 && normal[2] == 0) {
normal = Vec3f(0, 0, 1);
}
glNormal3f(normal[0], normal[1], normal[2]);

MD2TexCoord* texCoord = texCoords + triangle->texCoords[j];
glTexCoord2f(texCoord->texCoordX, texCoord->texCoordY);
glVertex3f(pos[0], pos[1], pos[2]);
}
}
glEnd();


What I'd like to do is to calculate all positions before hand, store them in a Vertex array and then draw them. This is what I am trying to replace it with (in the exact same part of the program)

int vCount = 0;
for(int i = 0; i < numTriangles; i++) {
MD2Triangle* triangle = triangles + i;
for(int j = 0; j < 3; j++) {
MD2Vertex* v1 = frame1->vertices + triangle->vertices[j];
MD2Vertex* v2 = frame2->vertices + triangle->vertices[j];
Vec3f pos = v1->pos * (1 - frac) + v2->pos * frac;
Vec3f normal = v1->normal * (1 - frac) + v2->normal * frac;
if (normal[0] == 0 && normal[1] == 0 && normal[2] == 0) {
normal = Vec3f(0, 0, 1);
}

indices[vCount] = normal[0];
vCount++;
indices[vCount] = normal[1];
vCount++;
indices[vCount] = normal[2];
vCount++;

MD2TexCoord* texCoord = texCoords + triangle->texCoords[j];
indices[vCount] = texCoord->texCoordX;
vCount++;
indices[vCount] = texCoord->texCoordY;
vCount++;

indices[vCount] = pos[0];
vCount++;
indices[vCount] = pos[1];
vCount++;
indices[vCount] = pos[2];
vCount++;
}

}

totalVertices = vCount;

glEnableClientState(GL_NORMAL_ARRAY);
glEnableClientState(GL_TEXTURE_COORD_ARRAY);
glEnableClientState(GL_VERTEX_ARRAY);

glNormalPointer(GL_FLOAT, 0, indices);
glTexCoordPointer(2, GL_FLOAT, sizeof(float)*3, indices);
glVertexPointer(3, GL_FLOAT, sizeof(float)*5, indices);

glDrawElements(GL_TRIANGLES, totalVertices, GL_UNSIGNED_BYTE, indices);

glDisableClientState(GL_VERTEX_ARRAY);  // disable vertex arrays
glEnableClientState(GL_TEXTURE_COORD_ARRAY);
glDisableClientState(GL_NORMAL_ARRAY);


First of all, does it look right?

Second, I get access violation error "Unhandled exception at 0x01455626 in Graphics_template_1.exe: 0xC0000005: Access violation reading location 0xed5243c0" pointing at line 7

Vec3f pos = v1->pos * (1 - frac) + v2->pos * frac;


where the two Vs seems to have no value in the debugger.... Till this point the function behaves in exactly the same way as the one above, I don't understand why this happens?

Thanks for any help you may be able to provide!

EDIT ----------------------------------------------------------------------------------

Thank you Zacharmarz for your guidance, I eventually got the code working with vertexArrays (since the regular array was causing the exception due to unknown size of vertices).... after all this work I was expecting an improvement in performance but my frame rate is almost half what it was when rendering twice with immediate mode.... Maybe I'm doing something extra here? I managed to implement a std::vector collection like recommended:

    for(int i = 0; i < numTriangles; i++) {
MD2Triangle* triangle = triangles + i;
for(int j = 0; j < 3; j++) {
MD2Vertex* v1 = frame1->vertices + triangle->vertices[j];
MD2Vertex* v2 = frame2->vertices + triangle->vertices[j];
Vec3f pos = v1->pos * (1 - frac) + v2->pos * frac;
Vec3f normal = v1->normal * (1 - frac) + v2->normal * frac;
if (normal[0] == 0 && normal[1] == 0 && normal[2] == 0) {
normal = Vec3f(0, 0, 1);
}

normals.push_back(normal[0]);
normals.push_back(normal[1]);
normals.push_back(normal[2]);

MD2TexCoord* texCoord = texCoords + triangle->texCoords[j];
textCoords.push_back(texCoord->texCoordX);
textCoords.push_back(texCoord->texCoordY);

vertices.push_back(pos[0]);
vertices.push_back(pos[1]);
vertices.push_back(pos[2]);
}

}

glEnableClientState(GL_NORMAL_ARRAY);
glEnableClientState(GL_TEXTURE_COORD_ARRAY);
glEnableClientState(GL_VERTEX_ARRAY);

glNormalPointer(GL_FLOAT, 0, &normals[0]);
glTexCoordPointer(2, GL_FLOAT, 0, &textCoords[0]);
glVertexPointer(3, GL_FLOAT, 0, &vertices[0]);

glDrawArrays(GL_TRIANGLES, 0, vertices.size()/3);

glDisableClientState(GL_VERTEX_ARRAY);  // disable vertex arrays
glDisableClientState(GL_TEXTURE_COORD_ARRAY);
glDisableClientState(GL_NORMAL_ARRAY);

vertices.clear();
textCoords.clear();
normals.clear();


Is there something extra that I am doing here? Cos this is really meant to be more efficient that glBegin()/End(), right?

Thanks for your time and help!

First of all: You misunderstood glDrawElements. This function takes as its last parameter indices to array of vertices/normals/texture_coordinates (not array with vertices / normals.....) - this can be used for indexed geometry.

Second of all: You misunderstood glNormalPointer / glVertexPointer / glTexCoordPointer. Its 3rd parameter is stride, which means stride in the whole array of vertices / normals / tex_coordinates. So its always 8*sizeof(float). And also its lasta parameter is pointer to first elements (vertex coordinate, texture coordinate, normal coordinate). So it should be for normals really indices, for textures &(indices[3]) (I think also indices + 3 should work) and for vertices &(indices[5]).

And for your first problem - you should use glDrawArrays, if you don't want to use indexed geometry. And you should use it in this way: glDrawArrays(GL_TRIANGLES, 0, totalVertices/8) - here "totalVertices/8" is your real count of vertices - each vertex has 8 parts - 3 normal coordinates + 2 texture coordinates + 3 vertex coordinates.

But I really don't understand how your Unhandled exception... can occur, when you use the same mechanism in first case and there it's working properly.

BTW: And this is not VBO. These are called vertex arrays - geometry data is transfered from CPU side to GPU side each time you call glDrawArrays. If you want to use VBOs (geometry is stored in GPU memory), you can try this tutorial. But what I have understood, you are doing some animation or interpolation (?) - you are using frame1 and frame2 and frac variable. So you can use vertex arrays of VBOs with some "streamed" flag - this means the data will be changed every frame (specified once and used once). –

• yes, I am interpolating between frames... Thanks for your very exhaustive answer! Clearly I was a bit confused about VBO, all I want to do is to repeat immediate drawing as I have to pass through the vertices twice for a rendering effect, and doing all the interpolation calulculation between glBegin/glEnd is a very bad idea I was told.... I'll try what you said, once I can spot this stupid error which makes absolutely no sense... – Alex Nov 23 '11 at 13:30
• Aren't you somewhere unload MD2 model? Do you have changed something else except code you have shown here? Is this doing also in first iteration or when does it appear? – zacharmarz Nov 23 '11 at 14:05
• yes it's on its very first iteration, all I did was to replace the code shown, nothing else has changed... somehow the MD2Vertex pointers are not being initialized when I replace the code.....and its quite a massive program so I wouldn't know what to post of it... really frustrating.... – Alex Nov 23 '11 at 14:23
• I seem to have localised the issue, it's to do with the indices[] array that I am trying to use for drawElements since the error doesn't happen when I remove it, I must have been writing past the end of it.... Since the size of the vertices is not quite known until the model is loaded, what would be the best way/container to store the vertices I iterate through for the vertex array draw call? – Alex Nov 23 '11 at 15:32
• The best and simplest way is to use ordinary dynamically alocated array. Something like: float* indices = new float[8*3*triangleCount]; (8 floats for each vertex, 3 vertices for each triangle). – zacharmarz Nov 23 '11 at 17:02