From 8845d7eed38974523bf081d78930ca24703f9237 Mon Sep 17 00:00:00 2001 From: Inho Lee Date: Tue, 3 Nov 2020 15:59:30 +0100 Subject: [PATCH 1/5] 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 e3083c21f0a7beae6c37a2265b7919a02cbf83c4 Mon Sep 17 00:00:00 2001 From: Inho Lee Date: Wed, 11 Nov 2020 20:50:10 +0100 Subject: [PATCH 2/5] 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 3/5] 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 14f79caf4157bf6b66ffc98288a4b0ef059b56bf Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Thu, 26 Nov 2020 16:29:37 +0000 Subject: [PATCH 4/5] 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 5/5] 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;