godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.5k stars 21.26k forks source link

UFBX Imports Empties with 100x Scale & Wrong Rotation #90314

Open Uradamus opened 7 months ago

Uradamus commented 7 months ago

Tested versions

v4.3.dev4.official [df78c0636]

System information

Godot v4.3.dev4 - Manjaro Linux #1 SMP PREEMPT_DYNAMIC Thu Apr 4 20:32:38 UTC 2024 - X11 - GLES3 (Compatibility) - AMD Radeon RX 570 Series (radeonsi, polaris10, LLVM 16.0.6, DRM 3.54, 6.6.25-1-MANJARO) () - AMD Ryzen 5 5600G with Radeon Graphics (12 Threads)

Issue description

Importing FBX files that contain empties with the new UFBX feature results in empties that are 100x bigger than they should be and with the wrong rotation.

I was testing to see if I could set up scenes in Blender that used empties as instance placeholders that retain their transformations upon import in Godot as node3d nodes that I can parent my instances to. It works fine with gltf, but fbx has the above mentioned issues.

I exported the instance_test scene as both file types with default export settings in Blender and created a new inherited scene for each, then instanced the instance_marker scene into each empty on each version of the scene for the MRP below. So the gltf.tscn shows the expected results while the ufbx.tscn shows the issues being experienced.

Steps to reproduce

Drop an fbx containing empties into a project and inspect the transforms on the resulting node3d.

Minimal reproduction project (MRP)

ufbx_test.zip

fire commented 7 months ago

@bqqbarbhg Any ideas?

bqqbarbhg commented 7 months ago

Blender stores by default all objects at 100x scale, using 1cm units, in Godot we use UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY unconditionally, which scales meshes, but leaves nodes be:

https://github.com/godotengine/godot/blob/e5b4ef8e9522e950033cbece39a31a4a76da19c1/modules/fbx/fbx_document.cpp#L1956

Using UFBX_SPACE_CONVERSION_ADJUST_TRANSFORMS would scale the nodes, but then cause other kinds of scaling issues in files exported by other exporters. This could be added as an import option, ie. allowing users to choose between UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY and UFBX_SPACE_CONVERSION_ADJUST_TRANSFORMS with better names, though I think last time the general consensus was that it's too much complexity.

Fortunately, Blender allows exporting with the setting "Apply Scalings" set to "FBX Units Scale", which will export the file as 1m units, removing any funky transforms that appear.

Anyways, I'll take a peek at what FBX2glTF does here and see if there's some clean solution, but I don't know if there is one.

bqqbarbhg commented 7 months ago

Looks like FBX2gLTF behaves exactly the same way here, so I'd say ufbx does the "correct" thing here.

Exposing the UFBX_SPACE_CONVERSION_ADJUST_TRANSFORMS/UFBX_SPACE_CONVERSION_MODIFY_GEOMETRY as some user-friendly option like Unit Scaling: Node Transforms / Geometry would work to fix these cases in my opinion.

But as I said for Blender this can also be avoided on the export side by using "FBX Units Scale".

Uradamus commented 7 months ago

Just to confirm - I tested exporting the fbx with that "apply transform" option checked and it indeed fixed both the scaling and rotation issues for the generated nodes after reimporting and creating a new inherited scene.

So one option would be to treat this as a documentation issue and make sure this edge case and its solution shows up in the documentation. It wouldn't be wise to recommend using that option outside of this use case though, since applying transforms can screw up armatures and animations from what I recall. But still, I can't see really any use cases that would need both empties and armatures in the same FBX, but you never know.

Uradamus commented 7 months ago

instance-test.zip Here is the latest export (using the apply transform option) of the fbx that had issues in the MRP on its own, in case any of you would like to further test with it.

fire commented 7 months ago

This is not very visible, but we placed the documentation here.

https://docs.godotengine.org/en/latest/classes/class_fbxdocument.html#description

Uradamus commented 7 months ago

I think perhaps the wording in the Blender exporter might have changed, if that documentation was accurate at one point. Because right now there are no options labeled "FBX Unit Scale" that I can see. I assume it is equivalent to the "Apply Transform" option I mentioned above, since that is the option that fixed this issue.

I haven't used FBX export much though, so not totally sure. There is a chance it could also be something in the commercial Better FBX Importer & Exporter add-on that is pretty popular, and perhaps the author of that page didn't realize they were looking at the wrong exporter, but that's purely speculation. But I know that add-on does have a field that lets you adjust the base FBX units.

Included is a screenshot of the default exporter settings. I've underlined the option in question in red. It's unchecked by default and has the warning about potential issues with armatures/animations I mentioned earlier in the form of that caution symbol that gives some context when you hover over it. Screenshot_20240408_225511

bqqbarbhg commented 7 months ago

The setting is in the "Apply Scalings" dropdown as I mentioned in my first reply:

Fortunately, Blender allows exporting with the setting "Apply Scalings" set to "FBX Units Scale", which will export the file as 1m units, removing any funky transforms that appear.

Using this is also fully safe (as far as ufbx is concerned), as the only thing it does is to set the file unit scale to be meters, and skips scaling all the nodes by 100. I think the default is what it is for broken importers that ignore the FBX file units, as centimeters are the de-facto standard unit for FBX.

Uradamus commented 7 months ago

Where is that drop down located? Is it part of the exporter? Or something that needs to be done in another part of Blender before using the exporter?

bqqbarbhg commented 7 months ago

It's here, a bit above the "Apply Transform" setting:

image

Uradamus commented 7 months ago

I have no idea how I wasn't seeing that, lol.

bqqbarbhg commented 7 months ago

Oh also of note, using that will not remove the 90 degree rotation from the nodes, as that's needed to convert the scene from Blender's Z-up to Godot's Y-up. "Apply Transform" will probably get rid of that at the cost of messing up armatures.

Uradamus commented 7 months ago

Ya, just noticed that. Though the weird part about that is that the Blender FBX exporter defaults to -z-forward and y-up. As can be seen in both of our screenshots.

bqqbarbhg commented 7 months ago

Yeah if you export with Y-up in Blender, Blender itself will add the 90 degree rotation to nodes. If you export Z-up the file metadata will contain the information that it is Z-up, but nodes will have no extra transforms. Unfortunately, when loading, ufbx will add the transforms back to make the file look correct :D

So in essence, changing the export space just changes where the conversion is done.

Uradamus commented 7 months ago

Ya, so after trying a ton of different combos, it seems the "apply transform" is the only one that fixes the rotation issue. It also technically can fix the scale issue as well, since as long as that is checked it doesn't seem to matter what the "Apply Scalings" is set to. Though the FBX Units Scale seems like a sensible default none the less for that option.

fire commented 7 months ago

As far as I know, this is an intentional preference compared to the Blender export of fbx, not sure if we can do anything about it. As far as I know there is no visual difference.

How can we best resolve this?

fire commented 2 months ago

As far as I know we're not planning on fixing this issue.

There are documentation workarounds.

ChildLearningClub commented 2 weeks ago

Creating Convex and Trimesh collisions from the imported .fbx files with the scale applied results in GodotPhysics issues, with the collisions themselves scaled to match the scaled .fbx files utilizing Godot's built in "Create Collsion Shape".

Using the MeshDataTool is required to create collision shapes from these scaled meshes with a scale of 1:1 to work with the physics engine. I think that it would be ideal that the FBX importer does this and reset the scale to 1:1 on the meshes?

    var mesh_node_instances: Array[Node] = imported_fbx_node.find_children("*", "MeshInstance3D", true, false)
    for mesh_node: MeshInstance3D in mesh_node_instances:

        var mesh_instance_3d_node = MeshInstance3D.new()
        var expanded_mesh = ArrayMesh.new()

        var original_mesh = mesh_node.mesh as ArrayMesh

        # Create a new ArrayMesh and MeshDataTool
        var mdt = MeshDataTool.new()
        var surface_count = original_mesh.get_surface_count()

    # Iterate through each surface in the mesh
        for surface_index in range(surface_count):
            # Create MeshDataTool from the current surface
            mdt.create_from_surface(original_mesh, surface_index)

            var vertex_count = mdt.get_vertex_count()

            # Expand the vertices for this surface
            for i in range(vertex_count):
                var vertex = mdt.get_vertex(i)
                # Expand the vertex by the expansion factor
                vertex *= mesh_node.scale
                mdt.set_vertex(i, vertex)

            # If the expanded mesh is empty, commit the first surface, otherwise append the surface
            if surface_index == 0:
                expanded_mesh.clear_surfaces()
                mdt.commit_to_surface(expanded_mesh, surface_index)
            else:
                mdt.commit_to_surface(expanded_mesh, surface_index)

https://github.com/user-attachments/assets/1691deee-bfa3-46fe-8904-b8b586546aa7

I'm not completely sure why I was still having issues when scaled down to 1:1 other then that maybe it was because the collision was created from a mesh that had been scaled, but utilizing the MeshDataTool fixes this.

NOTE:

The "Create Collision Shape" dropdown menu worked and I selected single convex collision shape, OBS just didn't pick it up in the recording.

fire commented 2 weeks ago

Although this bug is in the FBX importer, this step should be applied in the Scene Importer since in theory it would apply to gltf2 too.