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

Fix and clean up skeletal animation in Irrlicht #15722

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

appgurueu
Copy link
Contributor

Re-do-ish of #15354.

Fixes skeletal animation / attachment related jank, specifically fixes #14818 and fixes #14817 and refactors much of the code in preparation of e.g. #9218. See also #15350.

It works by doing things the way they should be done. Currently we have Irrlicht doing things wrong and 2-3 layers of workarounds, some in Irrlicht, some in Luanti, on top of it. This is a broken mess.

What should be done is conceptually pretty simple: First, we need to recursively update the absolute transforms of node hierarchies consisting of scene nodes (including bones) inside OnAnimate.
When rendering, we simply transfer these transforms to the mesh and let it do the CPU skinning. (Later we will want to do that on the GPU.)
Each animated scene node must store its own bone transformations for animation blending purposes (and later GPU skinning).

All the workarounds we currently have can essentially go out the window.

I believe my changes here should also optimize things, but I haven't benchmarked yet.

To do

This PR is ready for review. I ask you not to focus on minor style issues (for example I suppose I might not have applied casing consistently; sometimes I'm using Luanti, sometimes Irrlicht conventions) but rather broader issues until the very end of reviewing where I'll do one big sweeping pass to get rid of them.

How to test

  • Check that various models - .x, .gltf / .glb, .b3d - still animate correctly. As a starting point, the models included in devtest should still work correctly: Lava Flan, Cool Guy, Sam, Animated Frog, Animated Spider.
  • Check that bounding boxes are updated correctly with the animation. (See the CAnimatedMeshSceneNode rendering code, set DebugDataVisible = scene::EDS_BBOX.)
  • Check that attachment positions are always up to date, see e.g. the linked issue; ideally also throw in animations.
  • Check that animation blending works. See the linked issue. One simple way is to edit MTG player_api/api.lua and change the animation_blend (which is in seconds). Then observe e.g. how when going from standing to punching, your right arm is raised smoothly over the given timespan. In general, transitions between animations should appear smoothened.
  • Check that bone overrides still work. A good stress test is probably character_anim.

The commits are only semi-useful for reviewing this.

@appgurueu appgurueu added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Bugfix 🐛 PRs that fix a bug @ Client rendering Code quality labels Jan 27, 2025
moar fix

optimize cool guy texture
@lhofhansl
Copy link
Contributor

lhofhansl commented Feb 3, 2025

Tried on a random game. Got this:
.../luanti/irr/src/SkinnedMesh.cpp:64: std::vector<std::variant<irr::core::Transform, irr::core::CMatrix4<float> > > irr::scene::SkinnedMesh::animateMesh(irr::f32): Assertion 'HasAnimation' failed.
Don't have time to dig right, might get to it later this week.

@appgurueu
Copy link
Contributor Author

Tried on a random game. Got this: .../luanti/irr/src/SkinnedMesh.cpp:64: std::vector<std::variant<irr::core::Transform, irr::core::CMatrix4<float> > > irr::scene::SkinnedMesh::animateMesh(irr::f32): Assertion 'HasAnimation' failed.

Ah yes, someone is probably using an animatable mesh format for a static mesh. I've added a check to not call animateMesh, because I think that the mesh being animatable is a sensible precondition for animateMesh (otherwise you're just passing a throwaway frame number).

Don't have time to dig right, might get to it later this week.

Feel free to do it at a later point in time, I wouldn't rush it for 5.11 anyways. Thanks for the brief test, that's already helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Client rendering Code quality Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attachments lag behind bones under some circumstances Animation blending no longer works
2 participants