godot
godot copied to clipboard
Resolve bind poses from FBX clusters instead of FBX poses.
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.
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?
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.
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!
@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.
I think this needs a rebase on master.
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.
Thanks!