From 3deae8760c6ea327be35851f51393a8d57c82c69 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Mon, 2 Nov 2020 14:43:35 +0000 Subject: [PATCH 01/13] 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 02/13] 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 03/13] 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 8845d7eed38974523bf081d78930ca24703f9237 Mon Sep 17 00:00:00 2001 From: Inho Lee Date: Tue, 3 Nov 2020 15:59:30 +0100 Subject: [PATCH 04/13] Prevent to generate redundant morph targets for glTF2 --- code/AssetLib/glTF2/glTF2Importer.cpp | 13 ++++++--- code/Common/CreateAnimMesh.cpp | 38 +++++++++++++++------------ include/assimp/CreateAnimMesh.h | 14 ++++++++-- 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index b1ba5b67b..ecd16c9e0 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -453,11 +453,16 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { aim->mNumAnimMeshes = (unsigned int)targets.size(); aim->mAnimMeshes = new aiAnimMesh *[aim->mNumAnimMeshes]; for (size_t i = 0; i < targets.size(); i++) { - aim->mAnimMeshes[i] = aiCreateAnimMesh(aim); + bool needPositions = targets[i].position.size() > 0; + bool needNormals = targets[i].normal.size() > 0; + bool needTangents = targets[i].tangent.size() > 0; + // GLTF morph does not support colors and texCoords + aim->mAnimMeshes[i] = aiCreateAnimMesh(aim, + needPositions, needNormals, needTangents, false, false); aiAnimMesh &aiAnimMesh = *(aim->mAnimMeshes[i]); Mesh::Primitive::Target &target = targets[i]; - if (target.position.size() > 0) { + if (needPositions) { aiVector3D *positionDiff = nullptr; target.position[0]->ExtractData(positionDiff); for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; vertexId++) { @@ -465,7 +470,7 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { } delete[] positionDiff; } - if (target.normal.size() > 0) { + if (needNormals) { aiVector3D *normalDiff = nullptr; target.normal[0]->ExtractData(normalDiff); for (unsigned int vertexId = 0; vertexId < aim->mNumVertices; vertexId++) { @@ -473,7 +478,7 @@ void glTF2Importer::ImportMeshes(glTF2::Asset &r) { } delete[] normalDiff; } - if (target.tangent.size() > 0) { + if (needTangents) { Tangent *tangent = nullptr; attr.tangent[0]->ExtractData(tangent); diff --git a/code/Common/CreateAnimMesh.cpp b/code/Common/CreateAnimMesh.cpp index 05472529d..d2d7e1d14 100644 --- a/code/Common/CreateAnimMesh.cpp +++ b/code/Common/CreateAnimMesh.cpp @@ -44,42 +44,46 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace Assimp { -aiAnimMesh *aiCreateAnimMesh(const aiMesh *mesh) +aiAnimMesh *aiCreateAnimMesh(const aiMesh *mesh, bool needPositions, bool needNormals, bool needTangents, bool needColors, bool needTexCoords) { aiAnimMesh *animesh = new aiAnimMesh; animesh->mNumVertices = mesh->mNumVertices; - if (mesh->mVertices) { + if (needPositions && mesh->mVertices) { animesh->mVertices = new aiVector3D[animesh->mNumVertices]; std::memcpy(animesh->mVertices, mesh->mVertices, mesh->mNumVertices * sizeof(aiVector3D)); } - if (mesh->mNormals) { + if (needNormals && mesh->mNormals) { animesh->mNormals = new aiVector3D[animesh->mNumVertices]; std::memcpy(animesh->mNormals, mesh->mNormals, mesh->mNumVertices * sizeof(aiVector3D)); } - if (mesh->mTangents) { + if (needTangents && mesh->mTangents) { animesh->mTangents = new aiVector3D[animesh->mNumVertices]; std::memcpy(animesh->mTangents, mesh->mTangents, mesh->mNumVertices * sizeof(aiVector3D)); } - if (mesh->mBitangents) { + if (needTangents && mesh->mBitangents) { animesh->mBitangents = new aiVector3D[animesh->mNumVertices]; std::memcpy(animesh->mBitangents, mesh->mBitangents, mesh->mNumVertices * sizeof(aiVector3D)); } - for (int i = 0; i < AI_MAX_NUMBER_OF_COLOR_SETS; ++i) { - if (mesh->mColors[i]) { - animesh->mColors[i] = new aiColor4D[animesh->mNumVertices]; - std::memcpy(animesh->mColors[i], mesh->mColors[i], mesh->mNumVertices * sizeof(aiColor4D)); - } else { - animesh->mColors[i] = nullptr; + if (needColors) { + for (int i = 0; i < AI_MAX_NUMBER_OF_COLOR_SETS; ++i) { + if (mesh->mColors[i]) { + animesh->mColors[i] = new aiColor4D[animesh->mNumVertices]; + std::memcpy(animesh->mColors[i], mesh->mColors[i], mesh->mNumVertices * sizeof(aiColor4D)); + } else { + animesh->mColors[i] = nullptr; + } } } - for (int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) { - if (mesh->mTextureCoords[i]) { - animesh->mTextureCoords[i] = new aiVector3D[animesh->mNumVertices]; - std::memcpy(animesh->mTextureCoords[i], mesh->mTextureCoords[i], mesh->mNumVertices * sizeof(aiVector3D)); - } else { - animesh->mTextureCoords[i] = nullptr; + if (needTexCoords) { + for (int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i) { + if (mesh->mTextureCoords[i]) { + animesh->mTextureCoords[i] = new aiVector3D[animesh->mNumVertices]; + std::memcpy(animesh->mTextureCoords[i], mesh->mTextureCoords[i], mesh->mNumVertices * sizeof(aiVector3D)); + } else { + animesh->mTextureCoords[i] = nullptr; + } } } return animesh; diff --git a/include/assimp/CreateAnimMesh.h b/include/assimp/CreateAnimMesh.h index 01a118ba3..c327fa442 100644 --- a/include/assimp/CreateAnimMesh.h +++ b/include/assimp/CreateAnimMesh.h @@ -57,10 +57,20 @@ namespace Assimp { /** * Create aiAnimMesh from aiMesh. - * @param mesh The input mesh to create an animated mesh from. + * @param mesh The input mesh to create an animated mesh from. + * @param needPositions If true, positions will be copied from. + * @param needNormals If true, normals will be copied from. + * @param needTangents If true, tangents and bitangents will be copied from. + * @param needColors If true, colors will be copied from. + * @param needTexCoords If true, texCoords will be copied from. * @return The new created animated mesh. */ -ASSIMP_API aiAnimMesh *aiCreateAnimMesh(const aiMesh *mesh); +ASSIMP_API aiAnimMesh *aiCreateAnimMesh(const aiMesh *mesh, + bool needPositions = true, + bool needNormals = true, + bool needTangents = true, + bool needColors = true, + bool needTexCoords = true); } // end of namespace Assimp From 0d5e5790cb8bc2bcba15edefd9a5cf23b3d0c347 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sat, 14 Nov 2020 12:44:49 +0100 Subject: [PATCH 05/13] 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 06/13] 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; } From e3083c21f0a7beae6c37a2265b7919a02cbf83c4 Mon Sep 17 00:00:00 2001 From: Inho Lee Date: Wed, 11 Nov 2020 20:50:10 +0100 Subject: [PATCH 07/13] glTF2: import correct animation values for CUBICSPLINE CUBICSPLINE interpolation has tangent values with the animation data. Current import don't care this interpolation type but it will help not to fetch tangent values instead of animation data. Note: Assimp cannot support interpolation types yet. --- code/AssetLib/glTF2/glTF2Importer.cpp | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index ecd16c9e0..857154e88 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -1074,9 +1074,11 @@ aiNodeAnim *CreateNodeAnim(glTF2::Asset&, Node &node, AnimationSamplers &sampler samplers.translation->output->ExtractData(values); anim->mNumPositionKeys = static_cast(samplers.translation->input->count); anim->mPositionKeys = new aiVectorKey[anim->mNumPositionKeys]; + unsigned int ii = (samplers.translation->interpolation == Interpolation_CUBICSPLINE) ? 1 : 0; for (unsigned int i = 0; i < anim->mNumPositionKeys; ++i) { anim->mPositionKeys[i].mTime = times[i] * kMillisecondsFromSeconds; - anim->mPositionKeys[i].mValue = values[i]; + anim->mPositionKeys[i].mValue = values[ii]; + ii += (samplers.translation->interpolation == Interpolation_CUBICSPLINE) ? 3 : 1; } delete[] times; delete[] values; @@ -1096,12 +1098,14 @@ aiNodeAnim *CreateNodeAnim(glTF2::Asset&, Node &node, AnimationSamplers &sampler samplers.rotation->output->ExtractData(values); anim->mNumRotationKeys = static_cast(samplers.rotation->input->count); anim->mRotationKeys = new aiQuatKey[anim->mNumRotationKeys]; + unsigned int ii = (samplers.rotation->interpolation == Interpolation_CUBICSPLINE) ? 1 : 0; for (unsigned int i = 0; i < anim->mNumRotationKeys; ++i) { anim->mRotationKeys[i].mTime = times[i] * kMillisecondsFromSeconds; - anim->mRotationKeys[i].mValue.x = values[i].w; - anim->mRotationKeys[i].mValue.y = values[i].x; - anim->mRotationKeys[i].mValue.z = values[i].y; - anim->mRotationKeys[i].mValue.w = values[i].z; + anim->mRotationKeys[i].mValue.x = values[ii].w; + anim->mRotationKeys[i].mValue.y = values[ii].x; + anim->mRotationKeys[i].mValue.z = values[ii].y; + anim->mRotationKeys[i].mValue.w = values[ii].z; + ii += (samplers.rotation->interpolation == Interpolation_CUBICSPLINE) ? 3 : 1; } delete[] times; delete[] values; @@ -1122,9 +1126,11 @@ aiNodeAnim *CreateNodeAnim(glTF2::Asset&, Node &node, AnimationSamplers &sampler samplers.scale->output->ExtractData(values); anim->mNumScalingKeys = static_cast(samplers.scale->input->count); anim->mScalingKeys = new aiVectorKey[anim->mNumScalingKeys]; + unsigned int ii = (samplers.scale->interpolation == Interpolation_CUBICSPLINE) ? 1 : 0; for (unsigned int i = 0; i < anim->mNumScalingKeys; ++i) { anim->mScalingKeys[i].mTime = times[i] * kMillisecondsFromSeconds; - anim->mScalingKeys[i].mValue = values[i]; + anim->mScalingKeys[i].mValue = values[ii]; + ii += (samplers.scale->interpolation == Interpolation_CUBICSPLINE) ? 3 : 1; } delete[] times; delete[] values; @@ -1153,11 +1159,14 @@ aiMeshMorphAnim *CreateMeshMorphAnim(glTF2::Asset&, Node &node, AnimationSampler samplers.weight->output->ExtractData(values); anim->mNumKeys = static_cast(samplers.weight->input->count); - const unsigned int numMorphs = (unsigned int)samplers.weight->output->count / anim->mNumKeys; + // for Interpolation_CUBICSPLINE can have more outputs + const unsigned int weightStride = (unsigned int)samplers.weight->output->count / anim->mNumKeys; + const unsigned int numMorphs = (samplers.weight->interpolation == Interpolation_CUBICSPLINE) ? weightStride - 2 : weightStride; anim->mKeys = new aiMeshMorphKey[anim->mNumKeys]; - unsigned int k = 0u; + unsigned int ii = (samplers.weight->interpolation == Interpolation_CUBICSPLINE) ? 1 : 0; for (unsigned int i = 0u; i < anim->mNumKeys; ++i) { + unsigned int k = weightStride * i + ii; anim->mKeys[i].mTime = times[i] * kMillisecondsFromSeconds; anim->mKeys[i].mNumValuesAndWeights = numMorphs; anim->mKeys[i].mValues = new unsigned int[numMorphs]; From 7b59cc297e26f3e94b7c8a91b779fe21e355288f Mon Sep 17 00:00:00 2001 From: Neil Clifford Date: Thu, 19 Nov 2020 12:20:06 +0000 Subject: [PATCH 08/13] FBXParser.cpp - handle buffer over-read cases correctly --- code/AssetLib/FBX/FBXParser.cpp | 48 +++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/code/AssetLib/FBX/FBXParser.cpp b/code/AssetLib/FBX/FBXParser.cpp index f93f69d4d..8d4bbd866 100644 --- a/code/AssetLib/FBX/FBXParser.cpp +++ b/code/AssetLib/FBX/FBXParser.cpp @@ -641,7 +641,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * (type == 'd' ? 8 : 4)); + uint64_t dataToRead = static_cast(count) * (type == 'd' ? 8 : 4); + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } const uint32_t count3 = count / 3; out.reserve(count3); @@ -728,7 +732,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * (type == 'd' ? 8 : 4)); + uint64_t dataToRead = static_cast(count) * (type == 'd' ? 8 : 4); + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } const uint32_t count4 = count / 4; out.reserve(count4); @@ -807,7 +815,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * (type == 'd' ? 8 : 4)); + uint64_t dataToRead = static_cast(count) * (type == 'd' ? 8 : 4); + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } const uint32_t count2 = count / 2; out.reserve(count2); @@ -879,7 +891,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * 4); + uint64_t dataToRead = static_cast(count) * 4; + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } out.reserve(count); @@ -937,7 +953,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * (type == 'd' ? 8 : 4)); + uint64_t dataToRead = static_cast(count) * (type == 'd' ? 8 : 4); + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } if (type == 'd') { const double* d = reinterpret_cast(&buff[0]); @@ -998,7 +1018,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * 4); + uint64_t dataToRead = static_cast(count) * 4; + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } out.reserve(count); @@ -1063,7 +1087,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * 8); + uint64_t dataToRead = static_cast(count) * 8; + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } out.reserve(count); @@ -1121,7 +1149,11 @@ void ParseVectorDataArray(std::vector& out, const Element& el) ReadBinaryDataArray(type, count, data, end, buff, el); ai_assert(data == end); - ai_assert(buff.size() == count * 8); + uint64_t dataToRead = static_cast(count) * 8; + ai_assert(buff.size() == dataToRead); + if (dataToRead > buff.size()) { + ParseError("Invalid read size (binary)",&el); + } out.reserve(count); From 67abcb10ba19e793dacb9dae408b0c816abd25df Mon Sep 17 00:00:00 2001 From: Evangel Date: Sun, 22 Nov 2020 15:10:36 +1000 Subject: [PATCH 09/13] Added mName to aiScene. Primarily to provide access to the "name" member of glTF2 scenes. --- code/AssetLib/glTF2/glTF2Asset.h | 1 + code/AssetLib/glTF2/glTF2Asset.inl | 5 +++++ code/AssetLib/glTF2/glTF2Importer.cpp | 1 + include/assimp/scene.h | 7 +++++-- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.h b/code/AssetLib/glTF2/glTF2Asset.h index ead8ca1dd..23c19cc58 100644 --- a/code/AssetLib/glTF2/glTF2Asset.h +++ b/code/AssetLib/glTF2/glTF2Asset.h @@ -863,6 +863,7 @@ struct Sampler : public Object { }; struct Scene : public Object { + std::string name; std::vector> nodes; Scene() {} diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index 2c7e9fc5a..af9a4e94e 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -1400,6 +1400,11 @@ inline void Node::Read(Value &obj, Asset &r) { } inline void Scene::Read(Value &obj, Asset &r) { + if (Value *name = FindString(obj, "name")) { + if (name->IsString()) { + this->name = name->GetString(); + } + } if (Value *array = FindArray(obj, "nodes")) { for (unsigned int i = 0; i < array->Size(); ++i) { if (!(*array)[i].IsUint()) continue; diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index b1ba5b67b..ad0cea221 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -1386,6 +1386,7 @@ void glTF2Importer::InternReadFile(const std::string &pFile, aiScene *pScene, IO // read the asset file glTF2::Asset asset(pIOHandler); asset.Load(pFile, GetExtension(pFile) == "glb"); + pScene->mName = asset.scene->name; // // Copy the data out diff --git a/include/assimp/scene.h b/include/assimp/scene.h index a189f5700..2a9a77b02 100644 --- a/include/assimp/scene.h +++ b/include/assimp/scene.h @@ -335,12 +335,15 @@ struct aiScene /** * @brief The global metadata assigned to the scene itself. * - * This data contains global metadata which belongs to the scene like - * unit-conversions, versions, vendors or other model-specific data. This + * This data contains global metadata which belongs to the scene like + * unit-conversions, versions, vendors or other model-specific data. This * can be used to store format-specific metadata as well. */ C_STRUCT aiMetadata* mMetaData; + /** The name of the scene itself. + */ + C_STRUCT aiString mName; #ifdef __cplusplus From 30584c1ec178d9756be551287d9ab2e9eca63a7e Mon Sep 17 00:00:00 2001 From: Evangel Date: Sun, 22 Nov 2020 15:36:08 +1000 Subject: [PATCH 10/13] Replaced name with scene_name to avoid shadowing. --- code/AssetLib/glTF2/glTF2Asset.inl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index af9a4e94e..c892e6697 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -1400,9 +1400,9 @@ inline void Node::Read(Value &obj, Asset &r) { } inline void Scene::Read(Value &obj, Asset &r) { - if (Value *name = FindString(obj, "name")) { - if (name->IsString()) { - this->name = name->GetString(); + if (Value *scene_name = FindString(obj, "name")) { + if (scene_name->IsString()) { + this->name = scene_name->GetString(); } } if (Value *array = FindArray(obj, "nodes")) { From 98e42e22b8a922f96d5088db889d98bdd02129d9 Mon Sep 17 00:00:00 2001 From: Evangel Date: Sun, 22 Nov 2020 15:49:41 +1000 Subject: [PATCH 11/13] Added check around setting pScene->mName from asset.scene. --- code/AssetLib/glTF2/glTF2Importer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index ad0cea221..738186414 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -1386,7 +1386,9 @@ void glTF2Importer::InternReadFile(const std::string &pFile, aiScene *pScene, IO // read the asset file glTF2::Asset asset(pIOHandler); asset.Load(pFile, GetExtension(pFile) == "glb"); - pScene->mName = asset.scene->name; + if (asset.scene) { + pScene->mName = asset.scene->name; + } // // Copy the data out From 14f79caf4157bf6b66ffc98288a4b0ef059b56bf Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Thu, 26 Nov 2020 16:29:37 +0000 Subject: [PATCH 12/13] textures_converted keys can just be pointers --- code/AssetLib/FBX/FBXConverter.cpp | 10 +++++----- code/AssetLib/FBX/FBXConverter.h | 2 +- code/AssetLib/FBX/FBXDocument.h | 30 ------------------------------ 3 files changed, 6 insertions(+), 36 deletions(-) diff --git a/code/AssetLib/FBX/FBXConverter.cpp b/code/AssetLib/FBX/FBXConverter.cpp index a7f9e6832..efecf450f 100644 --- a/code/AssetLib/FBX/FBXConverter.cpp +++ b/code/AssetLib/FBX/FBXConverter.cpp @@ -1715,14 +1715,14 @@ aiString FBXConverter::GetTexturePath(const Texture *tex) { bool textureReady = false; //tells if our texture is ready (if it was loaded or if it was found) unsigned int index=0; - VideoMap::const_iterator it = textures_converted.find(*media); + VideoMap::const_iterator it = textures_converted.find(media); if (it != textures_converted.end()) { index = (*it).second; textureReady = true; } else { if (media->ContentLength() > 0) { index = ConvertVideo(*media); - textures_converted[*media] = index; + textures_converted[media] = index; textureReady = true; } } @@ -2221,12 +2221,12 @@ void FBXConverter::SetShadingPropertiesRaw(aiMaterial *out_mat, const PropertyTa if (media != nullptr && media->ContentLength() > 0) { unsigned int index; - VideoMap::const_iterator videoIt = textures_converted.find(*media); + VideoMap::const_iterator videoIt = textures_converted.find(media); if (videoIt != textures_converted.end()) { index = videoIt->second; } else { index = ConvertVideo(*media); - textures_converted[*media] = index; + textures_converted[media] = index; } // setup texture reference string (copied from ColladaLoader::FindFilenameForEffectTexture) @@ -3493,7 +3493,7 @@ void FBXConverter::ConvertOrphanedEmbeddedTextures() { if (realTexture) { const Video *media = realTexture->Media(); unsigned int index = ConvertVideo(*media); - textures_converted[*media] = index; + textures_converted[media] = index; } } } diff --git a/code/AssetLib/FBX/FBXConverter.h b/code/AssetLib/FBX/FBXConverter.h index 7db4b795b..0ae0da662 100644 --- a/code/AssetLib/FBX/FBXConverter.h +++ b/code/AssetLib/FBX/FBXConverter.h @@ -428,7 +428,7 @@ private: using MaterialMap = std::fbx_unordered_map; MaterialMap materials_converted; - using VideoMap = std::fbx_unordered_map; + using VideoMap = std::fbx_unordered_map; VideoMap textures_converted; using MeshMap = std::fbx_unordered_map >; diff --git a/code/AssetLib/FBX/FBXDocument.h b/code/AssetLib/FBX/FBXDocument.h index 165bb900e..67b953549 100644 --- a/code/AssetLib/FBX/FBXDocument.h +++ b/code/AssetLib/FBX/FBXDocument.h @@ -638,15 +638,6 @@ public: return ptr; } - bool operator==(const Video& other) const - { - return ( - type == other.type - && relativeFileName == other.relativeFileName - && fileName == other.fileName - ); - } - bool operator<(const Video& other) const { return std::tie(type, relativeFileName, fileName) < std::tie(other.type, other.relativeFileName, other.fileName); @@ -1192,25 +1183,4 @@ private: } // Namespace FBX } // Namespace Assimp -namespace std -{ - template <> - struct hash - { - std::size_t operator()(const Assimp::FBX::Video& video) const - { - using std::size_t; - using std::hash; - using std::string; - - size_t res = 17; - res = res * 31 + hash()(video.Name()); - res = res * 31 + hash()(video.RelativeFilename()); - res = res * 31 + hash()(video.Type()); - - return res; - } - }; -} - #endif // INCLUDED_AI_FBX_DOCUMENT_H From 149b8d1fd14df41962598956cc80d1d3929fdbd6 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Fri, 27 Nov 2020 14:26:51 +0000 Subject: [PATCH 13/13] Don't need operator< either. --- code/AssetLib/FBX/FBXDocument.h | 5 ----- 1 file changed, 5 deletions(-) diff --git a/code/AssetLib/FBX/FBXDocument.h b/code/AssetLib/FBX/FBXDocument.h index 67b953549..85ccca5d0 100644 --- a/code/AssetLib/FBX/FBXDocument.h +++ b/code/AssetLib/FBX/FBXDocument.h @@ -638,11 +638,6 @@ public: return ptr; } - bool operator<(const Video& other) const - { - return std::tie(type, relativeFileName, fileName) < std::tie(other.type, other.relativeFileName, other.fileName); - } - private: std::string type; std::string relativeFileName;