Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FBX Importer Rewrite for Godot 4.0 #40707

Conversation

RevoluPowered
Copy link
Contributor

@RevoluPowered RevoluPowered commented Jul 25, 2020

Adds the ability to load Autodesk FBX files, without requiring the SDK or third-party dependencies.

Previously due to FBX Pivot Transforms this was not possible, also inherit type (nodes which do not inherit parent scale, or do different multiplication orders) was previously complex to support and is required to support Maya fully.

We abstract most of Ref away and keep this simple for anyone who needs to make some edit to the importer. Pivot transform handles pivot's and also inherit type (scaling ignored, parent before scaling, etc)

Previously we had four layers, one for assimp one for godot, and one for the importer itself converting the data to godot, this removes assimp from the middle simplifying the method to

FBX Document -> Godot Importer (this is a direct importer now, and should be much more efficient in some cases over the previous importer)

Another advantage is we also only have a single set of core math types between FBX and Godot (Vector3, Vector2 and Transform), less conversions, less can go wrong.

This can be extended with as many features as we want now and there is now room to grow it into all the specific tools for Maya etc, or even possibly 3ds max.

Why the decision to drop assimp?

  • We didn't drop it entirely, we only use the FBX portion of the document parser.
  • We moved all the internal assimp types like aiMatrix aiVector3D to Vector3D and Transform respectively.
  • Multiple conversion layers caused issues which are harder to track down with three formats to handle it was harder to add the more complex FBX elements.

FBX Importer with viable support for commercial FBX files

  • pivots (Pivot transforms fully supported, excluding 3DS Max as of now)
  • inherit types (Segment Scale Compensation works ON or OFF; unity doesn't have this supported 🍰 )
  • pre and post-rotation (orient bones and skeleton correctly same as original FBX / Modeller application)
  • pivoted animations (Pivot transforms fully supported, excluding 3DS Max as of now)
  • meshes
  • normals
  • vertex colors
  • vertex weights
  • node animations
  • node locator animations
  • bone animations
  • blend shapes (as other importers use Normalize we use normalized blend shapes)
  • file scaling

Known bugs:

  • no plan to support 3ds max (but we can open this if requested)
    • requires intense computation and affine inverse on all children nodes with geometric pivots (expensive in code and time)
    • add a handler into pivot transform to affine the children inverse of the geometric pivot and apply the pivot to the current node, effectively undoing the geometric pivot entirely, giving the correct absolute position.

Known unsupported (will be added)

  • bone locators - will be added in the future, this will be easy enough to add, a bone targets a 'model id' the target has a parent target, this can be used to link the locator automatically to the right skeleton instead of making a locator skeleton, which is what the old importer did.

This work is sponsored by IMVU.

@RevoluPowered RevoluPowered requested a review from akien-mga as a code owner July 25, 2020 20:46
@RevoluPowered RevoluPowered force-pushed the fbx_port_to_godot_vulkan branch from 908d631 to 9222801 Compare July 25, 2020 21:59
@Calinou Calinou added this to the 4.0 milestone Jul 25, 2020
@RevoluPowered RevoluPowered force-pushed the fbx_port_to_godot_vulkan branch from 9222801 to fab0e31 Compare July 25, 2020 22:03
Co-authored-by: Gordon MacPherson <[email protected]>
Co-authored-by: Andrea Catania <[email protected]>
Co-authored-by: K. S. Ernest (iFire) Lee <[email protected]>

This is a complete rewrite of the importer. It will give more deterministic behaviour and has been sponsored by IMVU inc, over 1 year has gone into the development of this importer to remove the burden of the FBX SDK.

This was my project for 1 entire year and I really enjoyed the opportunity to add to Godot.

Along the road of implementing fixes we implemented fbx pivots, animations and inheritance type handling, which in most cases works properly.

We have implemented animation and mesh skinning too this should work out of the box, if there are issues let us know.

It's designed so that you can expand this with ease, and fix bugs easily too.

It can import from Autodesk Maya and import into Godot, with pivots.

There are bits we could polish but for now this is good enough.

This was sponsored by IMVU, and a special thanks to everyone who supported this project.

Signed-off-by: Gordon MacPherson <[email protected]>
@RevoluPowered RevoluPowered force-pushed the fbx_port_to_godot_vulkan branch from fab0e31 to a26ace1 Compare July 25, 2020 22:04
@RevoluPowered
Copy link
Contributor Author

RevoluPowered commented Jul 25, 2020

@reduz got a few questions about the new API:

  • what property do we use for transparency on SpatialMaterial3D?
  • do we no longer need texture flags?

Editor builds werror=yes added as fixes required

#warning "FIX Transparency mapping here it prob should not be clear coat"
 ^
modules/fbx/data/fbx_material.cpp:218:2: warning: FIX THESE SWITCHES :D [-W#warnings]
#warning FIX THESE SWITCHES :D
 ^
modules/fbx/data/fbx_material.cpp:191:11: error: 5 enumeration values not handled in switch: 'TEXTURE_HEIGHTMAP', 'TEXTURE_SUBSURFACE_SCATTERING', 'TEXTURE_SUBSURFACE_TRANSMITTANCE'... [-Werror,-Wswitch]
                switch (mapping_mode) {
                        ^
modules/fbx/data/fbx_material.cpp:191:11: note: add missing switch cases
                switch (mapping_mode) {
                        ^
modules/fbx/data/fbx_material.cpp:301:2: warning: FIX ME - Alpha transparency [-W#warnings]
#warning FIX ME - Alpha transparency
 ^
modules/fbx/data/fbx_material.cpp:411:2: warning: "Texture import flags are gone?" [-W#warnings]
#warning "Texture import flags are gone?"
 ^

@fire
Copy link
Member

fire commented Jul 26, 2020

 BaseMaterial3D::set_transparency(Transparency p_transparency)

	enum Transparency {
		TRANSPARENCY_DISABLED,
		TRANSPARENCY_ALPHA,
		TRANSPARENCY_ALPHA_SCISSOR,
		TRANSPARENCY_ALPHA_DEPTH_PRE_PASS,
		TRANSPARENCY_MAX,
	};

Mapping modes

	enum Feature {
		FEATURE_EMISSION,
		FEATURE_NORMAL_MAPPING,
		FEATURE_RIM,
		FEATURE_CLEARCOAT,
		FEATURE_ANISOTROPY,
		FEATURE_AMBIENT_OCCLUSION,
		FEATURE_HEIGHT_MAPPING,
		FEATURE_SUBSURFACE_SCATTERING,
		FEATURE_SUBSURFACE_TRANSMITTANCE,
		FEATURE_BACKLIGHT,
		FEATURE_REFRACTION,
		FEATURE_DETAIL,
		FEATURE_MAX
	};

Import flag are global settings now.

@Xrayez
Copy link
Contributor

Xrayez commented Jul 26, 2020

You'll have to move the tests to top level dir now: #40709 (git doesn't pick it up as a merge conflict), or implement the tests in the module directly with #40720 if merged.

@akien-mga
Copy link
Member

Superseded by #41985.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants