Skip to content
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.

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

Implement a base class SkeletonModifier3D as refactoring for nodes that may modify Skeleton3D #87888

Merged

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Feb 3, 2024

Add SkeletonModifier3D as base class for some Nodes

The base class for the following Nodes will be SkeletonModifier3D:

  • BoneAttachment3D
  • OpenXRHand
  • SkeletonIK3D
  • XRHandModifier3D
  • XRBodyModifier3D

image

Old description

Old description

What is SkeletonModifier3D

image

SkeletonModifier3D retrieves a Skeleton by either having a Skeleton parent or by specifying an external Skeleton. This is almost the same way as the current BoneAttachment.

SkeletonModifier3D, like AnimationMixer, allows the user to select a process as CallbackModeProcess. Also, by specifying a target AnimationMixer, it is possible to process the animation in the same process as the AnimationMixer. At this time, SkeletonModifier3D always performs modification after the AnimationMixer.

In other words, the actual SkeletonModifier3D process only registers its own ObjectID with AnimationMixer. AnimationMixer performs modification after the registered SkeletonModifier3D playback process. If AnimationMixer has already processed the animations at the time of registration, SkeletonModifier3D will not register it and will perform modification on the fly.

Q. Why instead of connecting the signal like AnimationMixer.mixer_applied to each modifier, the order is managed by registering object IDs?

A. Because the signal does not care / cannot handle about the order of processing when multiple objects are connected.

This allows the processing order of SkeletonModifier3D to depend on the scene tree order and ProcessPriority, and prevents modifications from overwriting animations.

BTW, for performance reasons, both NodePath object pointers are cached.

Separate the methods related to PhysicalBone from Skeleton3D as PhysicalBoneSimulator3D

PhysicalBoneSimulator3D has SkeletonModifier3D as its base class.

PhysicalBoneSimulator3D has the following roles:

  • Stores the simulated pose separately from the Skeleton3D.
  • Apply the results of the simulation to Skeleton3D

Physical simulation process is performed on the PhysicalBone side of the child node. PhysicalBoneSimulator3D's CallbackModeProcess only affects when the simulation results are applied to Skeleton3D. So it does not affect the child nodes or the physical simulation.

Finally, for compatibility, Skeleton3D has PhysicalBoneSimulator3D as a virtual child. This ensures compatibility without changing the tree structure of old projects.

Deprecate pose override

Considering why bone_pose_override exists, there are two main reasons:

  1. to create a pose that has priority over animation playback
  2. to manage Transform separately from pose

This PR makes pose override internally unused. For about 1, it is solved by the property target_animation_mixer. For about 2, it should not kept the simulation data on the skeleton side in the first place, and is solved by having the PhysicalBone stored in the PhysicalBoneSimulator.

This allows multiple IK to be applied to a single Skeleton and blended with the results of physics simulations.

Godot.Engine.Nvidia.Profile.2024.02.03.-.09.44.47.12.mp4
Godot.Engine.Nvidia.Profile.2024.02.03.-.09.44.47.11.mp4

However, pose override is left as a deprecated method because it would be completely incompatible with old projects if it were eliminated.

Breaking Compatibility

This PR may change the update timing and processing order of Nodes with SkeletonModifier3D as their base class, which may change the behavior of the program. You will need to set the target AnimationMixer and Process Priority as necessary. However, if Process Priority is already set in the old project, there should be no problem.

To take it to the extreme, if you set the ProcessPriority of IK and PhysicalBoneSimulator to INFINITY - 1 and the ProcessPriority of BoneAttachment to INFINITY, you should be compatible with previous projects as long as you have not set ProcessPriority = INFINITY to the Node in the Scene in most of the case.

The following properties will change:

  • OpenXRHand.hand_skeleton
  • Skeleton3D.animate_physical_bones

However, the setter and getter will remain, as compatibility methods exist.

Other concerns

  1. We may need a function like reset_on_save() in SkeletonModifire. Would this be sufficient to reset Pose back to Rest?
  2. It is possible to apply another IK to a pose after the IK has been applied, but if you want those IKs to be based on the same pose -- in other words, they cannot be blended in "parallel". For that, perhaps something like a SkeletonModifierTree like AnimationTree would be needed, but I don't think it is necessary at this time. Because for gaming applications, most use cases should be "serial" blending, such as applying IKs to poses after an animation clip is played.

The latest implementation has changed from the description, so I provide a diagram below to explain the current implementation.

Although, the basic philosophy has not changed:

  • Avoid to use bone_pose_override internally
  • SkeletonModifier must be processed after AnimationMixer

How to work?

Diagram of the process order between dirty(for global pose), mod_dirty(for modifiers order) and update(for NOTIFICATION_UPDATE_SKELETON):

Unlike previous implementations, this PR separates the dirty and update flags. The dirty flag may be solved several times during a frame to calculate the correct bone global pose. The update flag is solved only once per frame when the skin or modifier needs to be updated.

exp1-01

As can be seen from the figure, it is guaranteed that a pose with dirty resolved can be obtained at the beginning of each Modifiers and Skin calculation.

Also, in the deferred process, using the "updating" flag avoids setting the "update" flag again, thus preventing infinite loops.

How to retrieve the modified pose?

The pose set by SkeletonModifier3D is only valid during the deferred process, so to retrieve it, you must connect the SkeletonModifier3D.modification_processed signal to some method is using get_bone_pose().

SkeletonModifier3D.process_modification() method is executed during the deferred process, so to use set/get_bone_pose() in that method is guaranteed to be processed in the same order as in the scene tree. And we need to continue to discuss with reduz how to virtualize or expose this method to GDScript.

Then, the Skeleton3D.pose_updated signal, which is mostly used by Editor, is not fired during the deferred process except for the first time when the deferred process starts. This means that modifications made by the Modifier are not detected by the Editor. However, the Skeleton3D.bone_pose_changed signal used by BoneAttachment3D still be fired.

Demo project

test_for_skeleton_modifier.zip


Production edit: closes godotengine/godot-roadmap#81

@TokageItLab TokageItLab added this to the 4.3 milestone Feb 3, 2024
@TokageItLab TokageItLab requested review from a team as code owners February 3, 2024 02:06
@TokageItLab TokageItLab force-pushed the animation-post-processable branch 3 times, most recently from 125e807 to 34fb830 Compare February 3, 2024 02:15
@CrayolaEater

This comment was marked as resolved.

@TokageItLab

This comment was marked as resolved.

@CrayolaEater

This comment was marked as resolved.

@TokageItLab TokageItLab force-pushed the animation-post-processable branch from 34fb830 to f3cb251 Compare February 3, 2024 02:36
@TokageItLab

This comment was marked as resolved.

@TokageItLab TokageItLab force-pushed the animation-post-processable branch from f3cb251 to 40c2802 Compare February 3, 2024 02:38
@TokageItLab TokageItLab changed the title Implement a base class SkeletonModifire3D as refactoring for nodes that may modify Skeleton3D Implement a base class SkeletonModifier3D as refactoring for nodes that may modify Skeleton3D Feb 3, 2024
@TokageItLab TokageItLab force-pushed the animation-post-processable branch 4 times, most recently from 79a6ac4 to 171f102 Compare February 3, 2024 02:56
@Maveyyl
Copy link

Maveyyl commented May 3, 2024

I tried the demo in the new 4.3 dev 6 and when I move the target node around, while the animation tree and players are inactive, the skeleton moves visually but the rotations of the bones aren't changed in the skeleton. I'm not sure the original goal of this feature but it currently doesn't allow us to make animations using the bones' rotation as tracks.

@TokageItLab
Copy link
Member Author

TokageItLab commented May 3, 2024

@Maveyyl Read description and class reference.

How to retrieve the modified pose?
The pose set by SkeletonModifier3D is only valid during the deferred process, so to retrieve it, you must connect the SkeletonModifier3D.modification_processed signal to some method is using get_bone_pose().

You were looking at the changes made only by the AnimationPlayer without SkeletonModifier, since the poses affected by the Modifier are not output to the inspector even if during animation playback; the Modifier's transformations are only valid during the update process, so you must use the signals.

If you just want to get/set the final visual pose, you can use the skeleton_updated signal added in #90575 instead of modification_processed.

The effect of the Modifier is discarded immediately after it is applied to the skin. We may be able to cache modified pose value and display it in the inspector, but should not do so because the pose is one frame old and does not represent the actual current frame internal pose, which can be unsafe.

@Maveyyl
Copy link

Maveyyl commented May 3, 2024

Thanks for explaining, I don't really understand why the values wouldn't be displayed in the bones transforms though.

I try to do the process of creation animations in godot, by moving the target node and saving the bone transforms in the animation keyframe, I opened a ticket to signal the impossibility and you referenced your push request as a solution. I don't see how I should do that with the signals you've exposed to me here. Could you explain?

@TokageItLab
Copy link
Member Author

TokageItLab commented May 4, 2024

@Maveyyl One of the problems is that SkeletonIK3D is not made for creating animations (although it is deprecated).

What we (animation team and reduz) agreed on in this implementation was that there needs to be a clear distinction between “bone modification for post effects” and “bone modification for the editor”. It is the latter that you need.

For the case where the AnimationTrackEditor is not changed and the Bone pose is set using the existing inspector:

  • If the IK is exclusive, you can use the normal set_bone_pose instead of using the modifier process. In other words, you need an editor that ignores deformation by the AnimationPlayer during IK editing.
  • In other words, the "bone modification for the editor" should not inherit from SkeletonModifier.

For the case of enhance AnimationTrackEditor and keying the pose in the modification process:

  • Possibly BoneAttachment may be usable in there, but at least some enhancement to AnimationTrackEditor for keying should be needed.

In the end, this is a problem of mixed use, so unfortunately SkeletonModifier probably won't solve what you are wanting to do without making new editor plugin. If you want to use IK to create animations, we will need to create IK Editor as a separate function.

@Maveyyl
Copy link

Maveyyl commented May 4, 2024

@TokageItLab When we're creating the animation, the animation player isn't playing and shouldn't be modifying the skeleton, right? So if I extend SkeletonIK3D to call set_bone_pose when receiving the signal you talked me about, I could technically make it work?

Thanks for explaining this all though! I get it now that my needs are not where the work was at.

@TokageItLab
Copy link
Member Author

TokageItLab commented May 4, 2024

@Maveyyl It is a little hacky, but if you use it as a deferred process in that signal like:

func connected_method():
    skel.set_deferred("bones/" + str(idx) + "/pose", skel.get_bone_pose(idx))

it possibly give you the behavior you are looking for.

However, this may possibly break IK Interpolation as a post process, so we cannot guarantee that it will work.


Or, if you do not need to check the value in real time, you can store the pose retrieved in the signal at the your side and key that value in place of the current bone pose when keying.

var cached_visible_pose: Transform3D

func connected_method():
    cached_visible_pose = skel.get_bone_pose(idx)

func your_animation_track_editor_insert_key():
    anim.rotation_track_insert_key(track_idx, time, cached_visible_pose.basis.get_rotation_quaternion())

FYI, BoneAttachment may help you, as it has a similar behavior of that.

@sinewavey
Copy link

hi, I've just installed 4.3b1 - with no other changes, this entirely broke my IK setup, and I'd like to figure out a few things:

  1. why
  2. how i can fix this
  3. how i should be correctly doing things.

currently, I have a pair of arms with an IK3D each, left and right. Both are child to a node that interpolates its own position to match a physics body position.

on top of this node is an additional node with a weapon viewmodel, and two target marker nodes for the IK3D.

Upon opening to 4.3, first I noticed that the IK chain resulted in an entirely different outcome, and that both limb chains affected the other from a common root join.

I tweaked that, and while it gets close, another huge problem remained: physics interpolation.
No matter what combination I tried, in various modes and inheriting different transforms, I can't get this to work correctly.

I'm wondering what the suggested workflow for IK arms with posed hands is supposed to be now.

@TokageItLab
Copy link
Member Author

@sinewavey If you are using a Node outside of Skeleton that depends on a Physics process and you want to combine it with a SkeletonModifier, you must correctly set the Modifier Callback Mode Process in Skeleton.

image

The default is Idle, so try setting it to Physics. I remember that in the old IK was forced to work with the Physics process, but now it uses a process that depends on the Skeleton's settings.

@sinewavey
Copy link

sinewavey commented Jun 3, 2024

Oh - that explains why I couldn't find any related options despite your previous mentions. It's a property of the skeleton, which I had overlooked and didn't check.
Issue fixed! Thank you very much for the quick followup time.

@ZenPyro
Copy link

ZenPyro commented Jun 7, 2024

First, I want to say what a great job you guys have done! Really pushing Godot forward with this.

Anyway, I have had a question for quiet sometime now, and I thought now was as good a time as any to ask it! I never really knew what the "proper" workflow for using IK rigs to create procedural animations. I have always used the deprecated SkeletonIK3D Node, and had it follow a Marker3D Node which would act as the SkeletonIK3D Node's target. I wasn't sure how the 4.3.beta was going to change this (the workflow of setting up procedural animations), but I did notice that the documentation on the SkeletonIK3D Node has gotten more "aggressive" with what the plan for it is:

Deprecated: This class may be changed or removed in future versions.

So I realized I can't rely on the SkeletonIK3D Node forever for creating procedural animations (especially since this project will span years), so this is a good time to switch away from using it in my procedural animation pipeline. The problem is, I don't know what any alternative pipeline could be? I don't believe anything that was added in this beta would aid me in procedural animation (emphasis on "believe").

Basically I am trying to say is, you seem like the best person to ask, what would the "proper" procedural animation pipeline look like for Godot in a release where the SkeletonIK3D Node is not just deprecated, but totally removed!

@TokageItLab
Copy link
Member Author

TokageItLab commented Jun 11, 2024

We are just preparing an article about SkeletonModifier in godotengine/godot-website#854, and I have made some additions based on everyone's comments on here. It contains a tutorial on creating a custom SkeletonModifier, so it should be available in 4.3 (at least after beta2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done