8000 Fix: Add "preservePivots" condition when importing FBX animation by Nor-s · Pull Request #6115 · assimp/assimp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix: Add "preservePivots" condition when importing FBX animation #6115

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 31, 2025
Merged

Fix: Add "preservePivots" condition when importing FBX animation #6115

merged 3 commits into from
May 31, 2025

Conversation

Nor-s
Copy link
Contributor
@Nor-s Nor-s commented Apr 18, 2025 •
edited
Loading

Abstract

The animation importer part has no handling for the "preservePivots" setting.

This setting prevents dummy nodes when processing nodes in the model.

However, animations are not, so if you use this option, the animation will not work with the model.

See comments below.

needs to play nicely with GenerateTransformationNodeChain()

https://github.com/assimp/assimp/blob/ac5988422a7f1898b25c13fb5fb068c646a4c544/code/AssetLib/FBX/FBXConverter.cpp#L2957C1-L2984C73

https://github.com/assimp/assimp/blob/ac5988422a7f1898b25c13fb5fb068c646a4c544/code/AssetLib/FBX/FBXConverter.cpp#L747C49-L747C51

GenerateTransformationNodeChain

  • in this function use "preservePivots" setting

https://github.com/assimp/assimp/blob/ac5988422a7f1898b25c13fb5fb068c646a4c544/code/AssetLib/FBX/FBXConverter.cpp#L870C4-L875C1

*Note: NameTransformationChainNode creates dummy node names.

before after
issue issue_pr

@tellypresence tellypresence added FBX Bugs related to the FBX format Animation Bugs related to the animation data labels Apr 19, 2025
@volcoma
Copy link
volcoma commented Apr 21, 2025

This is very nice to be handled from inside assimp. Here is the same issue discussed - i have posted a workaround from outside. but it would be great if this is fixed in the library.
#4005

@volcoma volcoma mentioned this pull request Apr 21, 2025
Closed
@tellypresence tellypresence mentioned this pull request May 1, 2025
Closed
kimkulling
kimkulling approved these changes May 31, 2025
View reviewed changes
Copy link
Member
@kimkulling kimkulling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.

@kimkulling kimkulling merged commit c4515f5 into assimp:master May 31, 2025
11 checks passed
@kimkulling
Copy link
Member

Merged, thanks a lot for your contibution.

@kimkulling kimkulling added this to the The assimp 6.0 release milestone May 31, 2025
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kimkulling kimkulling kimkulling approved these changes

Assignees
No one assigned
Labels
Animation Bugs related to the animation data FBX Bugs related to the FBX format
Projects
The Asset-Importer-Lib DevOps Board
Status: ✅ Done
Milestone
The assimp 6.0 release
Development

Successfully merging this pull request may close these issues.
4 participants
@Nor-s @volcoma @kimkulling @tellypresence
0