Add ufbx for FBX importing

[fire]

This restores FBX imports in Godot.

I would need people to test their models, or downloaded models. Specifically, I want to test that:

• The model imports correctly, with textures and animation
• That the process is smooth and intuitive

To test this properly, a variety of models is required, so do not hesitate to try different ones. You can find free FBX models at https://sketchfab.com/3d-models?features=downloadable (be careful, you may see NSFW models).

If you run into an issue, please post here with your OS, and as much information as you can about the error.

Screen_Recording_2023-09-15_at_2.49.16_PM.mov

This work is supported as part of V-Sekai and with help from @bqqbarbhg.

The work was approved at the Animation / 3D Pipeline meeting and was also discussed with the production team as an experimental marked feature for 4.3.

1. https://godotengine.org/fbx-import/
2. https://godotengine.org/article/fbx-importer-rewritten-for-godot-3-2-4/
3. https://godotengine.org/article/here-comes-godot-3-2/#3d-assets

• Marked experimental
• Added ufbx to Godot Engine
• Correct uv1 and uv2 for import.
• Animation clips support
• Skeletal animation support
• Meshless fbx files are imported with animations
• Blend shapes are imported
• Allow Utf8 string name imports
• Postponed camera imports
• Postponed light imports
• Blend shape animations are imported
• Eight uv support
• Remove code scaffolding from GLTFDocument
• Prune extra files in thirdparty/ufbx
• Need help with the disable enable scheme with FBX2GLTF
• Handle scale ignore [HARD]
• Transparency is enabled
• HANA bug with clusters being parented to the root and failing import
• Postponed Camera imports again
• Postponed Light imports again
• Add documentation
• Adapt FBXDocumentExtension

No proposal exists because this restores FBX import with around five to eight repeated attempts.

See also https://github.com/V-Sekai/godot-ufbx

Finalization after 4.3 window is open

- [ ] Fix engine bugs on 2 gigabyte fbx samples (cc-by)

• Move ufbx into the godot/thirdparty folder
• Write licensing information

Finalization before 4.3 window closes

1. Cameras
2. Lights
3. Curve paths

Known Issues

When the fbx2gltf import has failed it will use the mostly recently cached scene from the ufbx import.

See #86309 for the related issue.

Production edit: closes godotengine/godot-roadmap#40

[bqqbarbhg]

By the way, ufbx also has optional support for importing Wavefront .obj files that is as exhaustively tested. I'm not sure what kind of support Godot has for .obj files currently, but it might be worth comparing.

[fire]

@bqqbarbhg It's a good idea, but I would prefer to merge ufbx for FBX importing first and then we can expose the obj importer later. For clarity and maintenance ease.

[fire]

The codebase is synced from https://github.com/V-Sekai/godot-ufbx and has commit logs.

[DigitalN8m4r3]

Tested with
*mixamo models
*rdyplayerme models
*custom models

so far no issues, good work 👍

[fire]

From @bqqbarbhg the latest update supports fixes bone scaling when using unusual inherit modes.

In addition to normal bones (RSrs, RS being rotation/scale matrix of parent, rs of the child), FBX supports bones that ignore the size of their parent (Rrs), or composite it into their own size (RrSs).

Added a new feature in ufbx to create helper nodes to support these inherit modes in a standard scene graph/skeleton.

[fire]

Needed to rebase because of https://github.com/godotengine/godot/pull/81968/files preventing me from testing mac.

[fire]

TL;DR we bias the importer towards Maya FBX because it is the pipeline lacking representation in Godot Engine,

Blender has a functional GLTF pipeline and if needed Blender has a workaround using the FBX exporter option "FBX Units Scale".

In-depth summary

Here's an ai summary of our conversation [iFire, and bqqbarbhg] why to bias Maya FBX vs Blender FBX:

In the context of the godot-ufbx, we bias towards Maya FBX files over Blender FBX files by using UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY avoiding UFBX_SPACE_CONVERSION_ADJUST_TRANSFORMS.

The UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY option in the ufbx library is used to scale the geometry of a model. This is similar to how the fbx2gltf tool operates. It's particularly useful for handling files that do not have the 100x scaling in nodes, which is common in many models exported from Maya (like without 100x scaling).

When you use UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY, the 100x scale factor applied by Blender during export will be retained. This means that if you import a model from Blender that was exported with this option enabled, the model will appear correct but scaled.

If you are exporting your models from Blender, you can select the "FBX Units Scale" option before exporting. This will ensure that the scale factor is correctly set during export, avoiding the need for manual adjustment when importing back into Blender.

blender_default.fbx Example

Export the test scene from Blender will save the file as:

Node (scale 100)
  Vertices [(1, 0, 0), (-1, 0, 0), (0, 1, 0)]

importing with UFBX_SPACE_CONVERSION_ADJUST_TRANSFORMS modifies it to

Node (scale 1)
  Vertices [(1, 0, 0), (-1, 0, 0), (0, 1, 0)]

and UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY modifies it to

Node (scale 100)
  Vertices [(0.01, 0, 0), (-0.01, 0, 0), (0, 0.01, 0)]

and on the other hand exporting a triangle like that from Maya would usually be like

Node_Maya (scale 1)
  Vertices [(100, 0, 0), (-100, 0, 0), (0, 100, 0)]

UFBX_SPACE_CONVERSION_ADJUST_TRANSFORMS
Node_Maya (scale 0.01)
  Vertices [(100, 0, 0), (-100, 0, 0), (0, 100, 0)]

UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY
Node_Maya (scale 1)
  Vertices [(1, 0, 0), (-1, 0, 0), (0, 1, 0)]

blender_default.fbx.zip

[fire]

I talked with @bqqbarbhg about it being good to have camera/light support back in and updating ufbx fixes to include.

My plan is to prepare for merge first and try get additional updates for camera/light and the ufbx library later. Our discussion mentioned that threading will help with performance but ufbx is far from the biggest bottleneck and have some ufbx fixes from fuzzing.

[akien-mga]

Added a check for whether the fbx2gltf importer is actually enabled. Needs testing.

[fire]

The proper architecture should be scene/3d/model_.h stuff as the abstraction for GLTF and FBX models. These CANNOT depend on any code in the gltf and fbx modules! Otherwise they're not abstractions at all. And that's a major problem currently so I had to move scene/3d/model_.h back to the gltf module, after removing its coupling to the fbx module...

If that's not possible, then the second best would be to have the fbx module depend fully on gltf (which it does in the current PR, despite the attempt at abstracting this away), reusing as much code as possible, instead of duplicating it like the original version of this PR did.

1. scene/3d/model_*.h's abstraction is not used by anything other than me forcing fbx and gltf into a base class. @lyuma proposed deleting it.
2. fbx module depend fully on gltf document in practice
3. fbx extends gltf seems to fit more closely the code architecture.
4. We were thinking about deleting scene/3d/model_*.h as a pull request on top of this one because of the compatibility breakage problem on GLTFDocument we didn't have a good solution for.

@lyuma discussed with me removing the scene/3d/model_*.h and that seems like it would lead to a design where fbx module depend fully on gltf document, we discussed yesterday about this design and it has some merits.

Thoughts?

[akien-mga]

@lyuma discussed with me removing the scene/3d/model_*.h and that seems like it would lead to a design where fbx module depend fully on gltf document, we discussed yesterday about this design and it has some merits.

Yeah I think it's the way to go if it's not doable to have a gltf/fbx agnostic abstraction 👍

Edit: I'm fine with merging this PR as is, and seeing the architecture refactored further in a follow-up PR.
We just need to confirm that my cleanup changes didn't break any functionality.

[fire]

More commentary:

1. In the original version of the pr, the code was unfortunately duplicated, but the importers were independent.
2. In the second version of the pr, we removed the duplication but fbx and gltf documents became heavily coupled despite my attempt to extract the skinning common code and other common code.
3. We want less duplicated code, it means the coupling increases.
4. Given GLTFDocument and FBXDocument are so coupled we could probably extract common code, but I think the most minimal change is modify the object hierarchy so that FBX extends GLTF.
5. The tricky part is fbx would inherit some of the gltf api's extraneous methods relating to export, gltf extensions and the FBXState extending GLTFState. Perhaps there's a better design.

Edited:

Extract common code like methods that don't depend on the internal state of the fbx or gltf documents.

[akien-mga]

This is not related to this PR, but I triggered that bug while renaming EditorSceneFormatImporterFBX to EditorSceneFormatImporterFBX2GLTF, so I figured I'd fix it too.

[fire]

We've been testing using the fbx: https://sketchfab.com/3d-models/shibahu-2e9dffd948f747668609a5a477daad86.

This Shibahu model is fairly complicated and has blend shape animations which broke previously.

I'll compile and test.

godot_engine_fbx_shibahu_d223e45e6e2d0067183529ef8dffd4f853c11b60.mov

On d223e45 with shibahu

Seems to work, the materials are expected not importing, so we're testing in normals debug mode.

[fire]

Updated to fix merge conflict in misc/extension_api_validation/4.2-stable.expected

[akien-mga]

Updated with the changes from #88680, and squashed all commits.

[akien-mga]

Should be good to merge now.

We finally settled down on an architecture where the fbx module extends and depends on the gltf module, reusing what it can.

[akien-mga]

Thanks! Amazing work @fire and @bqqbarbhg!

[adamscott]

Thanks! Amazing work @fire and @bqqbarbhg!

Yes indeed! Thanks both of you for your great job.

Yeah I think it's the way to go if it's not doable to have a gltf/fbx agnostic abstraction 👍

Maybe I don't understand something, but in the face of it, it will make it possible to test that a FBX document is a GLTF document. This seems wrong. Maybe it's super difficult to implement an agnostic abstraction, but it still seems wrong.