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

Inverse kinematic #18699

Merged
merged 2 commits into from
Aug 4, 2018
Merged

Inverse kinematic #18699

merged 2 commits into from
Aug 4, 2018

Conversation

AndreaCatania
Copy link
Contributor

@AndreaCatania AndreaCatania commented May 8, 2018

This is a short video that show IK in action https://youtu.be/hr2x5zWY_AQ

The code should be polished and the main thing to add are the constraints.
The constraints addition is not so important because the algorithm that I've used is FABRIK, and it doesn't need to be constrained to look "reasonably" and this mean that use it will be super easy.

In order to use it you don't need to attach a physical bone to the skeleton. You will need to add it only to apply a constraint (That it's not yer implemented).

How to use it:

To use it the first thing to do is create the IK task using this line of code:

var goal_transform = Transform()
var right_arm_chain = $Skeleton.create_ik_task("right_arm_bone", "right_hand_bone", goal_transform )

Then in the process function you have to call the function to solve the chain:

func _process(delta):
	$Skeleton.solve_ik_task( right_arm_chain );

Only if the position of task changes update it using:

var goal_transform = Transform()
$Skeleton.set_ik_task_goal(right_arm_chain , goal_transform )

@mysticfall
Copy link
Contributor

mysticfall commented May 8, 2018

First of all, thanks much for the contribution. I've been waiting for something like this for a while :)

I have a question though. Is it possible to specify 'influence' which determines how strong the IK would be applied (i.e. over the current animation)?

The use case I have in mind is modifying an animation, so that the character's hands or feet would move to the precise position.

Let's say the player throws a punch to another character, and in this case, what I expect would be the fist to land on the face at the end of the animation, rather than making the fist attached to the face through out the whole animation.

But I'm not sure how to achieve that using set_bone_ignore_animation or other APIs that this PR provides.

Is there something I'm missing, or should I open a separate issue after this gets merged?

@AndreaCatania
Copy link
Contributor Author

The main idea is that provide an API that allow us to use IK on skeleton, then implement some function (or node I hope) allow the animation player to use this API to dosome more "complex" operations (like animation blending with IK, or IK solving on event emitted, etc..).

What you want to do is not so difficult right now but it's very uncomfortable because you have to take care of all things like event managing manually.

In the video that I've posted you can see that the IK solver is running only when the body is inside the button area, when it go outside the solver stop running. Here I could integrate it with an animation but as I said above I should take care of all event managing manually.

@mysticfall
Copy link
Contributor

mysticfall commented May 8, 2018

In Unity, there's SetIKPositionWeight and SetIKRotationWeight pair with which we can specify how much the IK would affect the animation.

If there was such a function in Godot, I thought it could be a lot easier to deal with such problems that I described above.

For instance, I might add an additional function track in the punching animation which can gradually change the IK weight/influence at the correct moment (i.e. just before the fist hits the target) to achieve the ideal result.

I was wondering if something like this could be implemented with the current binary on/off flag that is included in this PR. And if not, I think it'd be nice to have an API to change IK weight before we can implement more advanced features like the one you described.

Of course, I'm not saying that you should make the change right away, but I just wanted to know if I should open a feature request for it.

The video looks really cool, by the way. Can't wait to try it myself :)

@AndreaCatania
Copy link
Contributor Author

The weight essentially is a parameter for blending animation. I'm imagine a scenario where you have an animation tree like this:

untitled

For this reason I think that we don't have to manage it inside the IK sover, in this way we can take the two things separated and leave to the proper parts makes their job.

I think that manage IK inside the animation tree rather using the code is the best thing to do

@mysticfall
Copy link
Contributor

That makes sense. Then I'll wait until we overhaul AnimationTreePlayer before 3.1, hopefully with the IK integration feature you suggested. Thanks for the explanation!

@akien-mga
Copy link
Member

akien-mga commented May 8, 2018

Build fails with clang (gcc and msvc are fine):

./scene/3d/skeleton.h:243:5: error: delegating constructors are permitted only in C++11
                                ChainTip(other_ct.chain_item, other_ct.end_effector) {}
                                ^~~~~~~~

@AndreaCatania
Copy link
Contributor Author

@akien-mga Sadly :| error: delegating constructors are permitted only in C++11

Compilation error fixed
Code polished
Improved functions naming
Improved ik API workflow

(Updated "How it works" on first comment)

@slapin
Copy link
Contributor

slapin commented May 10, 2018

About why I do need to keep some bones rigid in IK.

I have skeleton which I use for animation, it contains limbs of 4 bones, so 2 of bones act as one in case of IK (but that is not a case for animation - some poses simply require this to look ok, i.e. twisiting or muscule bulge simulation, small-scale bending). If IK will require all bones to be IKable, I will have to set up fake bones for IK and manually convert rotations to actual bones in frame loop which will eat-up CPU cycles. Swichig meshes/skeletons are also a problem (i.e. I want to use IK to fixup animation, not just posing, so I will need full skeleton for that).

@AndreaCatania
Copy link
Contributor Author

Talk about full skeleton IK is wrong. If you try this pr you will see that you have to create an IK task where you have to set root and tip bones. If the two bones to handle the muscle are outside of this chain its basis will not be touched.

When you create the task the chain is composed only of direct bones, all other appended bones doesn't partecipate to the solving and its basis is not touched

If this is not your case please send me an image that represent your skeleton

@slapin
Copy link
Contributor

slapin commented May 10, 2018 via email

@LeonardMeagher2
Copy link
Contributor

@slapin are you saying the "kneel" is the parent to lowerleg01.L?
I'd imagine upperleg02.L should be a parent to both lowerleg01.L and "kneel"

@slapin
Copy link
Contributor

slapin commented May 10, 2018 via email

@pwab
Copy link

pwab commented May 16, 2018

As discussed with @TwistedTwigleg in godotengine/godot-demo-projects/issues/211 I also tried to impement FABRIK but for 2d IK chains. Is your implementation somehow transferable for 2d skeletons, @AndreaCatania?

@AndreaCatania
Copy link
Contributor Author

Yes it's possible, it's not too difficult take the code that I written and change Vector3 with Vector2 etc.. Feel free to do it if you want

@Zireael07
Copy link
Contributor

Is this merge-ready or is it on hold like the soft body PR?

@AndreaCatania
Copy link
Contributor Author

It works, but reduz said that I need to add a function to blend with current animation pose. I think that I can do it today. I let you know

@AndreaCatania
Copy link
Contributor Author

I can't do it today I will finish it this weekend sorry

@AndreaCatania AndreaCatania force-pushed the ik branch 2 times, most recently from 2fa9a86 to d0376cf Compare June 9, 2018 10:16
@AndreaCatania
Copy link
Contributor Author

I've implemented the blending functionality and this PR is ready to be merged.

Now is possible to use it with animation player so you can use IK easily with your animations.
Here a video: https://youtu.be/PXhF78c6wRs
and here the project to test it, and see how I've implemented it using animation player:
ik.zip

@AndreaCatania AndreaCatania force-pushed the ik branch 3 times, most recently from c4a604c to a618c2b Compare June 17, 2018 11:53
@AndreaCatania
Copy link
Contributor Author

I've implemented the new node SkeletonIK as @reduz proposed.

Now is possible to set a root and tip bone and a target transform or a spatial node as target directly in the SkeletonIK node.
Then by calling the method start now the IK will be solved automatically and if you set a node as target the IK will follow it.

And also is possible to set the interpolation value, that can be changed at runtime, check this project to test it: ik.zip

Let me know what you think about!

@AndreaCatania
Copy link
Contributor Author

In this picture the error on the feet are visible only on the first line of people sitdown on the wall, and for all others you don't need the IK to fix it.

The same rule goes with Ubisoft video, do you think that all people that are so faraway that seems just a texture have the IK active? I don't think so... you need a perfect IK only for nearest person and should be activated only when required so when a person is out of focus but still near and visible you need to solve IK with just a good precision, but for all other you don't need to solve..

Also when you resolve the IK one time you don't need to re solve it again if you don't move the bones.

Anyway I think that for talk about what we have to change to optimize we first need a scene that runs at low FPS to check it... because in this moment we are talk about a real picture and what you could need to make it in godot

@AndreaCatania AndreaCatania force-pushed the ik branch 3 times, most recently from 2c99396 to 796f59a Compare June 30, 2018 10:21
@AndreaCatania
Copy link
Contributor Author

  • Rebased
  • Added exposed some internal settings
  • Added possibility to play IK directly in the editor
    ezgif com-video-to-gif

@AndreaCatania
Copy link
Contributor Author

The IK had the problem that when you wanted to make it works with the animation player, the animation player override the bone poistion if executed after IK solver.

In order to make it works correctly I've moved the execution code from process_internal to process, so I'm always sure that the IK solver is processed always after all animation players.

This doesn't make any differences on Editor side and is always possible to override the _process function by a script.

@hpvb
Copy link
Member

hpvb commented Jul 14, 2018

@AndreaCatania are you happy with the current state of this PR? I think it should be merged if you are happy.

@AndreaCatania
Copy link
Contributor Author

AndreaCatania commented Jul 14, 2018 via email

@karroffel
Copy link
Contributor

I mentioned this before in a private discussion, but I'll leave it here so others can see it too.

I tested this PR, and while it works really well, the interface and way to use it is quite different than in pretty much all other software that let's the user use IK.

With this PR, a target transform is applied to the bone on the tip of the chain, then the rest of the chain "follows". That means that the rotation of the target has to match the one of the desired pose.

This is a lot harder to get right.

The usual approach is to have a target (that only contributes position by default, rotation opt-in) and a pole that dictates how the chain will bend.

From what I have seen, there is an official IK demo that adds a "middle bone helper" (so a pole basically) on top of the FABRIK algorithm.

https://github.com/godotengine/godot-demo-projects/blob/c6b481aaf0cfb677b182b0e3711702c15b0cb413/3d/ik/addons/sade/ik_fabrik.gd

Maybe this PR can be modified so it supports the more "standard" target+pole workflow.

class SkeletonIK : public Node {
GDCLASS(SkeletonIK, Node);

String root_bone;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by the way, I think this should be StringName, not String

Copy link
Contributor Author

@AndreaCatania AndreaCatania Jul 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to me that it is not better, because the skeleton internally store bone names as String

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to set StringName anyway, but I got compilation error..

void set_target(const Transform &p_target);
const Transform &get_target() const;

void set_target_node_override(const NodePath &p_node);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this should be called override, if set it should be obvious (And docummented) that takes priority over a transform

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

void set_interpolation(real_t p_interpolation);
real_t get_interpolation() const;

void set_target(const Transform &p_target);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this set_target_transform

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

NodePath get_target_node_override();

void set_min_distance(real_t p_min_distance);
real_t get_min_distance() const { return min_distance; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont quite understand what this is for

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is used to control the precision of IK, it tells the minimum distance between target and tip before consider the process done.

@AndreaCatania
Copy link
Contributor Author

@reduz I've a problem with process_internal priority.

The execution order seems random, (or better seems related to the area of last mouse click), when I start the animation and IK in the editor the IK sometimes is processed before the animation player and sometimes later...
Even if the priority of AnimatorPlayer is 0 and should be executed before SkeletonIK that has the priority 1.

check video: https://www.youtube.com/watch?v=yBliluCY6M0

Ther's something other that I need to address to properly implement the priority?

@akien-mga
Copy link
Member

Needs a rebase to fix conflicts in skeleton.cpp.

@AndreaCatania
Copy link
Contributor Author

Rebased

@@ -72,7 +72,7 @@ class Node : public Object {

struct ComparatorWithPriority {

bool operator()(const Node *p_a, const Node *p_b) const { return p_b->has_priority_higher_than(p_a) || p_b->is_greater_than(p_a); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just request the priority (integer) and compare integers here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to add a function that no one else will use

@AndreaCatania AndreaCatania force-pushed the ik branch 2 times, most recently from 3cf871e to bfede82 Compare August 4, 2018 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants