From 213a9f9d55249d0fcf09fec32ec8cc87533672b1 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Tue, 17 Nov 2020 10:39:03 +0000 Subject: [PATCH 01/12] First pass at PotentialNode --- code/AssetLib/FBX/FBXConverter.cpp | 232 +++++++++++++++-------------- code/AssetLib/FBX/FBXConverter.h | 5 +- 2 files changed, 120 insertions(+), 117 deletions(-) diff --git a/code/AssetLib/FBX/FBXConverter.cpp b/code/AssetLib/FBX/FBXConverter.cpp index a7f9e6832..90082172d 100644 --- a/code/AssetLib/FBX/FBXConverter.cpp +++ b/code/AssetLib/FBX/FBXConverter.cpp @@ -185,6 +185,16 @@ std::string FBXConverter::MakeUniqueNodeName(const Model *const model, const aiN return unique_name; } +/// This struct manages nodes which may or may not end up in the node hierarchy. +/// When a node becomes a child of another node, that node becomes its owner and mOwnership should be released. +struct FBXConverter::PotentialNode +{ + PotentialNode(std::unique_ptr uptr) : mNode(uptr.get()), mOwnership(std::move(uptr)) {} + aiNode* operator->() { return mNode; } + aiNode* mNode; + std::unique_ptr mOwnership; +}; + /// todo: pre-build node hierarchy /// todo: get bone from stack /// todo: make map of aiBone* to aiNode* @@ -192,137 +202,129 @@ std::string FBXConverter::MakeUniqueNodeName(const Model *const model, const aiN void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node) { const std::vector &conns = doc.GetConnectionsByDestinationSequenced(id, "Model"); - std::vector nodes; + std::vector nodes; nodes.reserve(conns.size()); - std::vector nodes_chain; - std::vector post_nodes_chain; + std::vector nodes_chain; + std::vector post_nodes_chain; - try { - for (const Connection *con : conns) { - // ignore object-property links - if (con->PropertyName().length()) { - // really important we document why this is ignored. - FBXImporter::LogInfo("ignoring property link - no docs on why this is ignored"); - continue; //? + for (const Connection *con : conns) { + // ignore object-property links + if (con->PropertyName().length()) { + // really important we document why this is ignored. + FBXImporter::LogInfo("ignoring property link - no docs on why this is ignored"); + continue; //? + } + + // convert connection source object into Object base class + const Object *const object = con->SourceObject(); + if (nullptr == object) { + FBXImporter::LogError("failed to convert source object for Model link"); + continue; + } + + // FBX Model::Cube, Model::Bone001, etc elements + // This detects if we can cast the object into this model structure. + const Model *const model = dynamic_cast(object); + + if (nullptr != model) { + nodes_chain.clear(); + post_nodes_chain.clear(); + + aiMatrix4x4 new_abs_transform = parent->mTransformation; + std::string node_name = FixNodeName(model->Name()); + // even though there is only a single input node, the design of + // assimp (or rather: the complicated transformation chain that + // is employed by fbx) means that we may need multiple aiNode's + // to represent a fbx node's transformation. + + // generate node transforms - this includes pivot data + // if need_additional_node is true then you t + const bool need_additional_node = GenerateTransformationNodeChain(*model, node_name, nodes_chain, post_nodes_chain); + + // assert that for the current node we must have at least a single transform + ai_assert(nodes_chain.size()); + + if (need_additional_node) { + nodes_chain.emplace_back(PotentialNode(std::unique_ptr(new aiNode(node_name)))); } - // convert connection source object into Object base class - const Object *const object = con->SourceObject(); - if (nullptr == object) { - FBXImporter::LogError("failed to convert source object for Model link"); - continue; - } + //setup metadata on newest node + SetupNodeMetadata(*model, *nodes_chain.back().mNode); - // FBX Model::Cube, Model::Bone001, etc elements - // This detects if we can cast the object into this model structure. - const Model *const model = dynamic_cast(object); + // link all nodes in a row + aiNode *last_parent = parent; + for (PotentialNode& child : nodes_chain) { + ai_assert(child.mNode); - if (nullptr != model) { - nodes_chain.clear(); - post_nodes_chain.clear(); - - aiMatrix4x4 new_abs_transform = parent->mTransformation; - std::string node_name = FixNodeName(model->Name()); - // even though there is only a single input node, the design of - // assimp (or rather: the complicated transformation chain that - // is employed by fbx) means that we may need multiple aiNode's - // to represent a fbx node's transformation. - - // generate node transforms - this includes pivot data - // if need_additional_node is true then you t - const bool need_additional_node = GenerateTransformationNodeChain(*model, node_name, nodes_chain, post_nodes_chain); - - // assert that for the current node we must have at least a single transform - ai_assert(nodes_chain.size()); - - if (need_additional_node) { - nodes_chain.push_back(new aiNode(node_name)); + if (last_parent != parent) { + last_parent->mNumChildren = 1; + last_parent->mChildren = new aiNode *[1]; + last_parent->mChildren[0] = child.mOwnership.release(); } - //setup metadata on newest node - SetupNodeMetadata(*model, *nodes_chain.back()); + child->mParent = last_parent; + last_parent = child.mNode; - // link all nodes in a row - aiNode *last_parent = parent; - for (aiNode *child : nodes_chain) { - ai_assert(child); + new_abs_transform *= child->mTransformation; + } + + // attach geometry + ConvertModel(*model, nodes_chain.back().mNode, root_node, new_abs_transform); + + // check if there will be any child nodes + const std::vector &child_conns = doc.GetConnectionsByDestinationSequenced(model->ID(), "Model"); + + // if so, link the geometric transform inverse nodes + // before we attach any child nodes + if (child_conns.size()) { + for (PotentialNode& postnode : post_nodes_chain) { + ai_assert(postnode.mNode); if (last_parent != parent) { last_parent->mNumChildren = 1; last_parent->mChildren = new aiNode *[1]; - last_parent->mChildren[0] = child; + last_parent->mChildren[0] = postnode.mOwnership.release(); } - child->mParent = last_parent; - last_parent = child; + postnode->mParent = last_parent; + last_parent = postnode.mNode; - new_abs_transform *= child->mTransformation; + new_abs_transform *= postnode->mTransformation; } - - // attach geometry - ConvertModel(*model, nodes_chain.back(), root_node, new_abs_transform); - - // check if there will be any child nodes - const std::vector &child_conns = doc.GetConnectionsByDestinationSequenced(model->ID(), "Model"); - - // if so, link the geometric transform inverse nodes - // before we attach any child nodes - if (child_conns.size()) { - for (aiNode *postnode : post_nodes_chain) { - ai_assert(postnode); - - if (last_parent != parent) { - last_parent->mNumChildren = 1; - last_parent->mChildren = new aiNode *[1]; - last_parent->mChildren[0] = postnode; - } - - postnode->mParent = last_parent; - last_parent = postnode; - - new_abs_transform *= postnode->mTransformation; - } - } else { - // free the nodes we allocated as we don't need them - Util::delete_fun deleter; - std::for_each( - post_nodes_chain.begin(), - post_nodes_chain.end(), - deleter); - } - - // recursion call - child nodes - ConvertNodes(model->ID(), last_parent, root_node); - - if (doc.Settings().readLights) { - ConvertLights(*model, node_name); - } - - if (doc.Settings().readCameras) { - ConvertCameras(*model, node_name); - } - - nodes.push_back(nodes_chain.front()); - nodes_chain.clear(); + } else { + // free the nodes we allocated as we don't need them + post_nodes_chain.clear(); } + + // recursion call - child nodes + ConvertNodes(model->ID(), last_parent, root_node); + + if (doc.Settings().readLights) { + ConvertLights(*model, node_name); + } + + if (doc.Settings().readCameras) { + ConvertCameras(*model, node_name); + } + + nodes.push_back(std::move(nodes_chain.front())); + nodes_chain.clear(); } + } - if (nodes.size()) { - parent->mChildren = new aiNode *[nodes.size()](); - parent->mNumChildren = static_cast(nodes.size()); + if (nodes.size()) { + parent->mChildren = new aiNode *[nodes.size()](); + parent->mNumChildren = static_cast(nodes.size()); - std::swap_ranges(nodes.begin(), nodes.end(), parent->mChildren); - } else { - parent->mNumChildren = 0; - parent->mChildren = nullptr; + for (int i = 0; i < nodes.size(); ++i) + { + parent->mChildren[i] = nodes[i].mOwnership.release(); } - - } catch (std::exception &) { - Util::delete_fun deleter; - std::for_each(nodes.begin(), nodes.end(), deleter); - std::for_each(nodes_chain.begin(), nodes_chain.end(), deleter); - std::for_each(post_nodes_chain.begin(), post_nodes_chain.end(), deleter); + nodes.clear(); + } else { + parent->mNumChildren = 0; + parent->mChildren = nullptr; } } @@ -681,8 +683,8 @@ std::string FBXConverter::NameTransformationChainNode(const std::string &name, T return name + std::string(MAGIC_NODE_TAG) + "_" + NameTransformationComp(comp); } -bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std::string &name, std::vector &output_nodes, - std::vector &post_output_nodes) { +bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std::string &name, std::vector &output_nodes, + std::vector &post_output_nodes) { const PropertyTable &props = model.Props(); const Model::RotOrder rot = model.RotationOrder(); @@ -828,7 +830,7 @@ bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std chain[i] = chain[i].Inverse(); } - aiNode *nd = new aiNode(); + PotentialNode nd(std::unique_ptr(new aiNode())); nd->mName.Set(NameTransformationChainNode(name, comp)); nd->mTransformation = chain[i]; @@ -836,9 +838,9 @@ bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std if (comp == TransformationComp_GeometricScalingInverse || comp == TransformationComp_GeometricRotationInverse || comp == TransformationComp_GeometricTranslationInverse) { - post_output_nodes.push_back(nd); + post_output_nodes.emplace_back(std::move(nd)); } else { - output_nodes.push_back(nd); + output_nodes.emplace_back(std::move(nd)); } } @@ -847,8 +849,7 @@ bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std } // else, we can just multiply the matrices together - aiNode *nd = new aiNode(); - output_nodes.push_back(nd); + PotentialNode nd(std::unique_ptr(new aiNode())); // name passed to the method is already unique nd->mName.Set(name); @@ -857,6 +858,7 @@ bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std for (unsigned int i = TransformationComp_Translation; i < TransformationComp_MAXIMUM; i++) { nd->mTransformation = nd->mTransformation * chain[i]; } + output_nodes.push_back(std::move(nd)); return false; } diff --git a/code/AssetLib/FBX/FBXConverter.h b/code/AssetLib/FBX/FBXConverter.h index 7db4b795b..5c73663d1 100644 --- a/code/AssetLib/FBX/FBXConverter.h +++ b/code/AssetLib/FBX/FBXConverter.h @@ -171,9 +171,10 @@ private: // ------------------------------------------------------------------------------------------------ /** - * note: memory for output_nodes will be managed by the caller + * note: memory for output_nodes is managed by the caller, via the PotentialNode struct. */ - bool GenerateTransformationNodeChain(const Model& model, const std::string& name, std::vector& output_nodes, std::vector& post_output_nodes); + struct PotentialNode; + bool GenerateTransformationNodeChain(const Model& model, const std::string& name, std::vector& output_nodes, std::vector& post_output_nodes); // ------------------------------------------------------------------------------------------------ void SetupNodeMetadata(const Model& model, aiNode& nd); From c00153089a351ff097735475bf3fe701ba34a3d0 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Tue, 17 Nov 2020 10:45:10 +0000 Subject: [PATCH 02/12] Neater construction --- code/AssetLib/FBX/FBXConverter.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/code/AssetLib/FBX/FBXConverter.cpp b/code/AssetLib/FBX/FBXConverter.cpp index 90082172d..fb625599d 100644 --- a/code/AssetLib/FBX/FBXConverter.cpp +++ b/code/AssetLib/FBX/FBXConverter.cpp @@ -189,10 +189,11 @@ std::string FBXConverter::MakeUniqueNodeName(const Model *const model, const aiN /// When a node becomes a child of another node, that node becomes its owner and mOwnership should be released. struct FBXConverter::PotentialNode { - PotentialNode(std::unique_ptr uptr) : mNode(uptr.get()), mOwnership(std::move(uptr)) {} + PotentialNode() : mOwnership(new aiNode), mNode(mOwnership.get()) {} + PotentialNode(const std::string& name) : mOwnership(new aiNode(name)), mNode(mOwnership.get()) {} aiNode* operator->() { return mNode; } - aiNode* mNode; std::unique_ptr mOwnership; + aiNode* mNode; }; /// todo: pre-build node hierarchy @@ -246,7 +247,7 @@ void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node) ai_assert(nodes_chain.size()); if (need_additional_node) { - nodes_chain.emplace_back(PotentialNode(std::unique_ptr(new aiNode(node_name)))); + nodes_chain.emplace_back(PotentialNode(node_name)); } //setup metadata on newest node @@ -830,7 +831,7 @@ bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std chain[i] = chain[i].Inverse(); } - PotentialNode nd(std::unique_ptr(new aiNode())); + PotentialNode nd; nd->mName.Set(NameTransformationChainNode(name, comp)); nd->mTransformation = chain[i]; @@ -849,7 +850,7 @@ bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std } // else, we can just multiply the matrices together - PotentialNode nd(std::unique_ptr(new aiNode())); + PotentialNode nd; // name passed to the method is already unique nd->mName.Set(name); From 885a196c74cdfac3c0f12f8855270218d3774bde Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Thu, 19 Nov 2020 16:30:44 +0000 Subject: [PATCH 03/12] Unsigned --- code/AssetLib/FBX/FBXConverter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/FBX/FBXConverter.cpp b/code/AssetLib/FBX/FBXConverter.cpp index fb625599d..f8395543f 100644 --- a/code/AssetLib/FBX/FBXConverter.cpp +++ b/code/AssetLib/FBX/FBXConverter.cpp @@ -318,7 +318,7 @@ void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node) parent->mChildren = new aiNode *[nodes.size()](); parent->mNumChildren = static_cast(nodes.size()); - for (int i = 0; i < nodes.size(); ++i) + for (unsigned int i = 0; i < nodes.size(); ++i) { parent->mChildren[i] = nodes[i].mOwnership.release(); } From 4d4a3c42f6ad17ba63ed55b29525b3d968de82a4 Mon Sep 17 00:00:00 2001 From: Oleg Bogdanov Date: Sat, 28 Nov 2020 10:01:50 -0800 Subject: [PATCH 04/12] Fix for 3489 | Preserve morph targets when splitting by bone count --- .../SplitByBoneCountProcess.cpp | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/code/PostProcessing/SplitByBoneCountProcess.cpp b/code/PostProcessing/SplitByBoneCountProcess.cpp index 1fd26c757..b39444859 100644 --- a/code/PostProcessing/SplitByBoneCountProcess.cpp +++ b/code/PostProcessing/SplitByBoneCountProcess.cpp @@ -408,6 +408,45 @@ void SplitByBoneCountProcess::SplitMesh( const aiMesh* pMesh, std::vectormNumAnimMeshes > 0) { + newMesh->mNumAnimMeshes = pMesh->mNumAnimMeshes; + newMesh->mAnimMeshes = new aiAnimMesh*[newMesh->mNumAnimMeshes]; + + for (unsigned int morphIdx = 0; morphIdx < newMesh->mNumAnimMeshes; ++morphIdx) { + aiAnimMesh* origTarget = pMesh->mAnimMeshes[morphIdx]; + aiAnimMesh* newTarget = new aiAnimMesh; + newTarget->mName = origTarget->mName; + newTarget->mWeight = origTarget->mWeight; + newTarget->mNumVertices = numSubMeshVertices; + newTarget->mVertices = new aiVector3D[numSubMeshVertices]; + newMesh->mAnimMeshes[morphIdx] = newTarget; + + if (origTarget->HasNormals()) { + newTarget->mNormals = new aiVector3D[numSubMeshVertices]; + } + + if (origTarget->HasTangentsAndBitangents()) { + newTarget->mTangents = new aiVector3D[numSubMeshVertices]; + newTarget->mBitangents = new aiVector3D[numSubMeshVertices]; + } + + for( unsigned int vi = 0; vi < numSubMeshVertices; ++vi) { + // find the source vertex for it in the source mesh + unsigned int previousIndex = previousVertexIndices[vi]; + newTarget->mVertices[vi] = origTarget->mVertices[previousIndex]; + + if (newTarget->HasNormals()) { + newTarget->mNormals[vi] = origTarget->mNormals[previousIndex]; + } + if (newTarget->HasTangentsAndBitangents()) { + newTarget->mTangents[vi] = origTarget->mTangents[previousIndex]; + newTarget->mBitangents[vi] = origTarget->mBitangents[previousIndex]; + } + } + } + } + // I have the strange feeling that this will break apart at some point in time... } } From 6ed1488e6e6e05b7424245548b32b7efdb125291 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Mon, 30 Nov 2020 15:04:06 +0000 Subject: [PATCH 05/12] * Improved error messages * Throw instead of asserts on invalid file input --- code/AssetLib/glTF/glTFCommon.cpp | 4 +++- code/AssetLib/glTF/glTFCommon.h | 5 ++++- code/AssetLib/glTF2/glTF2Asset.h | 8 ++++++++ code/AssetLib/glTF2/glTF2Asset.inl | 22 +++++++++------------- code/AssetLib/glTF2/glTF2Importer.cpp | 5 ++++- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/code/AssetLib/glTF/glTFCommon.cpp b/code/AssetLib/glTF/glTFCommon.cpp index 6bea18a0a..01ba31209 100644 --- a/code/AssetLib/glTF/glTFCommon.cpp +++ b/code/AssetLib/glTF/glTFCommon.cpp @@ -49,7 +49,9 @@ using namespace glTFCommon::Util; namespace Util { size_t DecodeBase64(const char *in, size_t inLength, uint8_t *&out) { - ai_assert(inLength % 4 == 0); + if (inLength % 4 != 0) { + throw DeadlyImportError("Invalid base64 encoded data: \"", std::string(in, std::min(size_t(32), inLength)), "\", length:", inLength); + } if (inLength < 4) { out = 0; diff --git a/code/AssetLib/glTF/glTFCommon.h b/code/AssetLib/glTF/glTFCommon.h index 977fc0da4..bcb319c47 100644 --- a/code/AssetLib/glTF/glTFCommon.h +++ b/code/AssetLib/glTF/glTFCommon.h @@ -249,7 +249,10 @@ inline char EncodeCharBase64(uint8_t b) { } inline uint8_t DecodeCharBase64(char c) { - return DATA::tableDecodeBase64[size_t(c)]; // TODO faster with lookup table or ifs? + if (c & 0xF0) { + throw DeadlyImportError("Invalid base64 char value: ", size_t(c)); + } + return DATA::tableDecodeBase64[size_t(c & 0x0F)]; // TODO faster with lookup table or ifs? } size_t DecodeBase64(const char *in, size_t inLength, uint8_t *&out); diff --git a/code/AssetLib/glTF2/glTF2Asset.h b/code/AssetLib/glTF2/glTF2Asset.h index 23c19cc58..be149f86e 100644 --- a/code/AssetLib/glTF2/glTF2Asset.h +++ b/code/AssetLib/glTF2/glTF2Asset.h @@ -1124,6 +1124,14 @@ private: IOStream *OpenFile(std::string path, const char *mode, bool absolute = false); }; +inline std::string getContextForErrorMessages(const std::string &id, const std::string &name) { + std::string context = id; + if (!name.empty()) { + context += " (\"" + name + "\")"; + } + return context; +} + } // namespace glTF2 // Include the implementation of the methods diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index c892e6697..0c6ce6e6d 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -273,17 +273,21 @@ Ref LazyDict::Retrieve(unsigned int i) { } if (!mDict->IsArray()) { - throw DeadlyImportError("GLTF: Field is not an array \"", mDictId, "\""); + throw DeadlyImportError("GLTF: Field \"", mDictId, "\" is not an array"); + } + + if (i >= mDict->Size()) { + throw DeadlyImportError("GLTF: Array index ", i, " is out of bounds (", mDict->Size(), ") for \"", mDictId, "\""); } Value &obj = (*mDict)[i]; if (!obj.IsObject()) { - throw DeadlyImportError("GLTF: Object at index \"", to_string(i), "\" is not a JSON object"); + throw DeadlyImportError("GLTF: Object at index ", to_string(i), " is not a JSON object"); } if (mRecursiveReferenceCheck.find(i) != mRecursiveReferenceCheck.end()) { - throw DeadlyImportError("GLTF: Object at index \"", to_string(i), "\" has recursive reference to itself"); + throw DeadlyImportError("GLTF: Object at index ", to_string(i), " has recursive reference to itself"); } mRecursiveReferenceCheck.insert(i); @@ -741,14 +745,6 @@ inline void CopyData(size_t count, } } -inline std::string getContextForErrorMessages(const std::string& id, const std::string& name) { - std::string context = id; - if (!name.empty()) { - context += " (\"" + name + "\")"; - } - return context; -} - } // namespace template @@ -766,11 +762,11 @@ void Accessor::ExtractData(T *&outData) { const size_t targetElemSize = sizeof(T); if (elemSize > targetElemSize) { - throw DeadlyImportError("GLTF: elemSize > targetElemSize"); + throw DeadlyImportError("GLTF: elemSize ", elemSize, " > targetElemSize ", targetElemSize, " in ", getContextForErrorMessages(id, name)); } if (count*stride > (bufferView ? bufferView->byteLength : sparse->data.size())) { - throw DeadlyImportError("GLTF: count*stride out of range"); + throw DeadlyImportError("GLTF: count*stride ", (count * stride), " > bufferView->byteLength ", bufferView->byteLength, " in ", getContextForErrorMessages(id, name)); } outData = new T[count]; diff --git a/code/AssetLib/glTF2/glTF2Importer.cpp b/code/AssetLib/glTF2/glTF2Importer.cpp index 3fb7889af..672bac52d 100644 --- a/code/AssetLib/glTF2/glTF2Importer.cpp +++ b/code/AssetLib/glTF2/glTF2Importer.cpp @@ -918,7 +918,10 @@ aiNode *ImportNode(aiScene *pScene, glTF2::Asset &r, std::vector & if (!node.meshes.empty()) { // GLTF files contain at most 1 mesh per node. - assert(node.meshes.size() == 1); + if (node.meshes.size() > 1) + { + throw DeadlyImportError("GLTF: Invalid input, found ", node.meshes.size(), " meshes in ", getContextForErrorMessages(node.id, node.name), ", but only 1 mesh per node allowed."); + } int mesh_idx = node.meshes[0].GetIndex(); int count = meshOffsets[mesh_idx + 1] - meshOffsets[mesh_idx]; From 7d72c78c8e2c8333a46094a4385c85752a06a491 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Mon, 30 Nov 2020 15:20:51 +0000 Subject: [PATCH 06/12] Some improvements --- code/AssetLib/glTF2/glTF2Asset.inl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/code/AssetLib/glTF2/glTF2Asset.inl b/code/AssetLib/glTF2/glTF2Asset.inl index 0c6ce6e6d..8fc8bf24e 100644 --- a/code/AssetLib/glTF2/glTF2Asset.inl +++ b/code/AssetLib/glTF2/glTF2Asset.inl @@ -283,11 +283,11 @@ Ref LazyDict::Retrieve(unsigned int i) { Value &obj = (*mDict)[i]; if (!obj.IsObject()) { - throw DeadlyImportError("GLTF: Object at index ", to_string(i), " is not a JSON object"); + throw DeadlyImportError("GLTF: Object at index ", i, " in array \"", mDictId, "\" is not a JSON object"); } if (mRecursiveReferenceCheck.find(i) != mRecursiveReferenceCheck.end()) { - throw DeadlyImportError("GLTF: Object at index ", to_string(i), " has recursive reference to itself"); + throw DeadlyImportError("GLTF: Object at index ", i, " in array \"", mDictId, "\" has recursive reference to itself"); } mRecursiveReferenceCheck.insert(i); @@ -765,8 +765,9 @@ void Accessor::ExtractData(T *&outData) { throw DeadlyImportError("GLTF: elemSize ", elemSize, " > targetElemSize ", targetElemSize, " in ", getContextForErrorMessages(id, name)); } - if (count*stride > (bufferView ? bufferView->byteLength : sparse->data.size())) { - throw DeadlyImportError("GLTF: count*stride ", (count * stride), " > bufferView->byteLength ", bufferView->byteLength, " in ", getContextForErrorMessages(id, name)); + const size_t maxSize = (bufferView ? bufferView->byteLength : sparse->data.size()); + if (count*stride > maxSize) { + throw DeadlyImportError("GLTF: count*stride ", (count * stride), " > maxSize ", maxSize, " in ", getContextForErrorMessages(id, name)); } outData = new T[count]; From 53ff0702ce08d0f9f63b885eebdf2cbf1b516490 Mon Sep 17 00:00:00 2001 From: Max Vollmer Date: Mon, 30 Nov 2020 16:21:29 +0000 Subject: [PATCH 07/12] Fixed check for base64 char values --- code/AssetLib/glTF/glTFCommon.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/AssetLib/glTF/glTFCommon.h b/code/AssetLib/glTF/glTFCommon.h index bcb319c47..6d402b0e3 100644 --- a/code/AssetLib/glTF/glTFCommon.h +++ b/code/AssetLib/glTF/glTFCommon.h @@ -249,10 +249,10 @@ inline char EncodeCharBase64(uint8_t b) { } inline uint8_t DecodeCharBase64(char c) { - if (c & 0xF0) { + if (c & 0x80) { throw DeadlyImportError("Invalid base64 char value: ", size_t(c)); } - return DATA::tableDecodeBase64[size_t(c & 0x0F)]; // TODO faster with lookup table or ifs? + return DATA::tableDecodeBase64[size_t(c & 0x7F)]; // TODO faster with lookup table or ifs? } size_t DecodeBase64(const char *in, size_t inLength, uint8_t *&out); From b47c5c8c27b0fc3174eb4a9f30d86bdfeb962097 Mon Sep 17 00:00:00 2001 From: FRICOTEAUX Date: Mon, 30 Nov 2020 20:11:41 +0100 Subject: [PATCH 08/12] glTF2Exporter: fix crash when exporting a scene with several meshes per node (regression coming from commit e8d2b84 on 08/03/2020 "clean all warnings for vs-2019") --- code/AssetLib/glTF2/glTF2Exporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/AssetLib/glTF2/glTF2Exporter.cpp b/code/AssetLib/glTF2/glTF2Exporter.cpp index 872543ef2..637808877 100644 --- a/code/AssetLib/glTF2/glTF2Exporter.cpp +++ b/code/AssetLib/glTF2/glTF2Exporter.cpp @@ -1126,7 +1126,7 @@ void glTF2Exporter::MergeMeshes() unsigned int meshIndex = meshRef.GetIndex(); if (meshIndex == removedIndex) { - node->meshes.erase(curNode->meshes.begin() + mm); + curNode->meshes.erase(curNode->meshes.begin() + mm); } else if (meshIndex > removedIndex) { Ref newMeshRef = mAsset->meshes.Get(meshIndex - 1); From 542ca843befbbb8b094a2274d944dd195e065067 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Tue, 1 Dec 2020 16:38:18 +0100 Subject: [PATCH 09/12] Update Readme.md --- Readme.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Readme.md b/Readme.md index 87d5b40d8..c6212bcc0 100644 --- a/Readme.md +++ b/Readme.md @@ -16,13 +16,16 @@ A library to import and export various 3d-model-formats including scene-post-pro
APIs are provided for C and C++. There are various bindings to other languages (C#, Java, Python, Delphi, D). Assimp also runs on Android and iOS. - -[Check the latest doc](https://assimp-docs.readthedocs.io/en/latest/). - Additionally, assimp features various __mesh post processing tools__: normals and tangent space generation, triangulation, vertex cache locality optimization, removal of degenerate primitives and duplicate vertices, sorting by primitive type, merging of redundant materials and many more. -This is the development repo containing the latest features and bugfixes. For productive use though, we recommend one of the stable releases available from [Github Assimp Releases](https://github.com/assimp/assimp/releases). +### Latest Doc's ### +Please check the latest documents at [Asset-Importer-Lib-Doc](https://assimp-docs.readthedocs.io/en/latest/). +### Get involved ### +This is the development repo containing the latest features and bugfixes. For productive use though, we recommend one of the stable releases available from [Github Assimp Releases](https://github.com/assimp/assimp/releases). +
+You find a bug in the docs? Use [Doc-Repo](https://github.com/assimp/assimp-docs). +
Please check our Wiki as well: https://github.com/assimp/assimp/wiki If you want to check our Model-Database, use the following repo: https://github.com/assimp/assimp-mdb From 8d8ba521e2094d0da89f7446f3027c899a5e6be4 Mon Sep 17 00:00:00 2001 From: Biswapriyo Nath Date: Wed, 2 Dec 2020 02:05:14 +0530 Subject: [PATCH 10/12] Check _MSC_VER for MSVC specific pragma directives. This fixes build issues when compiled with compiles like Clang or GCC. --- code/AssetLib/Assjson/cencode.c | 14 +++++++++----- contrib/Open3DGC/o3dgcSC3DMCDecoder.inl | 8 ++++---- contrib/Open3DGC/o3dgcSC3DMCEncoder.inl | 8 ++++---- contrib/Open3DGC/o3dgcTimer.h | 2 ++ contrib/zip/src/zip.c | 4 ++++ include/assimp/mesh.h | 12 ++++++------ .../SimpleTexturedDirectx11/TextureLoader.cpp | 6 ++++++ .../SimpleTexturedDirectx11/TextureLoader.h | 6 ++++++ .../SimpleTexturedDirectx11/main.cpp | 2 ++ .../SimpleTexturedOpenGL/src/model_loading.cpp | 6 ++++++ 10 files changed, 49 insertions(+), 19 deletions(-) diff --git a/code/AssetLib/Assjson/cencode.c b/code/AssetLib/Assjson/cencode.c index 0ed979b01..614a2671f 100644 --- a/code/AssetLib/Assjson/cencode.c +++ b/code/AssetLib/Assjson/cencode.c @@ -9,8 +9,10 @@ For details, see http://sourceforge.net/projects/libb64 const int CHARS_PER_LINE = 72; +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable : 4244) +#endif // _MSC_VER void base64_init_encodestate(base64_encodestate* state_in) { @@ -33,9 +35,9 @@ int base64_encode_block(const char* plaintext_in, int length_in, char* code_out, char* codechar = code_out; char result; char fragment; - + result = state_in->result; - + switch (state_in->step) { while (1) @@ -74,7 +76,7 @@ int base64_encode_block(const char* plaintext_in, int length_in, char* code_out, *codechar++ = base64_encode_value(result); result = (fragment & 0x03f) >> 0; *codechar++ = base64_encode_value(result); - + ++(state_in->stepcount); if (state_in->stepcount == CHARS_PER_LINE/4) { @@ -90,7 +92,7 @@ int base64_encode_block(const char* plaintext_in, int length_in, char* code_out, int base64_encode_blockend(char* code_out, base64_encodestate* state_in) { char* codechar = code_out; - + switch (state_in->step) { case step_B: @@ -106,8 +108,10 @@ int base64_encode_blockend(char* code_out, base64_encodestate* state_in) break; } *codechar++ = '\n'; - + return (int)(codechar - code_out); } +#ifdef _MSC_VER #pragma warning(pop) +#endif // _MSC_VER diff --git a/contrib/Open3DGC/o3dgcSC3DMCDecoder.inl b/contrib/Open3DGC/o3dgcSC3DMCDecoder.inl index c965fcd50..8bd85f27b 100644 --- a/contrib/Open3DGC/o3dgcSC3DMCDecoder.inl +++ b/contrib/Open3DGC/o3dgcSC3DMCDecoder.inl @@ -27,10 +27,10 @@ THE SOFTWARE. #include "o3dgcArithmeticCodec.h" #include "o3dgcTimer.h" -#ifdef _WIN32 +#ifdef _MSC_VER # pragma warning(push) # pragma warning( disable : 4456) -#endif // _WIN32 +#endif // _MSC_VER //#define DEBUG_VERBOSE @@ -852,9 +852,9 @@ namespace o3dgc } } // namespace o3dgc -#ifdef _WIN32 +#ifdef _MSC_VER # pragma warning( pop ) -#endif // _WIN32 +#endif // _MSC_VER #endif // O3DGC_SC3DMC_DECODER_INL diff --git a/contrib/Open3DGC/o3dgcSC3DMCEncoder.inl b/contrib/Open3DGC/o3dgcSC3DMCEncoder.inl index 78d4b3e78..ca1e0ea76 100644 --- a/contrib/Open3DGC/o3dgcSC3DMCEncoder.inl +++ b/contrib/Open3DGC/o3dgcSC3DMCEncoder.inl @@ -32,10 +32,10 @@ THE SOFTWARE. //#define DEBUG_VERBOSE -#ifdef _WIN32 +#ifdef _MSC_VER # pragma warning(push) # pragma warning(disable : 4456) -#endif // _WIN32 +#endif // _MSC_VER namespace o3dgc { @@ -927,9 +927,9 @@ namespace o3dgc } } // namespace o3dgc -#ifdef _WIN32 +#ifdef _MSC_VER # pragma warning(pop) -#endif // _WIN32 +#endif // _MSC_VER #endif // O3DGC_SC3DMC_ENCODER_INL diff --git a/contrib/Open3DGC/o3dgcTimer.h b/contrib/Open3DGC/o3dgcTimer.h index d9a285968..f5ed0c83f 100644 --- a/contrib/Open3DGC/o3dgcTimer.h +++ b/contrib/Open3DGC/o3dgcTimer.h @@ -28,7 +28,9 @@ THE SOFTWARE. #ifdef _WIN32 /* Thank you, Microsoft, for file WinDef.h with min/max redefinition. */ +#ifndef NOMINMAX #define NOMINMAX +#endif #include #elif __APPLE__ #include diff --git a/contrib/zip/src/zip.c b/contrib/zip/src/zip.c index 0020df0ed..83e8e2a41 100644 --- a/contrib/zip/src/zip.c +++ b/contrib/zip/src/zip.c @@ -18,8 +18,10 @@ /* Win32, DOS, MSVC, MSVS */ #include +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable : 4706) +#endif // _MSC_VER #define MKDIR(DIRNAME) _mkdir(DIRNAME) #define STRCLONE(STR) ((STR) ? _strdup(STR) : NULL) @@ -968,4 +970,6 @@ out: return status; } +#ifdef _MSC_VER #pragma warning(pop) +#endif // _MSC_VER diff --git a/include/assimp/mesh.h b/include/assimp/mesh.h index 4b5af97ce..ec1062837 100644 --- a/include/assimp/mesh.h +++ b/include/assimp/mesh.h @@ -52,9 +52,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #pragma GCC system_header #endif -#ifdef _WIN32 +#ifdef _MSC_VER #pragma warning(disable : 4351) -#endif // _WIN32 +#endif // _MSC_VER #include #include @@ -458,8 +458,8 @@ struct aiAnimMesh { */ unsigned int mNumVertices; - /** - * Weight of the AnimMesh. + /** + * Weight of the AnimMesh. */ float mWeight; @@ -713,8 +713,8 @@ struct aiMesh { * Note! Currently only works with Collada loader.*/ C_STRUCT aiAnimMesh **mAnimMeshes; - /** - * Method of morphing when animeshes are specified. + /** + * Method of morphing when animeshes are specified. */ unsigned int mMethod; diff --git a/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.cpp b/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.cpp index c3269d290..01ba343e8 100644 --- a/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.cpp +++ b/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.cpp @@ -31,10 +31,16 @@ #include #include +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable : 4005) +#endif // _MSC_VER + #include + +#ifdef _MSC_VER #pragma warning(pop) +#endif // _MSC_VER #include diff --git a/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.h b/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.h index c2e0b5214..5448daf2a 100644 --- a/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.h +++ b/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/TextureLoader.h @@ -31,10 +31,16 @@ #include +#ifdef _MSC_VER #pragma warning(push) #pragma warning(disable : 4005) +#endif // _MSC_VER + #include + +#ifdef _MSC_VER #pragma warning(pop) +#endif // _MSC_VER HRESULT CreateWICTextureFromMemory(_In_ ID3D11Device* d3dDevice, _In_opt_ ID3D11DeviceContext* d3dContext, diff --git a/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/main.cpp b/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/main.cpp index 5cce7c376..02e2b6088 100644 --- a/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/main.cpp +++ b/samples/SimpleTexturedDirectx11/SimpleTexturedDirectx11/main.cpp @@ -25,10 +25,12 @@ #include "UTFConverter.h" #include "SafeRelease.hpp" +#ifdef _MSC_VER #pragma comment (lib, "d3d11.lib") #pragma comment (lib, "Dxgi.lib") #pragma comment(lib,"d3dcompiler.lib") #pragma comment (lib, "dxguid.lib") +#endif // _MSC_VER using namespace DirectX; using namespace AssimpSamples::SharedCode; diff --git a/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp b/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp index 806da8f8a..a36c792d4 100644 --- a/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp +++ b/samples/SimpleTexturedOpenGL/SimpleTexturedOpenGL/src/model_loading.cpp @@ -18,10 +18,16 @@ #include #include +#ifdef _MSC_VER #pragma warning(disable: 4100) // Disable warning 'unreferenced formal parameter' +#endif // _MSC_VER + #define STB_IMAGE_IMPLEMENTATION #include "contrib/stb_image/stb_image.h" + +#ifdef _MSC_VER #pragma warning(default: 4100) // Enable warning 'unreferenced formal parameter' +#endif // _MSC_VER #include From 1910bbbdd9c91ee69708b129db5861dd1ac50736 Mon Sep 17 00:00:00 2001 From: huiji12321 <15117966074@163.com> Date: Fri, 4 Dec 2020 16:47:43 +0800 Subject: [PATCH 11/12] Additional Compiler Options for mips64el --- CMakeLists.txt | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 102783ba3..b739674af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -249,9 +249,15 @@ IF ((CMAKE_C_COMPILER_ID MATCHES "GNU") AND NOT CMAKE_COMPILER_IS_MINGW) SET(CMAKE_POSITION_INDEPENDENT_CODE ON) ENDIF() # hide all not-exported symbols - 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++) + 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++) + 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++) + ENDIF() ELSEIF(MSVC) # enable multi-core compilation with MSVC IF( "${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" ) # clang-cl From 6db5b7046f816474614ba5cd6e0f38c4287df730 Mon Sep 17 00:00:00 2001 From: tellypresence Date: Wed, 9 Dec 2020 20:21:43 +0900 Subject: [PATCH 12/12] Provide option for rapidjson no-member-iterator define Originally added to fix warning on MSVC but breaks android build --- code/CMakeLists.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index a0bad4e10..bb83a5f97 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -1033,7 +1033,10 @@ ELSE() INCLUDE_DIRECTORIES( "../contrib" ) INCLUDE_DIRECTORIES( "../contrib/pugixml/src" ) ADD_DEFINITIONS( -DRAPIDJSON_HAS_STDSTRING=1 ) - ADD_DEFINITIONS( -DRAPIDJSON_NOMEMBERITERATORCLASS ) + option( ASSIMP_RAPIDJSON_NO_MEMBER_ITERATOR "Suppress rapidjson warning on MSVC (NOTE: breaks android build)" ON ) + if(ASSIMP_RAPIDJSON_NO_MEMBER_ITERATOR) + ADD_DEFINITIONS( -DRAPIDJSON_NOMEMBERITERATORCLASS ) + endif() ENDIF() # VC2010 fixes