# Vertex Arrays, Am I Doing This Right?

This topic is 3840 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hello, I am converting my rendering code over to Vertex Arrays, to improve performance. However, I have encountered a problem, because there is a "This program needs to close" error that comes up after the render code runs once. I've just learned about vertex arrays so I could be doing it completely wrong. I'll show you how I build the arrays, and show you how I am rendering. Look:
Model::Model() : ObjHandle("")
{
OutVertexArray = NULL;
OutNormalArray = NULL;
OutTexCoordArray = NULL;
}
Model::Model(const std::string& file) : ObjHandle(file)
{
OutVertexArray = NULL;
OutNormalArray = NULL;
OutTexCoordArray = NULL;
}
Model::~Model()
{
delete [] OutVertexArray;
delete [] OutNormalArray;
delete [] OutTexCoordArray;
VertexArray.clear();
NormalArray.clear();
TexCoordArray.clear();
}
{
ObjHandle.SetFilename(file);
for(int a = 0; a < ObjHandle.GetNumVertices(); a++)
{
VertexArray.push_back(ObjHandle.GetVertex(a));
}
for(int b = 0; b < ObjHandle.GetNumNormals(); b++)
{
NormalArray.push_back(ObjHandle.GetNormal(b));
}
for(int c = 0; c < ObjHandle.GetNumTexCoords(); c++)
{
TexCoordArray.push_back(ObjHandle.GetTexCoords(c));
}
OutVertexArray = new double[VertexArray.size() * 3];
OutNormalArray = new double[NormalArray.size() * 3];
OutTexCoordArray = new double[TexCoordArray.size() * 2];
int e = 0;
int f = 0;
Triangle temptri;
for(int z = 0; z < ObjHandle.GetNumTriangles(); z++)
{
if(VertexArray.size() <= 0)
{
temptri = ObjHandle.GetIndices(z);
OutVertexArray[e] = VertexArray[temptri.Vertices[0]].x;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[0]].x;
if(TexCoordArray.size() !=0) OutTexCoordArray[f] = TexCoordArray[temptri.TexCoords[0]].x;
e++;
f++;
OutVertexArray[e] = VertexArray[temptri.Vertices[0]].y;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[0]].y;
if(TexCoordArray.size() !=0) OutTexCoordArray[f] = TexCoordArray[temptri.TexCoords[0]].y;
e++;
f++;
OutVertexArray[e] = VertexArray[temptri.Vertices[0]].z;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[0]].z;
e++;
OutVertexArray[e] = VertexArray[temptri.Vertices[1]].x;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[1]].x;
if(TexCoordArray.size() !=0) OutTexCoordArray[f] = TexCoordArray[temptri.TexCoords[1]].x;
e++;
f++;
OutVertexArray[e] = VertexArray[temptri.Vertices[1]].y;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[1]].y;
if(TexCoordArray.size() !=0) OutTexCoordArray[f] = TexCoordArray[temptri.TexCoords[1]].y;
e++;
f++;
OutVertexArray[e] = VertexArray[temptri.Vertices[1]].z;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[1]].z;
e++;
OutVertexArray[e] = VertexArray[temptri.Vertices[2]].x;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[2]].x;
if(TexCoordArray.size() != 0) OutTexCoordArray[f] = TexCoordArray[temptri.TexCoords[2]].x;
e++;
f++;
OutVertexArray[e] = VertexArray[temptri.Vertices[2]].y;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[2]].y;
if(TexCoordArray.size() !=0) OutTexCoordArray[f] = TexCoordArray[temptri.TexCoords[2]].y;
e++;
OutVertexArray[e] = VertexArray[temptri.Vertices[2]].z;
if(NormalArray.size() != 0) OutNormalArray[e] = NormalArray[temptri.Normals[2]].z;
}
}
ModelMaterial = ObjHandle.GetMaterial();
}
void Model::Render()
{
glPushAttrib(GL_LIGHTING_BIT);
glMaterialfv(GL_FRONT, GL_AMBIENT, ModelMaterial.Ambient.GetArray());
glMaterialfv(GL_FRONT, GL_DIFFUSE, ModelMaterial.Diffuse.GetArray());
glMaterialfv(GL_FRONT, GL_SPECULAR, ModelMaterial.Specular.GetArray());
glMaterialf(GL_FRONT, GL_SHININESS, ModelMaterial.Shininess);
glEnableClientState(GL_VERTEX_ARRAY);
glEnableClientState(GL_NORMAL_ARRAY);
glEnableClientState(GL_TEXTURE_COORD_ARRAY);
glVertexPointer(3, GL_DOUBLE, 0, OutVertexArray);
glNormalPointer(GL_DOUBLE, 0, OutNormalArray);
glTexCoordPointer(2, GL_DOUBLE, 0, OutTexCoordArray);
glDrawArrays(GL_TRIANGLES, 0, VertexArray.size());
glDisableClientState(GL_TEXTURE_COORD_ARRAY);
glDisableClientState(GL_NORMAL_ARRAY);
glDisableClientState(GL_VERTEX_ARRAY);
glPopAttrib();
}



##### Share on other sites
Your rendering code is fine, so I would guess your problem lies in the loading code, particularly where you weld the vertices together. Check and make sure that you are ending up with the vertex, normal and texcoord arrays all the same length (otherwise one is going out of bounds).

##### Share on other sites
ah right, I have 296 Vertices, 588 normals, and 0 tex coords. So that is what is causing the problem. Alright, but since I am loading from an .obj file, what do I do with the extra normals, and also why is there almost 1:2 ration of vertices to normals?
Edit: Ohh, I need the size of OutVertexArray. Hmm, can you find the size of an array like that?

##### Share on other sites
Quote:
 Original post by Niddlesah right, I have 296 Vertices, 588 normals, and 0 tex coords. So that is what is causing the problem. Alright, but since I am loading from an .obj file, what do I do with the extra normals, and also why is there almost 1:2 ration of vertices to normals?

Having more normals than vertices is not indicative of a problem. Your problem likely lies elsewhere. I'd suggest stepping through your program with a debugger.

Obj files have their vertices, normals, and texture coordinates indexed starting at 1. Perhaps you have not compensated for that and you are attempting to access a coordinate out of bounds (ie, your max vertex index will be 295, but the obj file calls for vertex 296)?

##### Share on other sites
Quote:
 Original post by Niddlesah right, I have 296 Vertices, 588 normals, and 0 tex coords. So that is what is causing the problem. Alright, but since I am loading from an .obj file, what do I do with the extra normals, and also why is there almost 1:2 ration of vertices to normals?

Honestly, I have no idea what you are doing there in your welding stage... it seems hopelessly over-complicated for a simple operation :)
Just loop through each face, as you are already. Then inside that loop through the 3 vertices of the current face, and write out the vertex, normal and texcoord to their respective arrays.

A few cautions:
Indices in .obj files are 1 based (not zero)... this is a common mistake to make so watch out.
.obj files are not necessarily made from triangles, they may be arbitrary n-gons, so make sure you triangulate before exporting from your modeller.
To maximise the performance of vertex arrays, you need to 'interleave' your vertex elements. Allocate a single array, and then place each vertex into the array as a (vertex, normal, texcoord) triple. This takes advantage of cache coherency and can greatly improve render speed with large vertex arrays.

##### Share on other sites
I have with DevC++ debugger, and this is the area where it shows me that there is a problem, with glDrawArrays().
Edit: I always make sure I triangulate, since my code is triangle-based.
And by interleave them, do you mean:
float a[] = {0, 0, 0,      0, 1, 0,    0, 0,              0, 1, 0,      0, 1, 0,    0, 1,              1, 1, 0,      0, 1, 0,    1, 1};//For one triangle? in vertex, normal, texcoord format//And then to do the array do it like so:glEnableClientState(GL_VERTEX_ARRAY | GL_NORMAL_ARRAY | GL_TEXTURE_COORD_ARRAY);glDrawArrays(GL_TRIANGLES, 0, 25);glDisableClientState();

Oh and I switched it so it subtracts one off of the array indices, I had it that way in my previous code, but I forgot to do it there.

I put all of the vertices, normals and texcoords into vectors, because I am planning for Culling, and then I have the out arrays to define what is actually being seen, at the moment I have it all being seen so it seems redundant, but I promise I have a plan for it =).

##### Share on other sites
Ohhh, just found glInterleavedArrays(), converting to that now.

##### Share on other sites
I now have this:
    void Model::LoadObj(const std::string& file)    {        ObjHandle.SetFilename(file);        ObjLoaderHandle.LoadObj(ObjHandle);        for(int a = 0; a < ObjHandle.GetNumVertices(); a++)        {            VertexArray.push_back(ObjHandle.GetVertex(a));        }        for(int b = 0; b < ObjHandle.GetNumNormals(); b++)        {            NormalArray.push_back(ObjHandle.GetNormal(b));        }        for(int c = 0; c < ObjHandle.GetNumTexCoords(); c++)        {            TexCoordArray.push_back(ObjHandle.GetTexCoords(c));        }        int mult = 0;        if(VertexArray.size() > 0)        {            mult+=3;            if(NormalArray.size() > 0) mult+=3;            if(TexCoordArray.size() > 0) mult+=2;            OutVertexArray = new double[ObjHandle.GetNumTriangles() * mult];            int iter = 0;            for(int a=0; a < ObjHandle.GetNumTriangles(); a++)            {                if(TexCoordArray.size() > 0)                {                    OutVertexArray[iter] = ObjHandle.GetTexCoords(ObjHandle.GetIndices(a).TexCoords.x - 1);                    iter++;                    OutVertexArray[iter] = ObjHandle.GetTexCoords(ObjHandle.GetIndices(a).TexCoords.y - 1);                    iter++;                }                if(NormalArray.size() > 0)                {                    OutVertexArray[iter] = ObjHandle.GetNormal(ObjHandle.GetIndices(a).Normals.x - 1);                    iter++;                    OutVertexArray[iter] = ObjHandle.GetNormal(ObjHandle.GetIndices(a).Normals.y - 1);                    iter++;                    OutVertexArray[iter] = ObjHandle.GetNormal(ObjHandle.GetIndices(a).Normals.z - 1);                    iter++;                }                OutVertexArray[iter] = ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.x - 1);                iter++;                OutVertexArray[iter] = ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.y - 1);                iter++;                OutVertexArray[iter] = ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.z - 1);                iter++;            }        }        ModelMaterial = ObjHandle.GetMaterial();    }    void Model::Render()    {        glPushAttrib(GL_LIGHTING_BIT);        glPushClientAttrib(GL_CLIENT_VERTEX_ARRAY_BIT);        glMaterialfv(GL_FRONT, GL_AMBIENT, ModelMaterial.Ambient.GetArray());        glMaterialfv(GL_FRONT, GL_DIFFUSE, ModelMaterial.Diffuse.GetArray());        glMaterialfv(GL_FRONT, GL_SPECULAR, ModelMaterial.Specular.GetArray());        glMaterialf(GL_FRONT, GL_SHININESS, ModelMaterial.Shininess);        glInterleavedArrays(GL_T2F_N3F_V3F, 0, OutVertexArray);        glDrawArrays(GL_TRIANGLES, 0, );        glPopClientAttrib();        glPopAttrib();    }

##### Share on other sites
Hm Interesting ...?

##### Share on other sites
I suspect a bit that lines like these
OutVertexArray[iter] = ObjHandle.GetTexCoords(ObjHandle.GetIndices(a).TexCoords.x - 1);iter++;OutVertexArray[iter] = ObjHandle.GetTexCoords(ObjHandle.GetIndices(a).TexCoords.y - 1);iter++;

are what you actually want.

W/o knowledge of your ObjHandle code, I assume that ObjHandle.GetIndices(a) accesses the line that returns the indices for the a-th (0 based indexing) triangle (presumbly an instance of Triangle class). That structure should present a location index, a normal index and a texture co-ordinate index for each of the 3 vertices used by the triangle, like so:
struct Triangle {   int Locations[3]; // indices of locations for vertices 1,2,3   int Normals[3]; // indices of normals for vertices 1,2,3   int TexCoords[3]; // indices of texture co-ordinates for vertices 1,2,3];

Those indices, if fetched from an .obj file as are, are 1-based.

With that background, I assume what you want to do looks like this (w/o having proven it being workable):
          for(int a=0; a < ObjHandle.GetNumTriangles(); a++)          {              const Triangle& indices = ObjHandle.GetIndices(a);              int index;              for(int b=0; b<3; ++b) // inner iteration over the 3 vertices of the current triangle              {                if(TexCoordArray.size() > 0)                {                    index = indices.TexCoords[b] - 1;                    OutVertexArray[iter] = ObjHandle.GetTexCoords(index).x;                    iter++;                    OutVertexArray[iter] = ObjHandle.GetTexCoords(index).y;                    iter++;                }                if(NormalArray.size() > 0)                {                    index = indices.Normals[b] - 1;                    OutVertexArray[iter] = ObjHandle.GetNormal(index).x;                    iter++;                    OutVertexArray[iter] = ObjHandle.GetNormal(index).y;                    iter++;                    OutVertexArray[iter] = ObjHandle.GetNormal(index).z;                    iter++;                }                index = indices.Locations[b] - 1;                OutVertexArray[iter] = ObjHandle.GetVertex(index).x;                iter++;                OutVertexArray[iter] = ObjHandle.GetVertex(index).y;                iter++;                OutVertexArray[iter] = ObjHandle.GetVertex(index).z;                iter++;              } // for b          } // for a

That code presumbly requires you to adapt the ObjHandle.GetVertex, ObjHandle.GetNormal, and ObjHandle.GetTexCoords routines accordingly.
However, you should consider to post code of the class of ObjHandle, especially the 3 routines above and ObjHandle.GetIndices, of course.

Although you mentioned the Triangle class in your OP, you may also use 3 consecutive index triples as an implicit triangle (what would be closer to how the .obj file stores the indices). In that case my code snippet above has to look different, of course. But nevertheless you have to push NumTriangles * 3 many vertices to the array.

Besides this, you are copying the locations, normals, and texture co-ordinates into arrays (VertexArray, ...) and later use ObjHandle.GetVertex, ... to fetch them again. That seems me a waste of resources.

##### Share on other sites
Quote:
 Original post by Niddlesah right, I have 296 Vertices, 588 normals, and 0 tex coords. So that is what is causing the problem. Alright, but since I am loading from an .obj file, what do I do with the extra normals, and also why is there almost 1:2 ration of vertices to normals?Edit: Ohh, I need the size of OutVertexArray. Hmm, can you find the size of an array like that?

(I assume you mean for the glDrawArrays() call.)

No; but, since you allocated it, you already know (from the code that makes the allocation) - so you would have to just save that information. Or, you could, you know, use a std::vector (like you've been smart enough to do with the "in arrays"), since they remember their size :) The std::vector wraps just such a dynamic array allocation, so you can just pass the address of element 0 to the GL calls.

Now, let's do some clean up :) The first thing to note is that the input vectors are *temporary*; i.e. we don't need their contents for rendering, but just for loading. Therefore, there is no reason to store them as members of the object; they duplicate the information of the "out" arrays. Meanwhile, though, 'OutVertexArray' is not a great name for a member of the Model - especially since it's not going to be an array any more (actually, though, it never was - it was a *pointer* to an array *allocation*). [smile] And while I'm on the point, I'd like to say that you should almost never call .clear() yourself on any standard library container, for the same reason not to call .close() yourself on file streams: it rarely actually does anything useful. The vector members of your class will get destructed - automatically - when your class does, and the destructor of vectors does all the work that .clear() does (and more). (Oh, and it's not important any more, but since you were using an initializer list for the ObjHandle, you do know you could have done it for the pointers, too? :) )

// There's nothing wrong with your indentation style; I'm just switching to mine// as I go because I find it easier to work with when I type code directly// into the text input box.// Although I do strongly recommend putting a space between 'if', 'for', etc.// and the open parenthesis: if you ever have to look at your code without// syntax colouring, it will make them visually distinct from function calls.Model::Model(): ObjHandle("") {}Model::Model(const std::string& file): ObjHandle(file) {  LoadObj(file);}// No destructor any more: we got rid of the stuff we were managing ourselves,// and the vectors will take care of themselves.// From here on, I'm going to work with the code from your most recent post,// since I assume it has fixed real problems. ;)void Model::LoadObj(const std::string& file) {  ObjHandle.SetFilename(file);  ObjLoaderHandle.LoadObj(ObjHandle);  // Now we create the 'in' vectors locally...  std::vector<Vertex> vertices;  for (int a = 0; a < ObjHandle.GetNumVertices(); a++) {    vertices.push_back(ObjHandle.GetVertex(a));  }  std::vector<Normal> normals;  for (int b = 0; b < ObjHandle.GetNumNormals(); b++) {    normals.push_back(ObjHandle.GetNormal(b));  }  std::vector<TexCoord> texcoords;  for (int c = 0; c < ObjHandle.GetNumTexCoords(); c++) {    texcoords.push_back(ObjHandle.GetTexCoords(c));  }  int mult = 0;  if (vertices.size() > 0) {    mult += 3;    if (normals.size() > 0) { mult += 3; }    if (texcoords.size() > 0) { mult += 2; }    // And then we populate the vertex array, which is a data member.    vertexData.resize(ObjHandle.GetNumTriangles() * mult);    int iter = 0;    for (int a=0; a < ObjHandle.GetNumTriangles(); a++) {      if (texcoords.size() > 0) {        vertexData[iter] = ObjHandle.GetTexCoords(ObjHandle.GetIndices(a).TexCoords.x - 1);        iter++;        vertexData[iter] = ObjHandle.GetTexCoords(ObjHandle.GetIndices(a).TexCoords.y - 1);        iter++;      }      if (NormalArray.size() > 0) {        vertexData[iter] = ObjHandle.GetNormal(ObjHandle.GetIndices(a).Normals.x - 1);        iter++;        vertexData[iter] = ObjHandle.GetNormal(ObjHandle.GetIndices(a).Normals.y - 1);        iter++;        vertexData[iter] = ObjHandle.GetNormal(ObjHandle.GetIndices(a).Normals.z - 1);        iter++;      }      vertexData[iter] = ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.x - 1);      iter++;      vertexData[iter] = ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.y - 1);      iter++;      vertexData[iter] = ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.z - 1);      iter++;    }  }  ModelMaterial = ObjHandle.GetMaterial();}void Model::Render() {  glPushAttrib(GL_LIGHTING_BIT);  glPushClientAttrib(GL_CLIENT_VERTEX_ARRAY_BIT);  glMaterialfv(GL_FRONT, GL_AMBIENT, ModelMaterial.Ambient.GetArray());  glMaterialfv(GL_FRONT, GL_DIFFUSE, ModelMaterial.Diffuse.GetArray());  glMaterialfv(GL_FRONT, GL_SPECULAR, ModelMaterial.Specular.GetArray());  glMaterialf(GL_FRONT, GL_SHININESS, ModelMaterial.Shininess);  glInterleavedArrays(GL_T2F_N3F_V3F, 0, vertexData);  glDrawArrays(GL_TRIANGLES, 0, vertexData.size()); // typo?  glPopClientAttrib();  glPopAttrib();}

Now, the next thing I notice is how the ObjHandle.get* calls are being managed. There's a bit of a logical disconnect here: we build arrays of all normals, vertices and texcoords, but they're not (any longer) actually used (although I imagine you used to just store them before you started trying to make the glDrawArrays stuff work :) ) Instead, we continually reach back in to the ObjHandle when we're loading stuff. This is actually OK: it's just that we ought to make some wrapper functions. Right now, we have stuff like this:

ObjHandle.GetVertex(ObjHandle.GetIndices(a).Vertices.x - 1);//  ^^^^     ^^^^^^     ^^^^                ^^^^^^^^

You can see how it's a bit repetitive. The problem is that the ObjHandle doesn't provide a useful interface: the client (Model) is expected to think of the ObjHandle in terms of a collection of data vectors, instead of a device that can tell it the coordinates of a given vertex (/normal/texcoord).

class ObjectHandle {  // etc. etc. etc.  public:  double vertexXFor(int triangleId) {    // Inside member functions, we can refer to data members directly and have    // an implicit "this" to operate upon. :)    return vertices[indices[triangleId].Vertices.x - 1];  }  double vertexYFor(int triangleId) {    return vertices[indices[triangleId].Vertices.y - 1];  }  // etc. etc. I'm sure you can imagine the other six :)};

In fact, it's likely that we can get rid of the old interface once the new one is in place, and thus present a cleaner abstraction to the rest of the code. :)

OK, actually it's not quite true that we weren't using the other vectors before. We checked their sizes, simply in order to figure out if we had tex coords or normals to work with. Of course, we were kind of assuming that there would be the correct numbers of each, so there was lots of redundant information going on around there ;) But anyway, it's up to the ObjHandle to check that those sizes are all "in sync", and all the client code cares about is whether normals/texcoords are being used at all. So, we add functions to ask:

  bool hasNormals() { return !normals.empty(); }  bool hasTexCoords() { return !texcoords.empty(); }

As for vertices, there's no need to check: if there are no vertices, then there couldn't possibly be any *triangles*, so our loop will just bail out right away anyway. :)

So, let's see the effect on the Model class. I'm also going to make one more small change here: instead of maintaining an "iterator" (really an index value) into the vertex array, I'm going to add the values with .push_back(), after .reserve()ing the necessary capacity. (We don't even have to .reserve(); it's an optimization to make use of knowing how many elements we have so that the vector doesn't resize a few times as more elements come in.)

Model::Model(): ObjHandle("") {}Model::Model(const std::string& file): ObjHandle(file) {  LoadObj(file);}void Model::LoadObj(const std::string& file) {  ObjHandle.SetFilename(file);  ObjLoaderHandle.LoadObj(ObjHandle);  // not building those vectors any more :)    int mult = 3; // base allowance for vertices.  if (ObjHandle.hasNormals()) { mult += 3; }  if (ObjHandle.hasTexCoords()) { mult += 2; }  // And then we populate the vertex array, which is a data member.  vertexData.reserve(ObjHandle.GetNumTriangles() * mult);  for (int a = 0; a < ObjHandle.GetNumTriangles(); a++) {    if (ObjHandle.hasTexCoords()) {      vertexData.push_back(ObjHandle.texcoordXFor(a));       vertexData.push_back(ObjHandle.texcoordYFor(a));     }    if (ObjHandle.hasNormals()) {      vertexData.push_back(ObjHandle.normalXFor(a));       vertexData.push_back(ObjHandle.normalYFor(a));       vertexData.push_back(ObjHandle.normalZFor(a));     }    vertexData.push_back(ObjHandle.vertexXFor(a));     vertexData.push_back(ObjHandle.vertexYFor(a));     vertexData.push_back(ObjHandle.vertexZFor(a));   }  ModelMaterial = ObjHandle.GetMaterial();}void Model::Render() {  glPushAttrib(GL_LIGHTING_BIT);  glPushClientAttrib(GL_CLIENT_VERTEX_ARRAY_BIT);  glMaterialfv(GL_FRONT, GL_AMBIENT, ModelMaterial.Ambient.GetArray());  glMaterialfv(GL_FRONT, GL_DIFFUSE, ModelMaterial.Diffuse.GetArray());  glMaterialfv(GL_FRONT, GL_SPECULAR, ModelMaterial.Specular.GetArray());  glMaterialf(GL_FRONT, GL_SHININESS, ModelMaterial.Shininess);  glInterleavedArrays(GL_T2F_N3F_V3F, 0, vertexData);  glDrawArrays(GL_TRIANGLES, 0, vertexData.size());  glPopClientAttrib();  glPopAttrib();}

We could move more of the operation into the ObjLoader (i.e. .createVertexArray() ;) ), but this seems like a reasonable split for now. Now it's just a matter of cleanup:

- loadObj is private, I hope. It's only a helper for construction; you probably shouldn't load models into an existing object. (You could always assign from a newly-created object if you need that functionality.)

- As such, the filename storage is redundant, as is keeping the ObjHandle around as a data member. Or so it would seem, anyway. What else does your Model *do* besides being rendered?

- I don't see where the ObjLoaderHandle gets set at all, anyway. I would think that the ObjHandle should be able to do its own loading? :s

- Some variable names could be improved, and some values that are used more than once can be saved in variables for cleanliness.

Putting those together, we get something like:

Model::Model() {}Model::Model(const std::string& file) {  LoadObj(file);}void Model::LoadObj(const std::string& file) {  ObjectHandle ObjHandle(file);  // it probably doesn't need a .SetFilename member, BTW  ObjHandle.Load(); // create its own ObjectLoaderHandle and load itself.    int valuesPerTriangle = 3;  int triangleCount = ObjHandle.GetNumTriangles();  bool hasNormals = ObjHandle.hasNormals();  bool hasTexCoords = ObjHandle.hasTexCoords();  if (hasNormals) { valuesPerTriangle += 3; }  if (hasTexCoords) { valuesPerTriangle += 2; }  vertexData.reserve(triangleCount * valuesPerTriangle);  for (int a = 0; a < triangleCount; a++) {    if (hasTexCoords) {      vertexData.push_back(ObjHandle.texcoordXFor(a));       vertexData.push_back(ObjHandle.texcoordYFor(a));     }    if (hasNormals) {      vertexData.push_back(ObjHandle.normalXFor(a));       vertexData.push_back(ObjHandle.normalYFor(a));       vertexData.push_back(ObjHandle.normalZFor(a));     }    vertexData.push_back(ObjHandle.vertexXFor(a));     vertexData.push_back(ObjHandle.vertexYFor(a));     vertexData.push_back(ObjHandle.vertexZFor(a));   }  ModelMaterial = ObjHandle.GetMaterial();}void Model::Render() {  glPushAttrib(GL_LIGHTING_BIT);  glPushClientAttrib(GL_CLIENT_VERTEX_ARRAY_BIT);  glMaterialfv(GL_FRONT, GL_AMBIENT, ModelMaterial.Ambient.GetArray());  glMaterialfv(GL_FRONT, GL_DIFFUSE, ModelMaterial.Diffuse.GetArray());  glMaterialfv(GL_FRONT, GL_SPECULAR, ModelMaterial.Specular.GetArray());  glMaterialf(GL_FRONT, GL_SHININESS, ModelMaterial.Shininess);  glInterleavedArrays(GL_T2F_N3F_V3F, 0, vertexData);  glDrawArrays(GL_TRIANGLES, 0, vertexData.size());  glPopClientAttrib();  glPopAttrib();}

There's one last problem, though: That 'GL_T2F_N3F_V3F' constant is telling OpenGL that there are always texcoords and normals in the vertex data - even if there aren't. What you'll want to do is look at the values of hasTexCoords and hasNormals, and have a data member 'vertexDataType' or something, which you set accordingly and then use in the .Render() function.

##### Share on other sites
Quote:
 Original post by NiddlesOhhh, just found glInterleavedArrays(), converting to that now.

In actual fact, glInterleavedArrays() is a legacy function, and just calls glEnableClient() and glVertexPointer()/glNormalPointer()/glTexCoordPointer() internally. You should probably be doing the vertex pointer approach yourself, which can be done for interleaved arrays by using the 'stride' argument.

##### Share on other sites
Without a vector with all of the vertices, how can I do hidden surface removal? That is my only problem with your code, Zahlman, is I was using first arrays to hold all of the vertices' data, and then (unimplemented) do some HSR, and then copy to the out vectors, and then send that to OpenGL. Other than that, your code looks beautiful =)