Resolve bind poses from FBX clusters instead of FBX poses.

[bqqbarbhg]

See #90986 for more context.

Skeletons imported with ufbx sometimes have broken bind poses:

ufbx previous

ufbx previous bind pose

this PR (both bind pose and initial pose are the same)

Turns out this was not a bug, either in ufbx or in the Godot ufbx importer. The issue is broken data in files, the Godot ufbx importer read the bind poses from FBX Pose objects. The rest pose matrices in these were correct for most files, but they seem to be incorrect in a significant minority of files.

FBX2glTF does not attempt to import bind poses, so it avoids this problem. This PR introduces an alternative method of reading the bind pose data, and initial testing shows improved skeletons in all tested files. I'll do more rigorous testing in the background using the same process as in #90986.

One thing to consider is whether we should have an option whether to import the bind pose data or not. This would allow compatibility with FBX2glTF and allow users to hotfix potentially broken bind poses in files, though if testing indicates that the new method is fully robust, it may not be as necessary.

[fire]

FBX2GLTF doesn't use bind poses because of invalid data, this alternative approach is cool, and I like how it fetches the bind pose from either two successive skin clusters or a top-level bone.

@lyuma mentioned wanted to review it

[bqqbarbhg]

Ran through all the dataset files containing skeletons:

Loaded 2264 files in 598min 50s.
Processed 5.41GB at 0.15MB/s.
Ignored 0 invalid files.

After this fix there were no more obviously broken skeletons, and most seem to match FBX2glTF 100% accurately. Result categories out of 2264 files:

• 2135 equal: Again some with non-important material or light differences
• 80 with broken skinned meshes in FBX2glTF, these seem to be caused by blend shapes
• 25 meshes with relatively significant differences in skeletons, but this time no obvious issues. Mostly the differences are in the roll of the bones, probably caused by the bind pose data in the file.
• 6 files with various ufbx issues for me to look at later:
  • ufbx bad spine: FBX2glTF, ufbx
  • ufbx missing hair and shoes: FBX2glTF, ufbx
  • ufbx missing spears: FBX2glTF, ufbx
  • ufbx missing triangle: FBX2glTF, ufbx
  • ufbx triangulation issue: FBX2glTF, ufbx
  • ufbx extra bones: FBX2glTF, ufbx (wrist bones, the face here is the usual FBX2glTF "missing skinned mesh" issue)

Example of the extent of skeleton difference caused by loading the bind pose:

FBX2glTF

ufbx

All in all, the testing indicates that this fix seems to be robust and fixes all the significant issues encountered before, without breaking any of the previously working cases.

The roll issue and the occasional bone distortion is still significant enough, that I'd propose adding an importer option to disable attempting to import the bind pose, treating the initial pose as rest pose, like FBX2glTF does. Any thoughts?

[fire]

We need to debate adding a new option, but that can review that in a separate pr. That means you can stack the two prs together into that pr.

[bqqbarbhg]

We need to debate adding a new option, but that can review that in a separate pr. That means you can stack the two prs together into that pr.

Makes sense, in that case this PR is fine to merge by me!

[bqqbarbhg]

@lyuma provided some test cases with multiple bind poses per bone.

Added explicit support for this in the form of warning the user if a bone has multiple conflicting bind pose definitions. Note that this may still mix up different bind poses for different parts of the mesh, but at least now bones do not get completely messed up and users should see a warning.

I'd still highly recommend adding a setting for disabling importing bind poses (maybe even not importing them by default), as they seem to have quite a few problems in the wild, and even this solution is a bit hacky.

[fire]

I think this needs a rebase on master.

[lyuma]

Looks great! The added check gives me more confidence it will not tear my armature apart in the rest.
We still need to discuss making an option for this behavior and what the default should be but again we should do this in a separate change.

please rebase as ifire said and squash your commits into one. I kept a copy of the history in my own fork so don't worry about losing history.

[lyuma]

A little worried this will logspam but let's keep it and it will help us diagnose issues during the beta test.

Not something we have to fix here, but we should figure out a better approach for import warnings. I want to show warnings and errors per-file in the import dock or in some dedicated place.

[bqqbarbhg]

Yeah was a little concerned about this as well, but it feels like something that should be informed to the user. Of course collecting them would be ideal.

On that note, we should also probably expose ufbx_metadata.warnings[] there, as ufbx can report a bunch of suspicious stuff in the file.

[bqqbarbhg]

Thanks! I'll squash the commits after seeing that the CI passes etc.

Also ran through all the skeletons in my dataset (+ the entirety of the public dataset) again with the new changes. Seems like the new bind pose resolution method loads files correctly.

29 of 2636 files had multiple bind poses, and on manual review all seemed to be alright. One file had only a 4% error in bone pose, so that might have been a false positive. I just set the 1% relative error threshold arbitrarily, so could be looked into if necessary.

[akien-mga]

Thanks!