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 XR Body crouching and climbing #89103

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

Malcolmnixon
Copy link
Contributor

@Malcolmnixon Malcolmnixon commented Mar 3, 2024

The XRBodyModifier3D node has a bone-update option of "Rotation Only" which is intended to preserve the lengths of bones. This is achieved by only setting the bone rotation rather than applying both rotation and position.

Unfortunately this option also skipped setting the "Hips" bone position which is the first actual bone of the body. As a result the players hips are fixed at the height of the T-Posed character.

The fix is to always set the Hips bone position and rotation to position the character, and then all child bones are updated as configured by the bone-update setting.

image

The following video demonstrates how this change affects character models performing different actions which affect vertical height:

Hips.Joint.PR.mp4

@Malcolmnixon Malcolmnixon requested a review from a team as a code owner March 3, 2024 03:27
@fire fire requested a review from a team March 3, 2024 03:29
@TokageItLab
Copy link
Member

TokageItLab commented Mar 3, 2024

How is this handled with respect to root bone?

Also, not directly related to this, but for about the position value movement, probably it needs to be multiplied by the Skeleton3D.motion_scale.

@dsnopek
Copy link
Contributor

dsnopek commented Mar 3, 2024

How is the root bone "defined"? If it's defined at the bottom center of the avatar (or something similar), then I think positioning the hip bone makes sense. Since all bone positions are relative (except the root itself), that means the hip would be positioned relative to the root, and so we shouldn't have issues with that allowing the player to move through walls: it should be mostly Y position, and maybe some small X/Z variations (I guess as far as you can shift your hips over your center).

@TokageItLab
Copy link
Member

TokageItLab commented Mar 3, 2024

I am not familiar with the specs of the XR controller, but if the XR tracker does not have the concept of root, then there should be an option for the movement of the XZ axis of the Hips that can be transferred to the Root.

At this point, whether or not the root_bone is actually moved should also be an option. Perhaps we should be able to specify a Modifier for the RootMotionView.

Well these may be a bit off-topic, but we need to consider that the option to use Hips or specify Root is missing and if it is added it may be included in the BONE_UPDATE enum.


By the way, regarding motion_scale, since GodotHumanoid has the waist height as motion_scale when retargeting, it is necessary to multiply the tracker's movement delta value by motion_scale.

In other words, the movement by GodotHumanoid's set_bone_pose_position() without multiplication is based on the assumption that the model has a hips height of 1 meter. Since the movement of a model with a hips height of 0.5 meter can be simply estimated to be a movement * 0.5, the model's slip can be reduced by multiplying by the model's hips height. Well, it may be necessary to have the actual tracked human hips height as a calibration factor in XRServer. So, the calcuration for applying the correct position value should be: pose_position_delta = tracker_motion_delta * (1.0 / actual_tracked_human_hips_height) * motion_scale.

@Malcolmnixon
Copy link
Contributor Author

How is the root bone "defined"? If it's defined at the bottom center of the avatar (or something similar), then I think positioning the hip bone makes sense. Since all bone positions are relative (except the root itself), that means the hip would be positioned relative to the root, and so we shouldn't have issues with that allowing the player to move through walls: it should be mostly Y position, and maybe some small X/Z variations (I guess as far as you can shift your hips over your center).

All the models I've put through the Godot Humanoid Skeleton Retargeting have the Root bone at ground level under the hips. The SkeletonProfileHumanoid has the "Root" at 0,0,0 and any minor positional issues may be automatically handled by the "Rest Fixer" or the "Fix Silhouette" features.

@Malcolmnixon
Copy link
Contributor Author

By the way, regarding motion_scale, since GodotHumanoid has the waist height as motion_scale when retargeting, it is necessary to multiply the tracker's movement delta value by motion_scale.

The trackers don't provide movement deltas - only the tracked absolute positions and rotations.

There appear to be two settings of interest with regards to scaling:

  • Skeleton3D.motion_scale - useful for fitting the tracked player/actor joints to the size of the avatar
  • XRServer.world_scale - useful for fitting the avatar to the world

I believe the correct set of actions is:

  1. Apply the Skeleton3D.motion_scale to the set_bone_pose_position position parameter
  2. Scale the XRBodyModifier3D transform by XRServer.world_scale

The intended result would be that differences in player/actor size could be adjusted to fit the size of the avatars skeleton, and then the entire avatar - mesh and skeleton would be scaled to fit the world (assuming the avatar is a child of the XRBodyModifier3D).

@TokageItLab
Copy link
Member

The trackers don't provide movement deltas - only the tracked absolute positions and rotations.

Then, I think the delta needs to be calculated on the XRBodyModifier3D in the same way as AnimationMixer for root_motion. It will be possible to follow up later.

@dsnopek dsnopek requested a review from a team March 3, 2024 18:17
Copy link
Contributor

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

@Malcolmnixon:

All the models I've put through the Godot Humanoid Skeleton Retargeting have the Root bone at ground level under the hips. The SkeletonProfileHumanoid has the "Root" at 0,0,0 and any minor positional issues may be automatically handled by the "Rest Fixer" or the "Fix Silhouette" features.

Ok, then this change makes sense to me conceptually! I haven't tested it, though.

@Malcolmnixon
Copy link
Contributor Author

Malcolmnixon commented Mar 4, 2024

This PR attempts to resolve the scaling issue by:

  1. Applying the Skeleton3D.motion_scale to the incoming joint positions
  2. Applying the XRServer.world_scale to the XRBodyModifier3D transform (which includes the scaled root joint)

This gives the developer the ability to modify Skeleton3D.motion_scale to better match the tracking data to the skeleton, and then adjust XRServer.world_scale to uniformly scale the entire avatar against the world.

The following video demonstrates playing with these effects:

Skeleton.and.World.Scale.mp4

Note: In the first half of the video I play with Skeleton3D.motion_scale to stretch the skeletons resulting in stick-figures. In this state the ground movement is still correct (feet in contact with the ground are stationary).

In the second half of the video I restore the Skeleton3D.motion_scale back to a sensible value and then play with the XRServer.world_scale. This scales the XRBodyModifier3D thus scaling the entire avatar. Again the ground movement is still correct (feet in contact with the ground are stationary).

scene/3d/xr_body_modifier_3d.cpp Outdated Show resolved Hide resolved
scene/3d/xr_body_modifier_3d.cpp Outdated Show resolved Hide resolved
@Malcolmnixon
Copy link
Contributor Author

@BastiaanOlij Could you check the updated documentation with regards to node scaling and positioning. It's similar to the proposed XRHandModifier3D changes.

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.

Looks good Malcolm.

We could (ab)use this opportunity to also mark this class as experimental. Best to get that out of the way before 4.3 releases :)

… set to "Rotation Only". Apply appropriate world and skeleton scaling.
@Malcolmnixon
Copy link
Contributor Author

We could (ab)use this opportunity to also mark this class as experimental. Best to get that out of the way before 4.3 releases :)

Switched XRBodyTracker, XRBodyModifier3D, XRFaceTracker, and XRFaceModifier3D to experimental.

@akien-mga akien-mga merged commit ee3c010 into godotengine:master Mar 6, 2024
16 checks passed
@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.

6 participants