First pass at PotentialNode

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<aiNode> uptr) : mNode(uptr.get()), mOwnership(std::move(uptr)) {}
+    aiNode* operator->() { return mNode; }
+    aiNode* mNode;
+    std::unique_ptr<aiNode> mOwnership;
+};
+
 /// todo: pre-build node hierarchy
 /// todo: get bone from stack
 /// todo: make map of aiBone* to aiNode*
@@ -192,13 +202,12 @@ std::string FBXConverter::MakeUniqueNodeName(const Model *const model, const aiN
 void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node) {
     const std::vector<const Connection *> &conns = doc.GetConnectionsByDestinationSequenced(id, "Model");
 
-    std::vector<aiNode *> nodes;
+    std::vector<PotentialNode> nodes;
     nodes.reserve(conns.size());
 
-    std::vector<aiNode *> nodes_chain;
+    std::vector<PotentialNode> nodes_chain;
-    std::vector<aiNode *> post_nodes_chain;
+    std::vector<PotentialNode> post_nodes_chain;
 
-    try {
     for (const Connection *con : conns) {
         // ignore object-property links
         if (con->PropertyName().length()) {
@@ -237,31 +246,31 @@ void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node)
             ai_assert(nodes_chain.size());
 
             if (need_additional_node) {
-                    nodes_chain.push_back(new aiNode(node_name));
+                nodes_chain.emplace_back(PotentialNode(std::unique_ptr<aiNode>(new aiNode(node_name))));
             }
 
             //setup metadata on newest node
-                SetupNodeMetadata(*model, *nodes_chain.back());
+            SetupNodeMetadata(*model, *nodes_chain.back().mNode);
 
             // link all nodes in a row
             aiNode *last_parent = parent;
-                for (aiNode *child : nodes_chain) {
+            for (PotentialNode& child : nodes_chain) {
-                    ai_assert(child);
+                ai_assert(child.mNode);
 
                 if (last_parent != parent) {
                     last_parent->mNumChildren = 1;
                     last_parent->mChildren = new aiNode *[1];
-                        last_parent->mChildren[0] = child;
+                    last_parent->mChildren[0] = child.mOwnership.release();
                 }
 
                 child->mParent = last_parent;
-                    last_parent = child;
+                last_parent = child.mNode;
 
                 new_abs_transform *= child->mTransformation;
             }
 
             // attach geometry
-                ConvertModel(*model, nodes_chain.back(), root_node, new_abs_transform);
+            ConvertModel(*model, nodes_chain.back().mNode, root_node, new_abs_transform);
 
             // check if there will be any child nodes
             const std::vector<const Connection *> &child_conns = doc.GetConnectionsByDestinationSequenced(model->ID(), "Model");
@@ -269,27 +278,23 @@ void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node)
             // if so, link the geometric transform inverse nodes
             // before we attach any child nodes
             if (child_conns.size()) {
-                    for (aiNode *postnode : post_nodes_chain) {
+                for (PotentialNode& postnode : post_nodes_chain) {
-                        ai_assert(postnode);
+                    ai_assert(postnode.mNode);
 
                     if (last_parent != parent) {
                         last_parent->mNumChildren = 1;
                         last_parent->mChildren = new aiNode *[1];
-                            last_parent->mChildren[0] = postnode;
+                        last_parent->mChildren[0] = postnode.mOwnership.release();
                     }
 
                     postnode->mParent = last_parent;
-                        last_parent = postnode;
+                    last_parent = postnode.mNode;
 
                     new_abs_transform *= postnode->mTransformation;
                 }
             } else {
                 // free the nodes we allocated as we don't need them
-                    Util::delete_fun<aiNode> deleter;
+                post_nodes_chain.clear();
-                    std::for_each(
-                            post_nodes_chain.begin(),
-                            post_nodes_chain.end(),
-                            deleter);
             }
 
             // recursion call - child nodes
@@ -303,7 +308,7 @@ void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node)
                 ConvertCameras(*model, node_name);
             }
 
-                nodes.push_back(nodes_chain.front());
+            nodes.push_back(std::move(nodes_chain.front()));
             nodes_chain.clear();
         }
     }
@@ -312,18 +317,15 @@ void FBXConverter::ConvertNodes(uint64_t id, aiNode *parent, aiNode *root_node)
         parent->mChildren = new aiNode *[nodes.size()]();
         parent->mNumChildren = static_cast<unsigned int>(nodes.size());
 
-            std::swap_ranges(nodes.begin(), nodes.end(), parent->mChildren);
+        for (int i = 0; i < nodes.size(); ++i)
+        {
+            parent->mChildren[i] = nodes[i].mOwnership.release();
+        }
+        nodes.clear();
     } else {
         parent->mNumChildren = 0;
         parent->mChildren = nullptr;
     }
-
-    } catch (std::exception &) {
-        Util::delete_fun<aiNode> 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);
-    }
 }
 
 void FBXConverter::ConvertLights(const Model &model, const std::string &orig_name) {
@@ -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<aiNode *> &output_nodes,
+bool FBXConverter::GenerateTransformationNodeChain(const Model &model, const std::string &name, std::vector<PotentialNode> &output_nodes,
-        std::vector<aiNode *> &post_output_nodes) {
+        std::vector<PotentialNode> &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<aiNode>(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();
+    PotentialNode nd(std::unique_ptr<aiNode>(new aiNode()));
-    output_nodes.push_back(nd);
 
     // 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;
 }
   

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<aiNode*>& output_nodes, std::vector<aiNode*>& post_output_nodes);
+    struct PotentialNode;
+    bool GenerateTransformationNodeChain(const Model& model, const std::string& name, std::vector<PotentialNode>& output_nodes, std::vector<PotentialNode>& post_output_nodes);
 
     // ------------------------------------------------------------------------------------------------
     void SetupNodeMetadata(const Model& model, aiNode& nd);