Made unittest for obj crash when colors are requested but not available

Made OBJ reader aware of user requests (Warning! Old default behaviour was wrong, because the reader read everything, without checking for the user options!)

git-svn-id: http://www.openmesh.org/svnrepo/OpenMesh/trunk@738 fdac6126-5c0c-442c-9429-916003d36597

src/OpenMesh/Core/IO/Options.hh

@@ -105,7 +105,8 @@ public:
       EdgeColor      = 0x0080, ///< Has (r) / store (w) edge colors
       FaceNormal     = 0x0100, ///< Has (r) / store (w) face normals
       FaceColor      = 0x0200, ///< Has (r) / store (w) face colors
-      ColorAlpha     = 0x0400  ///< Has (r) / store (w) alpha values for colors
+      FaceTexCoord   = 0x0400, ///< Has (r) / store (w) face texture coordinates
+      ColorAlpha     = 0x0800  ///< Has (r) / store (w) alpha values for colors
   };
 
 public:
@@ -199,6 +200,7 @@ public:
   bool edge_has_color()      const { return check(EdgeColor); }
   bool face_has_normal()     const { return check(FaceNormal); }
   bool face_has_color()      const { return check(FaceColor); }
+  bool face_has_texcoord()   const { return check(FaceTexCoord); }
   bool color_has_alpha()     const { return check(ColorAlpha); }
 
 

src/OpenMesh/Core/IO/reader/OBJReader.cc

@@ -283,6 +283,13 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
   std::string               matname;
 
 
+  // Options supplied by the user
+  Options userOptions = _opt;
+
+  // Options collected via file parsing
+  Options fileOptions;
+
+
   while( _in && !_in.eof() )
   {
     std::getline(_in,line);
@@ -363,11 +370,13 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
 
         if ( !stream.fail() )
         {
-          _opt += Options::VertexColor;
+          if (  userOptions.vertex_has_color() ) {
+            fileOptions += Options::VertexColor;
             colors.push_back(OpenMesh::Vec3uc((unsigned char)r,(unsigned char)g,(unsigned char)b));
           }
         }
       }
+    }
 
     // texture coord
     else if (keyWrd == "vt")
@@ -376,8 +385,13 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
 
       if ( !stream.fail()  ){
 
+        if ( userOptions.vertex_has_texcoord() || userOptions.face_has_texcoord() ) {
           texcoords.push_back(OpenMesh::Vec2f(u, v));
-        _opt += Options::VertexTexCoord;
+
+          // Can be used for both!
+          fileOptions += Options::VertexTexCoord;
+          fileOptions += Options::FaceTexCoord;
+        }
 
       }else{
 
@@ -393,8 +407,10 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
       stream >> r; stream >> g; stream >> b;
 
       if ( !stream.fail()   ){
+        if ( userOptions.vertex_has_color() ) {
           colors.push_back(OpenMesh::Vec3uc((unsigned char)r,(unsigned char)g,(unsigned char)b));
-        _opt += Options::VertexColor;
+          fileOptions += Options::VertexColor;
+        }
       }
     }
 
@@ -404,8 +420,10 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
       stream >> x; stream >> y; stream >> z;
 
       if ( !stream.fail() ) {
+        if (userOptions.vertex_has_normal() ){
           normals.push_back(OpenMesh::Vec3f(x,y,z));
-        _opt += Options::VertexNormal;
+          fileOptions += Options::VertexNormal;
+        }
       }
     }
 
@@ -491,7 +509,7 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
               // Obj counts from 1 and not zero .. array counts from zero therefore -1
               vhandles.push_back(VertexHandle(value-1));
               faceVertices.push_back(VertexHandle(value-1));
-              if (_opt.vertex_has_color())
+              if (fileOptions.vertex_has_color() )
                 _bi.set_color(vhandles.back(), colors[value-1]);
               break;
 	      
@@ -503,13 +521,28 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
                 value = texcoords.size() + value + 1;
               }
               assert(!vhandles.empty());
+
+
+              if ( fileOptions.vertex_has_texcoord() && userOptions.vertex_has_texcoord() ) {
+
                 if (!texcoords.empty() && (unsigned int) (value - 1) < texcoords.size()) {
                   // Obj counts from 1 and not zero .. array counts from zero therefore -1
                   _bi.set_texcoord(vhandles.back(), texcoords[value - 1]);
+                } else {
+                  omerr() << "Error setting Texture coordinates" << std::endl;
+                }
+
+                }
+
+                if (fileOptions.face_has_texcoord() && userOptions.face_has_texcoord() ) {
+
+                  if (!texcoords.empty() && (unsigned int) (value - 1) < texcoords.size()) {
                     face_texcoords.push_back( texcoords[value-1] );
                   } else {
                     omerr() << "Error setting Texture coordinates" << std::endl;
                   }
+                }
+
 
               break;
 
@@ -520,10 +553,13 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
                 // As obj counts from 1 and not zero add +1
                 value = normals.size() + value + 1;
               }
+
+              // Obj counts from 1 and not zero .. array counts from zero therefore -1
+              if (fileOptions.vertex_has_normal() ) {
                 assert(!vhandles.empty());
                 assert((unsigned int)(value-1) < normals.size());
-              // Obj counts from 1 and not zero .. array counts from zero therefore -1
                 _bi.set_normal(vhandles.back(), normals[value-1]);
+              }
               break;
           }
 
@@ -561,25 +597,36 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
         if ( mat.has_Kd() ) {
           Vec3uc fc = color_cast<Vec3uc, Vec3f>(mat.Kd());
 
-          for (std::vector<FaceHandle>::iterator it  = newfaces.begin();
+          if ( userOptions.face_has_color()) {
-                                                 it != newfaces.end(); ++it)
+
+            for (std::vector<FaceHandle>::iterator it = newfaces.begin(); it != newfaces.end(); ++it)
               _bi.set_color(*it, fc);
 
-          _opt += Options::FaceColor;
+            fileOptions += Options::FaceColor;
+          }
         }
 
         // Set the texture index in the face index property
         if ( mat.has_map_Kd() ) {
 
-          for (std::vector<FaceHandle>::iterator it  = newfaces.begin();
+          if (userOptions.face_has_texcoord()) {
-                                                 it != newfaces.end(); ++it)
+
+            for (std::vector<FaceHandle>::iterator it = newfaces.begin(); it != newfaces.end(); ++it)
               _bi.set_face_texindex(*it, mat.map_Kd_index());
 
+            fileOptions += Options::FaceTexCoord;
+
+          }
+
         } else {
+
           // If we don't have the info, set it to no texture
-          for (std::vector<FaceHandle>::iterator it  = newfaces.begin();
+          if (userOptions.face_has_texcoord()) {
-                                                 it != newfaces.end(); ++it)
+
+            for (std::vector<FaceHandle>::iterator it = newfaces.begin(); it != newfaces.end(); ++it)
               _bi.set_face_texindex(*it, 0);
+
+          }
         }
 
       } else {
@@ -589,8 +636,8 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
           newfaces.push_back(FaceHandle(n_faces+i));
 
         // Set the texture index to zero as we don't have any information
-        for (std::vector<FaceHandle>::iterator it  = newfaces.begin();
+        if ( userOptions.face_has_texcoord() )
-                                              it != newfaces.end(); ++it)
+          for (std::vector<FaceHandle>::iterator it = newfaces.begin(); it != newfaces.end(); ++it)
             _bi.set_face_texindex(*it, 0);
       }
 
@@ -604,18 +651,19 @@ read(std::istream& _in, BaseImporter& _bi, Options& _opt)
   {
     int i = 0;
     // add normal per vertex
-    if ( normals.size() == _bi.n_vertices() )
+
-      for (std::vector<VertexHandle>::iterator it  = vertexHandles.begin();
+    if (normals.size() == _bi.n_vertices()) {
-          it != vertexHandles.end(); ++it, i++) {
+      if ( fileOptions.vertex_has_normal() && userOptions.vertex_has_normal() ) {
+        for (std::vector<VertexHandle>::iterator it = vertexHandles.begin(); it != vertexHandles.end(); ++it, i++)
           _bi.set_normal(*it, normals[i]);
       }
-
+    }
 
     // add color per vertex
     i = 0;
     if (colors.size() >= _bi.n_vertices())
-      for (std::vector<VertexHandle>::iterator it  = vertexHandles.begin();
+      if (fileOptions.vertex_has_color() && userOptions.vertex_has_color()) {
-          it != vertexHandles.end(); ++it, i++) {
+        for (std::vector<VertexHandle>::iterator it = vertexHandles.begin(); it != vertexHandles.end(); ++it, i++)
           _bi.set_color(*it, colors[i]);
       }
 

src/Unittests/unittests_loading.hh

@@ -149,9 +149,12 @@ TEST_F(OpenMeshLoader, LoadSimpleOBJCheckHalfEdgeAndVertexNormals) {
   mesh_.request_halfedge_normals();
   mesh_.request_vertex_normals();
 
+  OpenMesh::IO::Options options;
+  options += OpenMesh::IO::Options::VertexNormal;
+
   std::string file_name = "cube-minimal.obj";
 
-  bool ok = OpenMesh::IO::read_mesh(mesh_, file_name);
+  bool ok = OpenMesh::IO::read_mesh(mesh_, file_name,options);
 
   EXPECT_TRUE(ok) << file_name;
 
@@ -209,6 +212,32 @@ TEST_F(OpenMeshLoader, LoadSimpleOBJCheckHalfEdgeAndVertexNormals) {
 
 }
 
+/*
+ * Just load a obj file and set vertex color option before loading
+ */
+TEST_F(OpenMeshLoader, LoadSimpleOBJForceVertexColorsAlthoughNotAvailable) {
+
+  mesh_.clear();
+
+  mesh_.request_vertex_colors();
+
+  std::string file_name = "cube-minimal.obj";
+
+  OpenMesh::IO::Options options;
+  options += OpenMesh::IO::Options::VertexColor;
+
+  bool ok = OpenMesh::IO::read_mesh(mesh_, file_name,options);
+
+  EXPECT_TRUE(ok) << file_name;
+
+  EXPECT_EQ(8u  , mesh_.n_vertices()) << "The number of loaded vertices is not correct!";
+  EXPECT_EQ(18u , mesh_.n_edges())     << "The number of loaded edges is not correct!";
+  EXPECT_EQ(12u , mesh_.n_faces())     << "The number of loaded faces is not correct!";
+  EXPECT_EQ(36u , mesh_.n_halfedges())  << "The number of loaded halfedges is not correct!";
+
+}
+
+
 /*
  * Just load a obj file of a cube and checks the halfedge texCoords
  */
@@ -218,9 +247,12 @@ TEST_F(OpenMeshLoader, LoadSimpleOBJCheckTexCoords) {
 
   mesh_.request_halfedge_texcoords2D();
 
+  OpenMesh::IO::Options options;
+  options += OpenMesh::IO::Options::FaceTexCoord;
+
   std::string file_name = "cube-minimal-texCoords.obj";
 
-  bool ok = OpenMesh::IO::read_mesh(mesh_, file_name);
+  bool ok = OpenMesh::IO::read_mesh(mesh_, file_name,options);
 
   EXPECT_TRUE(ok) << file_name;
 
@@ -255,7 +287,10 @@ TEST_F(OpenMeshLoader, LoadSimpleOBJWithVertexColorsAfterVertices) {
 
     mesh_.request_vertex_colors();
 
-    bool ok = OpenMesh::IO::read_mesh(mesh_, "cube-minimal-vertex-colors-after-vertex-definition.obj");
+    OpenMesh::IO::Options options;
+    options += OpenMesh::IO::Options::VertexColor;
+
+    bool ok = OpenMesh::IO::read_mesh(mesh_, "cube-minimal-vertex-colors-after-vertex-definition.obj",options);
 
     EXPECT_TRUE(ok) << "Unable to load cube-minimal-vertex-colors-after-vertex-definition.obj";
 
@@ -291,7 +326,10 @@ TEST_F(OpenMeshLoader, LoadSimpleOBJWithVertexColorsAsVCLines) {
 
     mesh_.request_vertex_colors();
 
-    bool ok = OpenMesh::IO::read_mesh(mesh_, "cube-minimal-vertex-colors-as-vc-lines.obj");
+    OpenMesh::IO::Options options;
+    options += OpenMesh::IO::Options::VertexColor;
+
+    bool ok = OpenMesh::IO::read_mesh(mesh_, "cube-minimal-vertex-colors-as-vc-lines.obj",options);
 
     EXPECT_TRUE(ok) << "Unable to load cube-minimal-vertex-colors-as-vc-lines.obj";