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

Parenting Assumption in Skin.BindJoints() and Skin._FindCommonAncestor() #193

Open
effs opened this issue Aug 23, 2023 · 1 comment
Open
Labels
bug Something isn't working

Comments

@effs
Copy link

effs commented Aug 23, 2023

This might not be exactly a bug, not sure what the desired behavior is but it caught me by surprise so I figured I'd raise it:

_FindCommonAncestor() will not traverse to a parent of a single node.

I think it could be argued both ways whether this is correct behavior for the function itself (it does travel to parents outside of the working set if there are multiple nodes) but because BindJoints(params Node[] joints) uses it directly, it means that for single nodes the IBM is calculated against itself which may or may not be desirable.

For my purposes, I fixed the problem by explicitly passing in the parent's WorldMatrix using another overload but I think either

  1. making the behavior consistent so that BindJoints will always find the closest shared parent, not the node itself, would be ideal if it doesn't cause problems; or

  2. the function or even parameter naming could be changed to help understand the intent if this is intended behavior

What's your take on this?

@vpenades
Copy link
Owner

Hmm... I would need some kind of simple case scenario to see how this is a fix, I mean, the whole thing related to skinning is already mind boggling 😅

@vpenades vpenades added the bug Something isn't working label Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants