From 3deae8760c6ea327be35851f51393a8d57c82c69 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Mon, 2 Nov 2020 14:43:35 +0000 Subject: [PATCH 1/5] Optimize FindDegenerates so it doesn't explode --- code/PostProcessing/FindDegenerates.cpp | 69 +++++++++++++------------ test/unit/utFindDegenerates.cpp | 59 +++++++++++++++++++++ 2 files changed, 94 insertions(+), 34 deletions(-) diff --git a/code/PostProcessing/FindDegenerates.cpp b/code/PostProcessing/FindDegenerates.cpp index e5defdeb0..db275540d 100644 --- a/code/PostProcessing/FindDegenerates.cpp +++ b/code/PostProcessing/FindDegenerates.cpp @@ -52,12 +52,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "FindDegenerates.h" #include +#include + using namespace Assimp; -//remove mesh at position 'index' from the scene -static void removeMesh(aiScene* pScene, unsigned const index); //correct node indices to meshes and remove references to deleted mesh -static void updateSceneGraph(aiNode* pNode, unsigned const index); +static void updateSceneGraph(aiNode* pNode, const std::unordered_map& meshMap); // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer @@ -91,50 +91,51 @@ void FindDegeneratesProcess::SetupProperties(const Importer* pImp) { // Executes the post processing step on the given imported data. void FindDegeneratesProcess::Execute( aiScene* pScene) { ASSIMP_LOG_DEBUG("FindDegeneratesProcess begin"); + + std::unordered_map meshMap; + meshMap.reserve(pScene->mNumMeshes); + + const unsigned int originalNumMeshes = pScene->mNumMeshes; + unsigned int targetIndex = 0; for (unsigned int i = 0; i < pScene->mNumMeshes;++i) { - //Do not process point cloud, ExecuteOnMesh works only with faces data + // Do not process point cloud, ExecuteOnMesh works only with faces data if ((pScene->mMeshes[i]->mPrimitiveTypes != aiPrimitiveType::aiPrimitiveType_POINT) && ExecuteOnMesh(pScene->mMeshes[i])) { - removeMesh(pScene, i); - --i; //the current i is removed, do not skip the next one + delete pScene->mMeshes[i]; + // Not strictly required, but clean: + pScene->mMeshes[i] = nullptr; + } + else { + meshMap[i] = targetIndex; + pScene->mMeshes[targetIndex] = pScene->mMeshes[i]; + ++targetIndex; } } + pScene->mNumMeshes = targetIndex; + + if (meshMap.size() < originalNumMeshes) + { + updateSceneGraph(pScene->mRootNode, meshMap); + } + ASSIMP_LOG_DEBUG("FindDegeneratesProcess finished"); } -static void removeMesh(aiScene* pScene, unsigned const index) { - //we start at index and copy the pointers one position forward - //save the mesh pointer to delete it later - auto delete_me = pScene->mMeshes[index]; - for (unsigned i = index; i < pScene->mNumMeshes - 1; ++i) { - pScene->mMeshes[i] = pScene->mMeshes[i+1]; - } - pScene->mMeshes[pScene->mNumMeshes - 1] = nullptr; - --(pScene->mNumMeshes); - delete delete_me; - - //removing a mesh also requires updating all references to it in the scene graph - updateSceneGraph(pScene->mRootNode, index); -} - -static void updateSceneGraph(aiNode* pNode, unsigned const index) { +static void updateSceneGraph(aiNode* pNode, const std::unordered_map& meshMap) { + unsigned int targetIndex = 0; for (unsigned i = 0; i < pNode->mNumMeshes; ++i) { - if (pNode->mMeshes[i] > index) { - --(pNode->mMeshes[i]); - continue; - } - if (pNode->mMeshes[i] == index) { - for (unsigned j = i; j < pNode->mNumMeshes -1; ++j) { - pNode->mMeshes[j] = pNode->mMeshes[j+1]; - } - --(pNode->mNumMeshes); - --i; - continue; + const unsigned int sourceMeshIndex = pNode->mMeshes[i]; + auto it = meshMap.find(sourceMeshIndex); + if (it != meshMap.end()) + { + pNode->mMeshes[targetIndex] = it->second; + ++targetIndex; } } + pNode->mNumMeshes = targetIndex; //recurse to all children for (unsigned i = 0; i < pNode->mNumChildren; ++i) { - updateSceneGraph(pNode->mChildren[i], index); + updateSceneGraph(pNode->mChildren[i], meshMap); } } diff --git a/test/unit/utFindDegenerates.cpp b/test/unit/utFindDegenerates.cpp index 88ad9640a..31f1abb69 100644 --- a/test/unit/utFindDegenerates.cpp +++ b/test/unit/utFindDegenerates.cpp @@ -40,6 +40,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ #include "UnitTestPCH.h" +#include "../../include/assimp/scene.h" #include "PostProcessing/FindDegenerates.h" using namespace std; @@ -147,3 +148,61 @@ TEST_F(FindDegeneratesProcessTest, testDegeneratesRemovalWithAreaCheck) { EXPECT_EQ(mMesh->mNumUVComponents[1] - 100, mMesh->mNumFaces); } + +namespace +{ + std::unique_ptr getDegenerateMesh() + { + std::unique_ptr mesh = std::make_unique(); + mesh->mNumVertices = 2; + mesh->mVertices = new aiVector3D[2]; + mesh->mVertices[0] = aiVector3D{ 0.0f, 0.0f, 0.0f }; + mesh->mVertices[1] = aiVector3D{ 1.0f, 0.0f, 0.0f }; + mesh->mNumFaces = 1; + mesh->mFaces = new aiFace[1]; + mesh->mFaces[0].mNumIndices = 3; + mesh->mFaces[0].mIndices = new unsigned int[3]; + mesh->mFaces[0].mIndices[0] = 0; + mesh->mFaces[0].mIndices[1] = 1; + mesh->mFaces[0].mIndices[2] = 0; + return mesh; + } +} + +TEST_F(FindDegeneratesProcessTest, meshRemoval) { + mProcess->EnableAreaCheck(true); + mProcess->EnableInstantRemoval(true); + mProcess->ExecuteOnMesh(mMesh); + + std::unique_ptr scene = std::make_unique(); + scene->mNumMeshes = 5; + scene->mMeshes = new aiMesh*[5]; + + /// Use the mesh which doesn't get completely stripped of faces from the main test. + aiMesh* meshWhichSurvives = mMesh; + mMesh = nullptr; + + scene->mMeshes[0] = getDegenerateMesh().release(); + scene->mMeshes[1] = getDegenerateMesh().release(); + scene->mMeshes[2] = meshWhichSurvives; + scene->mMeshes[3] = getDegenerateMesh().release(); + scene->mMeshes[4] = getDegenerateMesh().release(); + + scene->mRootNode = new aiNode; + scene->mRootNode->mNumMeshes = 5; + scene->mRootNode->mMeshes = new unsigned int[5]; + scene->mRootNode->mMeshes[0] = 0; + scene->mRootNode->mMeshes[1] = 1; + scene->mRootNode->mMeshes[2] = 2; + scene->mRootNode->mMeshes[3] = 3; + scene->mRootNode->mMeshes[4] = 4; + + mProcess->Execute(scene.get()); + + EXPECT_EQ(scene->mNumMeshes, 1); + EXPECT_EQ(scene->mMeshes[0], meshWhichSurvives); + EXPECT_EQ(scene->mRootNode->mNumMeshes, 1); + EXPECT_EQ(scene->mRootNode->mMeshes[0], 0); + + EXPECT_EQ(mMesh->mNumUVComponents[1] - 100, mMesh->mNumFaces); +} From 75570307d65f5ec86b69987b278c99840dcb3a9e Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Mon, 2 Nov 2020 14:50:20 +0000 Subject: [PATCH 2/5] Remove unneeded line --- test/unit/utFindDegenerates.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/utFindDegenerates.cpp b/test/unit/utFindDegenerates.cpp index 31f1abb69..9ad9a275d 100644 --- a/test/unit/utFindDegenerates.cpp +++ b/test/unit/utFindDegenerates.cpp @@ -203,6 +203,4 @@ TEST_F(FindDegeneratesProcessTest, meshRemoval) { EXPECT_EQ(scene->mMeshes[0], meshWhichSurvives); EXPECT_EQ(scene->mRootNode->mNumMeshes, 1); EXPECT_EQ(scene->mRootNode->mMeshes[0], 0); - - EXPECT_EQ(mMesh->mNumUVComponents[1] - 100, mMesh->mNumFaces); } From a68f78ab94c6738ba94174164c38edb3dfdf8387 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Mon, 2 Nov 2020 15:03:17 +0000 Subject: [PATCH 3/5] C++11 --- test/unit/utFindDegenerates.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/utFindDegenerates.cpp b/test/unit/utFindDegenerates.cpp index 9ad9a275d..280a5199d 100644 --- a/test/unit/utFindDegenerates.cpp +++ b/test/unit/utFindDegenerates.cpp @@ -43,6 +43,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "../../include/assimp/scene.h" #include "PostProcessing/FindDegenerates.h" +#include + using namespace std; using namespace Assimp; @@ -153,7 +155,7 @@ namespace { std::unique_ptr getDegenerateMesh() { - std::unique_ptr mesh = std::make_unique(); + std::unique_ptr mesh(new aiMesh); mesh->mNumVertices = 2; mesh->mVertices = new aiVector3D[2]; mesh->mVertices[0] = aiVector3D{ 0.0f, 0.0f, 0.0f }; @@ -174,7 +176,7 @@ TEST_F(FindDegeneratesProcessTest, meshRemoval) { mProcess->EnableInstantRemoval(true); mProcess->ExecuteOnMesh(mMesh); - std::unique_ptr scene = std::make_unique(); + std::unique_ptr scene(new aiScene); scene->mNumMeshes = 5; scene->mMeshes = new aiMesh*[5]; From 0d5e5790cb8bc2bcba15edefd9a5cf23b3d0c347 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sat, 14 Nov 2020 12:44:49 +0100 Subject: [PATCH 4/5] Fix findings. --- code/PostProcessing/FindDegenerates.cpp | 31 +++++++++++-------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/code/PostProcessing/FindDegenerates.cpp b/code/PostProcessing/FindDegenerates.cpp index db275540d..2137f40c2 100644 --- a/code/PostProcessing/FindDegenerates.cpp +++ b/code/PostProcessing/FindDegenerates.cpp @@ -5,8 +5,6 @@ Open Asset Import Library (assimp) Copyright (c) 2006-2020, assimp team - - All rights reserved. Redistribution and use of this software in source and binary forms, @@ -45,25 +43,23 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * @brief Implementation of the FindDegenerates post-process step. */ - - -// internal headers #include "ProcessHelper.h" #include "FindDegenerates.h" + #include #include using namespace Assimp; -//correct node indices to meshes and remove references to deleted mesh +// Correct node indices to meshes and remove references to deleted mesh static void updateSceneGraph(aiNode* pNode, const std::unordered_map& meshMap); // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer -FindDegeneratesProcess::FindDegeneratesProcess() -: mConfigRemoveDegenerates( false ) -, mConfigCheckAreaOfTriangle( false ){ +FindDegeneratesProcess::FindDegeneratesProcess() : + mConfigRemoveDegenerates( false ), + mConfigCheckAreaOfTriangle( false ){ // empty } @@ -91,21 +87,22 @@ void FindDegeneratesProcess::SetupProperties(const Importer* pImp) { // Executes the post processing step on the given imported data. void FindDegeneratesProcess::Execute( aiScene* pScene) { ASSIMP_LOG_DEBUG("FindDegeneratesProcess begin"); - + if ( null == pScene) { + return; + } + std::unordered_map meshMap; meshMap.reserve(pScene->mNumMeshes); const unsigned int originalNumMeshes = pScene->mNumMeshes; unsigned int targetIndex = 0; - for (unsigned int i = 0; i < pScene->mNumMeshes;++i) - { + for (unsigned int i = 0; i < pScene->mNumMeshes; ++i) { // Do not process point cloud, ExecuteOnMesh works only with faces data if ((pScene->mMeshes[i]->mPrimitiveTypes != aiPrimitiveType::aiPrimitiveType_POINT) && ExecuteOnMesh(pScene->mMeshes[i])) { delete pScene->mMeshes[i]; // Not strictly required, but clean: pScene->mMeshes[i] = nullptr; - } - else { + } else { meshMap[i] = targetIndex; pScene->mMeshes[targetIndex] = pScene->mMeshes[i]; ++targetIndex; @@ -113,8 +110,7 @@ void FindDegeneratesProcess::Execute( aiScene* pScene) { } pScene->mNumMeshes = targetIndex; - if (meshMap.size() < originalNumMeshes) - { + if (meshMap.size() < originalNumMeshes) { updateSceneGraph(pScene->mRootNode, meshMap); } @@ -126,8 +122,7 @@ static void updateSceneGraph(aiNode* pNode, const std::unordered_mapmNumMeshes; ++i) { const unsigned int sourceMeshIndex = pNode->mMeshes[i]; auto it = meshMap.find(sourceMeshIndex); - if (it != meshMap.end()) - { + if (it != meshMap.end()) { pNode->mMeshes[targetIndex] = it->second; ++targetIndex; } From d9b90f714afdeb2ea3dc215d88c9c358590ffac4 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Mon, 16 Nov 2020 11:06:39 +0000 Subject: [PATCH 5/5] Fix typo --- code/PostProcessing/FindDegenerates.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/PostProcessing/FindDegenerates.cpp b/code/PostProcessing/FindDegenerates.cpp index 2137f40c2..364a75308 100644 --- a/code/PostProcessing/FindDegenerates.cpp +++ b/code/PostProcessing/FindDegenerates.cpp @@ -87,7 +87,7 @@ void FindDegeneratesProcess::SetupProperties(const Importer* pImp) { // Executes the post processing step on the given imported data. void FindDegeneratesProcess::Execute( aiScene* pScene) { ASSIMP_LOG_DEBUG("FindDegeneratesProcess begin"); - if ( null == pScene) { + if ( nullptr == pScene) { return; }