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

Add tips detection to auto mapping in bone mapper #90050

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Mar 30, 2024

Fixes #89725.

GodotHumanoid does not have a metacarpal for fingers other than the thumb. To avoid assigning them incorrectly, it is necessary to assign from the ends. However, some models might have "Tips" or "Leaf" bones to provide length information, so assigning from the ends is problematic.

This PR, make auto mapping can detected Tips and the mapping will be offset when candidate bones for the fingers has the name Tip or Leaf, or if none of the meshes have weights bound to them.

If the joint count of fingers excluding the thumb is the same, if even one Tip is detected, it will be apply by all bones. This is because the ReadyPlayerMe model is weird since only the index finger tips has weight.

This becomes a problem in cases where you intentionally have tips on only one finger, so if the number of bones is different, determine the tips individually. Still it may be a problem if the number of fingers is the same and you forget to bind only a specific finger, but I consider it is a rare case.

Make the detection based on the name match and the number of children in the thumb method by @lyuma.

@TokageItLab TokageItLab added this to the 4.3 milestone Mar 30, 2024
@TokageItLab TokageItLab requested review from fire, lyuma and a team March 30, 2024 19:33
@TokageItLab TokageItLab force-pushed the add-tips-detection branch 3 times, most recently from a179e46 to 2483ebb Compare March 30, 2024 19:53
@lyuma
Copy link
Contributor

lyuma commented Mar 31, 2024

Added some more test files which do not contain any mesh.

I am skeptical of using mesh vertex-based approaches, since files such as these do not contain any mesh at all:
simple_vrm_rig.glb.gz - This one is in T-pose
simple_vrm_rig_anim4.blend
simple_vrm_rig_nlaonly2.fbx - this is animated and not in t-pose

I'll test the PR and see if I can offer any changes.
For example, I ran into a similar problem in Unidot (which uses a copy of Godot's skeleton profile guesser) and I came up with this approach:
V-Sekai/unidot_importer@4f4f62c
However, I am a little concerned that this does not sufficiently address cases where there are two _End bones (which happens when importing and re-exporting an animation FBX with the default setting of "Add Leaf Bone"
But maybe it will mitigate the most common cases

@TokageItLab
Copy link
Member Author

Indeed it is little skeptical, but since BoneMap is reusable, so I think this implementation would be a not big problem. So it would work as a workaround to use a BoneMap generated for a model with a mesh in an animation file without a mesh, as long as they have the same bone name.

@TokageItLab
Copy link
Member Author

TokageItLab commented Apr 1, 2024

From discussed with @lyuma, we think we can add a couple of decision points, so I will add them.

So the steps with the following priority order should be used for estimation:

  1. put "distal" in the search suggestions
  2. if the number of thumb bones is 4, there are Tips
  3. look at the mesh weights

@TokageItLab TokageItLab marked this pull request as draft April 1, 2024 06:31
@TokageItLab TokageItLab marked this pull request as ready for review April 5, 2024 22:16
@TokageItLab
Copy link
Member Author

Detection by the number of thumb children conflicts with detection from weights. Probably, given the existence of skeletons without skins, detection by the number of thumb children should be preferred. Therefore, I will adopt @lyuma's method overall, thanks.

Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

Reviewed on a call with fire. I think this is good. It can use more testing, but that's what beta is for.

What's important is this improves Mixamo and ReadyPlayerMe compatibility by adding a few heuristics, so I think it's fairly safe.

editor/plugins/bone_map_editor_plugin.h Outdated Show resolved Hide resolved
editor/plugins/bone_map_editor_plugin.h Outdated Show resolved Hide resolved
editor/plugins/bone_map_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/bone_map_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/bone_map_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/plugins/bone_map_editor_plugin.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga merged commit c205f02 into godotengine:master Apr 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@TokageItLab TokageItLab deleted the add-tips-detection branch June 29, 2024 11:44
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.

Skeleton Retarget for ReadyPlayerMe avatars picks incorrect bones
5 participants