From 9e484daa65dc7993d39e89fa612f91b59f43bc2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korbinian=20W=C3=BCrl?= Date: Mon, 30 Apr 2018 15:51:48 +0200 Subject: [PATCH 1/4] Unit test for Issue #1923: OBJ Exporter can't correctly export vertex colors --- .../models/OBJ/cube_with_vertexcolors_uni.obj | 30 +++++++++++++++++++ test/unit/utObjImportExport.cpp | 18 +++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/models/OBJ/cube_with_vertexcolors_uni.obj diff --git a/test/models/OBJ/cube_with_vertexcolors_uni.obj b/test/models/OBJ/cube_with_vertexcolors_uni.obj new file mode 100644 index 000000000..c698b9489 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/unit/utObjImportExport.cpp b/test/unit/utObjImportExport.cpp index 8aec9c443..e879c350e 100644 --- a/test/unit/utObjImportExport.cpp +++ b/test/unit/utObjImportExport.cpp @@ -267,6 +267,24 @@ 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 ); + +#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 +} + TEST_F( utObjImportExport, issue1453_segfault ) { static const std::string ObjModel = "v 0.0 0.0 0.0\n" From 9cfdb8d3658f306163d2eb33eaaeb16c5a2eb429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korbinian=20W=C3=BCrl?= Date: Mon, 30 Apr 2018 16:23:29 +0200 Subject: [PATCH 2/4] Fixed test .obj file: OBJ Vertex Colors are expected to be floats 0-1 See https://github.com/cnr-isti-vclab/vcglib/blob/master/wrap/io_trimesh/import_obj.h#L326 --- test/models/OBJ/cube_with_vertexcolors.obj | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/models/OBJ/cube_with_vertexcolors.obj b/test/models/OBJ/cube_with_vertexcolors.obj index d78d3a555..1541e0823 100644 --- a/test/models/OBJ/cube_with_vertexcolors.obj +++ b/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 1.0 24 0 121 -v 0.0 1.0 0.0 4 0 44 -v 0.0 1.0 1.0 224 0 10 -v 1.0 0.0 0.0 24 200 25 -v 1.0 0.0 1.0 124 10 56 -v 1.0 1.0 0.0 78 10 50 -v 1.0 1.0 1.0 23 0 200 +v 0.0 0.0 0.0 0.48627 0.43137 0.47059 +v 0.0 0.0 1.0 0.09412 0.00000 0.47451 +v 0.0 1.0 0.0 0.01569 0.00000 0.17255 +v 0.0 1.0 1.0 0.87843 0.00000 0.03922 +v 1.0 0.0 0.0 0.09412 0.78431 0.09804 +v 1.0 0.0 1.0 0.48627 0.03922 0.21961 +v 1.0 1.0 0.0 0.30588 0.03922 0.19608 +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 From 4b7b692e5e72f70ae001a274ccd78a4ffde35d9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korbinian=20W=C3=BCrl?= Date: Mon, 30 Apr 2018 16:27:34 +0200 Subject: [PATCH 3/4] Fix Issue #1923: OBJ Exporter can't correctly export vertex colors The indexMap for vertices now uses a combined vp + vc index --- code/ObjExporter.cpp | 90 ++++++++++------------------------------ code/ObjExporter.h | 99 +++++++++++++++++++++++++------------------- 2 files changed, 78 insertions(+), 111 deletions(-) diff --git a/code/ObjExporter.cpp b/code/ObjExporter.cpp index 6cd69f402..1542efebf 100644 --- a/code/ObjExporter.cpp +++ b/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() -, mVpMap() +, vp() +, 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 ); - mVcMap.getColors( vc ); - if ( vc.empty() ) { + mVpMap.getKeys( vp ); + if ( !useVc ) { mOutput << "# " << vp.size() << " vertex positions" << endl; - for ( const aiVector3D& v : vp ) { - mOutput << "v " << v.x << " " << v.y << " " << v.z << endl; + for ( const vertexData& v : vp ) { + 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 aiVector3D& v : vp ) { - if ( colIdx < vc.size() ) { - mOutput << "v " << v.x << " " << v.y << " " << v.z << " " << vc[ colIdx ].r << " " << vc[ colIdx ].g << " " << vc[ colIdx ].b << endl; - } - ++colIdx; + for ( const vertexData& v : vp ) { + mOutput << "v " << v.vp.x << " " << v.vp.y << " " << v.vp.z << " " << v.vc.r << " " << v.vc.g << " " << v.vc.b << endl; } } 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& 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 &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 { diff --git a/code/ObjExporter.h b/code/ObjExporter.h index 7920598d0..bd745b593 100644 --- a/code/ObjExporter.h +++ b/code/ObjExporter.h @@ -77,13 +77,12 @@ private: FaceVertex() : vp() , vn() - , vt() - , vc() { + , vt() { // empty } // one-based, 0 means: 'does not exist' - unsigned int vp, vn, vt, vc; + unsigned int vp, vn, vt; }; struct Face { @@ -106,66 +105,80 @@ private: private: std::string filename; const aiScene* const pScene; - std::vector vp, vn, vt; + + struct vertexData { + aiVector3D vp; + aiColor3D vc; // OBJ does not support 4D color + }; + + std::vector vn, vt; std::vector vc; + std::vector vp; + bool useVc; - struct aiVectorCompare { - bool operator() (const aiVector3D& a, const aiVector3D& b) const { - if(a.x < b.x) return true; - if(a.x > b.x) return false; - if(a.y < b.y) return true; - if(a.y > b.y) return false; - if(a.z < b.z) return true; + 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 aiColor4Compare { - bool operator() ( const aiColor4D& a, const aiColor4D& b ) const { - 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; + struct aiVectorCompare { + bool operator() (const aiVector3D& a, const aiVector3D& b) const { + if(a.x < b.x) return true; + if(a.x > b.x) return false; + if(a.y < b.y) return true; + if(a.y > b.y) return false; + if(a.z < b.z) return true; return false; } }; - class vecIndexMap { + template > + class indexMap { int mNextIndex; - typedef std::map dataType; + typedef std::map dataType; dataType vecMap; public: - vecIndexMap() + indexMap() : mNextIndex(1) { // empty } - int getIndex(const aiVector3D& vec); - void getVectors( std::vector& vecs ); + int getIndex(const T& key) { + 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& keys ) { + keys.resize(vecMap.size()); + for(typename dataType::iterator it = vecMap.begin(); it != vecMap.end(); ++it){ + keys[it->second-1] = it->first; + } + }; }; - class colIndexMap { - int mNextIndex; - typedef std::map dataType; - dataType colMap; - - public: - colIndexMap() - : mNextIndex( 1 ) { - // empty - } - - int getIndex( const aiColor4D& col ); - void getColors( std::vector &colors ); - }; - - vecIndexMap mVpMap, mVnMap, mVtMap; - colIndexMap mVcMap; + indexMap mVnMap, mVtMap; + indexMap mVpMap; std::vector mMeshes; // this endl() doesn't flush() the stream From 22475130649c3d356c507a7a420ce176d0b45d38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Korbinian=20W=C3=BCrl?= Date: Tue, 1 May 2018 11:23:38 +0200 Subject: [PATCH 4/4] Fix for TravisCI --- test/unit/utObjImportExport.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/unit/utObjImportExport.cpp b/test/unit/utObjImportExport.cpp index e879c350e..5a10ae17b 100644 --- a/test/unit/utObjImportExport.cpp +++ b/test/unit/utObjImportExport.cpp @@ -272,6 +272,8 @@ TEST_F( utObjImportExport, issue1923_vertex_color_Test ) { 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"); @@ -283,6 +285,8 @@ TEST_F( utObjImportExport, issue1923_vertex_color_Test ) { SceneDiffer differ; EXPECT_TRUE( differ.isEqual( scene, sceneReImport ) ); #endif // ASSIMP_BUILD_NO_EXPORT + + delete scene; } TEST_F( utObjImportExport, issue1453_segfault ) {