From 213a9f9d55249d0fcf09fec32ec8cc87533672b1 Mon Sep 17 00:00:00 2001 From: Malcolm Tyrrell Date: Tue, 17 Nov 2020 10:39:03 +0000 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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... } }