Merge pull request #53765 from reduz/skeleton-remove-rest-influence

Remove REST transform influence in skeleton bones

doc/classes/Skeleton3D.xml

@@ -47,6 +47,11 @@
 				Removes the local pose override on all bones in the skeleton.
 			</description>
 		</method>
+		<method name="create_skin_from_rest_transforms">
+			<return type="Skin" />
+			<description>
+			</description>
+		</method>
 		<method name="execute_modifications">
 			<return type="void" />
 			<argument index="0" name="delta" type="float" />
@@ -88,13 +93,6 @@
 				Returns the amount of bones in the skeleton.
 			</description>
 		</method>
-		<method name="get_bone_custom_pose" qualifiers="const">
-			<return type="Transform3D" />
-			<argument index="0" name="bone_idx" type="int" />
-			<description>
-				Returns the custom pose of the specified bone. Custom pose is applied on top of the rest pose.
-			</description>
-		</method>
 		<method name="get_bone_global_pose" qualifiers="const">
 			<return type="Transform3D" />
 			<argument index="0" name="bone_idx" type="int" />
@@ -214,13 +212,6 @@
 				Returns whether the bone pose for the bone at [code]bone_idx[/code] is enabled.
 			</description>
 		</method>
-		<method name="is_bone_rest_disabled" qualifiers="const">
-			<return type="bool" />
-			<argument index="0" name="bone_idx" type="int" />
-			<description>
-				Returns whether the bone rest for the bone at [code]bone_idx[/code] is disabled.
-			</description>
-		</method>
 		<method name="local_pose_to_global_pose">
 			<return type="Transform3D" />
 			<argument index="0" name="bone_idx" type="int" />
@@ -290,23 +281,6 @@
 				Sets the children for the passed in bone, [code]bone_idx[/code], to the passed-in array of bone indexes, [code]bone_children[/code].
 			</description>
 		</method>
-		<method name="set_bone_custom_pose">
-			<return type="void" />
-			<argument index="0" name="bone_idx" type="int" />
-			<argument index="1" name="custom_pose" type="Transform3D" />
-			<description>
-				Sets the custom pose transform, [code]custom_pose[/code], for the bone at [code]bone_idx[/code]. This pose is an addition to the bone rest pose.
-				[b]Note:[/b] The pose transform needs to be in bone space. Use [method world_transform_to_global_pose] to convert a world transform, like one you can get from a [Node3D], to bone space.
-			</description>
-		</method>
-		<method name="set_bone_disable_rest">
-			<return type="void" />
-			<argument index="0" name="bone_idx" type="int" />
-			<argument index="1" name="disable" type="bool" />
-			<description>
-				Disables the rest pose for the bone at [code]bone_idx[/code] if [code]true[/code], enables the bone rest if [code]false[/code].
-			</description>
-		</method>
 		<method name="set_bone_enabled">
 			<return type="void" />
 			<argument index="0" name="bone_idx" type="int" />

editor/import/editor_import_collada.cpp

@@ -120,6 +120,15 @@ Error ColladaImport::_populate_skeleton(Skeleton3D *p_skeleton, Collada::Node *p
 
 	skeleton_bone_map[p_skeleton][joint->sid] = r_bone;
 
+	{
+		Transform3D xform = joint->compute_transform(collada);
+		collada.fix_transform(xform) * joint->post_transform;
+
+		p_skeleton->set_bone_pose_position(r_bone, xform.origin);
+		p_skeleton->set_bone_pose_rotation(r_bone, xform.basis.get_rotation_quaternion());
+		p_skeleton->set_bone_pose_scale(r_bone, xform.basis.get_scale());
+	}
+
 	if (collada.state.bone_rest_map.has(joint->sid)) {
 		p_skeleton->set_bone_rest(r_bone, collada.fix_transform(collada.state.bone_rest_map[joint->sid]));
 		//should map this bone to something for animation?
@@ -1639,16 +1648,6 @@ void ColladaImport::create_animation(int p_clip, bool p_import_value_tracks) {
 			Transform3D xform = cn->compute_transform(collada);
 			xform = collada.fix_transform(xform) * cn->post_transform;
 
-			if (nm.bone >= 0) {
-				//make bone transform relative to rest (in case of skeleton)
-				Skeleton3D *sk = Object::cast_to<Skeleton3D>(nm.node);
-				if (sk) {
-					xform = sk->get_bone_rest(nm.bone).affine_inverse() * xform;
-				} else {
-					ERR_PRINT("Collada: Invalid skeleton");
-				}
-			}
-
 			Vector3 s = xform.basis.get_scale();
 			bool singular_matrix = Math::is_zero_approx(s.x) || Math::is_zero_approx(s.y) || Math::is_zero_approx(s.z);
 			Quaternion q = singular_matrix ? Quaternion() :

editor/plugins/skeleton_3d_editor_plugin.cpp

@@ -200,12 +200,7 @@ void BoneTransformEditor::_value_changed_transform(const String p_property_name,
 }
 
 void BoneTransformEditor::_change_transform(Transform3D p_new_transform) {
-	if (property.get_slicec('/', 0) == "bones" && property.get_slicec('/', 2) == "custom_pose") {
-		undo_redo->create_action(TTR("Set Custom Bone Pose Transform"), UndoRedo::MERGE_ENDS);
-		undo_redo->add_undo_method(skeleton, "set_bone_custom_pose", property.get_slicec('/', 1).to_int(), skeleton->get_bone_custom_pose(property.get_slicec('/', 1).to_int()));
-		undo_redo->add_do_method(skeleton, "set_bone_custom_pose", property.get_slicec('/', 1).to_int(), p_new_transform);
-		undo_redo->commit_action();
-	} else if (property.get_slicec('/', 0) == "bones") {
+	if (property.get_slicec('/', 0) == "bones") {
 		undo_redo->create_action(TTR("Set Bone Transform"), UndoRedo::MERGE_ENDS);
 		undo_redo->add_undo_property(skeleton, property, skeleton->get(property));
 		undo_redo->add_do_property(skeleton, property, p_new_transform);
@@ -236,21 +231,6 @@ void BoneTransformEditor::_update_properties() {
 	_update_transform_properties(tform);
 }
 
-void BoneTransformEditor::_update_custom_pose_properties() {
-	if (updating) {
-		return;
-	}
-
-	if (!skeleton) {
-		return;
-	}
-
-	updating = true;
-
-	Transform3D tform = skeleton->get_bone_custom_pose(property.to_int());
-	_update_transform_properties(tform);
-}
-
 void BoneTransformEditor::_update_transform_properties(Transform3D tform) {
 	Basis rotation_basis = tform.get_basis();
 	Vector3 rotation_radians = rotation_basis.get_rotation_euler();
@@ -463,9 +443,7 @@ void Skeleton3DEditor::pose_to_rest() {
 
 	ur->add_do_method(skeleton, "set_bone_pose", selected_bone, Transform3D());
 	ur->add_undo_method(skeleton, "set_bone_pose", selected_bone, skeleton->get_bone_pose(selected_bone));
-	ur->add_do_method(skeleton, "set_bone_custom_pose", selected_bone, Transform3D());
-	ur->add_undo_method(skeleton, "set_bone_custom_pose", selected_bone, skeleton->get_bone_custom_pose(selected_bone));
-	ur->add_do_method(skeleton, "set_bone_rest", selected_bone, skeleton->get_bone_rest(selected_bone) * skeleton->get_bone_custom_pose(selected_bone) * skeleton->get_bone_pose(selected_bone));
+	ur->add_do_method(skeleton, "set_bone_rest", selected_bone, skeleton->get_bone_rest(selected_bone) * skeleton->get_bone_pose(selected_bone));
 	ur->add_undo_method(skeleton, "set_bone_rest", selected_bone, skeleton->get_bone_rest(selected_bone));
 
 	ur->commit_action();
@@ -654,11 +632,9 @@ void Skeleton3DEditor::_joint_tree_selection_changed() {
 
 			pose_editor->set_target(bone_path + "pose");
 			rest_editor->set_target(bone_path + "rest");
-			custom_pose_editor->set_target(bone_path + "custom_pose");
 
 			pose_editor->set_visible(true);
 			rest_editor->set_visible(true);
-			custom_pose_editor->set_visible(true);
 
 			selected_bone = b_idx;
 		}
@@ -679,9 +655,6 @@ void Skeleton3DEditor::_update_properties() {
 	if (pose_editor) {
 		pose_editor->_update_properties();
 	}
-	if (custom_pose_editor) {
-		custom_pose_editor->_update_custom_pose_properties();
-	}
 	_update_gizmo_transform();
 }
 
@@ -820,12 +793,6 @@ void Skeleton3DEditor::create_editors() {
 	rest_editor->set_visible(false);
 	add_child(rest_editor);
 	rest_editor->set_transform_read_only(true);
-
-	custom_pose_editor = memnew(BoneTransformEditor(skeleton));
-	custom_pose_editor->set_label(TTR("Bone Custom Pose"));
-	custom_pose_editor->set_visible(false);
-	add_child(custom_pose_editor);
-	custom_pose_editor->set_transform_read_only(true);
 }
 
 void Skeleton3DEditor::_notification(int p_what) {
@@ -1289,7 +1256,6 @@ void Skeleton3DGizmoPlugin::set_subgizmo_transform(const EditorNode3DGizmo *p_gi
 	if (parent_idx >= 0) {
 		original_to_local = original_to_local * skeleton->get_bone_global_pose(parent_idx);
 	}
-	original_to_local = original_to_local * skeleton->get_bone_rest(p_id) * skeleton->get_bone_custom_pose(p_id);
 	Basis to_local = original_to_local.get_basis().inverse();
 
 	// Prepare transform.
@@ -1518,5 +1484,5 @@ void Skeleton3DGizmoPlugin::redraw(EditorNode3DGizmo *p_gizmo) {
 	}
 
 	Ref<ArrayMesh> m = surface_tool->commit();
-	p_gizmo->add_mesh(m, Ref<Material>(), Transform3D(), skeleton->register_skin(Ref<Skin>()));
+	p_gizmo->add_mesh(m, Ref<Material>(), Transform3D(), skeleton->register_skin(skeleton->create_skin_from_rest_transforms()));
 }

editor/plugins/skeleton_3d_editor_plugin.h

@@ -102,13 +102,12 @@ class BoneTransformEditor : public VBoxContainer {
 	void set_label(const String &p_label) { label = p_label; }
 
 	void _update_properties();
-	void _update_custom_pose_properties();
 	void _update_transform_properties(Transform3D p_transform);
 
 	// Transform can be keyed, whether or not to show the button.
 	void set_keyable(const bool p_keyable);
 
-	// When rest mode, pose and custom_pose editor are diasbled.
+	// When rest mode, pose editor are diasbled.
 	void set_properties_read_only(const bool p_readonly);
 	void set_transform_read_only(const bool p_readonly);
 
@@ -151,7 +150,6 @@ class Skeleton3DEditor : public VBoxContainer {
 	Tree *joint_tree = nullptr;
 	BoneTransformEditor *rest_editor = nullptr;
 	BoneTransformEditor *pose_editor = nullptr;
-	BoneTransformEditor *custom_pose_editor = nullptr;
 
 	VSeparator *separator;
 	MenuButton *skeleton_options = nullptr;

modules/fbx/data/fbx_skeleton.cpp

@@ -104,6 +104,13 @@ void FBXSkeleton::init_skeleton(const ImportState &state) {
 		print_verbose("working on bone: " + itos(bone_index) + " bone name:" + bone->bone_name);
 
 		skeleton->set_bone_rest(bone->godot_bone_id, get_unscaled_transform(bone->node->pivot_transform->LocalTransform, state.scale));
+		{
+			Transform3D base_xform = bone->node->pivot_transform->LocalTransform;
+
+			skeleton->set_bone_pose_position(bone_index, base_xform.origin);
+			skeleton->set_bone_pose_rotation(bone_index, base_xform.basis.get_rotation_quaternion());
+			skeleton->set_bone_pose_scale(bone_index, base_xform.basis.get_scale());
+		}
 
 		// lookup parent ID
 		if (bone->valid_parent && state.fbx_bone_map.has(bone->parent_bone_id)) {

modules/fbx/editor_scene_importer_fbx.cpp

@@ -1227,20 +1227,6 @@ Node3D *EditorSceneImporterFBX::_generate_scene(
 										AssetImportAnimation::INTERP_LINEAR);
 							}
 
-							// node animations must also include pivots
-							if (skeleton_bone >= 0) {
-								Transform3D xform = Transform3D();
-								xform.basis.set_quaternion_scale(rot, scale);
-								xform.origin = pos;
-								const Transform3D t = bone_rest.affine_inverse() * xform;
-
-								// populate	this again
-								rot = t.basis.get_rotation_quaternion();
-								rot.normalize();
-								scale = t.basis.get_scale();
-								pos = t.origin;
-							}
-
 							if (position_idx >= 0) {
 								animation->position_track_insert_key(position_idx, time, pos);
 							}

modules/gltf/gltf_document.cpp

@@ -4319,6 +4319,9 @@ Error GLTFDocument::_create_skeletons(Ref<GLTFState> state) {
 
 			skeleton->add_bone(node->get_name());
 			skeleton->set_bone_rest(bone_index, node->xform);
+			skeleton->set_bone_pose_position(bone_index, node->position);
+			skeleton->set_bone_pose_rotation(bone_index, node->rotation.normalized());
+			skeleton->set_bone_pose_scale(bone_index, node->scale);
 
 			if (node->parent >= 0 && state->nodes[node->parent]->skeleton == skel_i) {
 				const int bone_parent = skeleton->find_bone(state->nodes[node->parent]->get_name());
@@ -5470,7 +5473,7 @@ void GLTFDocument::_convert_skeleton_to_gltf(Skeleton3D *p_skeleton3d, Ref<GLTFS
 		// Note that we cannot use _gen_unique_bone_name here, because glTF spec requires all node
 		// names to be unique regardless of whether or not they are used as joints.
 		joint_node->set_name(_gen_unique_name(state, skeleton->get_bone_name(bone_i)));
-		Transform3D xform = skeleton->get_bone_rest(bone_i) * skeleton->get_bone_pose(bone_i);
+		Transform3D xform = skeleton->get_bone_pose(bone_i);
 		joint_node->scale = xform.basis.get_scale();
 		joint_node->rotation = xform.basis.get_rotation_quaternion();
 		joint_node->position = xform.origin;
@@ -5958,38 +5961,16 @@ void GLTFDocument::_import_animation(Ref<GLTFState> state, AnimationPlayer *ap,
 
 				if (position_idx >= 0) {
 					pos = _interpolate_track<Vector3>(track.position_track.times, track.position_track.values, time, track.position_track.interpolation);
+					animation->position_track_insert_key(position_idx, time, pos);
 				}
 
 				if (rotation_idx >= 0) {
 					rot = _interpolate_track<Quaternion>(track.rotation_track.times, track.rotation_track.values, time, track.rotation_track.interpolation);
+					animation->rotation_track_insert_key(rotation_idx, time, rot);
 				}
 
 				if (scale_idx >= 0) {
 					scale = _interpolate_track<Vector3>(track.scale_track.times, track.scale_track.values, time, track.scale_track.interpolation);
-				}
-
-				if (gltf_node->skeleton >= 0) {
-					Transform3D xform;
-					xform.basis.set_quaternion_scale(rot, scale);
-					xform.origin = pos;
-
-					const Skeleton3D *skeleton = state->skeletons[gltf_node->skeleton]->godot_skeleton;
-					const int bone_idx = skeleton->find_bone(gltf_node->get_name());
-					xform = skeleton->get_bone_rest(bone_idx).affine_inverse() * xform;
-
-					rot = xform.basis.get_rotation_quaternion();
-					rot.normalize();
-					scale = xform.basis.get_scale();
-					pos = xform.origin;
-				}
-
-				if (position_idx >= 0) {
-					animation->position_track_insert_key(position_idx, time, pos);
-				}
-				if (rotation_idx >= 0) {
-					animation->rotation_track_insert_key(rotation_idx, time, rot);
-				}
-				if (scale_idx >= 0) {
 					animation->scale_track_insert_key(scale_idx, time, scale);
 				}
 
@@ -6108,7 +6089,7 @@ void GLTFDocument::_convert_mesh_instances(Ref<GLTFState> state) {
 			} else {
 				if (skin.is_null()) {
 					// Note that gltf_skin_key should remain null, so these can share a reference.
-					skin = skeleton->register_skin(nullptr)->get_skin();
+					skin = skeleton->create_skin_from_rest_transforms();
 				}
 				gltf_skin.instantiate();
 				gltf_skin->godot_skin = skin;

scene/3d/bone_attachment_3d.cpp

@@ -215,8 +215,6 @@ void BoneAttachment3D::_transform_changed() {
 			sk->set_bone_global_pose_override(bone_idx, our_trans, 1.0, true);
 		} else if (override_mode == OVERRIDE_MODES::MODE_LOCAL_POSE) {
 			sk->set_bone_local_pose_override(bone_idx, sk->global_pose_to_local_pose(bone_idx, our_trans), 1.0, true);
-		} else if (override_mode == OVERRIDE_MODES::MODE_CUSTOM_POSE) {
-			sk->set_bone_custom_pose(bone_idx, sk->global_pose_to_local_pose(bone_idx, our_trans));
 		}
 	}
 }
@@ -273,8 +271,6 @@ void BoneAttachment3D::set_override_pose(bool p_override) {
 				sk->set_bone_global_pose_override(bone_idx, Transform3D(), 0.0, false);
 			} else if (override_mode == OVERRIDE_MODES::MODE_LOCAL_POSE) {
 				sk->set_bone_local_pose_override(bone_idx, Transform3D(), 0.0, false);
-			} else if (override_mode == OVERRIDE_MODES::MODE_CUSTOM_POSE) {
-				sk->set_bone_custom_pose(bone_idx, Transform3D());
 			}
 		}
 		_transform_changed();
@@ -294,8 +290,6 @@ void BoneAttachment3D::set_override_mode(int p_mode) {
 				sk->set_bone_global_pose_override(bone_idx, Transform3D(), 0.0, false);
 			} else if (override_mode == OVERRIDE_MODES::MODE_LOCAL_POSE) {
 				sk->set_bone_local_pose_override(bone_idx, Transform3D(), 0.0, false);
-			} else if (override_mode == OVERRIDE_MODES::MODE_CUSTOM_POSE) {
-				sk->set_bone_custom_pose(bone_idx, Transform3D());
 			}
 		}
 		override_mode = p_mode;

scene/3d/bone_attachment_3d.h

@@ -47,7 +47,6 @@ class BoneAttachment3D : public Node3D {
 	enum OVERRIDE_MODES {
 		MODE_GLOBAL_POSE,
 		MODE_LOCAL_POSE,
-		MODE_CUSTOM_POSE
 	};
 
 	bool use_external_skeleton = false;

scene/3d/mesh_instance_3d.cpp

@@ -146,11 +146,13 @@ void MeshInstance3D::_resolve_skeleton_path() {
 	if (!skeleton_path.is_empty()) {
 		Skeleton3D *skeleton = Object::cast_to<Skeleton3D>(get_node(skeleton_path));
 		if (skeleton) {
-			new_skin_reference = skeleton->register_skin(skin_internal);
 			if (skin_internal.is_null()) {
+				new_skin_reference = skeleton->register_skin(skeleton->create_skin_from_rest_transforms());
 				//a skin was created for us
 				skin_internal = new_skin_reference->get_skin();
 				notify_property_list_changed();
+			} else {
+				new_skin_reference = skeleton->register_skin(skin_internal);
 			}
 		}
 	}