From 5e7a48b2eb81023a8e2aa97bcb8a381125c06d28 Mon Sep 17 00:00:00 2001 From: Martin Felis Date: Thu, 14 Apr 2022 18:03:36 +0200 Subject: [PATCH] WIP: graph evaluations. Fixed various memory issues, better tests. --- src/AnimGraph/AnimGraph.cc | 7 +- src/AnimGraph/AnimGraph.h | 72 +++----------- src/AnimGraph/AnimGraphData.h | 154 +++++++++++++++++++++++------ src/AnimGraph/AnimGraphResource.cc | 34 +++++-- tests/AnimGraphEvalTests.cc | 19 ++++ tests/AnimGraphResourceTests.cc | 40 ++++---- 6 files changed, 211 insertions(+), 115 deletions(-) diff --git a/src/AnimGraph/AnimGraph.cc b/src/AnimGraph/AnimGraph.cc index 22060e4..46f47b3 100644 --- a/src/AnimGraph/AnimGraph.cc +++ b/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( - input_connection.m_source_socket.m_reference.ptr)); + m_anim_data_allocator.free(static_cast( + *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) { diff --git a/src/AnimGraph/AnimGraph.h b/src/AnimGraph/AnimGraph.h index 3379899..fb0a038 100644 --- a/src/AnimGraph/AnimGraph.h +++ b/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& getGraphOutputs() { return m_socket_accessor->m_inputs; } std::vector& getGraphInputs() { return m_socket_accessor->m_outputs; } - AnimDataWorkBuffer m_anim_data_work_buffer; + AnimDataAllocator m_anim_data_allocator; ~AnimGraph() { + std::vector& 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(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; diff --git a/src/AnimGraph/AnimGraphData.h b/src/AnimGraph/AnimGraphData.h index 5b1b060..c96ac01 100644 --- a/src/AnimGraph/AnimGraphData.h +++ b/src/AnimGraph/AnimGraphData.h @@ -5,17 +5,18 @@ #ifndef ANIMTESTBED_ANIMGRAPHDATA_H #define ANIMTESTBED_ANIMGRAPHDATA_H +#include + #include #include +#include #include #include -#include #include "SyncTrack.h" -#include "ozz/base/containers/vector.h" -#include -#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 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 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(socket->m_type) << " (" << SocketTypeNames[static_cast(socket->m_type)] << ")." << std::endl; -// *static_cast(socket->m_value.ptr) = value; + // *static_cast(socket->m_value.ptr) = value; } template @@ -248,7 +323,7 @@ struct NodeSocketAccessorBase { } template - 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 @@ -288,24 +363,32 @@ struct NodeSocketAccessorBase { // SetSocketReferenceValue<> specializations // template <> -inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* socket, const bool& value) { +inline void NodeSocketAccessorBase::SetSocketReferenceValue( + Socket* socket, + const bool& value) { *static_cast(socket->m_reference.ptr) = value; } template <> -inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* socket, const float& value) { +inline void NodeSocketAccessorBase::SetSocketReferenceValue( + Socket* socket, + const float& value) { *static_cast(socket->m_reference.ptr) = value; } template <> -inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* socket, const Vec3& value) { +inline void NodeSocketAccessorBase::SetSocketReferenceValue( + Socket* socket, + const Vec3& value) { static_cast(socket->m_reference.ptr)[0] = value[0]; static_cast(socket->m_reference.ptr)[1] = value[1]; static_cast(socket->m_reference.ptr)[2] = value[2]; } template <> -inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* socket, const Quat& value) { +inline void NodeSocketAccessorBase::SetSocketReferenceValue( + Socket* socket, + const Quat& value) { static_cast(socket->m_reference.ptr)[0] = value[0]; static_cast(socket->m_reference.ptr)[1] = value[1]; static_cast(socket->m_reference.ptr)[2] = value[2]; @@ -313,13 +396,17 @@ inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* } template <> -inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* socket, const std::string& value) { +inline void NodeSocketAccessorBase::SetSocketReferenceValue( + Socket* socket, + const std::string& value) { *static_cast(socket->m_reference.ptr) = value; } template <> -inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* socket, const char* value) { - std::string value_string (value); +inline void NodeSocketAccessorBase::SetSocketReferenceValue( + Socket* socket, + const char* value) { + std::string value_string(value); SetSocketReferenceValue(socket, value_string); } @@ -327,24 +414,32 @@ inline void NodeSocketAccessorBase::SetSocketReferenceValue(Socket* // SetSocketValue<> specializations // template <> -inline void NodeSocketAccessorBase::SetSocketValue(Socket* socket, const bool& value) { +inline void NodeSocketAccessorBase::SetSocketValue( + Socket* socket, + const bool& value) { socket->m_value.flag = value; } template <> -inline void NodeSocketAccessorBase::SetSocketValue(Socket* socket, const float& value) { +inline void NodeSocketAccessorBase::SetSocketValue( + Socket* socket, + const float& value) { socket->m_value.float_value = value; } template <> -inline void NodeSocketAccessorBase::SetSocketValue(Socket* socket, const Vec3& value) { +inline void NodeSocketAccessorBase::SetSocketValue( + 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(Socket* socket, const Quat& value) { +inline void NodeSocketAccessorBase::SetSocketValue( + 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(Socket* socket, } template <> -inline void NodeSocketAccessorBase::SetSocketValue(Socket* socket, const std::string& value) { +inline void NodeSocketAccessorBase::SetSocketValue( + 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(Socket* socket, const char* value) { +inline void NodeSocketAccessorBase::SetSocketValue( + Socket* socket, + const char* value) { SetSocketValue(socket, value); } @@ -368,6 +468,4 @@ struct NodeSocketAccessor : public NodeSocketAccessorBase { virtual ~NodeSocketAccessor() {} }; - - #endif //ANIMTESTBED_ANIMGRAPHDATA_H diff --git a/src/AnimGraph/AnimGraphResource.cc b/src/AnimGraph/AnimGraphResource.cc index 25a737f..f8f3089 100644 --- a/src/AnimGraph/AnimGraphResource.cc +++ b/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& resource_properties = node_resource.m_socket_accessor->m_properties; + std::vector& 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(name, property.m_value.flag); + node_instance_accessor->SetPropertyReferenceValue( + 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(property.m_type) << std::endl; + std::cerr << "Invalid socket type " + << static_cast(property.m_type) << std::endl; } } diff --git a/tests/AnimGraphEvalTests.cc b/tests/AnimGraphEvalTests.cc index c1ddd16..4c253bc 100644 --- a/tests/AnimGraphEvalTests.cc +++ b/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", diff --git a/tests/AnimGraphResourceTests.cc b/tests/AnimGraphResourceTests.cc index da86937..6fd4bdf 100644 --- a/tests/AnimGraphResourceTests.cc +++ b/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_work_buffer.size() == 0); + CHECK(graph.m_anim_data_allocator.m_num_allocations == 1); + 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_work_buffer.size() == 0); + CHECK(graph.m_anim_data_allocator.m_num_allocations == 2); + 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_work_buffer.size() == 0); + CHECK(graph.m_anim_data_allocator.m_num_allocations == 3); + 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_work_buffer.size() == 2); + CHECK(graph.m_anim_data_allocator.m_num_allocations == 3); + CHECK(graph.m_anim_data_allocator.size() == 2); - // TODO: rethink output node evaluation - graph.prepareNodeEval(graph_context, 0); - 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(*graph_output_socket->m_reference.ptr_ptr); + // Evaluate output node. + graph.evalOutputNode(); graph.finishNodeEval(0); + + + const Socket* graph_output_socket = graph.getOutputSocket("GraphOutput"); + AnimData* graph_output = + static_cast(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();