SkeletonIK rotates bones by 180 degrees

ghost commented 2 years ago

Godot version

3.4.stable

System information

Void Linux x86_64 5.13.19, GLES3, Mesa Intel(R) HD Graphics 5500 (BDW GT2)

Issue description

https://user-images.githubusercontent.com/79333625/141367831-a4409c55-8841-474c-b0a3-ee67a3451e30.mp4

Steps to reproduce

Simply move the target up.

Minimal reproduction project

iktest.zip

TwistedTwigleg commented 2 years ago

I downloaded the reproduction project and can confirm there appears to be an issue with the rotations and the bones incorrectly twisting. I'll see if I can figure out what is going on and make a solution.

TwistedTwigleg commented 2 years ago

I've tried to debug this and find a solution, but nothing has worked so far. Will try to look at it again in the future though.

In the meantime, you may be able to use Twisted IK 2 to get around the issue, as long as you are okay with C# for the inverse kinematics solution. It uses a different way of solving rotations, so it shouldn't get the bone twisting issue.

ghost commented 2 years ago

@TwistedTwigleg I was wondering about what's different about SkeletonIK from 3.4 and SkeletonIK3D from master, I've seen lots of renaming without any functional changes, except this one:

-       if (!ci->children.empty()) {
-           /// Rotate basis
-           const Vector3 initial_ori((ci->children[0].initial_transform.origin - ci->initial_transform.origin).normalized());
-           const Vector3 rot_axis(initial_ori.cross(ci->current_ori).normalized());
-
-           if (rot_axis[0] != 0 && rot_axis[1] != 0 && rot_axis[2] != 0) {
-               const real_t rot_angle(Math::acos(CLAMP(initial_ori.dot(ci->current_ori), -1, 1)));
-               new_bone_pose.basis.rotate(rot_axis, rot_angle);
-           }
+       if (!ci->children.is_empty()) {
+           p_task->skeleton->update_bone_rest_forward_vector(ci->bone);
+           Vector3 forward_vector = p_task->skeleton->get_bone_axis_forward_vector(ci->bone);
+           // Rotate the bone towards the next bone in the chain:
+           new_bone_pose.basis.rotate_to_align(forward_vector, new_bone_pose.origin.direction_to(ci->children[0].current_pos));

It appears you forgot to merge this bugfix https://github.com/godotengine/godot/commit/92a79ace5b3e58933ecdb0e21e3c3402073979b0 into 3.x

I didn't test that patch yet, so I can't claim it fixes my issue.

ghost commented 2 years ago

From #53358

I kinda narrowed it down a bit. It seems to be the current_ori calculation in solve_simple_forwards that is causing the issue. It seems it calculates it right, and then immediately calculates it wrong, leading to it jittering back and forth.

I experienced this kind of jittering while animating body, you can notice it in the video above from 19 to 21 seconds.

ghost commented 2 years ago

Ah, I can't even compile that patch because it depends on functions that don't exist in 3.x

TwistedTwigleg commented 2 years ago

I think it might be possible to make a private function that does the same thing as the Basis rotate_to_align function and try that. I thought I tried it, but maybe I am misremembering.

I do agree that it’s something in the rotation and may be related to #53358 but for 3.X. I did try a couple patches for rotation last time I took a look at this issue but none of them solved the issue. I’ll try to convert the rotate_to_align function soon (maybe tomorrow if there is time) and test it to see if it works 👍

ghost commented 2 years ago

update_bone_rest_forward_vector and get_bone_axis_forward_vector also need to be converted, right?

TwistedTwigleg commented 2 years ago

Ah yeah, probably. Not totally sure what the best solution for converting those would be. Maybe the forward directions could be calculated in the SkeletonIK ahead of time as well.

TwistedTwigleg commented 2 years ago

I did a quick test in Godot 4.0, to see if it has the same issue, since it has #53358. It sort of has the issue, but now instead of being above the shoulder that leads to the issue, it's when the rotation moves past the rest position. Here's a video showing what I mean:

https://user-images.githubusercontent.com/25082678/144317630-fac8a181-3153-4405-9860-1bbf080f570a.mp4

This makes me wonder if it's the initial orientation of the skeleton that has something to do with it, given that the issue in Godot 4.0 seems to only occur around the rest position, while the issue in Godot 3.x seems to occur around the rest position flipped on the Y axis.

The new IK system for Godot 4.0 behaves the same as the SkeletonIK node does in 4.0, where it can twist but only around the rest position. I'm not sure what is causing it though, as the new IK system uses a different solve method.

TwistedTwigleg commented 2 years ago

Good news though: The SkeletonIK node in Godot 3.2 stable does not have the issue, so there is at least hope! It may be possible to figure out what is going on by comparing the code from Godot 3.2.

ghost commented 2 years ago

I see it involves complex math :smiley: I wish it was easy to take code from Blender because it's strange to see a different behavior.

TwistedTwigleg commented 2 years ago

Okay, I found out some things!

This change is what caused the twisting issue at the pole. Specifically, the issue is created by using get_bone_global_pose_no_override instead of get_bone_global_pose.
- However, this change (PR) was made because of twisting because it takes whatever the current rotation is instead of keeping it relative to the initial rotation. So, we cannot simply revert it because then it the other issue will occur.
This also explains why it occurs with the new IK system as well! The issue is because when the desired rotation and the initial rotation are on the same line, the algorithms (SkeletonIK and new system) will rotate it around the pole as the solution.
- Technically this means the SkeletonIK is solving correctly, however this doesn't solve this issue. Ideally we need to find a solution that satisfies this issue and the issue in the PR.

Thankfully now that I have a better idea on what is causing the issue, it should be easier to find some sort of solution.

ghost commented 2 years ago

I don't understand the part about pole (which SkeletonIK prefers to call magnet), it sounds like the bones detach from the rest of the skeleton, rotate around pole and then translate back in order to attach to skeleton.

ghost commented 2 years ago

From the beginning what I visually observe is splitting a sphere with a radius of the perimeter of bones in IK chain into two connected semi-spheres and if the tip bone crosses semi-sphere in which it was initially placed, then there is no issue, but if it crosses its reflection, then each but tip bone in the chain will rotate by 180 degrees around its own axis.

TwistedTwigleg commented 2 years ago

The pole is the point when there joint rotates around when twisting. I think it's named after the poles on the earth, and it functions similarly. With a human upper arm, the pole would be the direction from the shoulder blade to the elbow (in other words, the direction the actual bone points in). In real life, we cannot really twist very much on the pole without muscle sheering, but in 3D animation it's used fairly frequently. The key is with any rotation in 3D, there is swing and twist rotation. Twist is around a pole, while swing is the rotation that affects the position of the next child bone (any rotation that isn't twist). So, in the case of this issue, the problem seems to be that when the swing rotation aligns with the pole in the initial rest rotation, it causes the twisting. In theory, what we can do is just reject the twist rotation, since we'll not want it anyway, but I'm not sure if it will work (am testing currently). Rotation in 3D with inverse kinematics is hard to explain! It's part of what makes it hard to work with, as the terms are confusing and hard to visualize.

The pole is different than the magnet position. What the magnet position does is applies an offset to certain bones in the FABRIK algorithm, which makes it bend towards the magnet position (but only the solve doesn't lead to a fully extended bone chain). The magnet basically applies an offset right before solving, it moves the bones towards the magnet, and then this forces the algorithm to correct the anomaly, which creates controllable bends. FABRIK takes the positions of bones into account when solving, which is why offsets we apply before solving are taken into account. Its kind of a side benefit of how FABRIK solves though, which is why not every IK algorithm can use magnet positions. CCDIK, for example, doesn't have any ability to support magnet positions because it solves using just rotations.

I probably explained it poorly, but hopefully it kinda helps explains the terms :sweat_smile:

Edit: Well, haven't solved it yet. The issue seems a tad more complicated than I was hoping initially.

ghost commented 2 years ago

I guess I have to forget about procedural animation for a while 🙁

TwistedTwigleg commented 2 years ago

In theory it just needs something like this DecomposeSwingTwist function to be called after applying the rotation. However, I have tried several different ways to attempt something like this without any success. Ideally what is needed is a way to rotate the bone without having any chance of twisting, but all of the solutions I've tried for this particular case have not worked.

I guess I have to forget about procedural animation for a while 🙁

It might be possible to work around the issue by changing the default rig of model from the arms down to the arms in a T position, which should shift the point where it has twisting issues to the inside of the arm (if I am correct) which would make it extremely unlikely to occur when animating. Not totally sure though, but that is what I would recommend trying if you are looking to work around this issue with the current code.

Another solution would be to switch this line of code back to p_chain_item->initial_transform = p_sk->get_bone_global_pose(p_chain_item->bone); and then compile Godot. It may have other issues (like the issue it was intended to fix), but if you are not getting those issues then it may be fine to just swap the code and use it that way.

ghost commented 2 years ago

It's not a single point where it has twisting issues, it's entire reflection plane which goes through the root bone: skeleton

T-pose is definitely not ok because most animations will cross that plane. For hands there is no default rig that would prevent them from breaking because unlike legs they need to go beyond 180 degrees.

Before updating I had similar issue with legs #47803 and as a workaround I tried changing the default rotation of bones which caused them to fail when they were moving front, back or left/right depending on default angle, by fixing that issue in geometrical sense you rotated the error plane by 90 degrees.

Also besides that something needs to be done about jittering, even if the only procedural animation was to bend spines when character looks up/down, it would still be a problem.

Zireael07 commented 2 years ago

Aside: speaking of magnets, it would be great if we could select a node (position 3D) as magnet - I tried entering various values and never really figured out how they work...

TwistedTwigleg commented 2 years ago

I am thoroughly stumped on a solution. I tried the decomposition of the quaternion to remove twist rotation, tried constructing it using LookingAt with various up directions, and tried using Basis-based rotation all with no luck. I’ve tried everything I can think of currently, and for now at least, I have given up.

Aside: speaking of magnets, it would be great if we could select a node (position 3D) as magnet - I tried entering various values and never really figured out how they work...

That should be a doable change for the SkeletonIK node. We just need to get a reference to the node, and then use something like magnet = skeleton.global_transform.xform_inv(magnet_node.global_transform.origin), at least I think it should look something like that. The only thing I’m not totally sure on is whether the magnet is an offset to the bone, or whether it’s a global position. I think it may be an offset, so some minor additional processing would be needed but I think it’s doable.

(It’s also doable with the IK system in Godot 4.0, but its harder due to it being a reference, magnets being per-bone rather than per IK chain, and having to access the skeleton/bones through the modification stack. It’s not impossible though, it would just be harder).

ghost commented 2 years ago

Magnet is definitely an offset and you can see that from the project I posted here (I used nodes as magnets), there are magnets on both legs and hands, however I didn't try figuring out what they are actually relative to and whether or not they obey scale.

As I understand you don't copy Blender's IK implementation because of the license.

TwistedTwigleg commented 2 years ago

Magnet is definitely an offset and you can see that from the project I posted here (I used nodes as magnets), there are magnets on both legs and hands, however I didn't try figuring out what they are actually relative to and whether or not they obey scale.

Ah okay, good to know! It should be doable then :+1:

As I understand you don't copy Blender's IK implementation because of the license.

There is the license that is a concern, but also the code is not terribly straightforward nor easy to understand and port over to Godot.

JoanPotatoes2021 commented 2 years ago

Aside: speaking of magnets, it would be great if we could select a node (position 3D) as magnet - I tried entering various values and never really figured out how they work...

👍 That would be ideal, that's how we work in 3d packages with poles, I wouldn't mind the magnets still be kept as Vector3 as long as they worked and maybe we could visualize them in the viewport, but magnets atm feel more like offsets rather than poles and sometimes they can be completely unpredictable.

GeorgeS2019 commented 1 year ago

@TokageItLab @TwistedTwigleg

Do check with this new SkeletonIK3D merged feature to feedback if this issue could be closed

SkeletonIK3D bone roll axis is not working correctly

lyuma commented 1 year ago

@GeorgeS2019 This issue is not about the regression in 4.0 that I fixed in #77469

Instead, this issue is about a longstanding problem present in >=3.3 (and >= 4.0) which relates to calculating a sub-optimal bone roll. I believe this issue is more of a design limitation of the use of FABRIK and the inability to set a pole vector in SkeletonIK(3D).

But it's still an issue, so we should keep it open for now.

GeorgeS2019 commented 1 year ago

Share a test project in zip

mojoyup1528 commented 9 months ago

Confirming same issue in v3.5.3.stable.official [6c814135b]. Using the ARVRCamera, the mesh head rotates 180 while my headset only rotates 90.

godotengine / godot