Skip to content

Optimize Skeleton3D #113441

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TokageItLab wants to merge 1 commit into
base: master
Choose a base branch
from
+43 −22
Open

Optimize Skeleton3D #113441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion core/object/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,7 @@ class Object {
// Store on each object a bitfield to quickly test whether it is derived from some "key" classes
// that are commonly tested in performance sensitive code.
// Ensure unsigned to bitpack.
// If you want to add more classes, please make sure to edit the Object::_ancestry field.
enum class AncestralClass : unsigned int {
REF_COUNTED = 1 << 0,
NODE = 1 << 1,
Expand All @@ -603,6 +604,8 @@ class Object {
COLLISION_OBJECT_3D = 1 << 12,
PHYSICS_BODY_3D = 1 << 13,
MESH_INSTANCE_3D = 1 << 14,
// Used a lot in AnimationMixer, SkeletonModifier3D and BoneAttachment3D.
SKELETON_3D = 1 << 15,
};

static constexpr AncestralClass static_ancestral_class = (AncestralClass)0;
Expand Down Expand Up @@ -653,7 +656,7 @@ class Object {
void _initialize();
void _postinitialize();

uint32_t _ancestry : 15;
uint32_t _ancestry : 16;

bool _block_signals : 1;
bool _can_translate : 1;
Expand Down
12 changes: 8 additions & 4 deletions scene/3d/skeleton_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ void Skeleton3D::_notification(int p_what) {

// Process modifiers.

LocalVector<BonePoseBackup> bones_backup;
_find_modifiers();
if (!modifiers.is_empty()) {
bones_backup.resize(bones.size());
Expand Down Expand Up @@ -840,12 +839,16 @@ void Skeleton3D::clear_bones() {
// Posing api

void Skeleton3D::set_bone_pose(int p_bone, const Transform3D &p_pose) {
set_bone_pose_components(p_bone, p_pose.origin, p_pose.basis.get_rotation_quaternion(), p_pose.basis.get_scale());
}

void Skeleton3D::set_bone_pose_components(int p_bone, const Vector3 &p_position, const Quaternion &p_rotation, const Vector3 &p_scale) {
const int bone_size = bones.size();
ERR_FAIL_INDEX(p_bone, bone_size);

bones[p_bone].pose_position = p_pose.origin;
bones[p_bone].pose_rotation = p_pose.basis.get_rotation_quaternion();
bones[p_bone].pose_scale = p_pose.basis.get_scale();
bones[p_bone].pose_position = p_position;
bones[p_bone].pose_rotation = p_rotation;
bones[p_bone].pose_scale = p_scale;
bones[p_bone].pose_cache_dirty = true;
if (is_inside_tree()) {
_make_dirty();
Expand Down Expand Up @@ -1416,6 +1419,7 @@ void Skeleton3D::physical_bones_remove_collision_exception(RID p_exception) {
#endif // _DISABLE_DEPRECATED

Skeleton3D::Skeleton3D() {
_define_ancestry(AncestralClass::SKELETON_3D);
}

Skeleton3D::~Skeleton3D() {
Expand Down
4 changes: 4 additions & 0 deletions scene/3d/skeleton_3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ class Skeleton3D : public Node3D {
#endif // _DISABLE_DEPRECATED && PHYSICS_3D_DISABLED

public:
static constexpr AncestralClass static_ancestral_class = AncestralClass::SKELETON_3D;

enum ModifierCallbackModeProcess {
MODIFIER_CALLBACK_MODE_PROCESS_PHYSICS,
MODIFIER_CALLBACK_MODE_PROCESS_IDLE,
Expand Down Expand Up @@ -160,6 +162,7 @@ class Skeleton3D : public Node3D {
HashSet<SkinReference *> skin_bindings;
void _skin_changed();

LocalVector<BonePoseBackup> bones_backup;
mutable LocalVector<Bone> bones;
mutable bool process_order_dirty = false;

Expand Down Expand Up @@ -267,6 +270,7 @@ class Skeleton3D : public Node3D {
Quaternion get_bone_pose_rotation(int p_bone) const;
Vector3 get_bone_pose_scale(int p_bone) const;
void set_bone_pose(int p_bone, const Transform3D &p_pose);
void set_bone_pose_components(int p_bone, const Vector3 &p_position, const Quaternion &p_rotation, const Vector3 &p_scale);
void set_bone_pose_position(int p_bone, const Vector3 &p_position);
void set_bone_pose_rotation(int p_bone, const Quaternion &p_rotation);
void set_bone_pose_scale(int p_bone, const Vector3 &p_scale);
Expand Down
44 changes: 27 additions & 17 deletions scene/animation/animation_mixer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1878,29 +1878,39 @@ void AnimationMixer::_blend_apply() {
if (!t_skeleton) {
return;
}
if (t->loc_used) {
t_skeleton->set_bone_pose_position(t->bone_idx, t->loc);
}
if (t->rot_used) {
t_skeleton->set_bone_pose_rotation(t->bone_idx, t->rot);
}
if (t->scale_used) {
t_skeleton->set_bone_pose_scale(t->bone_idx, t->scale);
}

if (t->loc_used && t->rot_used && t->scale_used) {
t_skeleton->set_bone_pose_components(t->bone_idx, t->loc, t->rot, t->scale);
} else {
Comment on lines +1882 to +1884
Copy link
Copy Markdown
Member Author

@TokageItLab TokageItLab Dec 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What concerns me is that cases where all loc/rot/scale components are enable are quite rare.

In cases where both position and rotation are active, setting them simultaneously might potentially lead to improvements. So if I were to implement this, I would try passing loc/rot/scale as a bitmask as an argument and performing conditional branching within the set_bone_pose_components() method to determine what to set and always use that method, like:

void Skeleton3D::set_bone_pose_components(int p_bone, int p_component_mask, const Vector3 &p_position, const Quaternion &p_rotation, const Vector3 &p_scale) {
	const int bone_size = bones.size();
	ERR_FAIL_INDEX(p_bone, bone_size);

	if (p_component_mask & COMPONENT_POSITION) {
		bones[p_bone].pose_position = p_position;
	}
	if (p_component_mask & COMPONENT_ROTATION) {
		bones[p_bone].pose_rotation = p_rotation;
	}
	if (p_component_mask & COMPONENT_SCALE) {
		bones[p_bone].pose_scale = p_scale;
	}

But I'm not sure which cost is lower until I test it.

Copy link
Copy Markdown
Contributor

@Ryan-000 Ryan-000 Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and think this is a good idea since most skeletal animations only use position and rotation.

If performance is a concern, marking the set_bone_pose_components method as inline should allow the compiler to eliminate the branches when it is in Skeleton3D::set_bone_pose.

Or we can have just both methods, one with flags and one without.

if (t->loc_used) {
t_skeleton->set_bone_pose_position(t->bone_idx, t->loc);
}
if (t->rot_used) {
t_skeleton->set_bone_pose_rotation(t->bone_idx, t->rot);
}
if (t->scale_used) {
t_skeleton->set_bone_pose_scale(t->bone_idx, t->scale);
}
}
} else if (!t->skeleton_id.is_valid()) {
Node3D *t_node_3d = ObjectDB::get_instance<Node3D>(t->object_id);
if (!t_node_3d) {
return;
}
if (t->loc_used) {
t_node_3d->set_position(t->loc);
}
if (t->rot_used) {
t_node_3d->set_rotation(t->rot.get_euler());
}
if (t->scale_used) {
t_node_3d->set_scale(t->scale);

if (t->loc_used && t->rot_used && t->scale_used) {
Transform3D transform = Transform3D(Basis(t->rot).scaled(t->scale), t->loc);
t_node_3d->set_transform(transform);
} else {
if (t->loc_used) {
t_node_3d->set_position(t->loc);
}
if (t->rot_used) {
t_node_3d->set_rotation(t->rot.get_euler());
}
if (t->scale_used) {
t_node_3d->set_scale(t->scale);
}
}
}
#endif // _3D_DISABLED
Expand Down