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: Update ufbx to v0.14.0 #91528

Merged
merged 1 commit into from
May 7, 2024
Merged

Conversation

bqqbarbhg
Copy link
Contributor

@bqqbarbhg bqqbarbhg commented May 3, 2024

Update to ufbx to version v0.14.0, which is currently being finished. I'm opening a draft PR to facilitate testing of the new version and so we can discuss a few of the newer features.

Some highlights of the ufbx updates up to v0.13.0:

  • Fixed some core parsing issues, meaning files that should load didn't
    • Fix failing to parse 'Z' type
    • Fix threaded parsing of 'b' arrays
    • Fix loading of files with missing mapping information
  • Internal optimizations
    • Fixed internal performance regression using slow little endian reads
    • Optimized the built binary to be ~30kB smaller
  • Baking animations with stepped tangents results in more robust interpolation

Fixes from nearly complete v0.14.0, many based on Godot testing (#90986):

  • Fix orthographic camera unit scaling
  • Add UFBX_INHERIT_MODE_HANDLING_COMPENSATE_NO_FALLBACK to fix some non-scaled inheritance cases
  • Fix handling of non-animated Blender blend shape weights
  • Fix FBX version 6100 pivot conversion failing with skinned meshes
  • Fix triangulation of N-gons with either between [32, 64] corners or having very complex topology

Tasks for me before I consider this safe for merging:

@fire
Copy link
Member

fire commented May 5, 2024

Seems like there's three tasks remaining:

  1. Run through an abridged dataset using the new ufbx version
  2. Merge ufbx Integration v0.14.0 ufbx/ufbx#121
  3. Press ready to review and merge here

@bqqbarbhg
Copy link
Contributor Author

The dataset looked fine at a glance, didn't have the time to do a proper labeling this time.

The only open issue is the use_blender_pbr_material mentioned above with the caveats discussed, I don't really have a strong opinion either way.

Ready to merge by me.

@bqqbarbhg bqqbarbhg marked this pull request as ready for review May 6, 2024 20:43
@bqqbarbhg bqqbarbhg requested review from a team as code owners May 6, 2024 20:43
@fire
Copy link
Member

fire commented May 6, 2024

I would not support blender pbr since it seems to not handle round tripping well?

@bqqbarbhg
Copy link
Contributor Author

Disabled it for now in that case

@fire
Copy link
Member

fire commented May 7, 2024

If it's ok can you rebase the pr so it's one commit?

@akien-mga
Copy link
Member

akien-mga commented May 7, 2024

Could you amend in this documentation change too?

diff --git a/thirdparty/README.md b/thirdparty/README.md
index 4a7ab7314a..8c07da5d24 100644
--- a/thirdparty/README.md
+++ b/thirdparty/README.md
@@ -894,7 +894,7 @@ number and run the script.
 ## ufbx
 
 - Upstream: https://github.com/ufbx/ufbx
-- Version: git (v0.11.1, 2024)
+- Version: 0.14.0 (80ff790ab36507b99ec7e4ef55b9cfb076ce821b, 2024)
 - License: MIT
 
 Files extracted from upstream source:

@akien-mga akien-mga modified the milestones: 4.x, 4.3 May 7, 2024
@bqqbarbhg
Copy link
Contributor Author

Could you amend in this documentation change too?

Sure, thanks! My bad for missing that one, should be fixed now.

@akien-mga akien-mga merged commit e8d5bdd into godotengine:master May 7, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

4 participants