Fix assertion when there is no normal/texcoord assigned to the face. Fixes #161.

Suppress clang warnings.

examples/viewer/viewer.cc

@@ -37,11 +37,11 @@ extern "C" {
 #include <windows.h>
 
 #ifdef max
- #undef max
+#undef max
 #endif
 
 #ifdef min
- #undef min
+#undef min
 #endif
 
 #include <mmsystem.h>
@@ -143,15 +143,15 @@ float eye[3], lookat[3], up[3];
 
 GLFWwindow* window;
 
-static std::string GetBaseDir(const std::string &filepath) {
+static std::string GetBaseDir(const std::string& filepath) {
   if (filepath.find_last_of("/\\") != std::string::npos)
     return filepath.substr(0, filepath.find_last_of("/\\"));
   return "";
 }
 
-static bool FileExists(const std::string &abs_filename) {
+static bool FileExists(const std::string& abs_filename) {
   bool ret;
-  FILE *fp = fopen(abs_filename.c_str(), "rb");
+  FILE* fp = fopen(abs_filename.c_str(), "rb");
   if (fp) {
     ret = true;
     fclose(fp);
@@ -195,10 +195,10 @@ static void CalcNormal(float N[3], float v0[3], float v1[3], float v2[3]) {
 }
 
 static bool LoadObjAndConvert(float bmin[3], float bmax[3],
-                       std::vector<DrawObject>* drawObjects,
+                              std::vector<DrawObject>* drawObjects,
-                       std::vector<tinyobj::material_t>& materials,
+                              std::vector<tinyobj::material_t>& materials,
-                       std::map<std::string, GLuint>& textures,
+                              std::map<std::string, GLuint>& textures,
-                       const char* filename) {
+                              const char* filename) {
   tinyobj::attrib_t attrib;
   std::vector<tinyobj::shape_t> shapes;
 
@@ -217,8 +217,8 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
 #endif
 
   std::string err;
-  bool ret =
+  bool ret = tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename,
-      tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename, base_dir.c_str());
+                              base_dir.c_str());
   if (!err.empty()) {
     std::cerr << err << std::endl;
   }
@@ -242,56 +242,62 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
   materials.push_back(tinyobj::material_t());
 
   for (size_t i = 0; i < materials.size(); i++) {
-    printf("material[%d].diffuse_texname = %s\n", int(i), materials[i].diffuse_texname.c_str());
+    printf("material[%d].diffuse_texname = %s\n", int(i),
+           materials[i].diffuse_texname.c_str());
   }
 
   // Load diffuse textures
   {
-      for (size_t m = 0; m < materials.size(); m++) {
+    for (size_t m = 0; m < materials.size(); m++) {
-          tinyobj::material_t* mp = &materials[m];
+      tinyobj::material_t* mp = &materials[m];
 
-          if (mp->diffuse_texname.length() > 0) {
+      if (mp->diffuse_texname.length() > 0) {
-              // Only load the texture if it is not already loaded
+        // Only load the texture if it is not already loaded
-              if (textures.find(mp->diffuse_texname) == textures.end()) {
+        if (textures.find(mp->diffuse_texname) == textures.end()) {
-                  GLuint texture_id;
+          GLuint texture_id;
-                  int w, h;
+          int w, h;
-                  int comp;
+          int comp;
 
-                  std::string texture_filename = mp->diffuse_texname;
+          std::string texture_filename = mp->diffuse_texname;
-                  if (!FileExists(texture_filename)) {
+          if (!FileExists(texture_filename)) {
-                    // Append base dir.
+            // Append base dir.
-                    texture_filename = base_dir + mp->diffuse_texname;
+            texture_filename = base_dir + mp->diffuse_texname;
-                    if (!FileExists(texture_filename)) {
+            if (!FileExists(texture_filename)) {
-                      std::cerr << "Unable to find file: " << mp->diffuse_texname << std::endl;
+              std::cerr << "Unable to find file: " << mp->diffuse_texname
-                      exit(1);
+                        << std::endl;
-                    }
+              exit(1);
-                  }
+            }
-                  
-                  unsigned char* image = stbi_load(texture_filename.c_str(), &w, &h, &comp, STBI_default);
-                  if (!image) {
-                      std::cerr << "Unable to load texture: " << texture_filename << std::endl;
-                      exit(1);
-                  }
-                  std::cout << "Loaded texture: " << texture_filename << ", w = " << w << ", h = " << h << ", comp = " << comp << std::endl;
-
-                  glGenTextures(1, &texture_id);
-                  glBindTexture(GL_TEXTURE_2D, texture_id);
-                  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
-                  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
-                  if (comp == 3) {
-                      glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, w, h, 0, GL_RGB, GL_UNSIGNED_BYTE, image);
-                  }
-                  else if (comp == 4) {
-                      glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, w, h, 0, GL_RGBA, GL_UNSIGNED_BYTE, image);
-                  } else {
-                      assert(0); // TODO
-                  }
-                  glBindTexture(GL_TEXTURE_2D, 0);
-                  stbi_image_free(image);
-                  textures.insert(std::make_pair(mp->diffuse_texname, texture_id));
-              }
           }
+
+          unsigned char* image =
+              stbi_load(texture_filename.c_str(), &w, &h, &comp, STBI_default);
+          if (!image) {
+            std::cerr << "Unable to load texture: " << texture_filename
+                      << std::endl;
+            exit(1);
+          }
+          std::cout << "Loaded texture: " << texture_filename << ", w = " << w
+                    << ", h = " << h << ", comp = " << comp << std::endl;
+
+          glGenTextures(1, &texture_id);
+          glBindTexture(GL_TEXTURE_2D, texture_id);
+          glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+          glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+          if (comp == 3) {
+            glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, w, h, 0, GL_RGB,
+                         GL_UNSIGNED_BYTE, image);
+          } else if (comp == 4) {
+            glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, w, h, 0, GL_RGBA,
+                         GL_UNSIGNED_BYTE, image);
+          } else {
+            assert(0);  // TODO
+          }
+          glBindTexture(GL_TEXTURE_2D, 0);
+          stbi_image_free(image);
+          textures.insert(std::make_pair(mp->diffuse_texname, texture_id));
+        }
       }
+    }
   }
 
   bmin[0] = bmin[1] = bmin[2] = std::numeric_limits<float>::max();
@@ -308,23 +314,40 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
 
         int current_material_id = shapes[s].mesh.material_ids[f];
 
-        if ((current_material_id < 0) || (current_material_id >= static_cast<int>(materials.size()))) {
+        if ((current_material_id < 0) ||
+            (current_material_id >= static_cast<int>(materials.size()))) {
           // Invaid material ID. Use default material.
-          current_material_id = materials.size() - 1; // Default material is added to the last item in `materials`.
+          current_material_id =
+              materials.size() -
+              1;  // Default material is added to the last item in `materials`.
         }
-        //if (current_material_id >= materials.size()) {
+        // if (current_material_id >= materials.size()) {
-        //    std::cerr << "Invalid material index: " << current_material_id << std::endl;
+        //    std::cerr << "Invalid material index: " << current_material_id <<
+        //    std::endl;
         //}
         //
         float diffuse[3];
         for (size_t i = 0; i < 3; i++) {
-            diffuse[i] = materials[current_material_id].diffuse[i];
+          diffuse[i] = materials[current_material_id].diffuse[i];
         }
         float tc[3][2];
         if (attrib.texcoords.size() > 0) {
-            assert(attrib.texcoords.size() > 2 * idx0.texcoord_index + 1);
+          if ((idx0.texcoord_index < 0) || (idx1.texcoord_index < 0) ||
-            assert(attrib.texcoords.size() > 2 * idx1.texcoord_index + 1);
+              (idx2.texcoord_index < 0)) {
-            assert(attrib.texcoords.size() > 2 * idx2.texcoord_index + 1);
+            // face does not contain valid uv index.
+            tc[0][0] = 0.0f;
+            tc[0][1] = 0.0f;
+            tc[1][0] = 0.0f;
+            tc[1][1] = 0.0f;
+            tc[2][0] = 0.0f;
+            tc[2][1] = 0.0f;
+          } else {
+            assert(attrib.texcoords.size() >
+                   size_t(2 * idx0.texcoord_index + 1));
+            assert(attrib.texcoords.size() >
+                   size_t(2 * idx1.texcoord_index + 1));
+            assert(attrib.texcoords.size() >
+                   size_t(2 * idx2.texcoord_index + 1));
 
             // Flip Y coord.
             tc[0][0] = attrib.texcoords[2 * idx0.texcoord_index];
@@ -333,13 +356,14 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
             tc[1][1] = 1.0f - attrib.texcoords[2 * idx1.texcoord_index + 1];
             tc[2][0] = attrib.texcoords[2 * idx2.texcoord_index];
             tc[2][1] = 1.0f - attrib.texcoords[2 * idx2.texcoord_index + 1];
+          }
         } else {
-            tc[0][0] = 0.0f;
+          tc[0][0] = 0.0f;
-            tc[0][1] = 0.0f;
+          tc[0][1] = 0.0f;
-            tc[1][0] = 0.0f;
+          tc[1][0] = 0.0f;
-            tc[1][1] = 0.0f;
+          tc[1][1] = 0.0f;
-            tc[2][0] = 0.0f;
+          tc[2][0] = 0.0f;
-            tc[2][1] = 0.0f;
+          tc[2][1] = 0.0f;
         }
 
         float v[3][3];
@@ -363,27 +387,40 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
         }
 
         float n[3][3];
-        if (attrib.normals.size() > 0) {
+        {
-          int f0 = idx0.normal_index;
+          bool invalid_normal_index = false;
-          int f1 = idx1.normal_index;
+          if (attrib.normals.size() > 0) {
-          int f2 = idx2.normal_index;
+            int nf0 = idx0.normal_index;
-          assert(f0 >= 0);
+            int nf1 = idx1.normal_index;
-          assert(f1 >= 0);
+            int nf2 = idx2.normal_index;
-          assert(f2 >= 0);
+
-          for (int k = 0; k < 3; k++) {
+            if ((nf0 < 0) || (nf1 < 0) || (nf2 < 0)) {
-            n[0][k] = attrib.normals[3 * f0 + k];
+              // normal index is missing from this face.
-            n[1][k] = attrib.normals[3 * f1 + k];
+              invalid_normal_index = true;
-            n[2][k] = attrib.normals[3 * f2 + k];
+            } else {
+              for (int k = 0; k < 3; k++) {
+                assert(size_t(3 * nf0 + k) < attrib.normals.size());
+                assert(size_t(3 * nf1 + k) < attrib.normals.size());
+                assert(size_t(3 * nf2 + k) < attrib.normals.size());
+                n[0][k] = attrib.normals[3 * nf0 + k];
+                n[1][k] = attrib.normals[3 * nf1 + k];
+                n[2][k] = attrib.normals[3 * nf2 + k];
+              }
+            }
+          } else {
+            invalid_normal_index = true;
+          }
+
+          if (invalid_normal_index) {
+            // compute geometric normal
+            CalcNormal(n[0], v[0], v[1], v[2]);
+            n[1][0] = n[0][0];
+            n[1][1] = n[0][1];
+            n[1][2] = n[0][2];
+            n[2][0] = n[0][0];
+            n[2][1] = n[0][1];
+            n[2][2] = n[0][2];
           }
-        } else {
-          // compute geometric normal
-          CalcNormal(n[0], v[0], v[1], v[2]);
-          n[1][0] = n[0][0];
-          n[1][1] = n[0][1];
-          n[1][2] = n[0][2];
-          n[2][0] = n[0][0];
-          n[2][1] = n[0][1];
-          n[2][2] = n[0][2];
         }
 
         for (int k = 0; k < 3; k++) {
@@ -396,11 +433,9 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
           // Combine normal and diffuse to get color.
           float normal_factor = 0.2;
           float diffuse_factor = 1 - normal_factor;
-          float c[3] = {
+          float c[3] = {n[k][0] * normal_factor + diffuse[0] * diffuse_factor,
-              n[k][0] * normal_factor + diffuse[0] * diffuse_factor,
+                        n[k][1] * normal_factor + diffuse[1] * diffuse_factor,
-              n[k][1] * normal_factor + diffuse[1] * diffuse_factor,
+                        n[k][2] * normal_factor + diffuse[2] * diffuse_factor};
-              n[k][2] * normal_factor + diffuse[2] * diffuse_factor
-          };
           float len2 = c[0] * c[0] + c[1] * c[1] + c[2] * c[2];
           if (len2 > 0.0f) {
             float len = sqrtf(len2);
@@ -422,19 +457,22 @@ static bool LoadObjAndConvert(float bmin[3], float bmax[3],
       o.numTriangles = 0;
 
       // OpenGL viewer does not support texturing with per-face material.
-      if (shapes[s].mesh.material_ids.size() > 0 && shapes[s].mesh.material_ids.size() > s) {
+      if (shapes[s].mesh.material_ids.size() > 0 &&
-          o.material_id = shapes[s].mesh.material_ids[0]; // use the material ID of the first face.
+          shapes[s].mesh.material_ids.size() > s) {
+        o.material_id = shapes[s].mesh.material_ids[0];  // use the material ID
+                                                         // of the first face.
       } else {
-          o.material_id = materials.size() - 1; // = ID for default material.
+        o.material_id = materials.size() - 1;  // = ID for default material.
       }
       printf("shape[%d] material_id %d\n", int(s), int(o.material_id));
 
       if (buffer.size() > 0) {
         glGenBuffers(1, &o.vb_id);
         glBindBuffer(GL_ARRAY_BUFFER, o.vb_id);
-        glBufferData(GL_ARRAY_BUFFER, buffer.size() * sizeof(float), &buffer.at(0),
+        glBufferData(GL_ARRAY_BUFFER, buffer.size() * sizeof(float),
-                     GL_STATIC_DRAW);
+                     &buffer.at(0), GL_STATIC_DRAW);
-        o.numTriangles = buffer.size() / (3 + 3 + 3 + 2) / 3; // 3:vtx, 3:normal, 3:col, 2:texcoord
+        o.numTriangles = buffer.size() / (3 + 3 + 3 + 2) /
+                         3;  // 3:vtx, 3:normal, 3:col, 2:texcoord
 
         printf("shape[%d] # of triangles = %d\n", static_cast<int>(s),
                o.numTriangles);
@@ -467,7 +505,7 @@ static void reshapeFunc(GLFWwindow* window, int w, int h) {
 }
 
 static void keyboardFunc(GLFWwindow* window, int key, int scancode, int action,
-                  int mods) {
+                         int mods) {
   (void)window;
   (void)scancode;
   (void)mods;
@@ -549,7 +587,9 @@ static void motionFunc(GLFWwindow* window, double mouse_x, double mouse_y) {
   prevMouseY = mouse_y;
 }
 
-static void Draw(const std::vector<DrawObject>& drawObjects, std::vector<tinyobj::material_t>& materials, std::map<std::string, GLuint>& textures) {
+static void Draw(const std::vector<DrawObject>& drawObjects,
+                 std::vector<tinyobj::material_t>& materials,
+                 std::map<std::string, GLuint>& textures) {
   glPolygonMode(GL_FRONT, GL_FILL);
   glPolygonMode(GL_BACK, GL_FILL);
 
@@ -572,7 +612,7 @@ static void Draw(const std::vector<DrawObject>& drawObjects, std::vector<tinyobj
     if ((o.material_id < materials.size())) {
       std::string diffuse_texname = materials[o.material_id].diffuse_texname;
       if (textures.find(diffuse_texname) != textures.end()) {
-          glBindTexture(GL_TEXTURE_2D, textures[diffuse_texname]);
+        glBindTexture(GL_TEXTURE_2D, textures[diffuse_texname]);
       }
     }
     glVertexPointer(3, GL_FLOAT, stride, (const void*)0);
@@ -668,7 +708,8 @@ int main(int argc, char** argv) {
   float bmin[3], bmax[3];
   std::vector<tinyobj::material_t> materials;
   std::map<std::string, GLuint> textures;
-  if (false == LoadObjAndConvert(bmin, bmax, &gDrawObjects, materials, textures, argv[1])) {
+  if (false == LoadObjAndConvert(bmin, bmax, &gDrawObjects, materials, textures,
+                                 argv[1])) {
     return -1;
   }
 

loader_example.cc

@@ -29,6 +29,13 @@ extern "C" {
 #endif
 #endif
 
+#ifdef __clang__
+#pragma clang diagnostic push
+#if __has_warning("-Wzero-as-null-pointer-constant")
+#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant"
+#endif
+#endif
+
 class timerutil {
  public:
 #ifdef _WIN32

models/issue-161-inconsistent-f.obj

@@ -0,0 +1,37 @@
+o cube
+mtllib cube.mtl
+
+v -0.500000 -0.500000 0.500000
+v 0.500000 -0.500000 0.500000
+v -0.500000 0.500000 0.500000
+v 0.500000 0.500000 0.500000
+v -0.500000 0.500000 -0.500000
+v 0.500000 0.500000 -0.500000
+v -0.500000 -0.500000 -0.500000
+v 0.500000 -0.500000 -0.500000
+
+vt 0.000000 0.000000
+vt 1.000000 0.000000
+vt 0.000000 1.000000
+vt 1.000000 1.000000
+
+g cube
+usemtl cube
+s 1
+f 1/1 2/2 3/3
+f 3/3 2/2 4/4
+s 2
+f 3/1 4/2 5/3
+f 5/3 4/2 6/4
+s 3
+f 5/4 6/3 7/2
+f 7/2 6/3 8/1
+s 4
+f 7/1 8/2 1/3
+f 1/3 8/2 2/4
+s 5
+f 2/1 8/2 4/3
+f 4/3 8/2 6/4
+s 6
+f 7/1 1/2 5/3
+f 5 1 3

tiny_obj_loader.h

@@ -51,6 +51,13 @@ THE SOFTWARE.
 
 namespace tinyobj {
 
+#ifdef __clang__
+#pragma clang diagnostic push
+#if __has_warning("-Wzero-as-null-pointer-constant")
+#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant"
+#endif
+#endif
+
 // https://en.wikipedia.org/wiki/Wavefront_.obj_file says ...
 //
 //  -blendu on | off                       # set horizontal texture blending
@@ -1026,9 +1033,9 @@ static bool exportFaceGroupToShape(
 				real_t e1x = v2x - v1x;
 				real_t e1y = v2y - v1y;
 				real_t e1z = v2z - v1z;
-				float cx = fabs(e0y*e1z - e0z*e1y);
+				float cx = std::fabs(e0y*e1z - e0z*e1y);
-				float cy = fabs(e0z*e1x - e0x*e1z);
+				float cy = std::fabs(e0z*e1x - e0x*e1z);
-				float cz = fabs(e0x*e1y - e0y*e1x);
+				float cz = std::fabs(e0x*e1y - e0y*e1x);
 				const float epsilon = 0.0001f;
 				if( cx > epsilon || cy > epsilon || cz > epsilon ) {
 					// found a corner
@@ -2237,6 +2244,10 @@ bool LoadObjWithCallback(std::istream &inStream, const callback_t &callback,
 
   return true;
 }
+
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
 }  // namespace tinyobj
 
 #endif