🔒 Locked 🎯 Unreal

Obj Loader Performance

Started by Ogre722 Jun 14, 2010 at 12:30 PM 5 replies 2.9k views

Original Post

Jun 14, 2010 at 12:30 PM

I have created an obj loader for my rendering engine and I am not very satisfied with its performance. I compared it to the loader used by Blender3d and it is only slightly slower. But still, I want to use this to support real time rendering so I need it to be fast. The code is all inside a class, so I will show the functions:

Quote:

int objModel::fileLines(char* filename){    int size = 0;    string* line = new string;    ifstream inData;    inData.open(filename);    if (!inData)        return 0;    if (inData.is_open())    {        while (!inData.eof())        {            getline(inData, *line);            size++;        }        inData.close();    }    delete line;    return size;}void objModel::numVertNormUVFace(char *filename, int &verts, int &norms, int &UVs, int &faces){    ifstream data;    string tmp;    data.open(filename);    if (data.is_open())    {        while (!data.eof())        {            getline(data, tmp);            if(tmp[0] == 'v' && tmp[1] == ' ') { verts++; }            if(tmp[0] == 'v' && tmp[1] == 't') { UVs++;   }            if(tmp[0] == 'v' && tmp[1] == 'n') { norms++; }            if(tmp[0] == 'f' && tmp[1] == ' ') { faces++; }        }        data.close();    }    if (verts < 1)        MessageBox(NULL, "Could not read obj file.", filename, MB_ICONERROR);}void objModel::getVert(string line, float &x, float &y, float &z){    string vString[3];    int c = 0;    for (int i = 2; i <= line.size(); i++)    {        if (line == ' ')        {            c++;        }        else        {            vString[c].push_back(line);        }    }    x = (float)atof(vString[0].c_str());    y = (float)atof(vString[1].c_str());    z = (float)atof(vString[2].c_str());}void objModel::getNorm(string line, float &x, float &y, float &z){    string vString[3];    int c = 0;    for (int i = 3; i <= line.size(); i++)    {        if (line == ' ')        {            c++;        }        else        {            vString[c].push_back(line);        }    }    x = (float)atof(vString[0].c_str());    y = (float)atof(vString[1].c_str());    z = (float)atof(vString[2].c_str());}void objModel::getUV(string line, float &x, float &y){    string vString[2];    int c = 0;    for (int i = 3; i <= line.size(); i++)    {        if (line == ' ')        {            c++;        }        else        {            vString[c].push_back(line);        }    }    x = (float)atof(vString[0].c_str());    y = (float)atof(vString[1].c_str());}Face objModel::getFace(string line){    string fString[4];    int c = 0;    for (int i = 2; i < line.size(); i++)    {        if (line == ' ')        {            c++;        }        else        {            fString[c].push_back(line);        }    }    string s1[3][4];    c = 0;    for (int x = 0; x < 4; x++)    {        for (int i = 0; i < fString[x].size(); i++)        {            if (fString[x].at(i) == '/')            {                c++;            }            else            {                s1[c][x].push_back(fString[x].at(i));            }        }        c = 0;    }    Face f;    f.v1 = atoi(s1[0][0].c_str());    f.v2 = atoi(s1[0][1].c_str());    f.v3 = atoi(s1[0][2].c_str());    f.v4 = atoi(s1[0][3].c_str());    f.uv1 = atoi(s1[1][0].c_str());    f.uv2 = atoi(s1[1][1].c_str());    f.uv3 = atoi(s1[1][2].c_str());    f.uv4 = atoi(s1[1][3].c_str());    f.n1 = atoi(s1[2][0].c_str());    f.n2 = atoi(s1[2][1].c_str());    f.n3 = atoi(s1[2][2].c_str());    f.n4 = atoi(s1[2][3].c_str());    return f;}bool objModel::Load(char* Filename, int meshType){    t1 = GetTickCount();    hasTexCoords = false;    hasNormals = false;    MeshType = meshType;    filename = Filename;    nVerts = nNorms = nUVs = nFaces = 0;    string tmp;    ifstream file(filename);    if (!file)    {        cout << endl << "could not load " << filename << endl;        return false;    }    numVertNormUVFace(filename, nVerts, nNorms, nUVs, nFaces);    cout << "Filename: " << filename << endl;    cout << "Verts: " << nVerts << endl;    cout << "Norms: " << nNorms << endl;    cout << "TexCoord: " << nUVs << endl;    cout << "Polygons: " << nFaces<< endl;    pVerts = new Vertex[nVerts];    pUVs = new texCoord[nUVs];    pNorms = new Normal[nNorms];    pfaces = new Face[nFaces];    int vLoop = 0;    int nLoop = 0;    int tLoop = 0;    int fLoop = 0;    if (file.is_open())    {        while (!file.eof())        {            getline(file, tmp);            if (tmp[0] == 'v' && tmp[1] == ' ')            {                getVert(tmp, pVerts[vLoop].x, pVerts[vLoop].y, pVerts[vLoop].z);                vLoop++;            }            if (tmp[0] == 'v' && tmp[1] == 'n')            {                hasNormals = true;                getNorm(tmp, pNorms[nLoop].x, pNorms[nLoop].y, pNorms[nLoop].z);                nLoop++;            }            if (tmp[0] == 'v' && tmp[1] == 't')            {                hasTexCoords = true;                getUV(tmp, pUVs[tLoop].U, pUVs[tLoop].V);                tLoop++;            }            if (tmp[0] == 'f' && tmp[1] == ' ')            {                pfaces[fLoop] = getFace(tmp);                fLoop++;            }        }        file.close();    }    t2 = GetTickCount();    cout << "ObjLoad: " << (t2-t1)*.001 << "s" << endl;    return true;}

I will greatly appreciate any suggestions that can be given.

cdoty

Jun 14, 2010 at 12:45 PM

This is an example of misdirected optimizing. The obj format isn't optimized for games; if you're worried about speed, use an external tool to convert the data to a binary file optimized for the system/API you are targeting.

If you're worried about flexibility, use a chunk system (see the IFF format for some ideas). Basically, for each data type you write out some sort of chunk header (usually a 4 byte identifier, ie 'POLY', 'TEXT', 'FACE', etc.), followed by a size, followed by the data. If you don't need a chunk, you can skip the data by adding the chunk size to the current position. In code you set up a switch statement based on the chunk type.

meisawesome

Jun 14, 2010 at 12:45 PM

Just a few suggestions, use an std::vector instead of reading the file twice. Using the vector means that you will only read through the file once, after all of the elements are copied into locally stored std::vectors, you can copy them to member structures. Also, I recommend using const string& as function paramaters to get around string copy overhead that most compilers wont optimize out.

MrPickle

Jun 14, 2010 at 12:52 PM

After a quick scan of your code, the biggest suggestion I (and many others) can give is to use std::vector instead of arrays.

These are wonderful things, they're (for the most part) as fast as arrays and you don't have to worry about allocating and deallocating the memory (which I can't see you do anywhere and so you probably have a memory leak), it's all handled automatically.

It will also cut out the need for you to count the amount of faces, vertices etc because you can just push the values onto the vector as needed. (which will cut down on the load time partially as you don't have to scan the file prior to loading it).

All necessary info on how to use std::vector can be found via a quick google search.

[EDIT] beaten to the punch I see...

ApochPiQ

1,378

ApochPiQ

Jun 14, 2010 at 1:05 PM

In addition to what has already been suggested, you should be using IO streams properly since you are using C++. You can simply do streamobject >> variable; to read almost any type of data, including strings; this will eliminate the need for calls to atoi() and all your unsafe string checking code.

A couple of other drive-by tips:

If you aren't already familiar with it, look up the use of else if and think carefully about where you could apply it to eliminate some redundant if checks

You don't need to use new and delete in your fileLines method. You can just keep the string local as a temporary. In fact, I strongly recommend not doing what you do currently, as it can leak memory pretty easily. At the very least use a smart pointer and not a raw pointer.

getVert and getNorm look suspiciously similar. They can be collapsed into a single function trivially. Avoid copy-paste code redundancy whenever possible.

It looks like your class leaks memory quite a bit. Be sure you always delete what you new and delete[] what you new[]. Look into smart pointers and the RAII idiom for some good advice on structuring your code to avoid these kinds of problems.

JonConley

Jun 14, 2010 at 1:19 PM

The beginning part of your post makes it seem as if you are trying to load models on the fly, which even with a large streaming game most of the assests are reused and can stay in memory(Granted not all). Real-time rendering doesn't need to worry itself on loading a model, only the rendering portion. There is no need to reload a model everytime a new entity is created.

I understand wanting to make the model load quicker but as has been pointed out OBJ is not an optimized format for games. It will load slower then other game models, and like I stated above loading a model should have no impact on whether it can be rendered in real-time.

Ogre722

Author

Ogre722

Jun 16, 2010 at 11:39 AM

Thank you to everyone who posted on this thread, all the suggestions were helpful. I was able to improve the performance of my loader and learn a few new things about programming and c++.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.