From ecf3b0fef2c37e2e696b50d9644c17e774b40e8c Mon Sep 17 00:00:00 2001 From: Martin Felis Date: Fri, 16 Jan 2026 15:27:33 +0100 Subject: [PATCH] Substantial performance improvements by refactoring AnimationData allocations. AnimationData is now a buffer and a hashmap with offsets of the TrackValues. During graph initialization all used TrackValues get registered and their offsets computed. AnimationData should now be allocated by the AnimationDataAllocator. It takes care of pooling already allocated AnimationDatas and also has a buffer block that contains the default values making it fast to allocate a new AnimationData with the initial pose / default values. --- synced_animation_graph.cpp | 36 +++---- synced_animation_graph.h | 1 - synced_animation_node.cpp | 106 +++++++++++++------- synced_animation_node.h | 149 +++++++++++++++++----------- tests/test_synced_animation_graph.h | 15 ++- 5 files changed, 188 insertions(+), 119 deletions(-) diff --git a/synced_animation_graph.cpp b/synced_animation_graph.cpp index 96deca9..c4fe5df 100644 --- a/synced_animation_graph.cpp +++ b/synced_animation_graph.cpp @@ -299,41 +299,37 @@ void SyncedAnimationGraph::_process_graph(double p_delta, bool p_update_only) { _update_properties(); + AnimationData *graph_output = graph_context.animation_data_allocator.allocate(); root_animation_node->activate_inputs(Vector>()); root_animation_node->calculate_sync_track(Vector>()); root_animation_node->update_time(p_delta); - root_animation_node->evaluate(graph_context, LocalVector(), graph_output); + root_animation_node->evaluate(graph_context, LocalVector(), *graph_output); - _apply_animation_data(graph_output); + _apply_animation_data(*graph_output); + + graph_context.animation_data_allocator.free(graph_output); } void SyncedAnimationGraph::_apply_animation_data(const AnimationData &output_data) const { GodotProfileZone("SyncedAnimationGraph::_apply_animation_data"); - for (const KeyValue &K : output_data.track_values) { - const AnimationData::TrackValue *track_value = K.value; + for (const KeyValue &K : output_data.value_buffer_offset) { + const AnimationData::TrackValue *track_value = output_data.get_value(K.key); switch (track_value->type) { case AnimationData::TrackType::TYPE_POSITION_3D: case AnimationData::TrackType::TYPE_ROTATION_3D: { const AnimationData::TransformTrackValue *transform_track_value = static_cast(track_value); - int bone_idx = -1; - NodePath path = transform_track_value->track->path; - - if (path.get_subname_count() == 1) { - bone_idx = graph_context.skeleton_3d->find_bone(path.get_subname(0)); - - if (bone_idx != -1) { - if (transform_track_value->loc_used) { - graph_context.skeleton_3d->set_bone_pose_position(transform_track_value->bone_idx, transform_track_value->loc); - } - - if (transform_track_value->rot_used) { - graph_context.skeleton_3d->set_bone_pose_rotation(transform_track_value->bone_idx, transform_track_value->rot); - } - } else { - assert(false && "Not yet implemented!"); + if (transform_track_value->bone_idx != -1) { + if (transform_track_value->loc_used) { + graph_context.skeleton_3d->set_bone_pose_position(transform_track_value->bone_idx, transform_track_value->loc); } + + if (transform_track_value->rot_used) { + graph_context.skeleton_3d->set_bone_pose_rotation(transform_track_value->bone_idx, transform_track_value->rot); + } + } else { + assert(false && "Not yet implemented!"); } break; diff --git a/synced_animation_graph.h b/synced_animation_graph.h index 589f1fb..091f9bd 100644 --- a/synced_animation_graph.h +++ b/synced_animation_graph.h @@ -14,7 +14,6 @@ private: NodePath skeleton_path; GraphEvaluationContext graph_context = {}; - AnimationData graph_output; mutable List properties; mutable AHashMap, StringName>> parameter_to_node_parameter_map; diff --git a/synced_animation_node.cpp b/synced_animation_node.cpp index 2575cfc..b74929c 100644 --- a/synced_animation_node.cpp +++ b/synced_animation_node.cpp @@ -142,9 +142,7 @@ void AnimationData::sample_from_animation(const Ref &animation, const int count = tracks.size(); for (int i = 0; i < count; i++) { - TrackValue *track_value = nullptr; const Animation::Track *animation_track = tracks_ptr[i]; - const NodePath &track_node_path = animation_track->path; if (!animation_track->enabled) { continue; } @@ -153,41 +151,29 @@ void AnimationData::sample_from_animation(const Ref &animation, const switch (ttype) { case Animation::TYPE_POSITION_3D: case Animation::TYPE_ROTATION_3D: { - TransformTrackValue *transform_track_value = nullptr; - if (track_values.has(animation_track->thash)) { - transform_track_value = static_cast(track_values[animation_track->thash]); - } else { - transform_track_value = memnew(AnimationData::TransformTrackValue); - } - int bone_idx = -1; + TransformTrackValue *transform_track_value = get_value(animation_track->thash); - if (track_node_path.get_subname_count() == 1) { - bone_idx = skeleton_3d->find_bone(track_node_path.get_subname(0)); - if (bone_idx != -1) { - transform_track_value->bone_idx = bone_idx; - switch (ttype) { - case Animation::TYPE_POSITION_3D: { - animation->try_position_track_interpolate(i, p_time, &transform_track_value->loc); - transform_track_value->loc_used = true; - break; - } - case Animation::TYPE_ROTATION_3D: { - animation->try_rotation_track_interpolate(i, p_time, &transform_track_value->rot); - transform_track_value->rot_used = true; - break; - } - default: { - assert(false && !"Not yet implemented"); - break; - } + if (transform_track_value->bone_idx != -1) { + switch (ttype) { + case Animation::TYPE_POSITION_3D: { + animation->try_position_track_interpolate(i, p_time, &transform_track_value->loc); + transform_track_value->loc_used = true; + break; + } + case Animation::TYPE_ROTATION_3D: { + animation->try_rotation_track_interpolate(i, p_time, &transform_track_value->rot); + transform_track_value->rot_used = true; + break; + } + default: { + assert(false && !"Not yet implemented"); + break; } } } else { // TODO assert(false && !"Not yet implemented"); } - - track_value = transform_track_value; break; } default: { @@ -196,12 +182,63 @@ void AnimationData::sample_from_animation(const Ref &animation, const break; } } - - track_value->track = tracks_ptr[i]; - set_value(animation_track->thash, track_value); } } +void AnimationData::allocate_track_value(const Animation::Track *animation_track, const Skeleton3D *skeleton_3d) { + switch (animation_track->type) { + case Animation::TrackType::TYPE_ROTATION_3D: + case Animation::TrackType::TYPE_POSITION_3D: { + size_t value_offset = 0; + AnimationData::TransformTrackValue *transform_track_value = nullptr; + if (value_buffer_offset.has(animation_track->thash)) { + value_offset = value_buffer_offset[animation_track->thash]; + transform_track_value = reinterpret_cast(&buffer[value_offset]); + } else { + value_offset = buffer.size(); + value_buffer_offset.insert(animation_track->thash, buffer.size()); + buffer.resize(buffer.size() + sizeof(AnimationData::TransformTrackValue)); + transform_track_value = new (reinterpret_cast(&buffer[value_offset])) AnimationData::TransformTrackValue(); + } + assert(transform_track_value != nullptr); + if (animation_track->path.get_subname_count() == 1) { + transform_track_value->bone_idx = skeleton_3d->find_bone(animation_track->path.get_subname(0)); + } + + if (animation_track->type == Animation::TrackType::TYPE_POSITION_3D) { + transform_track_value->loc_used = true; + } else if (animation_track->type == Animation::TrackType::TYPE_ROTATION_3D) { + transform_track_value->rot_used = true; + } + + break; + } + default: + break; + } +} + +void AnimationData::allocate_track_values(const Ref &animation, const Skeleton3D *skeleton_3d) { + GodotProfileZone("AnimationData::allocate_track_values"); + + const LocalVector tracks = animation->get_tracks(); + Animation::Track *const *tracks_ptr = tracks.ptr(); + + int count = tracks.size(); + for (int i = 0; i < count; i++) { + const Animation::Track *animation_track = tracks_ptr[i]; + if (!animation_track->enabled) { + continue; + } + + allocate_track_value(animation_track, skeleton_3d); + } +} + +void AnimationDataAllocator::register_track_values(const Ref &animation, const Skeleton3D *skeleton_3d) { + default_data.allocate_track_values(animation, skeleton_3d); +} + bool AnimationSamplerNode::initialize(GraphEvaluationContext &context) { SyncedAnimationNode::initialize(context); @@ -211,6 +248,8 @@ bool AnimationSamplerNode::initialize(GraphEvaluationContext &context) { return false; } + context.animation_data_allocator.register_track_values(animation, context.skeleton_3d); + node_time_info.loop_mode = animation->get_loop_mode(); // Initialize Sync Track from marker @@ -260,7 +299,6 @@ void AnimationSamplerNode::evaluate(GraphEvaluationContext &context, const Local node_time_info.position = node_time_info.sync_track.calc_ratio_from_sync_time(node_time_info.sync_position) * animation->get_length(); } - output.clear(); output.sample_from_animation(animation, context.skeleton_3d, node_time_info.position); } diff --git a/synced_animation_node.h b/synced_animation_node.h index 7067554..02c00bf 100644 --- a/synced_animation_node.h +++ b/synced_animation_node.h @@ -13,7 +13,11 @@ * @class AnimationData * Represents data that is transported via animation connections in the SyncedAnimationGraph. * - * Essentially, it is a hash map for all Animation::Track values that can are sampled from an Animation. + * In general AnimationData objects should be obtained using the AnimationDataAllocator. + * + * The class consists of a buffer containing the data and a hashmap that resolves the + * Animation::TypeHash of an Animation::Track to the corresponding AnimationData::TrackValue + * block within the buffer. */ struct AnimationData { enum TrackType : uint8_t { @@ -29,7 +33,6 @@ struct AnimationData { }; struct TrackValue { - Animation::Track *track = nullptr; TrackType type = TYPE_ANIMATION; virtual ~TrackValue() = default; @@ -91,69 +94,51 @@ struct AnimationData { const TransformTrackValue *other_value_casted = &static_cast(other_value); return bone_idx == other_value_casted->bone_idx && loc == other_value_casted->loc && rot == other_value_casted->rot && scale == other_value_casted->scale; } - - TrackValue *clone() const override { - TransformTrackValue *result = memnew(TransformTrackValue); - result->track = track; - result->bone_idx = bone_idx; - - result->loc_used = loc_used; - result->rot_used = rot_used; - result->scale_used = scale_used; - - result->init_loc = init_loc; - result->init_rot = init_rot; - result->init_scale = init_scale; - - result->loc = loc; - result->rot = rot; - result->scale = scale; - - return result; - } }; AnimationData() = default; ~AnimationData() = default; AnimationData(const AnimationData &other) { - for (const KeyValue &K : other.track_values) { - track_values.insert(K.key, K.value->clone()); - } + value_buffer_offset = other.value_buffer_offset; + buffer = other.buffer; } AnimationData(AnimationData &&other) noexcept : - track_values(std::exchange(other.track_values, AHashMap())) { + value_buffer_offset(std::exchange(other.value_buffer_offset, AHashMap())), + buffer(std::exchange(other.buffer, LocalVector())) { } AnimationData &operator=(const AnimationData &other) { AnimationData temp(other); - std::swap(track_values, temp.track_values); + std::swap(value_buffer_offset, temp.value_buffer_offset); + std::swap(buffer, temp.buffer); return *this; } AnimationData &operator=(AnimationData &&other) noexcept { - std::swap(track_values, other.track_values); + std::swap(value_buffer_offset, other.value_buffer_offset); + std::swap(buffer, other.buffer); return *this; } - void - set_value(const Animation::TypeHash &thash, TrackValue *value) { - if (!track_values.has(thash)) { - track_values.insert(thash, value); - } else { - track_values[thash] = value; - } + void allocate_track_value(const Animation::Track *animation_track, const Skeleton3D *skeleton_3d); + void allocate_track_values(const Ref &animation, const Skeleton3D *skeleton_3d); + + template + TrackValueType *get_value(const Animation::TypeHash &thash) { + return reinterpret_cast(&buffer[value_buffer_offset[thash]]); } - void clear() { - _clear_values(); + template + const TrackValueType *get_value(const Animation::TypeHash &thash) const { + return reinterpret_cast(&buffer[value_buffer_offset[thash]]); } bool has_same_tracks(const AnimationData &other) const { HashSet valid_track_hashes; - for (const KeyValue &K : track_values) { + for (const KeyValue &K : value_buffer_offset) { valid_track_hashes.insert(K.key); } - for (const KeyValue &K : other.track_values) { + for (const KeyValue &K : other.value_buffer_offset) { if (HashSet::Iterator entry = valid_track_hashes.find(K.key)) { valid_track_hashes.remove(entry); } else { @@ -172,9 +157,9 @@ struct AnimationData { return; } - for (const KeyValue &K : track_values) { - TrackValue *track_value = K.value; - TrackValue *other_track_value = to_data.track_values[K.key]; + for (const KeyValue &K : value_buffer_offset) { + TrackValue *track_value = get_value(K.key); + const TrackValue *other_track_value = to_data.get_value(K.key); track_value->blend(*other_track_value, lambda); } @@ -182,19 +167,63 @@ struct AnimationData { void sample_from_animation(const Ref &animation, const Skeleton3D *skeleton_3d, double p_time); - AHashMap track_values; // Animation::Track to TrackValue + AHashMap value_buffer_offset; + LocalVector buffer; +}; -protected: - void _clear_values() { - for (KeyValue &K : track_values) { - memdelete(K.value); +/** + * @class AnimationDataAllocator + * + * Allows reusing of already allocated AnimationData objects. Stores the default values for all + * tracks. An allocated AnimationData object always has a resetted state where all TrackValues + * have the default value. + * + * During SyncedAnimationGraph initialization all nodes that generate values for AnimationData + * must register their tracks in the AnimationDataAllocator to ensure all allocated AnimationData + * have corresponding tracks. + */ +class AnimationDataAllocator { + AnimationData default_data; + List allocated_data; + +public: + ~AnimationDataAllocator() { + while (!allocated_data.is_empty()) { + memfree(allocated_data.front()->get()); + allocated_data.pop_front(); } } + + /// @brief Registers all animation track values for the default_data value. + void register_track_values(const Ref &animation, const Skeleton3D *skeleton_3d); + + AnimationData *allocate() { + GodotProfileZone("AnimationDataAllocator::allocate_template"); + if (!allocated_data.is_empty()) { + GodotProfileZone("AnimationDataAllocator::allocate_from_list"); + AnimationData *result = allocated_data.front()->get(); + allocated_data.pop_front(); + + // We copy the whole block as the assignment operator copies entries element wise. + memcpy(result->buffer.ptr(), default_data.buffer.ptr(), default_data.buffer.size()); + + return result; + } + + AnimationData *result = memnew(AnimationData); + *result = default_data; + return result; + } + + void free(AnimationData *data) { + allocated_data.push_front(data); + } }; struct GraphEvaluationContext { AnimationPlayer *animation_player = nullptr; Skeleton3D *skeleton_3d = nullptr; + AnimationDataAllocator animation_data_allocator; }; /** @@ -242,14 +271,14 @@ public: return true; } - virtual void activate_inputs(Vector> input_nodes) { + virtual void activate_inputs(const Vector> &input_nodes) { // By default, all inputs nodes are activated. for (const Ref &node : input_nodes) { node->active = true; node->node_time_info.is_synced = node_time_info.is_synced; } } - virtual void calculate_sync_track(Vector> input_nodes) { + virtual void calculate_sync_track(const Vector> &input_nodes) { // By default, use the SyncTrack of the first input. if (input_nodes.size() > 0) { node_time_info.sync_track = input_nodes[0]->node_time_info.sync_track; @@ -339,16 +368,16 @@ public: return result; } - void activate_inputs(Vector> input_nodes) override { - for (const Ref &node : input_nodes) { - node->active = true; + void activate_inputs(const Vector> &input_nodes) override { + input_nodes[0]->active = true; + input_nodes[1]->active = true; - // If this Blend2 node is already synced then inputs are also synced. Otherwise, inputs are only set to synced if synced blending is active in this node. - node->node_time_info.is_synced = node_time_info.is_synced || sync; - } + // If this Blend2 node is already synced then inputs are also synced. Otherwise, inputs are only set to synced if synced blending is active in this node. + input_nodes[0]->node_time_info.is_synced = node_time_info.is_synced || sync; + input_nodes[1]->node_time_info.is_synced = node_time_info.is_synced || sync; } - void calculate_sync_track(Vector> input_nodes) override { + void calculate_sync_track(const Vector> &input_nodes) override { if (node_time_info.is_synced || sync) { assert(input_nodes[0]->node_time_info.loop_mode == input_nodes[1]->node_time_info.loop_mode); node_time_info.sync_track = SyncTrack::blend(blend_weight, input_nodes[0]->node_time_info.sync_track, input_nodes[1]->node_time_info.sync_track); @@ -715,7 +744,7 @@ public: return true; } - void activate_inputs(Vector> input_nodes) override { + void activate_inputs(const Vector> &input_nodes) override { GodotProfileZone("SyncedBlendTree::activate_inputs"); tree_graph.nodes[0]->active = true; @@ -731,7 +760,7 @@ public: } } - void calculate_sync_track(Vector> input_nodes) override { + void calculate_sync_track(const Vector> &input_nodes) override { GodotProfileZone("SyncedBlendTree::calculate_sync_track"); for (int i = tree_graph.nodes.size() - 1; i > 0; i--) { const Ref &node = tree_graph.nodes[i]; @@ -791,14 +820,14 @@ public: if (i == 1) { node_runtime_data.output_data = &output_data; } else { - node_runtime_data.output_data = memnew(AnimationData); + node_runtime_data.output_data = context.animation_data_allocator.allocate(); } node->evaluate(context, node_runtime_data.input_data, *node_runtime_data.output_data); // All inputs have been consumed and can now be freed. for (const int child_index : tree_graph.node_connection_info[i].connected_child_node_index_at_port) { - memfree(_node_runtime_data[child_index].output_data); + context.animation_data_allocator.free(_node_runtime_data[child_index].output_data); } } } diff --git a/tests/test_synced_animation_graph.h b/tests/test_synced_animation_graph.h index 9314da7..977ae33 100644 --- a/tests/test_synced_animation_graph.h +++ b/tests/test_synced_animation_graph.h @@ -222,20 +222,25 @@ TEST_CASE("[SyncedAnimationGraph] Test BlendTree construction") { TEST_CASE_FIXTURE(SyncedAnimationGraphFixture, "[SceneTree][SyncedAnimationGraph] Test AnimationData blending") { AnimationData data_t0; + data_t0.allocate_track_values(test_animation_a, skeleton_node); data_t0.sample_from_animation(test_animation_a, skeleton_node, 0.0); AnimationData data_t1; + data_t1.allocate_track_values(test_animation_a, skeleton_node); data_t1.sample_from_animation(test_animation_a, skeleton_node, 1.0); AnimationData data_t0_5; + data_t0_5.allocate_track_values(test_animation_a, skeleton_node); data_t0_5.sample_from_animation(test_animation_a, skeleton_node, 0.5); AnimationData data_blended = data_t0; data_blended.blend(data_t1, 0.5); REQUIRE(data_blended.has_same_tracks(data_t0_5)); - for (const KeyValue &K : data_blended.track_values) { - CHECK(K.value->operator==(*data_t0_5.track_values.find(K.key)->value)); + for (const KeyValue &K : data_blended.value_buffer_offset) { + AnimationData::TrackValue *blended_value = data_blended.get_value(K.key); + AnimationData::TrackValue *data_t0_5_value = data_t0_5.get_value(K.key); + CHECK(*blended_value == *data_t0_5_value); } // And also check that values do not match @@ -243,8 +248,10 @@ TEST_CASE_FIXTURE(SyncedAnimationGraphFixture, "[SceneTree][SyncedAnimationGraph data_blended.blend(data_t1, 0.3); REQUIRE(data_blended.has_same_tracks(data_t0_5)); - for (const KeyValue &K : data_blended.track_values) { - CHECK(K.value->operator!=(*data_t0_5.track_values.find(K.key)->value)); + for (const KeyValue &K : data_blended.value_buffer_offset) { + AnimationData::TrackValue *blended_value = data_blended.get_value(K.key); + AnimationData::TrackValue *data_t0_5_value = data_t0_5.get_value(K.key); + CHECK(*blended_value != *data_t0_5_value); } }