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

Bug: broken fbx animation #5449

Open
tigert1998 opened this issue Feb 1, 2024 · 27 comments
Open

Bug: broken fbx animation #5449

tigert1998 opened this issue Feb 1, 2024 · 27 comments
Labels
Bug Global flag to mark a deviation from expected behaviour

Comments

@tigert1998
Copy link
Contributor

Describe the bug
The FBX animation support is broken in the latest master branch. However, animation still works in v5.3.1. So the latest several commits must have made animation broken.

To Reproduce
My model loader code is available at https://github.com/tigert1998/tiger-game-engine/blob/main/engine/src/model.cc.
You could build the instancing-demo CMake target to test.
The two models with broken animation (fbx version) are (these two both work in v5.3.1):

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Platform (please complete the following information):

  • OS: Windows
  • Version: latest master

Additional context
Add any other context about the problem here.

@tigert1998 tigert1998 added the Bug Global flag to mark a deviation from expected behaviour label Feb 1, 2024
@tigert1998 tigert1998 changed the title Bug: Bug: broken fbx animation Feb 1, 2024
@tellypresence
Copy link
Contributor

Confirm broken animation.

Model tarisland-dragon-high-poly
AnimationScreenshot
Qishilong_skill06tarisland-dragon-high-poly_Qishilong_skill06_anim

@JulianKnodt
Copy link
Contributor

Can you double check if this is still broken? I broke the FBX animation with a change, and it was recently fixed.

@tellypresence
Copy link
Contributor

tellypresence commented Mar 27, 2024

Confirm that latest commit does not fix fbx animation; also confirm that animation was OK at release 5.3.1

Model tarisland-dragon-high-poly
AnimationAssimp
feb861f (21 Mar 2024)release 5.3.1
Qishilong_skill06tarisland-dragon-high-poly_Qishilong_skill06_anim_assimp_commit_feb861ftarisland-dragon-high-poly_Qishilong_skill06_anim_assimp_release_5 3 1

@tellypresence
Copy link
Contributor

tellypresence commented Mar 27, 2024

Updated Aug 2024: please refer to issue 5719 for detailed bisect reports which show that there was breakage at 27 Oct 2019 and 24 Mar 2020

Here are examples of fbx files where animation was working around release 5.0.1 but stopped working before release 5.3.1; models available using either

  • direct link
  • from renderpeople.com: under "3D Animated People" click "Free Download" button, then click the "FBX" button and download zip

Models of interest are

  • 22-rp_manuel_animated_001_dancing_fbx
  • 35-rp_sophia_animated_003_idling_fbx
  • 55-rp_nathan_animated_003_walking_fbx
ModelAssimp versionNotes
2d2889f (24 Sep 2019)release 5.3.1 (25 Sep 2023)feb861f (21 Mar 2024)
manuel dancing
22-rp_manuel_animated_001_dancing_fbx
22-rp_manuel_animated_001_dancing_fbx22-rp_manuel_animated_001_dancing_fbx_assimp_release_5_3_122-rp_manuel_animated_001_dancing_fbx_assimp_commit_feb861fWorked at commit 2d2889f and release 5.3.1, obviously wrong now
sophia idling
35-rp_sophia_animated_003_idling_fbx
22-rp_manuel_animated_001_dancing_fbx35-rp_sophia_animated_003_idling_fbx_assimp_release_5_3_135-rp_sophia_animated_003_idling_fbx_assimp_commit_feb861fWorked at commit 2d2889f, didn't work in release 5.3.1, obviously worse now
nathan walking
55-rp_nathan_animated_003_walking_fbx
22-rp_manuel_animated_001_dancing_fbx55-rp_nathan_animated_003_walking_fbx_release_5_3_155-rp_nathan_animated_003_walking_fbx_assimp_commit_feb861fWorked at commit 2d2889f, didn't work in release 5.3.1, no obvious difference now

Here's what they should look like...
render_people_free_3d_animated_people

@kimkulling
Copy link
Member

Oh my gosh, that looks bad. I got it!

@shinyclaw
Copy link

Confirmed at version from today's master branch. Animation "Hip Hop Dancing" on model "CH42_NOPBR" from Mixamo.

Hip.Hop.Dancing.mp4

@mwestphal
Copy link
Contributor

I've bisected the issue to #5349

@mwestphal
Copy link
Contributor

Please note this is NOT the same issue as #3390

@kikonen
Copy link

kikonen commented May 5, 2024

Just tried to update from 5.3.1 to 5.4.0, and well, as a result got kind of abstract art, which looks similar to samples here.

imdongye added a commit to imdongye/imtoys that referenced this issue Jun 11, 2024
before update assimp version for fbx animation bug
assimp/assimp#5449
@tellypresence
Copy link
Contributor

@kikonen Are you able to share your test FBX model file(s) for troubleshooting purposes?

@tellypresence
Copy link
Contributor

@shinyclaw Have you got a link to the Mixamo CH42_NOPBR model?

@kikonen
Copy link

kikonen commented Aug 18, 2024

@tellypresence I presume I can share files for testing, just can't share them publicly to everyone.

https://drive.google.com/drive/folders/1ZOLRK1X-lN3Jq6hxdJ6XPr-oFEk2xcng?usp=sharing

SK_Lion.FBX

  • Got broken with 5.4.0 (worked with 5.3.1)

"Lion with 5.3.1"
image

Lion with 5.4.0
image

SK_SkeletonKnight.FBX

  • Animation has not worked so far properly with assimp at all; aka. this may have separate issues, not related to 5.4.0, but just adding it here as secondary reference point

Skeleton animation, with assimp 5.3 - 5.4
image

@lxw404
Copy link

lxw404 commented Aug 18, 2024

@kikonen Please try: #5643. I'm fairly sure I've fixed this regression, but it would be good to have more people verify it (and encourage that it gets merged).

@kikonen
Copy link

kikonen commented Aug 18, 2024

@lxw404 seems to improve animation result at least in my Lion.FBX case

without fix
Nice abstract art...
image

With fix
Expected result
image

Since goal in my project is not establishing abstract art exhibition, I prefer "boring expected result" :D

EDIT
Bonus points for the fact that "skeleton_knight.FBX" is not totally really weird mess any longer (w/ fix). Clearly having some problems, but at least can recognize more-or-less now what it is...
image

@lxw404
Copy link

lxw404 commented Aug 18, 2024

@kikonen These abstract art ones are hilarious, but I agree the boring expected result is probably best long term.

I'm guessing the skeleton is a completely separate issue then if it has never worked with assimp in any version before. I could possibly take a look at that and see what's up at some point.

@kikonen
Copy link

kikonen commented Aug 18, 2024

Yes, that skeleton has some other issues. I don't know about earlier versions of assimp than 5.3.1, since that happened to be point of time when I introduced assimp into this project. But yes, that case had issues with 5.3.1 already, but as can be seen, with these latest fixes it has improved a lot (remaining issues, well, there is changes of bugs (gosh ,really) in my own code also, so to be investigated, I just gave up with that few months ago, but now changes to take another look).

@JulianKnodt
Copy link
Contributor

I think there can be issues with multiple meshes in a single node (specifically I found some when exporting meshes with nodes with multiple meshes), but it would be useful to know the hierarchy of your scene

@tellypresence
Copy link
Contributor

@kikonen
Thank you for the link. It looks like both the Lion and Skeleton Knight have 2 FBX files each, the main mesh plus a separate animation-only FBX file. Do you have any way to export a single FBX file containing both the mesh and animation data?

@kikonen
Copy link

kikonen commented Aug 19, 2024

If possible should separate that skeleton into another issue.

Regarding fbx files, not sure how easily I could get them into same file. I.e. original files were separate files (mesh + anim), so to merge them would propably have try importing them into Blender and merge them there (and Blender seems to have its own issues with fbx import).

@tellypresence
Copy link
Contributor

@kikonen Please try: #5643. I'm fairly sure I've fixed this regression, but it would be good to have more people verify it (and encourage that it gets merged).

@kikonen Please sync your PR branch (it's out of date)

@kikonen
Copy link

kikonen commented Aug 21, 2024

@tellypresence Did some trial with #5643 fix with current assimp master, and it was working for me same way as nonrebased version in PR. Noticed additionally that base transforms for few static mesh fbx files I had exported from unreal seemed to work now better, i.e. previously I had to apply -90 degree rotation along X axis to them after importing (i.e. generic improvement from some fix along the way).

You probably meant that sync req to @lxw404 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Global flag to mark a deviation from expected behaviour
Projects
None yet
Development

No branches or pull requests

10 participants