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

Fix XRBodyModifier3D hip driving for avatars no Root bone #89727

Merged
1 commit merged into from
Mar 24, 2024

Conversation

Malcolmnixon
Copy link
Contributor

The XRBodyModifier3D node fails to correctly drive avatars which have no Root bone. This occurs on common avatars such as ReadyPlayerMe, Mixamo, and AvaturnMe where the root bone of the skeleton is the hips. The importers Skeleton Retarget doesn't add a Root but instead leaves the Hips as the root bone but with the corrected name.

The XRBodyModifier3D code assumes their must always be a bone associated with the Root tracking joint, and picks bone 0 (the first bone) if it didn't find it by the correct bone name. Unfortunately if it does this with an avatar where bone 0 is the hips then it associates the hips with the root and locks the hips in place.

This PR removes the offending code:

// If the root bone is not found then use the skeleton root bone.
if (bones[XRBodyTracker::JOINT_ROOT] == -1) {
bones[XRBodyTracker::JOINT_ROOT] = 0;
}

With this removed the behavior is as follows:

  • The XRBodyModifier3D is still positioned at the trackers "root" location
  • The Hips bone is correctly positioned relative to the trackers "root" location

The following video demonstrates four different avatars moving correctly:

  1. An AvaturnMe avatar with no Root bone (bone 0 = Hips)
  2. A Mixamo avatar with no Root bone (bone 0 = Hips)
  3. A ReadyPlayerMe avatar with no Root bone (bone 0 = Hips)
  4. A VRM avatar with bone 0 = Root
Avatars.mp4

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

lgtm

@fire fire requested review from a team, TokageItLab and lyuma March 21, 2024 05:46
@fire
Copy link
Member

fire commented Mar 21, 2024

At a first glance a hips bone is not a root bone and shouldn’t be a root bone. I’ll try to give a proper answer or someone from the animation can be more detailed.

We’re at GDC.

@TokageItLab
Copy link
Member

TokageItLab commented Mar 21, 2024

I assume that adding RootBone would seem to be better than using Hips as RootBone. (If necessary, such a feature may need to be added to Retargeter as well)

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

I have seen a PR somewhere that tried to extract the root motion from the hips transformation at realtime in AnimationTree, but I remember that it was rejected because it would not be good for performance.

Also, just yesterday, reduz and I were discussing moving the root motion from AnimationMixer to Skeleton3D, and in this case, using Hips as Root is also not a good idea for the same reason as above.

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

Oh, sorry, I misunderstood the PR.

If this is a PR to "do nothing" instead of "include RootMotion in Hips", then it makes sense.

@Malcolmnixon
Copy link
Contributor Author

The purpose of this PR is indeed to prevents the hips from erroneously being driven with "root motion" rules.

My Godot Mocap plugins (VMC, Movella, AxisStudio) do synthesize root motion from the hips - because the mocap data-stream they're consuming doesn't include root motion information. It's just a Vector3.slide to project the hips onto the floor, followed by a double-cross-product to orient the joint facing upwards.

	# Get the hips transform
	var hips : Transform3D = _body_tracker.get_joint_transform(XRBodyTracker.JOINT_HIPS)

	# Construct the root under the hips pointing forwards
	var root_y = Vector3.UP
	var root_z = -hips.basis.x.cross(root_y)
	var root_x = root_y.cross(root_z)
	var root_o := hips.origin.slide(Vector3.UP)
	var root := Transform3D(root_x, root_y, root_z, root_o).orthonormalized()

@akien-mga akien-mga closed this pull request by merging all changes into godotengine:master in 06abc86 Mar 24, 2024
akien-mga added a commit that referenced this pull request Mar 24, 2024
…bone

Fix XRBodyModifier3D hip driving for avatars no Root bone
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants