Access Reading Violation Error - C++ Obj Loader

Question

I've written a basic .obj loader. It works with a cube and some other basic stuff. Once I give it a complex model it fails.

Here is some basic code:

Functions.h

#pragma once
#include <string>

struct face {
    bool triangle;
    int faceNumber;
    int faces[4];
    face(int faceNumber2,int f1,int f2, int f3, int f4) {
        triangle = false;
        faceNumber = faceNumber2;
        faces[0] = f1;
        faces[1] = f2;
        faces[2] = f3;
        faces[3] = f4;
    }
    face(int faceNumber2,int f1,int f2, int f3) {
        triangle = true;
        faceNumber = faceNumber2;
        faces[0] = f1;
        faces[1] = f2;
        faces[2] = f3;
    }


};

struct position {
    float x, y, z;
    position(float X,float Y, float Z) {
        x = X;
        y = Y;
        z = Z;
    }
};

class Functions
{
public:
    Functions(void);
    ~Functions(void);
    int loadObject(std::string fileName);
    void drawCube(float size);
    unsigned int loadTexture(const char* fileName);
};

Functions.cpp

int Functions::loadObject(string fileName) {
    ifstream file(fileName);
    vector<string*> line;
    vector<position*> normals;
    vector<face*> faces;
    vector<position*> vertices;



    if (file.is_open()) {
        char buffer[256];
        while (!file.eof()) {
            file.getline(buffer,256);
            line.push_back(new string(buffer));
        }
        for (int i = 0; i<line.size(); i++) {
            if ((*line[i])[0] == 'v' && (*line[i])[1] == ' ') { //Vertice
                float x,y,z;
                sscanf_s((*line[i]).c_str(),"v %f %f %f",&x,&y,&z);
                vertices.push_back(new position(x,y,z));
            }else if ((*line[i])[0] == 'v' && (*line[i])[1] == 'n') { //Normals
                float x,y,z;
                sscanf_s((*line[i]).c_str(),"vn %f %f %f",&x,&y,&z);
                normals.push_back(new position(x,y,z));
            } else if ((*line[i])[0] == 'f') {
                if (count((*line[i]).begin(),(*line[i]).end(),' ') == 4) {
                    float a,b,c,d,e;
                    sscanf_s((*line[i]).c_str(),"f %f//%f %f//%f %f//%f %f//%f",&a,&e,&b,&e,&c,&e,&d,&e);

                    faces.push_back(new face(e,a,b,c,d));
                } else {
                    float a,b,c,e;
                    sscanf_s((*line[i]).c_str(),"f %f//%f %f//%f %f//%f",&a,&e,&b,&e,&c,&e);
                    faces.push_back(new face(e,a,b,c));
                }

            }
        }
        file.close();
    } else {
        file.close();
        throw exception("Fail to load file");
        return -1;
    }

    int num;
    num = glGenLists(1);
    glNewList(num,GL_COMPILE);
        for (int i = 0; i<faces.size(); i++) {
            face tempFace = (*faces[i]);
            if (tempFace.triangle) {
                glBegin(GL_TRIANGLES);
                    glNormal3f((*normals[tempFace.faceNumber-1]).x, (*normals[tempFace.faceNumber-1]).y, (*normals[tempFace.faceNumber-1]).z);
                    glVertex3f((*vertices[tempFace.faces[0]-1]).x, (*vertices[tempFace.faces[0]-1]).y, (*vertices[tempFace.faces[0]-1]).z);
**//Errors below.**
                    glVertex3f((*vertices[tempFace.faces[1]-1]).x, (*vertices[tempFace.faces[1]-1]).y, (*vertices[tempFace.faces[1]-1]).z);
                    glVertex3f((*vertices[tempFace.faces[2]-1]).x, (*vertices[tempFace.faces[2]-1]).y, (*vertices[tempFace.faces[2]-1]).z);
                glEnd();
            } else {
            //glNormal3f((*normals[tempFace.faceNumber-1]).x,(*normals[tempFace.faceNumber-1]).y,(*normals[tempFace.faceNumber-1]).z)
                glBegin(GL_QUADS);
                    glNormal3f((*normals[tempFace.faceNumber-1]).x, (*normals[tempFace.faceNumber-1]).y, (*normals[tempFace.faceNumber-1]).z);
                    glVertex3f((*vertices[tempFace.faces[0]-1]).x, (*vertices[tempFace.faces[0]-1]).y, (*vertices[tempFace.faces[0]-1]).z);
                    glVertex3f((*vertices[tempFace.faces[1]-1]).x, (*vertices[tempFace.faces[1]-1]).y, (*vertices[tempFace.faces[1]-1]).z);
                    glVertex3f((*vertices[tempFace.faces[2]-1]).x, (*vertices[tempFace.faces[2]-1]).y, (*vertices[tempFace.faces[2]-1]).z);
                    glVertex3f((*vertices[tempFace.faces[3]-1]).x, (*vertices[tempFace.faces[3]-1]).y, (*vertices[tempFace.faces[3]-1]).z);
                glEnd();
            }
        }
    glEndList();


    for (int i = 0; i<line.size(); i++) {
        delete line[i];
    }
    for (int i = 0; i<normals.size(); i++) {
        delete normals[i];
    }
    for (int i = 0; i<vertices.size(); i++) {
        delete vertices[i];
    }
    for (int i = 0; i<faces.size(); i++) {
        delete faces[i];
    }

    return 1;
}

I've been trying this for like two days. Like I said it works with a few things just not complex models.

Answer 1

There are several problems with the code, each technically calling for a separate answer. But oh well, here it goes. If you want to use C++, then do it right. First of all we want our objects RAII capable. Which means we need a copy constructor and an assignment operator.

// A face should always be a triangle. Quads are ambigous
// when it comes to rendering, and most OpenGL implementations
// break them into triangles anyway. We can do this better.
// Also the "faceNumber" element doesn't do what you think it
// does. We get back to that later.
struct face {
    int i[3];

    void copy(face const &f_) {
        for(int j = 0; j < 3; j++) { // could use memcpy as well
            i[j] = f_.i[j]
        }
    }
    face const &operator=(face const &f_) {
        copy(f_);
        return *this;
    }
    face(face const &f_) { copy(f_); }
    face(int i_a, int i_b, int i_c) {
        i[0] = i_a;
        i[1] = i_b;
        i[2] = i_c;

    }
};

struct tuple3f {
    float x, y, z;

    tuple3f(float x_,float y_, float z_) : x(x_), y(y_), z(z_) {}
    tuple3f(tuple3f const &p_) : x(p_.x), y(p_.y), z(p_.z) {}
    tuple3f const &operator=(tuple3f const &p_) {
        x = p_.x; y = p_.y; z = p_.z;
        return *this;
    }
};

// Struct for unrolling the vertices into.
struct vertex3p3n {
     tuple3f position, normal;

     // Implementing the constructor, copy constructor and
     // assignment operator is left as en exercise.   
     vertex3p3n(tuple3 const &p_, tuple3 const &n_) : ... {}
};

namespace glhelpers {
    enum error {
        NoError = 0,
        FileNotOpened
    };

    error loadWavefrontOBJ(
            std::string fileName,
            std::vector<vertex3p3n> &out_unrolledvertexarray ) {
        ifstream file(fileName);

        vector<tuple3f> positions;
        vector<tuple3f> normals;

        vector<face>    faces_position;
        vector<face>    faces_normal;

        if (file.is_open()) {
            // It's totally inefficient to first load a file line by line
            // into memory, making a new allocation for each line's string
            // which internally does another new allocation,
            // only to process it line by line later.
            //
            // Just do the line by line processing on the read lines without
            // intermediary allocation. And there's a nice std::getline
            // function which does the job just nicely for us.
            while( !file.eof() ) {
                std::string line;
                std::getline(file, line);

                if( 'v' == line[0] && ' ' == line[1] ) { // Vertex
                    float x,y,z;
                    sscanf_s(line.c_str()+2, "%f %f %f", &x,&y,&z);
                    positions.push_back(tuple3f(x,y,z));
                    continue;
                }

                if( 'v' == line[0] && 'n' == line[1] ) { // Normal
                    float x,y,z;
                    sscanf_s(line[i].c_str()+3, "%f %f %f", &x,&y,&z);
                    normals.push_back(tuple3f(x,y,z));
                    continue;
                }

                // Here begins the nasty part. The Wavefront OBJ file format
                // treats positions, normals and texture coordinates as separate
                // entities. Which is fine, as long as you don't want to shove
                // them into a renderer like OpenGL is. You see, in OpenGL
                // every *vertex* consists of the whole combination of
                // [position, normal, texture coordinate, ...] and other attributes.
                // They're not separable (even if the immediate mode API may make
                // people think set). Which leads to a problem: Somehow we've to
                // unite those later on. There are several ways to do this.
                // Renumbering and unrolling are the easiest once. Putting the
                // numbers into immediate mode is a form of unrolling.
                //
                // What the Wavefront actually is giving you for each face is the
                // index into the separated position, texture coordinate and
                // normal arrays.
                if ( 'f' == line[0] ) {
                    if( count(line.begin(), line.end(), ' ') == 4) {
                        float pa, pb, pc, pd;
                        float na, nb, nc, nd;
                        sscanf_s(
                             line.c_str()+2
                             "%f//%f %f//%f %f//%f %f//%f",
                             &pa,&na, &pb,&nb, &pc,&nc, &pd,&nd);

                        faces_position.push_back(face(pa, pb, pc));
                        faces_normal.push_back(  face(na, nb, nc));

                        // Decompose the quad into two triangles

                        faces_position.push_back(face(pc, pd, pa));
                        faces_normal.push_back(  face(nc, nd, na));
                    } else {
                        float pa, pb, pc;
                        float na, nb, nc;
                        sscanf_s(
                            line.c_str()+2,"%f//%f %f//%f %f//%f",
                            &pa,&na, &pb,&nb, &pc,&nc);

                        faces_position.push_back(face(pa, pb, pc));
                        faces_normal.push_back(  face(na, nb, nc));
                    }
                    continue;
                }
            }
            file.close();
        } else {
            // If the file couldn't be opened, there's no point
            // in trying to close it.

            // throwing an exception diverts the regular control
            // flow, i.e. the function won't return.
            // My advice: Don't use exceptions, they're harmful
            // in most situations. Use proper error codes instead.
            return FileNotOpened;
        }

        out_unrolledvertexarray.resize(faces_position.size()*3),
        for(int j = 0; j < faces_position.size(); j++) {
            for(int k = 0; k < 3; k++ {
                out_unrolledvertexarray[j*3 + k] =
                    vertex3p3n(
                        positions[faces_positions[j].i[k]],
                        normals[  faces_normals  [j].i[k]]);
            }
        }
        return NoError;
    }
}

Now how do we draw this? Well, we use vertex arrays for this. In the old and dusted fixed function pipeline, they're accessed through the client state and gl…Pointer functions.

std::vector<vertex3p3n> model;

void loadmodel();
{  // Do this only one time at startup
    glhelpers::loadWavefrontOBJ("...", model);
}

void drawmodel()
{
    glEnableClientState(GL_VERTEX_ARRAY); // a much better name was GL_POSITION_ARRAY
    glEnableClientState(GL_NORMAL_ARRAY);

    glVertexPointer(
        3, // 3 elements per position tuple
        GL_FLOAT, // each element is a float
        sizeof(vertex3p3n), // distance between vertex position[0]
        &model[0].position );

    glNormalPointer( // a normal always has 3 elements
        GL_FLOAT, // each element is a float
        sizeof(vertex3p3n), // distance between vertex position[0]
        &model[0].normal );

    glDrawArrays(GL_TRIANGLES, 0, model.size();
 }