Implement Skeleton Editor Gizmo

[TokageItLab]

Skeleton's TreeEditor was implemented in #36409, but the gizmo for editing was not (ref: 584 and #1767). So, the gizmo in SkeletonEditor, which was developed in the 3.2.x branch, is now complete tentatively. This is a draft PR for porting to 4.0.

https://www.youtube.com/watch?v=RjJiEo8uqPw

Dev branch for 3.2.x is here.

Features

When you select skeleton and bone edit tool, marker circles will appear on the joints. If you select a marker, a transform gizmo will appear, which looks the same as SpatialTransformGizmo but behaves a bit differently. Basically, in character animation, there is no editing other than rotate. So to make the editing process smoother, you can make selections in all Move/Rotate/Scale modes. Instead, there is no free transform mode, which I may implement somewhere else. the behavior was unified by SubGizmo implementation.

There is a PoseMode and a RestMode; during RestMode, Pose and CustomPose are temporarily disabled, and RestMode will be canceled when you exit the SkeletonEditor. However, there is no CustomPoseMode, because there is no way to disable only the Pose while the CustomPose is enabled.

Add the following options to SkeletonButton's popups in toolber

• Init pose
• Insert key of all bone poses
• Insert key of bone poses already exist track
• Apply current pose to rest

All of these are basically applied to all bones. Init pose initializes the Pose, but not the CustomPose. Insert key is activated when the animation player has a track, similar to Spatial's KeyButton. Be careful with Apply current pose to rest. It initializes the current Pose and CustomPose, and changes Rest.

Changes/Bugfixes

SkeletonEditor

• Added int selected_bone
• Removed the Pose Enabled checkbox in SkeletonBoneEditor to prevent conflicts in RestMode

SpatialEditorGizmo

• Skeleton's gizmo is now at the front of the screen when skeleton is selected
• Show the local axis of the bones when selecting a skeleton
• Highlight the currently selected bone
• Added selected bone color and bone axis length to EditorSettings

SpatialEditorPlugin

• Added Vector<Transform> external
• Added ToolExternal to ToolMode
• forward_spatial_gui_input() passes the id of the Viewport
• Some private functions changed to public

These were superseded SubGizmo implementation.

AnimationTrackEditor

• Added has_transform_track()

Done

• Porting for 4.0
• Fix depth drawing order in gizmo (with the shader by @lyuma )
• Consider the size and shape of the joint marker
• Adopt SubGizmos
• Behavior for negative scales is not ensured
• Set readonly to bone pose in rest mode after marge Apply set_read_only() to child classes of EditorProperty elements #51722
• Octahedron as bone gizmo in SpatialEditorGizmo weight assignment and updated timing are strange (superseded selectable gizmo shape with Wire shape)

Other Issues (Todo in the future)

• Need to fix some function (inserting multiple tracks has been broken) Fixed behavior of insert track queue in AnimationTrackEditor is unstable #51459 Fixed
• SkeletonEditor does not support multiple selection
• Need independent bone handling (with changing skin pose) in edit rest option for adjust initial rest
• ImmediateMesh does not draw anything when there is only one vertex (or all vertices position are equaled) #52627 Fixed

[JFonS]

I think you missed most of the changes in your commit, I can only see a modification in skeleton_3d_editor_plugin.cpp :)

[fire]

Can you write a description of the ExternalToolMode system? This is the api addition to Node3d gizmos we wanted to add, but it's hard to describe to other contributors.

Edited:

We finalized with a different proposal.

[TokageItLab]

@JFonS @fire Porting was done. There are a few issues that need to be fixed, but you can check that it works for now.

• The depth drawing order in gizmo is messed up. In 3.2.x I used a hack with vertex shader to change the draw order, but this is no longer available in 4.0, so I need to find another way.
• It was decided in discussion with reduz and others to implement SubGizmos first instead of ExternalMode in Node3DEditor.

[TokageItLab]

Depth problem was solved. It's a bit hacky so this may be a problem for some rendering engines other than vulkan, but it maintains the drawing order in 3D space. If you want to avoid hack, we need to implement a separate drawing layer for gizmo (ref: #2138).

[TokageItLab]

It seems that reduz is thinking of getting rid of the bone rest. He has a point since it may be not good to allow users to easily handle posing with bone rests. However, when it comes to retargeting or adjusting axes, rest needs to be changed.

Therefore, it would be better to omit the implementation of rest mode bone manipulation, and make rest and custom poses read only in the inspector, and instead add a "Rest Option" in the 3D View menu, such as operating "Set global rest", "Unify roll axis to specific axis", "Swap axis" and etc (everything will do with changing the skin poses).

[lyuma]

As a followup to the idea of removing bone_rest as it currently is... and just jotting down some thoughts I had recently: I've been increasingly thinking that rest_pose should become an animation concept of some sort, and added to "rotation", "position" and "scale" type tracks.

Basically, Animations could use the rest matrix (transform/rotation/scale) applied one after the other, so there is no problem with non-uniform matrices but you can still assign "identity" tracks in the animation to reset the object to the rest pose.

Another option is for the animation transform track to be separated into animation position track, rotation track and scale track (note that a Transform track internally already stores data in this form, but it requires all 10 values to be set, even though most animations only change rotation)... that way, the position would remain unchanged and the bone rest does not need to be stored.

[TokageItLab]

Updated:

• Omit rest mode
• Rest and Custom Pose are forced readonly in inspector
• Bone shape is able to be changed in EditorSetting
• Implemented ShowRestOnly mode in Skeleton

[JFonS]

Looks good in general, amazing work :)

I left some comments on minor details and some changes that need to be made, but the overall design is good.

I also found a crash while testing. It can be reproduced with an animated skeleton:

1. Have a scene with a skeleton and an AnimationPlayer that contains an animation for the skeleton.
2. Open animation editor with and the skeleton animation.
3. Select the animated skeleton, so the gizmo is drawn (the animation doesn't need to be playing).
4. Close the scene.
5. Crash happens.

I fixed it with the following changes:

diff --git a/editor/plugins/skeleton_3d_editor_plugin.cpp b/editor/plugins/skeleton_3d_editor_plugin.cpp
--- a/editor/plugins/skeleton_3d_editor_plugin.cpp	
+++ b/editor/plugins/skeleton_3d_editor_plugin.cpp	
@@ -356,6 +356,7 @@
 
 void Skeleton3DEditor::_update_show_rest_only() {
 	_update_pose_enabled(-1);
+	_update_gizmo_visible();
 }
 
 void Skeleton3DEditor::_update_pose_enabled(int p_bone) {
@@ -370,7 +371,6 @@
 			pose_editor->set_transform_read_only(skeleton->is_show_rest_only() || !(skeleton->is_bone_enabled(selected)));
 		}
 	}
-	_update_gizmo_visible();
 }
 
 void Skeleton3DEditor::_on_click_skeleton_option(int p_skeleton_option) {
@@ -667,6 +667,7 @@
 	set_rest_options_enabled(selected);
 	_update_properties();
 	_update_pose_enabled();
+	_update_gizmo_visible();
 }
 
 // May be not used with single select mode.

But please check if the fix makes sense and feel free to change it if it doesn't.

[TokageItLab]

@JFonS Thank you for your reviewing and I fixed the part you reviewed.

Also, as for the crashing bug, it seems that the problem was caused by the editor generation during scene transition. That method seemed unnecessary, so I've solved it by removing it.

I'll squash it if it seems to be OK to you.

[JFonS]

Re-reviewed and all my concerns were addressed.

Thanks for your great work and for putting up with my delayed reviews :)

[JFonS]

Once the commits are squashed and CI passes it should be good to merge.

[akien-mga]

Thanks!

[Shadowblitz16]

Why does this use embeded bone data instead of BoneAttachment3D?
And why can't we convert our meshes as relative children to the BoneAttachment3D's?

[TokageItLab]

I do not understand what you are asking. FYI, as to why all bones should not be the children of the scene as an actual Node. It is written in godotengine/godot-proposals#1767 because it is mainly from a performance point of view.

If you want to generate BoneAttachment from multiple bones, I remember it is possible to add it when setting up PhysicalBone, but I don't think there was a way to add only BoneAttachment. You can open a proposal for that.

If you want to convert a gizmo to a MeshInstance, that doesn't make sense. Most 3D applications don't have that capability and Godot is the same. That doesn't have to be included in the core as a general feature. I know that Blender has such a thing as an 3rd party addon for that, so I suggest you look for an addon or make your own.