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

SkeletonIK rotates bones by 180 degrees #54891

Open
ghost opened this issue Nov 11, 2021 · 27 comments
Open

SkeletonIK rotates bones by 180 degrees #54891

ghost opened this issue Nov 11, 2021 · 27 comments

Comments

@ghost
Copy link

ghost commented Nov 11, 2021

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

2021-11-11.22-55-40.mp4

Steps to reproduce

Simply move the target up.

Minimal reproduction project

iktest.zip

@TwistedTwigleg
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

ghost commented Nov 30, 2021

@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 92a79ac into 3.x

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

@ghost
Copy link
Author

ghost commented Nov 30, 2021

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
Copy link
Author

ghost commented Dec 1, 2021

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

@TwistedTwigleg
Copy link
Contributor

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
Copy link
Author

ghost commented Dec 1, 2021

update_bone_rest_forward_vector and get_bone_axis_forward_vector also need to be converted, right?

@TwistedTwigleg
Copy link
Contributor

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
Copy link
Contributor

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:

Godot_4_SkeletonIK.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
Copy link
Contributor

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
Copy link
Author

ghost commented Dec 1, 2021

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

@TwistedTwigleg
Copy link
Contributor

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
Copy link
Author

ghost commented Dec 1, 2021

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
Copy link
Author

ghost commented Dec 1, 2021

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
Copy link
Contributor

TwistedTwigleg commented Dec 1, 2021

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 😅

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

@ghost
Copy link
Author

ghost commented Dec 3, 2021

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

@TwistedTwigleg
Copy link
Contributor

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
Copy link
Author

ghost commented Dec 4, 2021

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

ghost commented Dec 5, 2021

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
Copy link
Contributor

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 👍

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
Copy link

JoanPotatoes2021 commented Apr 18, 2022

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.

@ghost ghost mentioned this issue Sep 15, 2022
4 tasks
@GeorgeS2019
Copy link

GeorgeS2019 commented May 25, 2023

@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
Copy link
Contributor

lyuma commented May 25, 2023

@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
Copy link

Share a test project in zip

@mojoyup1528
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants