   OBJ Loader - fscan_f issue
  Hey all this is my first post, I recently became unemployed and decided to finally get into what i always wanted and started by creating a simple Direct3D program to take care of the simple functions for drawing objects and adding HLSL.I began to write my own OBJ Loader and am running into some warnings that i think will impact it when i get into larger file sizes.Here is my code.#include &lt;iostream&gt;using std::cerr;using std::cout;using std::cin;using std::endl;using std::fixed;using std::ios;using std::left;using std::right;using std::showpoint;#include &lt;fstream&gt;using std::ifstream;#include &lt;iomanip&gt;using std::setw;using std::setprecision;#include &lt;string&gt;using std::string;#include &lt;cstdlib&gt;using std::exit;#include &lt;stdio.h&gt;#include &lt;vector&gt;#include &lt;d3dx10.h&gt;#include &lt;d3d10.h&gt;#include "VertexFormats.h"#include "Main.h"//void ModelOBJ::importGeometryFirstPass(FILE *pFile)void main(){FILE *pFile;char *mode = "r";char inputFilename[] = "C:\\Users\\Smigss\\Documents\\Visual Studio 2008\\Projects\\OBJLoader\\OBJLoader\\ashtray.obj";pFile = fopen(inputFilename, mode);if (pFile == NULL) {cout &lt;&lt; "Error reading file";exit(1);}bool m_hasTextureCoords = false;    bool m_hasNormals = false;    int m_numberOfVertexCoords = 0;    int m_numberOfTextureCoords = 0;    int m_numberOfNormals = 0;    int m_numberOfTriangles = 0;    int v = 0;    int vt = 0;    int vn = 0;int i = 0;char thething;    char buffer[256] = {0};    string name;    while (fscanf(pFile, "%s", buffer) != EOF)    {switch (buffer[0]){case 'v':   // v, vt, or vnswitch (buffer[1]){case '\0':fgets(buffer, sizeof(buffer), pFile);++m_numberOfVertexCoords;break;case 'n':fgets(buffer, sizeof(buffer), pFile);++m_numberOfNormals;break;case 't':fgets(buffer, sizeof(buffer), pFile);++m_numberOfTextureCoords;default:break;}break;default:fgets(buffer, sizeof(buffer), pFile);break;case 'f':   // v, v//vn, v/vt, v/vt/vn.  fscanf(pFile, "%s", buffer);  if (strstr(buffer, "//")) // v//vn  { sscanf(buffer, "%d//%d", &v, &vn); fscanf(pFile, "%d//%d", &v, &vn); fscanf(pFile, "%d//%d", &v, &vn); ++m_numberOfTriangles; while (fscanf(pFile, "%d//%d", &v, &vn) &gt; 0)++m_numberOfTriangles;  }  else if (sscanf(buffer, "%d/%d/%d", &v, &vt, &vn) == 3) // v/vt/vn  { fscanf(pFile, "%d/%d/%d", &v, &vt, &vn); fscanf(pFile, "%d/%d/%d", &v, &vt, &vn); ++m_numberOfTriangles; while (fscanf(pFile, "%d/%d/%d", &v, &vt, &vn) &gt; 0)++m_numberOfTriangles;  }  else if (sscanf(buffer, "%d/%d", &v, &vt) == 2) // v/vt  { fscanf(pFile, "%d/%d", &v, &vt); fscanf(pFile, "%d/%d", &v, &vt); ++m_numberOfTriangles; while (fscanf(pFile, "%d/%d", &v, &vt) &gt; 0)++m_numberOfTriangles;  }  else // v  { fscanf(pFile, "%d", &v); fscanf(pFile, "%d", &v); ++m_numberOfTriangles; while (fscanf(pFile, "%d", &v) &gt; 0)++m_numberOfTriangles;  }  break;}  }cout &lt;&lt; "VertexCoordinates: " &lt;&lt; m_numberOfVertexCoords;cout &lt;&lt; "\nVertexNormals: " &lt;&lt; m_numberOfNormals;cout &lt;&lt; "\nTextureCoordinates: " &lt;&lt; m_numberOfTextureCoords;cout &lt;&lt; "\nTriangles: " &lt;&lt; m_numberOfTriangles;// create the vertices array large enough to hold all those neededVertexPosStruct vertices[sizeof(m_numberOfVertexCoords)];VertexPosStruct normals[sizeof(m_numberOfNormals)];VertexPosStruct textures[sizeof(m_numberOfTextureCoords)];int va[3] = {0};int vna[3] = {0};int vta[3] = {0};int numVertices = 0;int numNormals = 0;int numTextures = 0;int curIndex = 0;std::vector&lt;float&gt; m_vertexCoords;std::vector&lt;float&gt; m_vertexNormals;std::vector&lt;float&gt; m_textureCoords;// Allocate memory for the OBJ model data.    m_vertexCoords.resize(m_numberOfVertexCoords * 3);m_vertexNormals.resize(m_numberOfNormals * 3);m_textureCoords.resize(m_numberOfTextureCoords * 3);// Create the indices array//DWORD indices[sizeof(m_numberOfTriangles) * 9];rewind(pFile);while (fscanf(pFile, "%s", buffer) != EOF)    {switch (buffer[0]){   case 'f':  // v, vt and vnfscanf(pFile, "%s", buffer);if (sscanf(buffer, "%d/%d/%d", &va[0], &vta[0], &vna[0]) == 3) // v/vt/vn{fscanf(pFile, "%d/%d/%d", &va[1], &vta[1], &vna[1]);fscanf(pFile, "%d/%d/%d", &va[2], &vta[2], &vna[2]);//indices[curIndex] = va[0];//curIndex = curIndex + 9;}default:fgets(buffer, sizeof(buffer), pFile);break;case 'v':   // v, vt, or vnswitch (buffer[1]){case '\0':fscanf(pFile, "%f %f %f", &m_vertexCoords[3 * numVertices], &m_vertexCoords[3 * numVertices + 1],&m_vertexCoords[3 * numVertices + 2]);++numVertices;break;case 'n':fscanf(pFile, "%f %f %f",&m_vertexNormals[3 * numNormals],&m_vertexNormals[3 * numNormals + 1],&m_vertexNormals[3 * numNormals + 2]);++numNormals;break;case 't':fscanf(pFile, "%f %f %f",&m_textureCoords[3 * numTextures],&m_textureCoords[3 * numTextures + 1],&m_textureCoords[3 * numTextures + 2]);++numTextures;break;default:break;}break;}}for(int a = 0; a &lt; numVertices; ++a){vertices[a].Pos.x = m_vertexCoords[3 * a];vertices[a].Pos.y = m_vertexCoords[3 * a + 1];vertices[a].Pos.z = m_vertexCoords[3 * a + 2];}for(int b = 0; b &lt; numNormals; ++b){normals.Normal.x = m_vertexNormals[3 * b];normals.Normal.y = m_vertexNormals[3 * b + 1];normals.Normal.z = m_vertexNormals[3 * b + 2];}for(int c = 0; c &lt; numTextures; ++c){textures[c].Texture.x = m_textureCoords[3 * c];textures[c].Texture.y = m_textureCoords[3 * c + 1];textures[c].Texture.z = m_textureCoords[3 * c + 2];}}If anyone can suggest anything please feel free to comment.[Edited by - Smigss on May 9, 2010 12:06:23 PM]
  Couple suggestions:Use [source][/source] tags to post code on these forums.You're using C++; use C++'s facilities.main must return int, return integers instead of using exitVertexPosStruct vertices[sizeof(m_numberOfVertexCoords)]; doesn't do what you think it does.  sizeof(anyInteger) will always be a constant size.  Writing to elements [4 m_numberOfVertexCoords) will cause problems!Only parse your file once.  Using a dynamic array, such as std::vector, you don't need to know the number of elements before handUse std::string instead of raw char arrays; they'll make your life easierUse std::istream::operator >> instead of fscanf and fgets; it'll make your life easierLike so:#include <iostream>#include <string>#include <sstream>#include <fstream>struct Vector3 {float x, y, z;};struct Face {unsigned idx[3];};int main( int argc, char* argv[] ) {using namespace std;vector<Vector3> vertices;vector<Vector3> normals;vector<Face> faces;if ( argc < 2 ) {cout << "Usage:   test.exe OBJFILE";return 0;}ifstream fs( argv[1] );unsigned lineAt = 0;while ( fs ) {++lineAt;string line;getline( fs, line ); // get a line of textstringstream ss;ss << line;  // tokenize the linestring type;ss >> type; // first token denotes the type of data to followif ( type == "v" || type == "vn" ) { // vertex/normal// next three tokens should be floatsVector3 temp;if ( ss >> temp.x >> temp.y >> temp.z ) {if ( type == "v" )vertices.push_back( temp );elsenormals.push_back( temp );} elsecout << "Unable to determine vertex at line " << lineAt << endl;} else if ( type == "f" ) { // face// next three tokens should be intsFace temp;if ( ss >> temp.idx[0] >> temp.idx[1] >> temp.idx[2] ) face.push_back( temp );elsecout << "Unable to determine face indices at line " << lineAt << endl;} else {cout << "Unknown type at line " << lineAt << endl;}}};  I was tasked with writing a high performance obj loader recently. I started with a well-formed really lovely entirely C++-based .obj loader but found that under VC++2010 all three of std::fstream, std::stringstream, and std::string were all just way, way too slow to load the kinds of models I wanted; even under a release build with highest optimisations and appropriate preprocessor flags to disable debugging, etc, etc. Watching it in a profiler I found that my bottlenecks were critical sections that the stream objects like to enter, for whatever reason, and std::string allocations.I tried various things, such as line-by-line reading, shoving the entire file into a stringstream first then using that as the one and only stringstream object, and some others I'm sure.In the end I gave in, I fopen the file, determine its size, dump the entire thing into a pre-allocated std::vector, iterate through that, converting all values to doubles and longs via strtod and strtoul respectively and low and behold the whole thing is lightening fast.I'm not saying your experience will be the same, especially under different compilers or settings, and certainly the std:: objects have no inherent speed limitations in their specification (it's all about the underlying implementation), but it's certainly worth profiling [wink]In addition, for an added speed boost, you can implement an ad-hoc header in the obj file as a comment to pre-specify the file size, number of vertices, etc. You could look for this first and, if it exists, reserve all necessary memory upfront; otherwise you'll have to pay the price for expanding vectors beyond their capacity, or else using deques but still suffering allocation overhead.[Edited by - dmatter on May 7, 2010 10:23:23 PM];  Wow, i appreciate the comments they are some good advice.I noticed that in my obj type file that the size of each type is shown at the end of the statement i.eV 1 1 1V 2 2 2#2 verticesSo what you were saying dmatter is that maybe when i get to that point i should export in my own file type that defines those sizes upfront in the header so i only have to parse the file once like _fastcall was saying._fastcall thanks very much I'm working on things i obviously was unaware of and that mock code you wrote was extremely helpful in understanding some of the string functions such as the overload operator >>.  And how much easier it is to pull a whole line as a string then grabbing from that.Much thanks hopefully I'll have rewritten this by tommorow in a much more optimized fashion.One question though, will i have any problems with using the D3DXVECTOR3 type instead of putting into a std::vector first or do i need to convert the parameters from the STD to the D3DX?[Edited by - Smigss on May 9, 2010 12:05:34 PM];Quote:Original post by SmigssSo what you were saying dmatter is that maybe when i get to that point i should export in my own file type that defines those sizes upfront in the header so i only have to parse the file once like _fastcall was saying.Pretty much. It doesn't have to be a custom format though (it can be, if you want), it can still be an .obj file just with a comment at the top to act as a header, something like:#objHeader#file 9999#verts 123#normals 222#texs 321That way you can just read the initial part of the file for the #objHeader token; if it's present you can read in the header and perform all your memory allocations upfront. Otherwise you can't predict how much memory you require and so you can't help but pay the performance penalty for repeatedly resizing vectors. In that case I ran some tests to make a heuristic guess which is performed based on the size of the file; certainly the number of vertices/normals/texs/faces are not going to be bigger than the size of the file so the closer you guess to to that number you less likely you are to have to resize a vector but the more likely you are to be wasting memory.In the end I didn't need to delve into cache optimisations but one could order all vertices, normals, texs and faces into their distinct groups so that there's minimal changing between different array accesses.It was interesting to note, too, that a naive Java implementation significantly out-performed a fairly standard C++ implementation; I'd expect it would out-perform _fastcall's implementation for example. Managed languages have much, much cheaper allocation overheads and I assumed that a Java ArrayList is probably able to resize faster than a std::vector that is holding types by-value.This is probably mostly overkill for you though, the obj format is unsuitable really for high-performance asset loading anyway; it's just that in my case I did need to load massive obj files quickly.   ;  This is true but baby steps first.  I used to write in VB instead of C++ about a decade ago and i just picked up a D3DX book and C++ book a few weeks ago and so far I've been coming along pretty well with it.  My hope is by the end of the summer i will have written a complete graphics engine if not more.  Is there any suggestions on a specific file type that is most currently used or does each company create their own as i would imagine?   ;Quote:Original post by SmigssIs there any suggestions on a specific file type that is most currently used or does each company create their own as i would imagine?They might create their own for a specific application, canonical examples are Doom and Quake, or they might just pick an existing format that serves their purpose. By and large it doesn't matter really. Obj is good for beginner and no-budget projects because it's a nice simple format to use, there's plenty of free assets available online and there are free tools available, i.e. Blender, that can convert back and forth between Obj and pretty much every other format going. Milkshape, ms3d, files are similar in this respect although perhaps a tad more challenging to a beginner but are somewhat faster to load. Everything has tradeoffs.   ;  So i used _fastcall's example of the code and it's working really good for the V, VN, VT values as they are seperated by whitespaces but...trying to pull the Indices using the same format is only pulling the first number as they are seperated by "/" instead of a whitespace.  Any suggestions on how to format that input properly?bool importModelData(VertexPosNormalTexStruct *pVertices, DWORD *pIndices){using namespace std;FILE *pFile;char *mode = "r";char objFile[] = "C:\\Users\\Smigss\\Documents\\Visual Studio 2008\\Projects\\OBJLoader\\OBJLoader\\Box.obj";pFile = fopen(objFile, mode);if (!pFile){return false;}ifstream fs( pFile );int numVertex = 0;int numNormal = 0;int numTexture = 0;int numIndice = 0;unsigned lineAt = 0;while ( fs ){++lineAt;string line;getline(fs, line);  //get line of textstringstream ss;ss << line;  //tokenize the linestring type;ss >> type;  //firt token identifies type of dataif ( type == "v" ) //vertex coordinates, next 3 values should be floats{ss >> pVertices[numVertex].Vertice.x >> pVertices[numVertex].Vertice.y >> pVertices[numVertex].Vertice.z;++numVertex;}else if ( type == "vn" ){ss >> pVertices[numNormal].Normal.x >> pVertices[numNormal].Normal.y >> pVertices[numNormal].Normal.z;++numNormal;}if (type == "f" ){ss >> pIndices[numIndice] >> pIndices[numIndice + 1] >> pIndices[numIndice + 2];numIndice += 9;}}}Here is what it's like in the file:g Box01s 2f 1/1/1 2/2/1 3/3/1 f 3/3/1 4/4/1 1/1/1 s 4f 5/4/2 6/1/2 7/2/2 f 7/2/2 8/3/2 5/4/2 s 8f 1/4/3 4/1/3 6/2/3 f 6/2/3 5/3/3 1/4/3 s 16f 4/4/4 3/1/4 7/2/4 f 7/2/4 6/3/4 4/4/4 s 32f 3/4/5 2/1/5 8/2/5 f 8/2/5 7/3/5 3/4/5 s 64f 2/4/6 1/1/6 5/2/6 f 5/2/6 8/3/6 2/4/6 # 12 faces   ;  Ok so after much searching i found another post the solved the problem.  Albeit it seems like it could be optimized better.  Basically i just had to create a char type as a delimiter for the "/".Here is the adjusted code:if (type == "f" ){ss >> pIndices[numIndice] >> placeholder >> pIndices[numIndice + 1] >> placeholder >> pIndices[numIndice + 2] >> pIndices[numIndice + 3] >> placeholder >> pIndices[numIndice + 4] >> placeholder >> pIndices[numIndice + 5] >> pIndices[numIndice + 6] >> placeholder >> pIndices[numIndice + 7] >> placeholder >> pIndices[numIndice + 8];numIndice += 9;}It's not drawing it yet but I think I'll have to recheck the mesh because i used a different structure last time when manually creating these values.  I no longer account for Color but instead get Normal values.   