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

Allow Chain to include bodies have multiple children #1268

Closed
jslee02 opened this issue Mar 29, 2019 · 4 comments
Closed

Allow Chain to include bodies have multiple children #1268

jslee02 opened this issue Mar 29, 2019 · 4 comments

Comments

@jslee02
Copy link
Member

jslee02 commented Mar 29, 2019

Chain doesn't allow including bodies that has multiple child bodies, but this criteria seem too strict. To me, a chain shouldn't have a branch in it but should be fine to be in a tree.

The current behavior prevents creating an proper IK from an end-effector if the parent body of end-effector has another child body: the IK will use only the parent joint of the end-effector but not the other ancestor joints to the root. This could be workarounded by manually specifying the desired DOFs to be used, but it's not ideal.

@mxgrey
Copy link
Member

mxgrey commented Apr 1, 2019

The Chain class was specifically designed to grab portions of a Skeleton that contain no branching.

Why not use the Linkage class for the use case that you're talking about?

If the problem is that Linkage::Criteria isn't convenient enough, we could give it a constructor to match the Chain::Criteria class, like:

Linkage::Criteria::Criteria(BodyNode* start, BodyNode* target, bool includeBoth = false);

and it'll construct a Linkage the same way it would construct a Chain but without the no-branching condition.

@jslee02
Copy link
Member Author

jslee02 commented Apr 2, 2019

Yeah, Linkage can be used for my use case since Chain is a special case of Linkage. And the constructor looks great to me.

The reason I proposed the criteria relaxation (optionally by adding a flag) of Chain was that Linkage is to represent subtree (since it allows multiple targets) so it might be a bit too general for non-strict chains.

If we go with the option of not changing Chain class then it'd be good to add a function Linkage::isChain() that returns true when the number of targets is one.

@mxgrey
Copy link
Member

mxgrey commented Apr 2, 2019

If we go with the option of not changing Chain class then it'd be good to add a function Linkage::isChain() that returns true when the number of targets is one.

This is kind of a tricky semantic issue in my mind. The intention of the "chain" concept was to assure users that they've selected a subset of a skeleton where the selection can be abstracted as two transforms (the first transform in the chain and the last transform in the chain) without impacting anything outside of the selection.

In the case you're describing, the selection is a linear sequence, but there are bodies parallel to the sequence that are impacted by more than just the endpoints of the sequence.

What if the function were named isSequence() instead to distinguish it from the Chain concept?

@jslee02
Copy link
Member Author

jslee02 commented Apr 4, 2019

I see your point. Let's keep the Chain as it is and give some utility functionalities to Linkage to support "sequence" structure in a convenient way.

What if the function were named isSequence() instead to distinguish it from the Chain concept?

👍

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

No branches or pull requests

2 participants