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 chain solver operates differently in GDExtension and C++ Module #91061

Open
stevenlgreen00 opened this issue Apr 23, 2024 · 6 comments

Comments

@stevenlgreen00
Copy link

Tested versions

Tested 4.2.2

System information

Linux, x86_64, Kubuntu 22.04, Godot 4.2.2 custom build

Issue description

I decided to port my C++ extension into a C++ module. I discovered a discrepancy between the two versions and thought I would mention it. When mapping bones between the two versions their orientation is reversed.

Steps to reproduce

When SkeletonIK3D is in a C++ extension.

Vector3 forward_vector = (ci->initial_transform.get_origin() - ci->children[0].initial_transform.get_origin()) .normalized();

In a Module, and as it is in the engine's version.

Vector3 forward_vector = (ci->children[0].initial_transform.get_origin() - ci->initial_transform.get_origin()) .normalized();

The origins swap positions.

Minimal reproduction project (MRP)

Being a C++ project it is a bit unwieldy to create a small project with this, but I thought I would highlight the discrepancy. I have versions working as a Module and Extension. I just don't think there should be a difference between the versions.

@AThousandShips
Copy link
Member

The origins swap positions.

Swap how? Do you mean you have to change the code for it to be correct? The engine code doesn't change itself, did you copy the code?

@stevenlgreen00
Copy link
Author

Swap was in reference to switching the direction of the forward_vector. I implemented my own SkeletonIK3D node, but I started with Godot's implementation of SkeletonIK3D. I kept the forward_vector. In order for bone-attachments to draw properly the forward_vector is computed as "current_origin - child_origin" for the extension and "child_origin - current_origin" in a module.

@AThousandShips
Copy link
Member

AThousandShips commented Apr 23, 2024

Then you might need to look at the total code, looking at the general code in the engine there's nothing wrong about the vectors, so you need to make sure you haven't made any mistake anywhere

Without posting your actual code there's noting we can do to help, the math code in the engine is identical, so that specific line is causing this, something else must be broken in your code

@AThousandShips
Copy link
Member

If you need help debugging your code though you should probably ask in the forums

@stevenlgreen00
Copy link
Author

Thanks for taking the time. I made a lot more changes than I had realized and didn't mean to waste your time.

Just to let you know what I missed, in-case your curious.

I went back to the original SkeletonIK3D node an tried to identify the minimum changes necessary, and found another difference that effected this that I had not considered when I started this thread.

The initial transforms from bone poses needed to be multiplied by the skeletons global transform to get SkeletonIK3D to work as an extension. I expect how the initial_transforms were changed would have altered things, causing the swap in forward_vector.

The changes to initial_transform were similar to the following:

void FabrikInverseKinematic::_update_chain(const Skeleton3D *p_sk, ChainItem *p_chain_item) {
    if (!p_chain_item) {
        return;
    }

    Transform3D G = p_sk->get_global_transform(); // LINE ADDED
    p_chain_item->initial_transform = G * p_sk->get_bone_global_pose_no_override(p_chain_item->bone); // MULTIPLIED
    p_chain_item->current_pos = p_chain_item->initial_transform.origin;

    ChainItem *items = p_chain_item->children.ptrw();
    for (int i = 0; i < p_chain_item->children.size(); i += 1) {
        _update_chain(p_sk, items + i);
    }
}

@AWSUMSAS
Copy link

Try tampering with the code so that the C++ module matches the behavior of the extension. Your problem is that there's an issue between the calculation of the forward vector and the orientation of the bone mapping between the extension and the module.
Try replacing the module code with the following:
Vector3 forward_vector = (ci->initial_transform.get_origin() - ci->children[0].initial_transform.get_origin()).normalized()
This should align the calculation of the forward vector in the module with how it's done in the extension.

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

3 participants