From 74af523b3ef6a68fa07749ac974d4fd51c6324e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20R=C3=B6sner?= Date: Thu, 12 Jan 2023 13:13:46 +0100 Subject: [PATCH 1/7] Generalize JoinVerticesProcess for multiple UV and color channels --- code/PostProcessing/JoinVerticesProcess.cpp | 49 +++++++++++++-------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/code/PostProcessing/JoinVerticesProcess.cpp b/code/PostProcessing/JoinVerticesProcess.cpp index ef5999875..f752ed0eb 100644 --- a/code/PostProcessing/JoinVerticesProcess.cpp +++ b/code/PostProcessing/JoinVerticesProcess.cpp @@ -105,7 +105,11 @@ void JoinVerticesProcess::Execute( aiScene* pScene) { namespace { -bool areVerticesEqual(const Vertex &lhs, const Vertex &rhs, bool complex) { +bool areVerticesEqual( + const Vertex &lhs, + const Vertex &rhs, + unsigned numUVChannels, + unsigned numColorChannels) { // A little helper to find locally close vertices faster. // Try to reuse the lookup table from the last step. const static float epsilon = 1e-5f; @@ -124,10 +128,6 @@ bool areVerticesEqual(const Vertex &lhs, const Vertex &rhs, bool complex) { return false; } - if ((lhs.texcoords[0] - rhs.texcoords[0]).SquareLength() > squareEpsilon) { - return false; - } - if ((lhs.tangent - rhs.tangent).SquareLength() > squareEpsilon) { return false; } @@ -136,19 +136,18 @@ bool areVerticesEqual(const Vertex &lhs, const Vertex &rhs, bool complex) { return false; } - // Usually we won't have vertex colors or multiple UVs, so we can skip from here - // Actually this increases runtime performance slightly, at least if branch - // prediction is on our side. - if (complex) { - for (int i = 0; i < 8; i++) { - if (i > 0 && (lhs.texcoords[i] - rhs.texcoords[i]).SquareLength() > squareEpsilon) { - return false; - } - if (GetColorDifference(lhs.colors[i], rhs.colors[i]) > squareEpsilon) { - return false; - } + for (unsigned i = 0; i < numUVChannels; i++) { + if ((lhs.texcoords[i] - rhs.texcoords[i]).SquareLength() > squareEpsilon) { + return false; } } + + for (unsigned i = 0; i < numColorChannels; i++) { + if (GetColorDifference(lhs.colors[i], rhs.colors[i]) > squareEpsilon) { + return false; + } + } + return true; } @@ -241,9 +240,16 @@ struct std::hash { //template specialization for std::equal_to for Vertex template<> struct std::equal_to { + equal_to(unsigned numUVChannels, unsigned numColorChannels) : + mNumUVChannels(numUVChannels), + mNumColorChannels(numColorChannels) {} bool operator()(const Vertex &lhs, const Vertex &rhs) const { - return areVerticesEqual(lhs, rhs, false); + return areVerticesEqual(lhs, rhs, mNumUVChannels, mNumColorChannels); } + +private: + unsigned mNumUVChannels; + unsigned mNumColorChannels; }; // now start the JoinVerticesProcess int JoinVerticesProcess::ProcessMesh( aiMesh* pMesh, unsigned int meshIndex) { @@ -316,8 +322,13 @@ int JoinVerticesProcess::ProcessMesh( aiMesh* pMesh, unsigned int meshIndex) { uniqueAnimatedVertices[animMeshIndex].reserve(pMesh->mNumVertices); } } - // a map that maps a vertix to its new index - std::unordered_map vertex2Index; + // a map that maps a vertex to its new index + const auto numBuckets = pMesh->mNumVertices; + const auto hasher = std::hash(); + const auto comparator = std::equal_to( + pMesh->GetNumUVChannels(), + pMesh->GetNumColorChannels()); + std::unordered_map vertex2Index(numBuckets, hasher, comparator); // we can not end up with more vertices than we started with vertex2Index.reserve(pMesh->mNumVertices); // Now check each vertex if it brings something new to the table From 43c0f8bb3d41250d6b566e6cb5b9a5c79e36aae3 Mon Sep 17 00:00:00 2001 From: Martin Mory Date: Sun, 15 Jan 2023 23:03:41 +0100 Subject: [PATCH 2/7] Remove whitespace between a tag and the first number, otherwise first call to strtoul10() returns 0 and the indices are broken, leading to possible out-of-bound access and memory corruption/crash --- code/AssetLib/Collada/ColladaParser.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index 91f32f485..cce6a0db6 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -762,6 +762,7 @@ void ColladaParser::ReadControllerWeights(XmlNode &node, Collada::Controller &pC if (text == nullptr) { throw DeadlyImportError("Out of data while reading "); } + SkipSpacesAndLineEnd(&text); it->first = strtoul10(text, &text); SkipSpacesAndLineEnd(&text); if (*text == 0) { From 5cbc00a595db2166463bc494435a67b4080d28fc Mon Sep 17 00:00:00 2001 From: Krishty Date: Mon, 16 Jan 2023 08:29:49 +0100 Subject: [PATCH 3/7] Fix Build With M3D Import Only `M3DWrapper.h` is designed to omit the definition of `class M3DWrapper` if neither M3D import nor M3D export are compiled. 608bccd9cf3a28c55163c10e84aec025293b7ab0 touched the corresponding preprocessor checks and introduced a bug: ``` #ifndef ASSIMP_BUILD_NO_M3D_IMPORTER #if !(ASSIMP_BUILD_NO_EXPORT || ASSIMP_BUILD_NO_M3D_EXPORTER) class M3DWrapper { ``` When compiling - with M3D import enabled, - but with either export generally disabled or M3D export disabled specifically, These checks evaluate to the wrong result and skip the definition, leading to a build failure in dependent code. ``` #if 1 // import enabled #if !(1 || 1) // export disabled and M3D export disabled ``` This commit fixes the check to compile the definition if neither import is disabled. --- code/AssetLib/M3D/M3DWrapper.cpp | 4 +--- code/AssetLib/M3D/M3DWrapper.h | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/code/AssetLib/M3D/M3DWrapper.cpp b/code/AssetLib/M3D/M3DWrapper.cpp index 30452c776..05087d592 100644 --- a/code/AssetLib/M3D/M3DWrapper.cpp +++ b/code/AssetLib/M3D/M3DWrapper.cpp @@ -39,8 +39,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ---------------------------------------------------------------------- */ -#ifndef ASSIMP_BUILD_NO_M3D_IMPORTER -#if !(ASSIMP_BUILD_NO_EXPORT || ASSIMP_BUILD_NO_M3D_EXPORTER) +#if !defined ASSIMP_BUILD_NO_M3D_IMPORTER || !(defined ASSIMP_BUILD_NO_EXPORT || defined ASSIMP_BUILD_NO_M3D_EXPORTER) #include "M3DWrapper.h" @@ -149,4 +148,3 @@ void M3DWrapper::ClearSave() { } // namespace Assimp #endif -#endif diff --git a/code/AssetLib/M3D/M3DWrapper.h b/code/AssetLib/M3D/M3DWrapper.h index c75ff1027..880aca996 100644 --- a/code/AssetLib/M3D/M3DWrapper.h +++ b/code/AssetLib/M3D/M3DWrapper.h @@ -47,8 +47,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef AI_M3DWRAPPER_H_INC #define AI_M3DWRAPPER_H_INC -#ifndef ASSIMP_BUILD_NO_M3D_IMPORTER -#if !(ASSIMP_BUILD_NO_EXPORT || ASSIMP_BUILD_NO_M3D_EXPORTER) +#if !defined ASSIMP_BUILD_NO_M3D_IMPORTER || !(defined ASSIMP_BUILD_NO_EXPORT || defined ASSIMP_BUILD_NO_M3D_EXPORTER) #include #include @@ -126,7 +125,6 @@ inline m3d_t *M3DWrapper::M3D() const { } // namespace Assimp -#endif #endif // ASSIMP_BUILD_NO_M3D_IMPORTER #endif // AI_M3DWRAPPER_H_INC From ab0a119626a93d55944208b96be856c914900f65 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 20 Jan 2023 19:14:04 +0100 Subject: [PATCH 4/7] Update LimitBoneWeightsProcess.cpp - Removing empty bones only if AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES is enabled. - closes https://github.com/assimp/assimp/issues/4840 --- .../LimitBoneWeightsProcess.cpp | 70 ++++++++----------- 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/code/PostProcessing/LimitBoneWeightsProcess.cpp b/code/PostProcessing/LimitBoneWeightsProcess.cpp index 3192e07bc..1db9158ef 100644 --- a/code/PostProcessing/LimitBoneWeightsProcess.cpp +++ b/code/PostProcessing/LimitBoneWeightsProcess.cpp @@ -55,10 +55,7 @@ using namespace Assimp; // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer -LimitBoneWeightsProcess::LimitBoneWeightsProcess() -{ - mMaxWeights = AI_LMW_MAX_WEIGHTS; -} +LimitBoneWeightsProcess::LimitBoneWeightsProcess() : mMaxWeights(AI_LMW_MAX_WEIGHTS) {} // ------------------------------------------------------------------------------------------------ // Destructor, private as well @@ -66,15 +63,15 @@ LimitBoneWeightsProcess::~LimitBoneWeightsProcess() = default; // ------------------------------------------------------------------------------------------------ // Returns whether the processing step is present in the given flag field. -bool LimitBoneWeightsProcess::IsActive( unsigned int pFlags) const -{ +bool LimitBoneWeightsProcess::IsActive( unsigned int pFlags) const { return (pFlags & aiProcess_LimitBoneWeights) != 0; } // ------------------------------------------------------------------------------------------------ // Executes the post processing step on the given imported data. -void LimitBoneWeightsProcess::Execute( aiScene* pScene) -{ +void LimitBoneWeightsProcess::Execute( aiScene* pScene) { + ai_assert(pScene != nullptr); + ASSIMP_LOG_DEBUG("LimitBoneWeightsProcess begin"); for (unsigned int m = 0; m < pScene->mNumMeshes; ++m) { @@ -86,16 +83,28 @@ void LimitBoneWeightsProcess::Execute( aiScene* pScene) // ------------------------------------------------------------------------------------------------ // Executes the post processing step on the given imported data. -void LimitBoneWeightsProcess::SetupProperties(const Importer* pImp) -{ - // get the current value of the property +void LimitBoneWeightsProcess::SetupProperties(const Importer* pImp) { this->mMaxWeights = pImp->GetPropertyInteger(AI_CONFIG_PP_LBW_MAX_WEIGHTS,AI_LMW_MAX_WEIGHTS); } +// ------------------------------------------------------------------------------------------------ +static void removeEmptyBones(aiMesh *pMesh) { + ai_assert(pMesh != nullptr); + + unsigned int writeBone = 0; + for (unsigned int readBone = 0; readBone< pMesh->mNumBones; ++readBone) { + aiBone* bone = pMesh->mBones[readBone]; + if (bone->mNumWeights > 0) { + pMesh->mBones[writeBone++] = bone; + } else { + delete bone; + } + } +} + // ------------------------------------------------------------------------------------------------ // Unites identical vertices in the given mesh -void LimitBoneWeightsProcess::ProcessMesh(aiMesh* pMesh) -{ +void LimitBoneWeightsProcess::ProcessMesh(aiMesh* pMesh) { if (!pMesh->HasBones()) return; @@ -105,11 +114,9 @@ void LimitBoneWeightsProcess::ProcessMesh(aiMesh* pMesh) WeightsPerVertex vertexWeights(pMesh->mNumVertices); size_t maxVertexWeights = 0; - for (unsigned int b = 0; b < pMesh->mNumBones; ++b) - { + for (unsigned int b = 0; b < pMesh->mNumBones; ++b) { const aiBone* bone = pMesh->mBones[b]; - for (unsigned int w = 0; w < bone->mNumWeights; ++w) - { + for (unsigned int w = 0; w < bone->mNumWeights; ++w) { const aiVertexWeight& vw = bone->mWeights[w]; if (vertexWeights.size() <= vw.mVertexId) @@ -126,8 +133,7 @@ void LimitBoneWeightsProcess::ProcessMesh(aiMesh* pMesh) unsigned int removed = 0, old_bones = pMesh->mNumBones; // now cut the weight count if it exceeds the maximum - for (WeightsPerVertex::iterator vit = vertexWeights.begin(); vit != vertexWeights.end(); ++vit) - { + for (WeightsPerVertex::iterator vit = vertexWeights.begin(); vit != vertexWeights.end(); ++vit) { if (vit->size() <= mMaxWeights) continue; @@ -154,39 +160,25 @@ void LimitBoneWeightsProcess::ProcessMesh(aiMesh* pMesh) } // clear weight count for all bone - for (unsigned int a = 0; a < pMesh->mNumBones; ++a) - { + for (unsigned int a = 0; a < pMesh->mNumBones; ++a) { pMesh->mBones[a]->mNumWeights = 0; } // rebuild the vertex weight array for all bones - for (unsigned int a = 0; a < vertexWeights.size(); ++a) - { + for (unsigned int a = 0; a < vertexWeights.size(); ++a) { const VertexWeightArray& vw = vertexWeights[a]; - for (const Weight* it = vw.begin(); it != vw.end(); ++it) - { + for (const Weight* it = vw.begin(); it != vw.end(); ++it) { aiBone* bone = pMesh->mBones[it->mBone]; bone->mWeights[bone->mNumWeights++] = aiVertexWeight(a, it->mWeight); } } // remove empty bones - unsigned int writeBone = 0; +#ifdef AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES + removeEmptyBones(pMesh); +#endif // AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES - for (unsigned int readBone = 0; readBone< pMesh->mNumBones; ++readBone) - { - aiBone* bone = pMesh->mBones[readBone]; - if (bone->mNumWeights > 0) - { - pMesh->mBones[writeBone++] = bone; - } - else - { - delete bone; - } - } pMesh->mNumBones = writeBone; - if (!DefaultLogger::isNullLogger()) { ASSIMP_LOG_INFO("Removed ", removed, " weights. Input bones: ", old_bones, ". Output bones: ", pMesh->mNumBones); } From 81cf1369dbf905dffbedd6fb6cd427455025b73f Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 20 Jan 2023 19:20:06 +0100 Subject: [PATCH 5/7] Set correct number of bones in mesh instance --- .../LimitBoneWeightsProcess.cpp | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/code/PostProcessing/LimitBoneWeightsProcess.cpp b/code/PostProcessing/LimitBoneWeightsProcess.cpp index 1db9158ef..8820c0113 100644 --- a/code/PostProcessing/LimitBoneWeightsProcess.cpp +++ b/code/PostProcessing/LimitBoneWeightsProcess.cpp @@ -2,8 +2,7 @@ Open Asset Import Library (assimp) ---------------------------------------------------------------------- -Copyright (c) 2006-2022, assimp team - +Copyright (c) 2006-2023, assimp team All rights reserved. @@ -36,13 +35,7 @@ DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - ----------------------------------------------------------------------- -*/ - -/** Implementation of the LimitBoneWeightsProcess post processing step */ - - +---------------------------------------------------------------------- */ #include "LimitBoneWeightsProcess.h" #include #include @@ -51,7 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include -using namespace Assimp; +namespace Assimp { // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer @@ -88,7 +81,7 @@ void LimitBoneWeightsProcess::SetupProperties(const Importer* pImp) { } // ------------------------------------------------------------------------------------------------ -static void removeEmptyBones(aiMesh *pMesh) { +static unsigned int removeEmptyBones(aiMesh *pMesh) { ai_assert(pMesh != nullptr); unsigned int writeBone = 0; @@ -100,6 +93,8 @@ static void removeEmptyBones(aiMesh *pMesh) { delete bone; } } + + return writeBone; } // ------------------------------------------------------------------------------------------------ @@ -175,11 +170,12 @@ void LimitBoneWeightsProcess::ProcessMesh(aiMesh* pMesh) { // remove empty bones #ifdef AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES - removeEmptyBones(pMesh); + pMesh->mNumBones = removeEmptyBones(pMesh); #endif // AI_CONFIG_IMPORT_REMOVE_EMPTY_BONES - pMesh->mNumBones = writeBone; if (!DefaultLogger::isNullLogger()) { ASSIMP_LOG_INFO("Removed ", removed, " weights. Input bones: ", old_bones, ". Output bones: ", pMesh->mNumBones); } } + +} // namespace Assimp From 9e1de3ec6e6a5749ff23f3b8640da96bb32b1046 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 20 Jan 2023 19:45:45 +0100 Subject: [PATCH 6/7] Remove /Zi compiler flag for MSVC, release config - closes https://github.com/assimp/assimp/issues/4845 --- CMakeLists.txt | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e613a43cc..d20e6c9e0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ # Open Asset Import Library (assimp) # ---------------------------------------------------------------------- -# Copyright (c) 2006-2022, assimp team +# Copyright (c) 2006-2023, assimp team # # All rights reserved. # @@ -262,13 +262,13 @@ IF ((CMAKE_C_COMPILER_ID MATCHES "GNU") AND NOT MINGW) ENDIF() # hide all not-exported symbols IF(CMAKE_HOST_SYSTEM_PROCESSOR MATCHES "mips64" ) - SET(CMAKE_CXX_FLAGS "-mxgot -fvisibility=hidden -fno-strict-aliasing -Wall ${CMAKE_CXX_FLAGS}") - SET(CMAKE_C_FLAGS "-fno-strict-aliasing ${CMAKE_C_FLAGS}") - SET(LIBSTDC++_LIBRARIES -lstdc++) + SET(CMAKE_CXX_FLAGS "-mxgot -fvisibility=hidden -fno-strict-aliasing -Wall ${CMAKE_CXX_FLAGS}") + SET(CMAKE_C_FLAGS "-fno-strict-aliasing ${CMAKE_C_FLAGS}") + SET(LIBSTDC++_LIBRARIES -lstdc++) ELSE() - SET(CMAKE_CXX_FLAGS "-fvisibility=hidden -fno-strict-aliasing -Wall ${CMAKE_CXX_FLAGS}") - SET(CMAKE_C_FLAGS "-fno-strict-aliasing ${CMAKE_C_FLAGS}") - SET(LIBSTDC++_LIBRARIES -lstdc++) + SET(CMAKE_CXX_FLAGS "-fvisibility=hidden -fno-strict-aliasing -Wall ${CMAKE_CXX_FLAGS}") + SET(CMAKE_C_FLAGS "-fno-strict-aliasing ${CMAKE_C_FLAGS}") + SET(LIBSTDC++_LIBRARIES -lstdc++) ENDIF() ELSEIF(MSVC) # enable multi-core compilation with MSVC @@ -277,13 +277,15 @@ ELSEIF(MSVC) ELSE() # msvc ADD_COMPILE_OPTIONS(/MP /bigobj /W4 /WX) ENDIF() + # disable "elements of array '' will be default initialized" warning on MSVC2013 IF(MSVC12) ADD_COMPILE_OPTIONS(/wd4351) ENDIF() - ADD_COMPILE_OPTIONS(/wd4244) #supress warning for double to float conversion if Double precission is activated + # supress warning for double to float conversion if Double precission is activated + ADD_COMPILE_OPTIONS(/wd4244) SET(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /D_DEBUG /Zi /Od") - SET(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /Zi") + SET(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}") SET(CMAKE_SHARED_LINKER_FLAGS_RELEASE "${CMAKE_SHARED_LINKER_FLAGS_RELEASE} /DEBUG:FULL /PDBALTPATH:%_PDB% /OPT:REF /OPT:ICF") ELSEIF (CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) IF(NOT ASSIMP_HUNTER_ENABLED) From 5ed01bcfa386ec889d4840eaa399d30d082e09e6 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 20 Jan 2023 20:00:36 +0100 Subject: [PATCH 7/7] Ensure initializer exists - Fixing a Codaxy finding. --- code/PostProcessing/LimitBoneWeightsProcess.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/code/PostProcessing/LimitBoneWeightsProcess.cpp b/code/PostProcessing/LimitBoneWeightsProcess.cpp index 8820c0113..51fb43dfc 100644 --- a/code/PostProcessing/LimitBoneWeightsProcess.cpp +++ b/code/PostProcessing/LimitBoneWeightsProcess.cpp @@ -46,6 +46,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace Assimp { +// Make sure this value is set. +#ifndef AI_LMW_MAX_WEIGHTS +# define AI_LMW_MAX_WEIGHTS 16 +#endif + // ------------------------------------------------------------------------------------------------ // Constructor to be privately used by Importer LimitBoneWeightsProcess::LimitBoneWeightsProcess() : mMaxWeights(AI_LMW_MAX_WEIGHTS) {}