godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.06k stars 20.19k forks source link

SkeletonIK works incorrectly in Godot 3.3 #48078

Closed macryc closed 3 years ago

macryc commented 3 years ago

Godot version: 3.3

OS/device including version: Win10, PC, GLES3, Nvidia Geforce RTX2070

Issue description: I opened my 3.2.3 project in 3.3 and noticed that the SkeletonIK node behaves erratically. I had its target set to copy the camera's rotation so that the player's torso moves with the camera, which works perfectly fine in 3.2.3, but in 3.3 the SkeletonIK seems to be ignoring its target and instead looking in erratic directions (it's impossible to find the pattern here).

Steps to reproduce: Open attached tscn in Godot 3.3. Play IK on spine_ik (the SkeletonIK node under camera root). Move the target node on z axis (positive z) and watch the root bone twist around.

Minimal reproduction project: Player.zip

yaelatletl commented 3 years ago

Can confirm, the previously working IK setup is now twisted

macryc commented 3 years ago

Yes, something got messed up around RC5. Suggest returning to the RC4 commit for SkeletonIK - I believe it worked fine until RC5. Heard it also works OK in 4.0 which apparently is using the previous code for SkeletonIK (haven't tried 4.0).

smix8 commented 3 years ago

Can't reproduce your issue with my test Skeletons as all bones rotate as expected for me in Godot 3.3.

This issue needs a reproduction project with your Skeleton setup.

From your description I expect the bug fixes in #47738 for root bone rotation and solver update to be the issue for your skeleton setup no longer working as before.

akien-mga commented 3 years ago

CC @TwistedTwigleg @AndreaCatania

macryc commented 3 years ago

Can't reproduce your issue with my test Skeletons as all bones rotate as expected for me in Godot 3.3.

This issue needs a reproduction project with your Skeleton setup.

From your description I expect the bug fixes in #47738 for root bone rotation and solver update to be the issue for your skeleton setup no longer working as before.

The project is huge and messy - it would be difficult to extract just the minimum reproduction component. But I've recorded this bug, take a look (maybe that gives you a hint?): https://www.youtube.com/watch?v=titXIneUxIc

brakhane commented 3 years ago

The project is huge and messy - it would be difficult to extract just the minimum reproduction component. But I've recorded this bug, take a look (maybe that gives you a hint?): https://www.youtube.com/watch?v=titXIneUxIc

If you cannot create a minimal working example, could you make your project downloadable, at least for devs who want to try to debug this?

While a video is good, nothing beats a (non-)working example, because using that a developer can find the exact commit that broke it (instead of say "it broke between RC4 and RC5" you can see that commit abcdef broke it) and it can also confirm that the fix actually works.

smix8 commented 3 years ago

@macryc I don't need your entire project. Just your skeleton resource (maybe character skinned base mesh for visuals) and your SkeletonIK setup saved in a packedscene should be enough to debug this.

The visuals in the video only tells me that one or more of your bones in the bone chain are oriented in opposite directions of your target node. Since the tip bone is copying the transform rotation of your target node it gets flipped and twists invert.

You could test this by placing a rotated child Spatial under your camera node and use it as the IK chain target node.

TwistedTwigleg commented 3 years ago

Out of curiosity, is it only the bone twist that is the issue? Prior to #47738 the root/first bone wouldn’t rotate itself correctly. If all the rotation, not just the twist, is incorrect then #47738 may have not fixed the issue in this case.

macryc commented 3 years ago

It looks like the bone is twisting. Overall the posture seems correct - it works the same way it did in 3.2.3. It's the root bone (the 1st spine bone, in this case) that is rotating incorrectly. The tip and the bone in the middle seem to be working fine, otherwise the character wouldn't be looking at the target mark (I'm referring to the video).

Described it fully in Godot group on FB: The skeletonIK starts at the spine bone (which is the root bone) and goes 2 spine bones up until the chest bone (which is the tip). The target is set to look at the collision point of the camera raycast. So clearly, the chest bone is doing that as the character is aiming towards the camera raycast collider correctly. But the lumbar spine bone, which is the root of the SkeletonIK is rotating so erratically can't even tell what it's trying to do. This must be a bug with the root bone rotation (I think they updated it in RC5 and looks like they created a bug).

macryc commented 3 years ago

@macryc I don't need your entire project. Just your skeleton resource (maybe character skinned base mesh for visuals) and your SkeletonIK setup saved in a packedscene should be enough to debug this.

The visuals in the video only tells me that one or more of your bones in the bone chain are oriented in opposite directions of your target node. Since the tip bone is copying the transform rotation of your target node it gets flipped and twists invert.

You could test this by placing a rotated child Spatial under your camera node and use it as the IK chain target node.

@macryc I don't need your entire project. Just your skeleton resource (maybe character skinned base mesh for visuals) and your SkeletonIK setup saved in a packedscene should be enough to debug this.

The visuals in the video only tells me that one or more of your bones in the bone chain are oriented in opposite directions of your target node. Since the tip bone is copying the transform rotation of your target node it gets flipped and twists invert.

You could test this by placing a rotated child Spatial under your camera node and use it as the IK chain target node.

Here is the player scene with the rig and SkeletonIK (and a bunch of other mess). Spine_IK_looker node is the one that I was rotating to match camera's rotation. It has a child of spine_ik_target, which is the target for SkeletonIK. I hoped to solve the issue by rotating the target by 180 degrees but that is not the problem. The problem is literally with the root bone twisting itself around in relation to its child. Looks like using look_at for that is not the right solution. Player.zip

Btw, I went back over the RC's and: RC4 - all good RC5-6 - SkeletonIK is not working at all RC7-8 - some work on the SkeletonIK - the root bone is oddly rotated towards its child (major scoliosis) but it's not twisting around as the target rotates RC9 - the root bone is correctly rotated towards its child (like in RC4) but it's twisting around as the target rotates

smix8 commented 3 years ago

I managed to get your Player.zip working (was broken on load) and removed dependencies and everything that is unnecessary for this issue.

here is a new MRP for debug Player.zip

Your skeleton has an inverted spine bone rest pose but this alone is not the problem. Your inverted bone gets twisted when SkeletonIK kicks in and rotates the bone to -180 degree. I think the problem is that parts of the SkeletonIK source are made with Euler rotations while in a sane world we could use Quat's to prevent such rotation errors.

TwistedTwigleg commented 3 years ago

... I think the problem is that parts of the SkeletonIK source are made with Euler rotations while in a sane world we could use Quat's to prevent such rotation errors.

I’m not sure Euler angle use is the direct problem. Whether you are using Euler or Quat, the rotation of a bone to a target position has infinite possibilities, so you’d have to resolve using some method. How you resolve that rotation is where it makes a bit of a difference, but more from an implementation perspective than an end result. (Unless you are interpolating, then yeah, Quat rotation is MUCH better 🙂)

My hunch is that the issue is caused by using look_at to generate the Basis for rotation, and the calculation there is leading to -180 degrees of twist rotation rather than reaching the same point using swing rotation. I wrote the code to use look_at for the root bone because otherwise the BoneAttachment3D node wouldn’t work when assigned to the root bone.

That said, I have a hunch on how to revert the code so the root bone is rotated like the other bones while also having BoneAttachment3D nodes working, I just haven’t had the time to try it. I think that executing the IK code after _process will fix the issue with BoneAttachment3D nodes while still using the old code, but the code in SkeletonIK will need some minor adjusting to get there. I plan to look at it soon, probably this weekend assuming nothing unexpected happens with my schedule.

macryc commented 3 years ago

I managed to get your Player.zip working (was broken on load) and removed dependencies and everything that is unnecessary for this issue.

here is a new MRP for debug Player.zip

Your skeleton has an inverted spine bone rest pose but this alone is not the problem. Your inverted bone gets twisted when SkeletonIK kicks in and rotates the bone to -180 degree. I think the problem is that parts of the SkeletonIK source are made with Euler rotations while in a sane world we could use Quat's to prevent such rotation errors.

Thanks mate. Yea, I just posted the player resource on its own - should have mentioned there's bound to be tons of dependencies to ignore/remove. This is a Mixamo skeleton. I did think that there may be some issues with the original rest pose of the spine bone but it looked fine in Blender (perhaps something to do with the importer). Anyway, as @TwistedTwigleg said, that isn't the issue. I'm curious to see what he finds. Thanks both for looking at it.

Naurk commented 3 years ago

Hi everyone, I have the same issue with a Mixamo skeleton. Here is the minimal project: https://github.com/Naurk/TP-Minimal-Controller

Just apply a SkeletonIK and you will see the twisted root bone, this didn't happen in 3.2.3 and previous. Sorry if I'm redundant, probably it depends on orientation issues, but do you know how can be solved (preferably directly in Godot)?

P.S.: sorry, didn't see TwistedTwigleg post.

macryc commented 3 years ago

Hi everyone, I have the same issue with a Mixamo skeleton. Here is the minimal project: https://github.com/Naurk/TP-Minimal-Controller

Just apply a SkeletonIK and you will see the twisted root bone, this didn't happen in 3.2.3 and previous. Sorry if I'm redundant, probably it depends on orientation issues, but do you know how can be solved (preferably directly in Godot)?

TwistedTwigleg has already fixed it. There is a request to pull the fix into the next release (3.3.1., I believe). Just need to wait a bit.

Zireael07 commented 3 years ago

The PR @macryc mentioned is this one: https://github.com/godotengine/godot/pull/48166

okay300 commented 3 years ago

Apart from turning, there's a problem with scaling the bone. Godot version: v3.2.3 / 3.3

It was the №2 problem in my report, but the topic was closed. https://github.com/godotengine/godot/issues/48358 Scale________1

Godot and blender file. Ik_test.zip

TwistedTwigleg commented 3 years ago

@okay300 - are you scaling the bone, the Skeleton, or the SkeletonIK node? Also, was this issue occurring prior to Godot 3.2.3? Edit: Just tested with the latest version/build in #48251 and it is fixed there, so that PR seems to fix both issues.

okay300 commented 3 years ago

The defect appears if you use SkeletonIK along with zoom animation. I zoom in through the animation of Bone 1.

Scale__

Godot and blender file: Ik_test.zip

okay300 commented 3 years ago

Can I recreate a theme that touches bone scaling?

TwistedTwigleg commented 3 years ago

@okay300 - thank you for the clarification on the issue and additional pictures! Sorry I didn't get back sooner, things have been busy on my end.

Can you test the artifact Godot editor builds here (Windows link) to see if they fix the scaling issue? The PR still has an issue with interpolation that I need to fix (planning to try and tackle it this weekend). You can find the build by clicking the "Artifacts" button in the top right corner of the screen and then downloading the editor. In my testing the issue was fixed in that build, but if you can still reproduce it, then it is probably a separate issue, in which case we can reopen the issue you made previously and/or make a new issue.

okay300 commented 3 years ago

Now animation does not zoom bone 1 heirs. Testing in fixed version - Fixes the SkeletonIK Scale__2_1 if you scale in a godo, scaling works.

godo and blender project.zip

akien-mga commented 3 years ago

I tested the above project in the latest version of #48251 (updated yesterday) and I can confirm that there still appears to be an issue with the scaling animation when IK is playing.

For the reference, @okay300's MRP has problems on 3.2.1, 3.2.2, 3.2.3 and 3.3-stable (slightly different from one version to another), so this final issue with scaling might not be a regression in 3.3 (like @macryc's original issue) and might also not be related to #48251. So we were maybe a bit too quick to close #48358 as duplicate, there's some overlap but also a unique issue issue it seems.

TwistedTwigleg commented 3 years ago

Sounds like the scaling problem is a separate issue then and not a direct regression, or at least not a regression caused by the BoneAttachment changes. I'd suggest we reopen #48358 and close this issue since #48251 fixes the OP problem.

akien-mga commented 3 years ago

Agreed, I'll reopen #48358 and copy https://github.com/godotengine/godot/issues/48078#issuecomment-833992369 there.