WIP: graph evaluations. Fixed various memory issues, better tests.

src/AnimGraph/AnimGraph.cc

@@ -105,7 +105,7 @@ void AnimGraph::prepareNodeEval(
     assert (*output_connection.m_source_socket.m_reference.ptr_ptr == nullptr);
 
     (*output_connection.m_source_socket.m_reference.ptr_ptr) =
-        m_anim_data_work_buffer.allocate(graph_context);
+        m_anim_data_allocator.allocate(graph_context.m_skeleton);
   }
 
   for (size_t i = 0, n = m_node_input_connections[node_index].size(); i < n;
@@ -132,8 +132,8 @@ void AnimGraph::finishNodeEval(size_t node_index) {
       continue;
     }
 
-    m_anim_data_work_buffer.free(static_cast<AnimData*>(
+    m_anim_data_allocator.free(static_cast<AnimData*>(
-        input_connection.m_source_socket.m_reference.ptr));
+        *input_connection.m_source_socket.m_reference.ptr_ptr));
     (*input_connection.m_source_socket.m_reference.ptr_ptr) = nullptr;
   }
 }
@@ -241,6 +241,7 @@ void AnimGraph::evaluate(AnimGraphContext& context) {
   }
 
   evalOutputNode();
+  finishNodeEval(0);
 }
 
 Socket* AnimGraph::getInputSocket(const std::string& name) {

src/AnimGraph/AnimGraph.h

@@ -8,61 +8,6 @@
 #include "AnimGraphData.h"
 #include "AnimGraphNodes.h"
 
-struct AnimDataWorkBuffer {
-  struct AnimDataList {
-    AnimData* m_anim_data = nullptr;
-    AnimDataList* next = nullptr;
-  };
-
-  AnimDataList* m_anim_data_list = nullptr;
-  size_t m_num_allocations = 0;
-
-  ~AnimDataWorkBuffer() {
-    AnimDataList* list_node = m_anim_data_list;
-    while (list_node != nullptr) {
-      AnimDataList* current_node = list_node;
-      list_node = list_node->next;
-      //delete current_node->m_anim_data;
-      current_node->m_anim_data = nullptr;
-      delete current_node;
-    }
-  }
-
-  AnimData* allocate(AnimGraphContext& graph_context) {
-    if (m_anim_data_list == nullptr) {
-      AnimData* result = new AnimData();
-      result->m_local_matrices.resize(graph_context.m_skeleton->num_soa_joints());
-      m_num_allocations++;
-      return result;
-    }
-
-    AnimData* result = m_anim_data_list->m_anim_data;
-    m_anim_data_list = m_anim_data_list->next;
-    delete m_anim_data_list;
-
-    return result;
-  }
-
-  void free(AnimData* anim_data) {
-    AnimDataList* list_node = new AnimDataList;
-    list_node->next = m_anim_data_list;
-    list_node->m_anim_data = anim_data;
-    m_anim_data_list = list_node;
-  }
-
-  size_t size() {
-    size_t result = 0;
-
-    AnimDataList* node = m_anim_data_list;
-    while (node != nullptr) {
-      result ++;
-      node = node->next;
-    }
-
-    return result;
-  }
-};
-
 //
 // AnimGraph (Runtime)
 //
@@ -80,9 +25,24 @@ struct AnimGraph {
   std::vector<Socket>& getGraphOutputs() { return m_socket_accessor->m_inputs; }
   std::vector<Socket>& getGraphInputs() { return m_socket_accessor->m_outputs; }
 
-  AnimDataWorkBuffer m_anim_data_work_buffer;
+  AnimDataAllocator m_anim_data_allocator;
 
   ~AnimGraph() {
+    std::vector<AnimGraphConnection>& graph_outputs = m_node_input_connections[0];
+
+    for (size_t i = 0, n = graph_outputs.size(); i < n; i++) {
+      AnimGraphConnection& connection = graph_outputs[i];
+      if (connection.m_target_socket.m_type == SocketType::SocketTypeAnimation) {
+        AnimData* graph_anim_output =
+            static_cast<AnimData*>(connection.m_target_socket.m_reference.ptr);
+        assert(graph_anim_output != nullptr);
+
+        // we have to explicitly call the destructor as the AnimData* was
+        // initialized using a placement new operator.
+        graph_anim_output->m_local_matrices.vector::~vector();
+      }
+    }
+
     delete[] m_input_buffer;
     delete[] m_output_buffer;
 

src/AnimGraph/AnimGraphData.h

@@ -5,17 +5,18 @@
 #ifndef ANIMTESTBED_ANIMGRAPHDATA_H
 #define ANIMTESTBED_ANIMGRAPHDATA_H
 
+#include <ozz/base/maths/soa_transform.h>
+
 #include <cstring>
 #include <iostream>
+#include <map>
 #include <string>
 #include <vector>
-#include <map>
 
 #include "SyncTrack.h"
-#include "ozz/base/containers/vector.h"
-#include <ozz/base/maths/soa_transform.h>
-#include "ozz/animation/runtime/skeleton.h"
 #include "ozz/animation/runtime/animation.h"
+#include "ozz/animation/runtime/skeleton.h"
+#include "ozz/base/containers/vector.h"
 
 //
 // Data types
@@ -23,6 +24,84 @@
 
 struct AnimGraph;
 
+struct AnimData {
+  ozz::vector<ozz::math::SoaTransform> m_local_matrices;
+};
+
+struct AnimDataAllocator {
+  struct AnimDataList {
+    AnimData* m_anim_data = nullptr;
+    AnimDataList* next = nullptr;
+  };
+
+  AnimDataList* m_anim_data_list = nullptr;
+  size_t m_num_allocations = 0;
+
+  ~AnimDataAllocator() {
+    AnimDataList* list_node = m_anim_data_list;
+    while (list_node != nullptr) {
+      AnimDataList* current_node = list_node;
+      list_node = list_node->next;
+#ifdef ANIM_DATA_ALLOCATOR_DEBUG
+      std::cout << "about to delete with size "
+                << current_node->m_anim_data->m_local_matrices.size()
+                << current_node->m_anim_data << std::endl;
+#endif
+      delete current_node->m_anim_data;
+      current_node->m_anim_data = nullptr;
+      delete current_node;
+    }
+  }
+
+  AnimData* allocate(ozz::animation::Skeleton* skeleton) {
+    if (m_anim_data_list == nullptr) {
+      AnimData* result = new AnimData();
+      result->m_local_matrices.resize(skeleton->num_soa_joints());
+#ifdef ANIM_DATA_ALLOCATOR_DEBUG
+      std::cout << "Allocated with size " << result->m_local_matrices.size()
+                << " " << result << std::endl;
+#endif
+      m_num_allocations++;
+      return result;
+    }
+
+    AnimData* result = m_anim_data_list->m_anim_data;
+    m_anim_data_list = m_anim_data_list->next;
+    delete m_anim_data_list;
+
+#ifdef ANIM_DATA_ALLOCATOR_DEBUG
+    std::cout << "Reusing buffer with size " << result->m_local_matrices.size()
+              << " " << result << std::endl;
+#endif
+
+    return result;
+  }
+
+  void free(AnimData* anim_data) {
+#ifdef ANIM_DATA_ALLOCATOR_DEBUG
+    std::cout << "Storing buffer with size " << anim_data->m_local_matrices.size()
+              << " " << anim_data << std::endl;
+#endif
+
+    AnimDataList* list_node = new AnimDataList;
+    list_node->next = m_anim_data_list;
+    list_node->m_anim_data = anim_data;
+    m_anim_data_list = list_node;
+  }
+
+  size_t size() {
+    size_t result = 0;
+
+    AnimDataList* node = m_anim_data_list;
+    while (node != nullptr) {
+      result++;
+      node = node->next;
+    }
+
+    return result;
+  }
+};
+
 struct AnimGraphContext {
   AnimGraph* m_graph = nullptr;
   ozz::animation::Skeleton* m_skeleton = nullptr;
@@ -40,10 +119,6 @@ struct AnimGraphContext {
   }
 };
 
-struct AnimData {
-  ozz::vector<ozz::math::SoaTransform> m_local_matrices;
-};
-
 typedef float Vec3[3];
 typedef float Quat[4];
 
@@ -75,12 +150,12 @@ struct Socket {
     float quat[4];
     char str[cSocketStringValueMaxLength];
   };
-  SocketValue m_value = { 0 };
+  SocketValue m_value = {0};
   union SocketReference {
     void* ptr;
     void** ptr_ptr;
   };
-  SocketReference m_reference = { 0 };
+  SocketReference m_reference = {0};
   int m_flags = 0;
   size_t m_type_size = 0;
 };
@@ -162,7 +237,7 @@ struct NodeSocketAccessorBase {
               << static_cast<int>(socket->m_type) << " ("
               << SocketTypeNames[static_cast<int>(socket->m_type)] << ")."
               << std::endl;
-//    *static_cast<T*>(socket->m_value.ptr) = value;
+    //    *static_cast<T*>(socket->m_value.ptr) = value;
   }
 
   template <typename T>
@@ -248,7 +323,7 @@ struct NodeSocketAccessorBase {
   }
 
   template <typename T>
-  bool RegisterInput(const std::string& name, T** value, int flags = 0) {
+  bool RegisterInput(const std::string& name, T* value, int flags = 0) {
     return RegisterSocket(m_inputs, name, value, flags);
   }
   template <typename T>
@@ -288,24 +363,32 @@ struct NodeSocketAccessorBase {
 // SetSocketReferenceValue<> specializations
 //
 template <>
-inline void NodeSocketAccessorBase::SetSocketReferenceValue<const bool&>(Socket* socket, const bool& value) {
+inline void NodeSocketAccessorBase::SetSocketReferenceValue<const bool&>(
+    Socket* socket,
+    const bool& value) {
   *static_cast<bool*>(socket->m_reference.ptr) = value;
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketReferenceValue<const float&>(Socket* socket, const float& value) {
+inline void NodeSocketAccessorBase::SetSocketReferenceValue<const float&>(
+    Socket* socket,
+    const float& value) {
   *static_cast<float*>(socket->m_reference.ptr) = value;
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketReferenceValue<const Vec3&>(Socket* socket, const Vec3& value) {
+inline void NodeSocketAccessorBase::SetSocketReferenceValue<const Vec3&>(
+    Socket* socket,
+    const Vec3& value) {
   static_cast<float*>(socket->m_reference.ptr)[0] = value[0];
   static_cast<float*>(socket->m_reference.ptr)[1] = value[1];
   static_cast<float*>(socket->m_reference.ptr)[2] = value[2];
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketReferenceValue<const Quat&>(Socket* socket, const Quat& value) {
+inline void NodeSocketAccessorBase::SetSocketReferenceValue<const Quat&>(
+    Socket* socket,
+    const Quat& value) {
   static_cast<float*>(socket->m_reference.ptr)[0] = value[0];
   static_cast<float*>(socket->m_reference.ptr)[1] = value[1];
   static_cast<float*>(socket->m_reference.ptr)[2] = value[2];
@@ -313,13 +396,17 @@ inline void NodeSocketAccessorBase::SetSocketReferenceValue<const Quat&>(Socket*
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketReferenceValue<const std::string&>(Socket* socket, const std::string& value) {
+inline void NodeSocketAccessorBase::SetSocketReferenceValue<const std::string&>(
+    Socket* socket,
+    const std::string& value) {
   *static_cast<std::string*>(socket->m_reference.ptr) = value;
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketReferenceValue<const char*>(Socket* socket, const char* value) {
+inline void NodeSocketAccessorBase::SetSocketReferenceValue<const char*>(
-  std::string value_string (value);
+    Socket* socket,
+    const char* value) {
+  std::string value_string(value);
   SetSocketReferenceValue<const std::string&>(socket, value_string);
 }
 
@@ -327,24 +414,32 @@ inline void NodeSocketAccessorBase::SetSocketReferenceValue<const char*>(Socket*
 // SetSocketValue<> specializations
 //
 template <>
-inline void NodeSocketAccessorBase::SetSocketValue<const bool&>(Socket* socket, const bool& value) {
+inline void NodeSocketAccessorBase::SetSocketValue<const bool&>(
+    Socket* socket,
+    const bool& value) {
   socket->m_value.flag = value;
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketValue<const float&>(Socket* socket, const float& value) {
+inline void NodeSocketAccessorBase::SetSocketValue<const float&>(
+    Socket* socket,
+    const float& value) {
   socket->m_value.float_value = value;
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketValue<const Vec3&>(Socket* socket, const Vec3& value) {
+inline void NodeSocketAccessorBase::SetSocketValue<const Vec3&>(
+    Socket* socket,
+    const Vec3& value) {
   socket->m_value.vec3[0] = value[0];
   socket->m_value.vec3[1] = value[1];
   socket->m_value.vec3[2] = value[2];
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketValue<const Quat&>(Socket* socket, const Quat& value) {
+inline void NodeSocketAccessorBase::SetSocketValue<const Quat&>(
+    Socket* socket,
+    const Quat& value) {
   socket->m_value.quat[0] = value[0];
   socket->m_value.quat[1] = value[1];
   socket->m_value.quat[2] = value[2];
@@ -352,14 +447,19 @@ inline void NodeSocketAccessorBase::SetSocketValue<const Quat&>(Socket* socket,
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketValue<const std::string&>(Socket* socket, const std::string& value) {
+inline void NodeSocketAccessorBase::SetSocketValue<const std::string&>(
+    Socket* socket,
+    const std::string& value) {
   constexpr size_t string_max_length = sizeof(socket->m_value.str) - 1;
   strncpy(socket->m_value.str, value.data(), string_max_length);
-  socket->m_value.str[value.size() > string_max_length ? string_max_length : value.size() ] = 0;
+  socket->m_value.str
+      [value.size() > string_max_length ? string_max_length : value.size()] = 0;
 }
 
 template <>
-inline void NodeSocketAccessorBase::SetSocketValue<const char*>(Socket* socket, const char* value) {
+inline void NodeSocketAccessorBase::SetSocketValue<const char*>(
+    Socket* socket,
+    const char* value) {
   SetSocketValue<const std::string&>(socket, value);
 }
 
@@ -368,6 +468,4 @@ struct NodeSocketAccessor : public NodeSocketAccessorBase {
   virtual ~NodeSocketAccessor() {}
 };
 
-
-
 #endif  //ANIMTESTBED_ANIMGRAPHDATA_H

src/AnimGraph/AnimGraphResource.cc

@@ -146,10 +146,12 @@ AnimNodeResource sAnimGraphNodeFromJson(const json& json_node) {
         size_t string_length = value_str.size();
         constexpr size_t string_max_length = sizeof(property.m_value.str) - 1;
         if (string_length > string_max_length) {
-          std::cerr << "Warning: string '" << value_str << "' too long, truncating to " << string_max_length << " bytes." << std::endl;
+          std::cerr << "Warning: string '" << value_str
+                    << "' too long, truncating to " << string_max_length
+                    << " bytes." << std::endl;
           string_length = string_max_length;
         }
-        memcpy (property.m_value.str, value_str.data(), string_length);
+        memcpy(property.m_value.str, value_str.data(), string_length);
         property.m_value.str[string_length] = 0;
       } else {
         std::cerr << "Invalid type for property '" << property.m_name
@@ -404,7 +406,7 @@ void AnimGraphResource::prepareGraphIOData(AnimGraph& instance) const {
   int output_block_offset = 0;
   for (int i = 0; i < graph_outputs.size(); i++) {
     graph_outputs[i].m_reference.ptr =
-        (void*)&instance.m_output_buffer[output_block_offset];
+        &instance.m_output_buffer[output_block_offset];
     output_block_offset += graph_outputs[i].m_type_size;
   }
 }
@@ -524,29 +526,41 @@ void AnimGraphResource::setRuntimeNodeProperties(AnimGraph& instance) const {
     NodeSocketAccessorBase* node_instance_accessor =
         AnimNodeAccessorFactory(node_resource.m_type_name, instance.m_nodes[i]);
 
-    std::vector<Socket>& resource_properties = node_resource.m_socket_accessor->m_properties;
+    std::vector<Socket>& resource_properties =
+        node_resource.m_socket_accessor->m_properties;
     for (size_t j = 0, n = resource_properties.size(); j < n; j++) {
       const Socket& property = resource_properties[j];
       const std::string& name = property.m_name;
 
       switch (property.m_type) {
         case SocketType::SocketTypeBool:
-          node_instance_accessor->SetPropertyReferenceValue<const bool&>(name, property.m_value.flag);
+          node_instance_accessor->SetPropertyReferenceValue<const bool&>(
+              name,
+              property.m_value.flag);
           break;
         case SocketType::SocketTypeFloat:
-          node_instance_accessor->SetPropertyReferenceValue(name, property.m_value.float_value);
+          node_instance_accessor->SetPropertyReferenceValue(
+              name,
+              property.m_value.float_value);
           break;
         case SocketType::SocketTypeVec3:
-          node_instance_accessor->SetPropertyReferenceValue(name, property.m_value.vec3);
+          node_instance_accessor->SetPropertyReferenceValue(
+              name,
+              property.m_value.vec3);
           break;
         case SocketType::SocketTypeQuat:
-          node_instance_accessor->SetPropertyReferenceValue(name, property.m_value.quat);
+          node_instance_accessor->SetPropertyReferenceValue(
+              name,
+              property.m_value.quat);
           break;
         case SocketType::SocketTypeString:
-          node_instance_accessor->SetPropertyReferenceValue(name, property.m_value.str);
+          node_instance_accessor->SetPropertyReferenceValue(
+              name,
+              property.m_value.str);
           break;
         default:
-          std::cerr << "Invalid socket type " << static_cast<int>(property.m_type) << std::endl;
+          std::cerr << "Invalid socket type "
+                    << static_cast<int>(property.m_type) << std::endl;
       }
     }
 

tests/AnimGraphEvalTests.cc

@@ -120,6 +120,25 @@ TEST_CASE_METHOD(
       sampled_translation.z[0] == Approx(translation_key.value.z).margin(0.01));
 }
 
+TEST_CASE("AnimDataPlacementNew", "[AnimGraphEval]") {
+  std::cout << "blamp" << std::endl;
+
+  int anim_data_size = sizeof(AnimData);
+  char* buf = new char[anim_data_size];
+
+  AnimData* anim_data_newed = new AnimData;
+  anim_data_newed->m_local_matrices.resize(2);
+  delete anim_data_newed;
+
+  AnimData* anim_data_ptr = new (buf) AnimData;
+  anim_data_ptr->m_local_matrices.resize(4);
+  anim_data_ptr->m_local_matrices.resize(0);
+  anim_data_ptr->m_local_matrices.vector::~vector();
+
+  delete[] buf;
+
+}
+
 TEST_CASE_METHOD(
     SimpleAnimFixture,
     "AnimGraphSimpleEval",

tests/AnimGraphResourceTests.cc

@@ -129,40 +129,44 @@ TEST_CASE("BasicGraph", "[AnimGraphResource]") {
   REQUIRE(anim_sampler_run->m_animation != nullptr);
 
   WHEN("Emulating Graph Evaluation") {
-    CHECK(graph.m_anim_data_work_buffer.size() == 0);
+    CHECK(graph.m_anim_data_allocator.size() == 0);
     graph.prepareNodeEval(graph_context, walk_node_index);
     graph.finishNodeEval(walk_node_index);
-    CHECK(graph.m_anim_data_work_buffer.m_num_allocations == 1);
+    CHECK(graph.m_anim_data_allocator.m_num_allocations == 1);
-    CHECK(graph.m_anim_data_work_buffer.size() == 0);
+    CHECK(graph.m_anim_data_allocator.size() == 0);
 
     graph.prepareNodeEval(graph_context, run_node_index);
     graph.finishNodeEval(run_node_index);
-    CHECK(graph.m_anim_data_work_buffer.m_num_allocations == 2);
+    CHECK(graph.m_anim_data_allocator.m_num_allocations == 2);
-    CHECK(graph.m_anim_data_work_buffer.size() == 0);
+    CHECK(graph.m_anim_data_allocator.size() == 0);
 
     graph.prepareNodeEval(graph_context, blend_node_index);
     CHECK(blend2_instance->i_input0 == anim_sampler_walk->o_output);
     CHECK(blend2_instance->i_input1 == anim_sampler_run->o_output);
-    CHECK(graph.m_anim_data_work_buffer.m_num_allocations == 3);
+    CHECK(graph.m_anim_data_allocator.m_num_allocations == 3);
-    CHECK(graph.m_anim_data_work_buffer.size() == 0);
+    CHECK(graph.m_anim_data_allocator.size() == 0);
 
     graph.finishNodeEval(blend_node_index);
     CHECK(anim_sampler_walk->o_output == nullptr);
     CHECK(anim_sampler_run->o_output == nullptr);
-    CHECK(graph.m_anim_data_work_buffer.m_num_allocations == 3);
+    CHECK(graph.m_anim_data_allocator.m_num_allocations == 3);
-    CHECK(graph.m_anim_data_work_buffer.size() == 2);
+    CHECK(graph.m_anim_data_allocator.size() == 2);
 
-    // TODO: rethink output node evaluation
+    // Evaluate output node.
-    graph.prepareNodeEval(graph_context, 0);
+    graph.evalOutputNode();
-    const Socket* graph_output_socket = graph.getOutputSocket("GraphOutput");
-    CHECK(blend2_instance->o_output == (*graph_output_socket->m_reference.ptr_ptr));
-    AnimData* graph_output =
-        static_cast<AnimData*>(*graph_output_socket->m_reference.ptr_ptr);
     graph.finishNodeEval(0);
+
+
+    const Socket* graph_output_socket = graph.getOutputSocket("GraphOutput");
+    AnimData* graph_output =
+        static_cast<AnimData*>(graph_output_socket->m_reference.ptr);
+
+    CHECK(graph_output->m_local_matrices.size() == graph_context.m_skeleton->num_soa_joints());
+
+    CHECK(graph.m_anim_data_allocator.m_num_allocations == 3);
+    CHECK(graph.m_anim_data_allocator.size() == 3);
+
     CHECK(blend2_instance->o_output == nullptr);
-    CHECK(graph_output == (*graph_output_socket->m_reference.ptr_ptr));
-    CHECK(graph.m_anim_data_work_buffer.m_num_allocations == 3);
-    CHECK(graph.m_anim_data_work_buffer.size() == 3);
   }
 
   graph_context.freeAnimations();