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/3] 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/3] 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/3] 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