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 BonePicker and simple auto mapping to BoneMapper #63854

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Aug 2, 2022

Co-authored-by: K. S. Ernest (iFire) Lee [email protected]

Section 6 and 7 of godotengine/godot-proposals#4510.
Closes godotengine/godot-proposals#4510.

image

Bones are estimated based on commonly used bone names and parent-child relationships. They are based on my experience and data collected by @fire.

Note: For now, this only works if you use SkeletonProfileHumanoid, which is the default preset for SkeletonProfile in Godot and it is for the most part hard coded.

@fire
Copy link
Member

fire commented Aug 3, 2022

Will be providing models and such to tokage.

@TokageItLab TokageItLab marked this pull request as draft August 3, 2022 00:53
@TokageItLab TokageItLab force-pushed the auto-bone-mapping branch 2 times, most recently from 30abd4c to 63b6e4c Compare August 3, 2022 03:00
@TokageItLab TokageItLab force-pushed the auto-bone-mapping branch 2 times, most recently from 86c5073 to f537819 Compare August 3, 2022 16:20
@TokageItLab TokageItLab marked this pull request as ready for review August 3, 2022 16:21
@TokageItLab TokageItLab requested a review from a team as a code owner August 3, 2022 16:21
@TokageItLab
Copy link
Member Author

Update usability.

image

@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 3, 2022

Enum list generation is too heavy (not related to this PR), I will send improvement later.

@TokageItLab TokageItLab force-pushed the auto-bone-mapping branch 3 times, most recently from 7d88f34 to c0cf792 Compare August 3, 2022 22:14
@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 3, 2022

Added BonePicker as Section 6 of godotengine/godot-proposals#4510.
By implementing the BonePicker and eliminating the Enum, the generation of the GUI has been made lighter.

image

@TokageItLab TokageItLab changed the title Add simple auto mapping to BoneMapper Add BonePicker and simple auto mapping to BoneMapper Aug 3, 2022
@TokageItLab TokageItLab force-pushed the auto-bone-mapping branch 3 times, most recently from 1dcda8b to d151aad Compare August 4, 2022 00:04
@JohanAR
Copy link
Contributor

JohanAR commented Aug 5, 2022

@TokageItLab I don't know which one caused it to fail, but it no longer happens with the updated commit. Is it still relevant to know? Now it looks like it autodetects everything for group Body except upper chest and root, while nothing is detected for the head (model has eyes but no jaw) or for the hands.

This is an example of how the fingers are named:
image

Maybe you can send me a message on the discord server if you need any more info about the model.

As for general usability feedback, it would be nice if it was possible to keep the bone picker open, for example replacing the scene tree in the left view. This would drastically reduce the amount of clicking per bone one wants to map.

It would also be nice if the skeleton was drawn on top of the model while working with the Skeleton3D, similar to how it looks in Unity, with the currently selected bone (in the picker) being highlighted of course.

@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 5, 2022

@JohanAR Thanks, finger0, finger1 are indeed undetectable cases as I expected.

As for general usability feedback, it would be nice if it was possible to keep the bone picker open, for example replacing the scene tree in the left view. This would drastically reduce the amount of clicking per bone one wants to map.

It would also be nice if the skeleton was drawn on top of the model while working with the Skeleton3D, similar to how it looks in Unity, with the currently selected bone (in the picker) being highlighted of course.

We discussed with @lyuma the same thing about it, but it is hard work and has been put on the back burner for now.

@JohanAR
Copy link
Contributor

JohanAR commented Aug 5, 2022

@TokageItLab would it be possible for the bone picker popup to at least remember which tree nodes were expanded the last time, so it doesn't collapse the entire tree every time it is opened?

@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 5, 2022

@JohanAR Okay, I made it recording the folding state.

@JohanAR
Copy link
Contributor

JohanAR commented Aug 5, 2022

@TokageItLab Awesome! Feels much quicker to manually assign bones already :)

Another minor suggestion: Make double clicking a bone in the popup select it, so I don't have to press the ok button. And maybe make it possible to double click the circles in the in the group illustration, so that opens the picker for the selected item.

@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 5, 2022

@JohanAR Fixed. Tree class has a double-click signal, so it can be confirmed with a double-click now. However, Button class does not have a double-click signal, so the opening picker still needs to press the button for the picker.

@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 5, 2022

So far, I consider sequential number naming such as finger0, finger1 is a corner case.

The reason for using such numbers for fingers is that in some of the models I have seen, there are some cases where the fingers are merged.

For example, there are models that bind the thumb as finger0 and the other four fingers as finger1. In other cases, there are models that bind the thumb as finger0, the index and middle fingers as finger1, and the ring and little fingers as finger2.

In these cases, it would be correct to treat finger0 as the thumb, but for the other fingers, there is no clear name because they are merged. In other words, auto-mapping cannot make the correct decision.

It would be correct to map each bone from thumb to little finger if finger0-finger4 existed as an extension of that. However, for now that is pretty much a corner case that only the proprietary Biped system in 3dx max might generate. Auto-mapping is just an assist tool, and its accuracy is not guaranteed for all cases. Manual mapping and reusing BoneMap resources should be sufficient for those cases IMO.

@TokageItLab
Copy link
Member Author

TokageItLab commented Aug 6, 2022

As a subplan for cases where finger detection cannot be done by generic name, I added a string sorting of the bone names containing "finger" to perform the assignment. This should probably work for 3ds max Biped models @JohanAR.

This means that it could break if there are 4 sequentially numbered fingers and one accessory name containing "finger" is on the palm, but I think that is a extreme-extreme corner case.

@TokageItLab TokageItLab force-pushed the auto-bone-mapping branch 6 times, most recently from 081dbf0 to e7b234f Compare August 12, 2022 21:28
@akien-mga
Copy link
Member

Needs rebase.

@TokageItLab TokageItLab marked this pull request as draft August 22, 2022 20:40
@TokageItLab TokageItLab marked this pull request as ready for review August 23, 2022 07:02
Co-authored-by: K. S. Ernest (iFire) Lee <[email protected]>
@akien-mga akien-mga merged commit 5c5bc21 into godotengine:master Aug 23, 2022
@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.

Implement Retarget feature in the importer
6 participants