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 animations and attachments once and for all #15354

Closed
wants to merge 9 commits into from

Conversation

appgurueu
Copy link
Contributor

@appgurueu appgurueu commented Oct 28, 2024

This is supposed to fix skeletal animation / attachment related jank, specifically #15186, #14818, #14817 and to refactor a part 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.

To do

This PR is WIP. I'm currently aiming for 5.11.0 since this will need a decent amount of testing.

  • Figure out why the heck .x has just ceased working it has unceased working
  • Replace hints with binary search, refactor this to use a sort of "keyframes" struct (I have this around somewhere) Should be all nice and tidy now :)
  • Fix stale nametag positions (figure out where to best update their position) can't reproduce (anymore?), they should be updated at the right point in time anyways (after rendering of everything 3d and thus after all absolute transforms have been computed)
  • Pull out debug remnants
  • Check whether build times are still comparable
  • Do proper bounding box calculations
  • Do bone overrides properly

How to test

See the related issues.

@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 @ Engine Core What happens inside the very engine labels Oct 28, 2024
@appgurueu appgurueu marked this pull request as draft October 28, 2024 14:18
@appgurueu appgurueu force-pushed the unfuck-irrlicht branch 2 times, most recently from b835db7 to 745accc Compare November 2, 2024 23:19
@appgurueu appgurueu changed the title Fix skeletal animations and attachments once and for all Fix animations and attachments once and for all Dec 2, 2024
@hecktest
Copy link
Contributor

hecktest commented Dec 6, 2024

For what it's worth, the hatlag is gone again but all root entities now jitter around a lot, all the time. As if interpolation wasn't working properly.
Other than that it looks like everything is on the right track.
jitter
jitter2

@appgurueu
Copy link
Contributor Author

appgurueu commented Dec 7, 2024

I have started to cleanly split this up into the "refactor" and "fix" parts, otherwise it's unmanageable for me to get to a reviewable state.

As if interpolation wasn't working properly.

Indeed it isn't. Maybe you have an idea what's going wrong? Here's the PR with just the refactoring changes in relatively clean commits: #15522. Nevermind, it was just a dumb argument swap due to inconsistent Irrlicht conventions.

@appgurueu
Copy link
Contributor Author

Closing in favor of #15722.

@appgurueu appgurueu closed this Jan 27, 2025
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 @ Engine Core What happens inside the very engine Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Testing needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants