Merge pull request #1931 from kwuerl/issue-1923

Fix Issue #1923: OBJ Exporter can't correctly export vertex colors

code/ObjExporter.cpp

@@ -114,14 +114,13 @@ static const std::string MaterialExt = ".mtl";
 ObjExporter::ObjExporter(const char* _filename, const aiScene* pScene, bool noMtl)
 : filename(_filename)
 , pScene(pScene)
-, vp()
 , vn()
 , vt()
-, vc()
+, vp()
-, mVpMap()
+, useVc(false)
 , mVnMap()
 , mVtMap()
-, mVcMap()
+, mVpMap()
 , mMeshes()
 , endl("\n") {
     // make sure that all formatting happens using the standard, C locale and not the user's current locale
@@ -268,27 +267,22 @@ void ObjExporter::WriteGeometryFile(bool noMtl) {
     AddNode(pScene->mRootNode, mBase);
 
     // write vertex positions with colors, if any
-    mVpMap.getVectors( vp );
+    mVpMap.getKeys( vp );
-    mVcMap.getColors( vc );
+    if ( !useVc ) {
-    if ( vc.empty() ) {
         mOutput << "# " << vp.size() << " vertex positions" << endl;
-        for ( const aiVector3D& v : vp ) {
+        for ( const vertexData& v : vp ) {
-            mOutput << "v  " << v.x << " " << v.y << " " << v.z << endl;
+            mOutput << "v  " << v.vp.x << " " << v.vp.y << " " << v.vp.z << endl;
         }
     } else {
         mOutput << "# " << vp.size() << " vertex positions and colors" << endl;
-        size_t colIdx = 0;
+        for ( const vertexData& v : vp ) {
-        for ( const aiVector3D& v : vp ) {
+            mOutput << "v  " << v.vp.x << " " << v.vp.y << " " << v.vp.z << " " << v.vc.r << " " << v.vc.g << " " << v.vc.b << endl;
-            if ( colIdx < vc.size() ) {
-                mOutput << "v  " << v.x << " " << v.y << " " << v.z << " " << vc[ colIdx ].r << " " << vc[ colIdx ].g << " " << vc[ colIdx ].b << endl;
-            }
-            ++colIdx;
         }
     }
     mOutput << endl;
 
     // write uv coordinates
-    mVtMap.getVectors(vt);
+    mVtMap.getKeys(vt);
     mOutput << "# " << vt.size() << " UV coordinates" << endl;
     for(const aiVector3D& v : vt) {
         mOutput << "vt " << v.x << " " << v.y << " " << v.z << endl;
@@ -296,7 +290,7 @@ void ObjExporter::WriteGeometryFile(bool noMtl) {
     mOutput << endl;
 
     // write vertex normals
-    mVnMap.getVectors(vn);
+    mVnMap.getKeys(vn);
     mOutput << "# " << vn.size() << " vertex normals" << endl;
     for(const aiVector3D& v : vn) {
         mOutput << "vn " << v.x << " " << v.y << " " << v.z << endl;
@@ -337,54 +331,15 @@ void ObjExporter::WriteGeometryFile(bool noMtl) {
     }
 }
 
-// ------------------------------------------------------------------------------------------------
-int ObjExporter::vecIndexMap::getIndex(const aiVector3D& vec) {
-    vecIndexMap::dataType::iterator vertIt = vecMap.find(vec);
-    // vertex already exists, so reference it
-    if(vertIt != vecMap.end()){
-        return vertIt->second;
-    }
-    vecMap[vec] = mNextIndex;
-    int ret = mNextIndex;
-    mNextIndex++;
-    return ret;
-}
-
-// ------------------------------------------------------------------------------------------------
-void ObjExporter::vecIndexMap::getVectors( std::vector<aiVector3D>& vecs ) {
-    vecs.resize(vecMap.size());
-    for(vecIndexMap::dataType::iterator it = vecMap.begin(); it != vecMap.end(); ++it){
-        vecs[it->second-1] = it->first;
-    }
-}
-
-// ------------------------------------------------------------------------------------------------
-int ObjExporter::colIndexMap::getIndex( const aiColor4D& col ) {
-    colIndexMap::dataType::iterator vertIt = colMap.find( col );
-    // vertex already exists, so reference it
-    if ( vertIt != colMap.end() ) {
-        return vertIt->second;
-    }
-    colMap[ col ] = mNextIndex;
-    int ret = mNextIndex;
-    mNextIndex++;
-
-    return ret;
-}
-
-// ------------------------------------------------------------------------------------------------
-void ObjExporter::colIndexMap::getColors( std::vector<aiColor4D> &colors ) {
-    colors.resize( colMap.size() );
-    for ( colIndexMap::dataType::iterator it = colMap.begin(); it != colMap.end(); ++it ) {
-        colors[ it->second - 1 ] = it->first;
-    }
-}
-
 // ------------------------------------------------------------------------------------------------
 void ObjExporter::AddMesh(const aiString& name, const aiMesh* m, const aiMatrix4x4& mat) {
     mMeshes.push_back(MeshInstance() );
     MeshInstance& mesh = mMeshes.back();
 
+    if ( nullptr != m->mColors[ 0 ] ) {
+        useVc = true;
+    }
+
     mesh.name = std::string( name.data, name.length );
     mesh.matname = GetMaterialName(m->mMaterialIndex);
 
@@ -410,7 +365,13 @@ void ObjExporter::AddMesh(const aiString& name, const aiMesh* m, const aiMatrix4
             const unsigned int idx = f.mIndices[a];
 
             aiVector3D vert = mat * m->mVertices[idx];
-            face.indices[a].vp = mVpMap.getIndex(vert);
+
+            if ( nullptr != m->mColors[ 0 ] ) {
+                aiColor4D col4 = m->mColors[ 0 ][ idx ];
+                face.indices[a].vp = mVpMap.getIndex({vert, aiColor3D(col4.r, col4.g, col4.b)});
+            } else {
+                face.indices[a].vp = mVpMap.getIndex({vert, aiColor3D(0,0,0)});
+            }
 
             if (m->mNormals) {
                 aiVector3D norm = aiMatrix3x3(mat) * m->mNormals[idx];
@@ -419,13 +380,6 @@ void ObjExporter::AddMesh(const aiString& name, const aiMesh* m, const aiMatrix4
                 face.indices[a].vn = 0;
             }
 
-            if ( nullptr != m->mColors[ 0 ] ) {
-                aiColor4D col4 = m->mColors[ 0 ][ idx ];
-                face.indices[ a ].vc = mVcMap.getIndex( col4 );
-            } else {
-                face.indices[ a ].vc = 0;
-            }
-
             if ( m->mTextureCoords[ 0 ] ) {
                 face.indices[a].vt = mVtMap.getIndex(m->mTextureCoords[0][idx]);
             } else {

code/ObjExporter.h

@@ -77,13 +77,12 @@ private:
         FaceVertex()
         : vp()
         , vn()
-        , vt()
+        , vt() {
-        , vc() {
             // empty
         }
 
         // one-based, 0 means: 'does not exist'
-        unsigned int vp, vn, vt, vc;
+        unsigned int vp, vn, vt;
     };
 
     struct Face {
@@ -106,8 +105,37 @@ private:
 private:
     std::string filename;
     const aiScene* const pScene;
-    std::vector<aiVector3D> vp, vn, vt;
+
+    struct vertexData {
+        aiVector3D vp;
+        aiColor3D vc; // OBJ does not support 4D color
+    };
+
+    std::vector<aiVector3D> vn, vt;
     std::vector<aiColor4D> vc;
+    std::vector<vertexData> vp;
+    bool useVc;
+
+    struct vertexDataCompare {
+        bool operator() ( const vertexData& a, const vertexData& b ) const {
+            // position
+            if (a.vp.x < b.vp.x) return true;
+            if (a.vp.x > b.vp.x) return false;
+            if (a.vp.y < b.vp.y) return true;
+            if (a.vp.y > b.vp.y) return false;
+            if (a.vp.z < b.vp.z) return true;
+            if (a.vp.z > b.vp.z) return false;
+
+            // color
+            if (a.vc.r < b.vc.r) return true;
+            if (a.vc.r > b.vc.r) return false;
+            if (a.vc.g < b.vc.g) return true;
+            if (a.vc.g > b.vc.g) return false;
+            if (a.vc.b < b.vc.b) return true;
+            if (a.vc.b > b.vc.b) return false;
+            return false;
+        }
+    };
 
     struct aiVectorCompare { 
         bool operator() (const aiVector3D& a, const aiVector3D& b) const { 
@@ -120,52 +148,37 @@ private:
         }
     };
 
-    struct aiColor4Compare {
+    template <class T, class Compare = std::less<T>>
-        bool operator() ( const aiColor4D& a, const aiColor4D& b ) const {
+    class indexMap {
-            if ( a.r < b.r ) return true;
-            if ( a.r > b.r ) return false;
-            if ( a.g < b.g ) return true;
-            if ( a.g > b.g ) return false;
-            if ( a.b < b.b ) return true;
-            if ( a.b > b.b ) return false;
-            if ( a.a < b.a ) return true;
-            if ( a.a > b.a ) return false;
-            return false;
-        }
-    };
-
-    class vecIndexMap {
         int mNextIndex;
-        typedef std::map<aiVector3D, int, aiVectorCompare> dataType;
+        typedef std::map<T, int, Compare> dataType;
         dataType vecMap;
     
     public:
-        vecIndexMap()
+        indexMap()
         : mNextIndex(1) {
             // empty
         }
 
-        int getIndex(const aiVector3D& vec);
+        int getIndex(const T& key) {
-        void getVectors( std::vector<aiVector3D>& vecs );
+            typename dataType::iterator vertIt = vecMap.find(key);
+            // vertex already exists, so reference it
+            if(vertIt != vecMap.end()){
+                return vertIt->second;
+            }
+            return vecMap[key] = mNextIndex++;
+        };
+
+        void getKeys( std::vector<T>& keys ) {
+            keys.resize(vecMap.size());
+            for(typename dataType::iterator it = vecMap.begin(); it != vecMap.end(); ++it){
+                keys[it->second-1] = it->first;
+            }
+        };
     };
 
-    class colIndexMap {
+    indexMap<aiVector3D, aiVectorCompare> mVnMap, mVtMap;
-        int mNextIndex;
+    indexMap<vertexData, vertexDataCompare> mVpMap;
-        typedef std::map<aiColor4D, int, aiColor4Compare> dataType;
-        dataType colMap;
-
-    public:
-        colIndexMap()
-        : mNextIndex( 1 ) {
-            // empty
-        }
-
-        int getIndex( const aiColor4D& col );
-        void getColors( std::vector<aiColor4D> &colors );
-    };
-
-    vecIndexMap mVpMap, mVnMap, mVtMap;
-    colIndexMap mVcMap;
     std::vector<MeshInstance> mMeshes;
 
     // this endl() doesn't flush() the stream

test/models/OBJ/cube_with_vertexcolors.obj

@@ -1,13 +1,13 @@
 g cube
 
-v  0.0  0.0  0.0  124  110  120
+v  0.0  0.0  0.0  0.48627  0.43137  0.47059
-v  0.0  0.0  1.0  24  0  121
+v  0.0  0.0  1.0  0.09412  0.00000  0.47451
-v  0.0  1.0  0.0  4  0  44
+v  0.0  1.0  0.0  0.01569  0.00000  0.17255
-v  0.0  1.0  1.0  224  0  10
+v  0.0  1.0  1.0  0.87843  0.00000  0.03922
-v  1.0  0.0  0.0  24  200  25
+v  1.0  0.0  0.0  0.09412  0.78431  0.09804
-v  1.0  0.0  1.0  124  10  56
+v  1.0  0.0  1.0  0.48627  0.03922  0.21961
-v  1.0  1.0  0.0  78  10  50
+v  1.0  1.0  0.0  0.30588  0.03922  0.19608
-v  1.0  1.0  1.0  23  0  200
+v  1.0  1.0  1.0  0.09020  0.00000  0.78431
 
 vn 0.0 0.0 1.0
 vn 0.0 0.0 -1.0

test/models/OBJ/cube_with_vertexcolors_uni.obj

@@ -0,0 +1,30 @@
+g cube
+
+v  0.0  0.0  0.0  0.0  0.0  0.0
+v  0.0  0.0  1.0  1.0  0.6  0.3
+v  0.0  1.0  0.0  0.0  0.0  0.0
+v  0.0  1.0  1.0  0.3  0.6  1.0
+v  1.0  0.0  0.0  0.0  0.0  0.0
+v  1.0  0.0  1.0  1.0  0.6  0.3
+v  1.0  1.0  0.0  0.0  0.0  0.0
+v  1.0  1.0  1.0  0.3  0.6  1.0
+
+vn 0.0 0.0 1.0
+vn 0.0 0.0 -1.0
+vn 0.0 1.0 0.0
+vn 0.0 -1.0 0.0
+vn 1.0 0.0 0.0
+vn -1.0 0.0 0.0
+
+f 1//2 7//2 5//2
+f 1//2 3//2 7//2
+f 1//6 4//6 3//6
+f 1//6 2//6 4//6
+f 3//3 8//3 7//3
+f 3//3 4//3 8//3
+f 5//5 7//5 8//5
+f 5//5 8//5 6//5
+f 1//4 5//4 6//4
+f 1//4 6//4 2//4
+f 2//1 6//1 8//1
+f 2//1 8//1 4//1

test/unit/utObjImportExport.cpp

@@ -267,6 +267,28 @@ TEST_F( utObjImportExport, issue809_vertex_color_Test ) {
 #endif // ASSIMP_BUILD_NO_EXPORT
 }
 
+TEST_F( utObjImportExport, issue1923_vertex_color_Test ) {
+    ::Assimp::Importer importer;
+    const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/OBJ/cube_with_vertexcolors_uni.obj", aiProcess_ValidateDataStructure );
+    EXPECT_NE( nullptr, scene );
+
+    scene = importer.GetOrphanedScene();
+
+#ifndef ASSIMP_BUILD_NO_EXPORT
+    ::Assimp::Exporter exporter;
+    const aiExportDataBlob* blob = exporter.ExportToBlob( scene, "obj");
+    EXPECT_NE( nullptr, blob );
+
+    const aiScene *sceneReImport = importer.ReadFileFromMemory( blob->data, blob->size, aiProcess_ValidateDataStructure );
+    EXPECT_NE( nullptr, scene );
+
+    SceneDiffer differ;
+    EXPECT_TRUE( differ.isEqual( scene, sceneReImport ) );
+#endif // ASSIMP_BUILD_NO_EXPORT
+
+    delete scene;
+}
+
 TEST_F( utObjImportExport, issue1453_segfault ) {
     static const std::string ObjModel =
         "v  0.0  0.0  0.0\n"