From 30e06f7437d3da72b38e452283e163890215fec4 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Thu, 9 Nov 2017 21:31:16 +0100 Subject: [PATCH 1/3] closes https://github.com/assimp/assimp/issues/216: check the area of a triangle to check if its degenerated or not. --- code/FBXBinaryTokenizer.cpp | 1 - code/FindDegenerates.cpp | 101 ++++++++++++++++++++++++------------ code/STLLoader.cpp | 6 --- code/XFileImporter.h | 5 +- 4 files changed, 68 insertions(+), 45 deletions(-) diff --git a/code/FBXBinaryTokenizer.cpp b/code/FBXBinaryTokenizer.cpp index 9ae38386b..519f2e176 100644 --- a/code/FBXBinaryTokenizer.cpp +++ b/code/FBXBinaryTokenizer.cpp @@ -422,7 +422,6 @@ bool ReadScope(TokenList& output_tokens, const char* input, const char*& cursor, return true; } - } // ------------------------------------------------------------------------------------------------ diff --git a/code/FindDegenerates.cpp b/code/FindDegenerates.cpp index 32a09f0c0..5b321312a 100644 --- a/code/FindDegenerates.cpp +++ b/code/FindDegenerates.cpp @@ -56,98 +56,131 @@ using namespace Assimp; // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer FindDegeneratesProcess::FindDegeneratesProcess() -: configRemoveDegenerates (false) -{} +: configRemoveDegenerates (false) { + // empty +} // ------------------------------------------------------------------------------------------------ // Destructor, private as well -FindDegeneratesProcess::~FindDegeneratesProcess() -{ +FindDegeneratesProcess::~FindDegeneratesProcess() { // nothing to do here } // ------------------------------------------------------------------------------------------------ // Returns whether the processing step is present in the given flag field. -bool FindDegeneratesProcess::IsActive( unsigned int pFlags) const -{ +bool FindDegeneratesProcess::IsActive( unsigned int pFlags) const { return 0 != (pFlags & aiProcess_FindDegenerates); } // ------------------------------------------------------------------------------------------------ // Setup import configuration -void FindDegeneratesProcess::SetupProperties(const Importer* pImp) -{ +void FindDegeneratesProcess::SetupProperties(const Importer* pImp) { // Get the current value of AI_CONFIG_PP_FD_REMOVE configRemoveDegenerates = (0 != pImp->GetPropertyInteger(AI_CONFIG_PP_FD_REMOVE,0)); } // ------------------------------------------------------------------------------------------------ // Executes the post processing step on the given imported data. -void FindDegeneratesProcess::Execute( aiScene* pScene) -{ +void FindDegeneratesProcess::Execute( aiScene* pScene) { DefaultLogger::get()->debug("FindDegeneratesProcess begin"); for (unsigned int i = 0; i < pScene->mNumMeshes;++i){ - ExecuteOnMesh( pScene->mMeshes[i]); + ExecuteOnMesh( pScene->mMeshes[ i ] ); } DefaultLogger::get()->debug("FindDegeneratesProcess finished"); } +static ai_real heron( ai_real a, ai_real b, ai_real c ) { + ai_real s = (a + b + c) / 2; + ai_real area = pow((s * ( s - a ) * ( s - b ) * ( s - c ) ), 0.5 ); + return area; +} + +static ai_real distance3D( const aiVector3D &vA, aiVector3D &vB ) { + const ai_real lx = ( vB.x - vA.x ); + const ai_real ly = ( vB.y - vA.y ); + const ai_real lz = ( vB.z - vA.z ); + ai_real a = lx*lx + ly*ly + lz*lz; + ai_real d = pow( a, 0.5 ); + + return d; +} + +static ai_real calculateAreaOfTriangle( const aiFace& face, aiMesh* mesh ) { + ai_real area = 0; + + aiVector3D vA( mesh->mVertices[ face.mIndices[ 0 ] ] ); + aiVector3D vB( mesh->mVertices[ face.mIndices[ 1 ] ] ); + aiVector3D vC( mesh->mVertices[ face.mIndices[ 2 ] ] ); + + ai_real a( distance3D( vA, vB ) ); + ai_real b( distance3D( vB, vC ) ); + ai_real c( distance3D( vC, vA ) ); + area = heron( a, b, c ); + + return area; +} + // ------------------------------------------------------------------------------------------------ // Executes the post processing step on the given imported mesh -void FindDegeneratesProcess::ExecuteOnMesh( aiMesh* mesh) -{ +void FindDegeneratesProcess::ExecuteOnMesh( aiMesh* mesh) { mesh->mPrimitiveTypes = 0; std::vector remove_me; - if (configRemoveDegenerates) - remove_me.resize(mesh->mNumFaces,false); + if (configRemoveDegenerates) { + remove_me.resize( mesh->mNumFaces, false ); + } unsigned int deg = 0, limit; - for (unsigned int a = 0; a < mesh->mNumFaces; ++a) - { + for ( unsigned int a = 0; a < mesh->mNumFaces; ++a ) { aiFace& face = mesh->mFaces[a]; bool first = true; // check whether the face contains degenerated entries - for (unsigned int i = 0; i < face.mNumIndices; ++i) - { + for (unsigned int i = 0; i < face.mNumIndices; ++i) { // Polygons with more than 4 points are allowed to have double points, that is // simulating polygons with holes just with concave polygons. However, // double points may not come directly after another. limit = face.mNumIndices; - if (face.mNumIndices > 4) - limit = std::min(limit,i+2); + if (face.mNumIndices > 4) { + limit = std::min( limit, i+2 ); + } - for (unsigned int t = i+1; t < limit; ++t) - { - if (mesh->mVertices[face.mIndices[i]] == mesh->mVertices[face.mIndices[t]]) - { + for (unsigned int t = i+1; t < limit; ++t) { + if (mesh->mVertices[face.mIndices[ i ] ] == mesh->mVertices[ face.mIndices[ t ] ]) { // we have found a matching vertex position // remove the corresponding index from the array - --face.mNumIndices;--limit; - for (unsigned int m = t; m < face.mNumIndices; ++m) - { - face.mIndices[m] = face.mIndices[m+1]; + --face.mNumIndices; + --limit; + for (unsigned int m = t; m < face.mNumIndices; ++m) { + face.mIndices[ m ] = face.mIndices[ m+1 ]; } --t; // NOTE: we set the removed vertex index to an unique value // to make sure the developer gets notified when his // application attemps to access this data. - face.mIndices[face.mNumIndices] = 0xdeadbeef; + face.mIndices[ face.mNumIndices ] = 0xdeadbeef; - if(first) - { + if(first) { ++deg; first = false; } - if (configRemoveDegenerates) { - remove_me[a] = true; + if ( configRemoveDegenerates ) { + remove_me[ a ] = true; goto evil_jump_outside; // hrhrhrh ... yeah, this rocks baby! } } } + ai_real area = calculateAreaOfTriangle( face, mesh ); + if ( area < 1e-6 ) { + if ( configRemoveDegenerates ) { + remove_me[ a ] = true; + goto evil_jump_outside; + } + + // todo: check for index which is corrupt. + } } // We need to update the primitive flags array of the mesh. diff --git a/code/STLLoader.cpp b/code/STLLoader.cpp index 600c4275d..41f8aad03 100644 --- a/code/STLLoader.cpp +++ b/code/STLLoader.cpp @@ -207,12 +207,6 @@ void STLImporter::InternReadFile( const std::string& pFile, aiScene* pScene, IOS throw DeadlyImportError( "Failed to determine STL storage representation for " + pFile + "."); } - // add all created meshes to the single node - /*pScene->mRootNode->mNumMeshes = pScene->mNumMeshes; - pScene->mRootNode->mMeshes = new unsigned int[pScene->mNumMeshes]; - for (unsigned int i = 0; i < pScene->mNumMeshes; i++) - pScene->mRootNode->mMeshes[i] = i; - */ // create a single default material, using a white diffuse color for consistency with // other geometric types (e.g., PLY). aiMaterial* pcMat = new aiMaterial(); diff --git a/code/XFileImporter.h b/code/XFileImporter.h index 528dcb851..0fa6b52eb 100644 --- a/code/XFileImporter.h +++ b/code/XFileImporter.h @@ -65,14 +65,11 @@ struct Node; /** The XFileImporter is a worker class capable of importing a scene from a * DirectX file .x */ -class XFileImporter : public BaseImporter -{ +class XFileImporter : public BaseImporter { public: XFileImporter(); ~XFileImporter(); - -public: // ------------------------------------------------------------------- /** Returns whether the class can handle the format of the given file. * See BaseImporter::CanRead() for details. */ From ad2ff9fd713e9bf7a21bf552bec4fdd6e5cee9db Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Thu, 9 Nov 2017 21:40:10 +0100 Subject: [PATCH 2/3] check for area test if the face is a triangle. --- code/FindDegenerates.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/code/FindDegenerates.cpp b/code/FindDegenerates.cpp index 5b321312a..83dce22ba 100644 --- a/code/FindDegenerates.cpp +++ b/code/FindDegenerates.cpp @@ -172,14 +172,16 @@ void FindDegeneratesProcess::ExecuteOnMesh( aiMesh* mesh) { } } } - ai_real area = calculateAreaOfTriangle( face, mesh ); - if ( area < 1e-6 ) { - if ( configRemoveDegenerates ) { - remove_me[ a ] = true; - goto evil_jump_outside; - } + if ( face.mNumIndices == 3 ) { + ai_real area = calculateAreaOfTriangle( face, mesh ); + if ( area < 1e-6 ) { + if ( configRemoveDegenerates ) { + remove_me[ a ] = true; + goto evil_jump_outside; + } - // todo: check for index which is corrupt. + // todo: check for index which is corrupt. + } } } From 24b728b3ea20d795ad3299cc7884392b514fa69b Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sun, 12 Nov 2017 22:09:18 +0100 Subject: [PATCH 3/3] FindDegeneratives: adapt unittests and add configs --- code/FindDegenerates.cpp | 31 +++++++++-------- code/FindDegenerates.h | 59 ++++++++++++++++++++++----------- code/X3DExporter.cpp | 8 +++-- include/assimp/config.h.in | 9 +++++ include/assimp/vector3.h | 9 +++-- test/unit/utFindDegenerates.cpp | 23 +++++++------ 6 files changed, 89 insertions(+), 50 deletions(-) diff --git a/code/FindDegenerates.cpp b/code/FindDegenerates.cpp index 83dce22ba..5a4a132ef 100644 --- a/code/FindDegenerates.cpp +++ b/code/FindDegenerates.cpp @@ -56,7 +56,8 @@ using namespace Assimp; // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer FindDegeneratesProcess::FindDegeneratesProcess() -: configRemoveDegenerates (false) { +: mConfigRemoveDegenerates( false ) +, mConfigCheckAreaOfTriangle( false ){ // empty } @@ -76,7 +77,8 @@ bool FindDegeneratesProcess::IsActive( unsigned int pFlags) const { // Setup import configuration void FindDegeneratesProcess::SetupProperties(const Importer* pImp) { // Get the current value of AI_CONFIG_PP_FD_REMOVE - configRemoveDegenerates = (0 != pImp->GetPropertyInteger(AI_CONFIG_PP_FD_REMOVE,0)); + mConfigRemoveDegenerates = (0 != pImp->GetPropertyInteger(AI_CONFIG_PP_FD_REMOVE,0)); + mConfigCheckAreaOfTriangle = ( 0 != pImp->GetPropertyInteger(AI_CONFIG_PP_FD_CHECKAREA) ); } // ------------------------------------------------------------------------------------------------ @@ -126,7 +128,7 @@ void FindDegeneratesProcess::ExecuteOnMesh( aiMesh* mesh) { mesh->mPrimitiveTypes = 0; std::vector remove_me; - if (configRemoveDegenerates) { + if (mConfigRemoveDegenerates) { remove_me.resize( mesh->mNumFaces, false ); } @@ -166,21 +168,24 @@ void FindDegeneratesProcess::ExecuteOnMesh( aiMesh* mesh) { first = false; } - if ( configRemoveDegenerates ) { + if ( mConfigRemoveDegenerates ) { remove_me[ a ] = true; goto evil_jump_outside; // hrhrhrh ... yeah, this rocks baby! } } } - if ( face.mNumIndices == 3 ) { - ai_real area = calculateAreaOfTriangle( face, mesh ); - if ( area < 1e-6 ) { - if ( configRemoveDegenerates ) { - remove_me[ a ] = true; - goto evil_jump_outside; - } - // todo: check for index which is corrupt. + if ( mConfigCheckAreaOfTriangle ) { + if ( face.mNumIndices == 3 ) { + ai_real area = calculateAreaOfTriangle( face, mesh ); + if ( area < 1e-6 ) { + if ( mConfigRemoveDegenerates ) { + remove_me[ a ] = true; + goto evil_jump_outside; + } + + // todo: check for index which is corrupt. + } } } } @@ -206,7 +211,7 @@ evil_jump_outside: } // If AI_CONFIG_PP_FD_REMOVE is true, remove degenerated faces from the import - if (configRemoveDegenerates && deg) { + if (mConfigRemoveDegenerates && deg) { unsigned int n = 0; for (unsigned int a = 0; a < mesh->mNumFaces; ++a) { diff --git a/code/FindDegenerates.h b/code/FindDegenerates.h index 9bd410dcd..cf03a24bc 100644 --- a/code/FindDegenerates.h +++ b/code/FindDegenerates.h @@ -54,15 +54,11 @@ namespace Assimp { // --------------------------------------------------------------------------- /** FindDegeneratesProcess: Searches a mesh for degenerated triangles. */ -class ASSIMP_API FindDegeneratesProcess : public BaseProcess -{ +class ASSIMP_API FindDegeneratesProcess : public BaseProcess { public: - FindDegeneratesProcess(); ~FindDegeneratesProcess(); -public: - // ------------------------------------------------------------------- // Check whether step is active bool IsActive( unsigned int pFlags) const; @@ -79,28 +75,53 @@ public: // Execute step on a given mesh void ExecuteOnMesh( aiMesh* mesh); + // ------------------------------------------------------------------- + /// @brief Enable the instant removal of degenerated primitives + /// @param enabled true for enabled. + void EnableInstantRemoval(bool enabled); // ------------------------------------------------------------------- - /** @brief Enable the instant removal of degenerated primitives - * @param d hm ... difficult to guess what this means, hu!? - */ - void EnableInstantRemoval(bool d) { - configRemoveDegenerates = d; - } + /// @brief Check whether instant removal is currently enabled + /// @return The instant removal state. + bool IsInstantRemoval() const; // ------------------------------------------------------------------- - /** @brief Check whether instant removal is currently enabled - * @return ... - */ - bool IsInstantRemoval() const { - return configRemoveDegenerates; - } + /// @brief Enable the area check for triangles. + /// @param enabled true for enabled. + void EnableAreaCheck( bool enabled ); + + // ------------------------------------------------------------------- + /// @brief Check whether the area check is enabled. + /// @return The area check state. + bool isAreaCheckEnabled() const; private: - //! Configuration option: remove degenerates faces immediately - bool configRemoveDegenerates; + bool mConfigRemoveDegenerates; + //! Configuration option: check for area + bool mConfigCheckAreaOfTriangle; }; + +inline +void FindDegeneratesProcess::EnableInstantRemoval(bool enabled) { + mConfigRemoveDegenerates = enabled; } +inline +bool FindDegeneratesProcess::IsInstantRemoval() const { + return mConfigRemoveDegenerates; +} + +inline +void FindDegeneratesProcess::EnableAreaCheck( bool enabled ) { + mConfigCheckAreaOfTriangle = enabled; +} + +inline +bool FindDegeneratesProcess::isAreaCheckEnabled() const { + return mConfigCheckAreaOfTriangle; +} + +} // Namespace Assimp + #endif // !! AI_FINDDEGENERATESPROCESS_H_INC diff --git a/code/X3DExporter.cpp b/code/X3DExporter.cpp index df688fd79..b560855fb 100644 --- a/code/X3DExporter.cpp +++ b/code/X3DExporter.cpp @@ -240,8 +240,12 @@ list attr_list; if((rotate_angle != 0) && (rotate_axis.Length() > 0)) attr_list.push_back({"rotation", Rotation2String(rotate_axis, rotate_angle)}); - if(!scale.Equal({1, 1, 1})) attr_list.push_back({"scale", Vector2String(scale)}); - if(translate.Length() > 0) attr_list.push_back({"translation", Vector2String(translate)}); + if(!scale.Equal({1.0,1.0,1.0})) { + attr_list.push_back({"scale", Vector2String(scale)}); + } + if(translate.Length() > 0) { + attr_list.push_back({"translation", Vector2String(translate)}); + } } // Begin node if need. diff --git a/include/assimp/config.h.in b/include/assimp/config.h.in index bafd96189..1dabae7cf 100644 --- a/include/assimp/config.h.in +++ b/include/assimp/config.h.in @@ -266,6 +266,15 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #define AI_CONFIG_PP_FD_REMOVE \ "PP_FD_REMOVE" +// --------------------------------------------------------------------------- +/** + * @brief Configures the #aiProcess_FindDegenerates to check the area of a + * trinagle to be greates than e-6. If this is not the case the triangle will + * be removed if #AI_CONFIG_PP_FD_REMOVE is set to true. + */ +#define AI_CONFIG_PP_FD_CHECKAREA \ + "PP_FD_CHECKAREA" + // --------------------------------------------------------------------------- /** @brief Configures the #aiProcess_OptimizeGraph step to preserve nodes * matching a name in a given list. diff --git a/include/assimp/vector3.h b/include/assimp/vector3.h index 946e36cc5..641dab795 100644 --- a/include/assimp/vector3.h +++ b/include/assimp/vector3.h @@ -65,11 +65,10 @@ template class aiVector3t { public: - - aiVector3t () : x(), y(), z() {} - aiVector3t (TReal _x, TReal _y, TReal _z) : x(_x), y(_y), z(_z) {} - explicit aiVector3t (TReal _xyz) : x(_xyz), y(_xyz), z(_xyz) {} - aiVector3t (const aiVector3t& o) : x(o.x), y(o.y), z(o.z) {} + aiVector3t() : x(), y(), z() {} + aiVector3t(TReal _x, TReal _y, TReal _z) : x(_x), y(_y), z(_z) {} + explicit aiVector3t (TReal _xyz ) : x(_xyz), y(_xyz), z(_xyz) {} + aiVector3t( const aiVector3t& o ) : x(o.x), y(o.y), z(o.z) {} public: diff --git a/test/unit/utFindDegenerates.cpp b/test/unit/utFindDegenerates.cpp index 8040a83a1..dce42732c 100644 --- a/test/unit/utFindDegenerates.cpp +++ b/test/unit/utFindDegenerates.cpp @@ -58,8 +58,7 @@ protected: }; // ------------------------------------------------------------------------------------------------ -void FindDegeneratesProcessTest::SetUp() -{ +void FindDegeneratesProcessTest::SetUp() { mesh = new aiMesh(); process = new FindDegeneratesProcess(); @@ -107,16 +106,12 @@ void FindDegeneratesProcessTest::SetUp() mesh->mNumUVComponents[1] = numFaces; } -// ------------------------------------------------------------------------------------------------ -void FindDegeneratesProcessTest::TearDown() -{ +void FindDegeneratesProcessTest::TearDown() { delete mesh; delete process; } -// ------------------------------------------------------------------------------------------------ -TEST_F(FindDegeneratesProcessTest, testDegeneratesDetection) -{ +TEST_F(FindDegeneratesProcessTest, testDegeneratesDetection) { process->EnableInstantRemoval(false); process->ExecuteOnMesh(mesh); @@ -135,12 +130,18 @@ TEST_F(FindDegeneratesProcessTest, testDegeneratesDetection) mesh->mPrimitiveTypes); } -// ------------------------------------------------------------------------------------------------ -TEST_F(FindDegeneratesProcessTest, testDegeneratesRemoval) -{ +TEST_F(FindDegeneratesProcessTest, testDegeneratesRemoval) { + process->EnableAreaCheck(false); process->EnableInstantRemoval(true); process->ExecuteOnMesh(mesh); EXPECT_EQ(mesh->mNumUVComponents[1], mesh->mNumFaces); } +TEST_F(FindDegeneratesProcessTest, testDegeneratesRemovalWithAreaCheck) { + process->EnableAreaCheck(true); + process->EnableInstantRemoval(true); + process->ExecuteOnMesh(mesh); + + EXPECT_EQ(mesh->mNumUVComponents[1]-100, mesh->mNumFaces); +}