Add ufbx for FBX importing #81746

fire · 2023-09-16T17:29:49Z

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.

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

Cameras
Lights
~~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 · 2023-09-17T03:13:31Z

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 · 2023-09-17T03:56:01Z

@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 · 2023-09-17T19:07:40Z

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

DigitalN8m4r3 · 2023-09-18T07:14:27Z

Tested with
*mixamo models
*rdyplayerme models
*custom models

so far no issues, good work 👍

fire · 2023-09-27T01:07:55Z

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.

modules/fbx/fbx_document.cpp

fire · 2023-10-07T18:51:54Z

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

fire · 2023-10-07T19:30:35Z

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

modules/fbx/structures/fbx_skeleton.h

modules/fbx/doc_classes/FBXDocument.xml

modules/fbx/LICENSE

modules/fbx/.github/ISSUE_TEMPLATE/bug_report.yml

modules/fbx/.gitrepo

modules/fbx/thirdparty/ufbx/.github/workflows/ci.yml

modules/fbx/doc_classes/EditorSceneFormatImporterUFBX.xml

fire · 2023-12-04T15:42:47Z

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.

modules/fbx/.all-contributorsrc

akien-mga · 2024-02-22T13:12:50Z

modules/fbx/editor/editor_scene_importer_ufbx.cpp

-	if (p_options.has("fbx/importer") && int(p_options["fbx/importer"]) == FBX_IMPORTER_FBX2GLTF) {
-		Ref<EditorSceneFormatImporterFBX> fbx2gltf_importer;
+	// FIXME: Hack to work around GH-86309.
+	if (p_options.has("fbx/importer") && int(p_options["fbx/importer"]) == FBX_IMPORTER_FBX2GLTF && GLOBAL_GET("filesystem/import/fbx2gltf/enabled")) {


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

fire · 2024-02-22T15:32:15Z

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.

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.
fbx module depend fully on gltf document in practice
fbx extends gltf seems to fit more closely the code architecture.
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 · 2024-02-22T15:33:00Z

@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 · 2024-02-22T15:50:44Z

More commentary:

In the original version of the pr, the code was unfortunately duplicated, but the importers were independent.
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.
We want less duplicated code, it means the coupling increases.
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.
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 · 2024-02-22T15:50:57Z

editor/project_converter_3_to_4.cpp

@@ -1168,7 +1168,7 @@ bool ProjectConverter3To4::test_array_names() {

 			// Callable is special class, to which normal classes may be renamed.
 			if (!ClassDB::class_exists(StringName(new_class)) && new_class != "Callable") {
-				ERR_PRINT(vformat("Class \"%s\" does not exist in Godot 4, so it cannot be used in the conversion.", old_class));
+				ERR_PRINT(vformat("Class \"%s\" does not exist in Godot 4, so it cannot be used in the conversion.", new_class));


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 · 2024-02-22T18:16:32Z

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 · 2024-02-23T14:39:11Z

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

This update introduces a new import method for FBX files using ufbx. If the fbx2gltf import fails, it will use the most recently cached scene from the ufbx import. The process is sped up by introducing threads to load the ufbx portion. Key changes include: - Support for importing geometry helper nodes in FBX files. - Addition of cameras and lights with updated names. - Removal of the fbx importer manager. - Introduction of ModelDocument3D and updates to its methods. - Changes to FBX import options and visibility. - Updating the documentation and handling some errors. - Store the original non-unique node, mesh and animation names in FBX and glTF. Co-Authored-By: bqqbarbhg <bqqbarbhg@gmail.com>

akien-mga · 2024-02-23T21:33:36Z

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 · 2024-02-23T22:09:01Z

Thanks! Amazing work @fire and @bqqbarbhg!

adamscott · 2024-02-23T22:54:41Z

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.

Calinou added enhancement topic:import topic:3d labels Sep 16, 2023

Calinou added this to the 4.x milestone Sep 16, 2023

fire mentioned this pull request Sep 17, 2023

Can't build on MacOS 13 godotengine/FBX2glTF#47

Open

fire mentioned this pull request Sep 20, 2023

Importing FBX with empty animation doesn't create Skeleton3D node at root node #81935

Closed

This comment was marked as outdated.

Sign in to view

AThousandShips reviewed Oct 7, 2023

View reviewed changes

fire force-pushed the vsk-ufbx branch 2 times, most recently from 8b0e914 to be02fa7 Compare October 7, 2023 18:51

fire force-pushed the vsk-ufbx branch from 46f318d to ca164b3 Compare October 8, 2023 02:41

fire commented Nov 14, 2023

View reviewed changes

modules/fbx/structures/fbx_skeleton.h Outdated Show resolved Hide resolved

fire commented Nov 14, 2023

View reviewed changes

modules/fbx/doc_classes/FBXDocument.xml Outdated Show resolved Hide resolved

fire commented Nov 14, 2023

View reviewed changes

modules/fbx/doc_classes/FBXDocument.xml Outdated Show resolved Hide resolved

viksl mentioned this pull request Nov 27, 2023

Godot stuck in import loop of FBX model with error -1073740791 #85402

Closed

adamscott reviewed Dec 4, 2023

View reviewed changes

modules/fbx/LICENSE Outdated Show resolved Hide resolved

fire commented Dec 4, 2023

View reviewed changes

modules/fbx/.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved

fire commented Dec 4, 2023

View reviewed changes

modules/fbx/.gitrepo Outdated Show resolved Hide resolved

fire commented Dec 4, 2023

View reviewed changes

modules/fbx/thirdparty/ufbx/.github/workflows/ci.yml Outdated Show resolved Hide resolved

adamscott requested a review from a team December 4, 2023 15:38

fire commented Dec 4, 2023

View reviewed changes

modules/fbx/doc_classes/EditorSceneFormatImporterUFBX.xml Show resolved Hide resolved

adamscott reviewed Dec 4, 2023

View reviewed changes

modules/fbx/.all-contributorsrc Outdated Show resolved Hide resolved

akien-mga reviewed Feb 22, 2024

View reviewed changes

akien-mga force-pushed the vsk-ufbx branch 3 times, most recently from da1f04c to 0ffb10d Compare February 22, 2024 15:32

akien-mga force-pushed the vsk-ufbx branch from 0ffb10d to d223e45 Compare February 22, 2024 15:50

akien-mga reviewed Feb 22, 2024

View reviewed changes

fire mentioned this pull request Feb 22, 2024

Remove scene/3d/model_*.h from FBX pull request #88680

Closed

fire force-pushed the vsk-ufbx branch from d223e45 to 46044ff Compare February 23, 2024 14:35

akien-mga force-pushed the vsk-ufbx branch from 46044ff to 04d4394 Compare February 23, 2024 21:33

akien-mga approved these changes Feb 23, 2024

View reviewed changes

akien-mga merged commit 36008be into godotengine:master Feb 23, 2024
16 checks passed

fire deleted the vsk-ufbx branch February 23, 2024 22:11

tdaven mentioned this pull request Feb 24, 2024

Fix crash during import bone remap. #87905

Closed

akien-mga mentioned this pull request Mar 13, 2024

Cannot import FBX models in the Android editor #89427

Closed

This was referenced Apr 26, 2024

Fix crash when there is no bones to rename when retargeting an imported skeleton #88109

Closed

Editor crashes when importing a skeleton with bone names that already match the retargeting profile #88086

Closed

fire mentioned this pull request Apr 28, 2024

Fbx GeometricTransformation support is problematic godotengine/FBX2glTF#56

Open

akien-mga mentioned this pull request May 30, 2024

Rename FBX2glTF binary path setting back to 4.2 name #92571

Merged

This was referenced Jun 18, 2024

Update import pages for ufbx in 4.3 godotengine/godot-docs#9501

Closed

Update import pages for ufbx in 4.3 godotengine/godot-docs#9509

Merged

akien-mga mentioned this pull request Aug 8, 2024

Crashing during bone remap #87904

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ufbx for FBX importing #81746

Add ufbx for FBX importing #81746

fire commented Sep 16, 2023 •

edited

Loading

bqqbarbhg commented Sep 17, 2023 •

edited

Loading

fire commented Sep 17, 2023

fire commented Sep 17, 2023

DigitalN8m4r3 commented Sep 18, 2023

fire commented Sep 27, 2023

This comment was marked as outdated.

fire commented Oct 7, 2023

fire commented Oct 7, 2023 •

edited

Loading

fire commented Dec 4, 2023

akien-mga Feb 22, 2024

fire commented Feb 22, 2024 •

edited

Loading

akien-mga commented Feb 22, 2024 •

edited

Loading

fire commented Feb 22, 2024 •

edited

Loading

akien-mga Feb 22, 2024

fire commented Feb 22, 2024 •

edited

Loading

fire commented Feb 23, 2024

akien-mga commented Feb 23, 2024

akien-mga left a comment

akien-mga commented Feb 23, 2024

adamscott commented Feb 23, 2024

Add ufbx for FBX importing #81746

Add ufbx for FBX importing #81746

Conversation

fire commented Sep 16, 2023 • edited Loading

Finalization after 4.3 window is open

Finalization before 4.3 window closes

Known Issues

bqqbarbhg commented Sep 17, 2023 • edited Loading

fire commented Sep 17, 2023

fire commented Sep 17, 2023

DigitalN8m4r3 commented Sep 18, 2023

fire commented Sep 27, 2023

This comment was marked as outdated.

fire commented Oct 7, 2023

fire commented Oct 7, 2023 • edited Loading

In-depth summary

blender_default.fbx Example

fire commented Dec 4, 2023

akien-mga Feb 22, 2024

Choose a reason for hiding this comment

fire commented Feb 22, 2024 • edited Loading

akien-mga commented Feb 22, 2024 • edited Loading

fire commented Feb 22, 2024 • edited Loading

akien-mga Feb 22, 2024

Choose a reason for hiding this comment

fire commented Feb 22, 2024 • edited Loading

fire commented Feb 23, 2024

akien-mga commented Feb 23, 2024

akien-mga left a comment

Choose a reason for hiding this comment

akien-mga commented Feb 23, 2024

adamscott commented Feb 23, 2024

fire commented Sep 16, 2023 •

edited

Loading

bqqbarbhg commented Sep 17, 2023 •

edited

Loading

fire commented Oct 7, 2023 •

edited

Loading

fire commented Feb 22, 2024 •

edited

Loading

akien-mga commented Feb 22, 2024 •

edited

Loading

fire commented Feb 22, 2024 •

edited

Loading

fire commented Feb 22, 2024 •

edited

Loading