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

Skinning influenced by more than 4 joints #1381

Merged
merged 3 commits into from
Nov 14, 2018

Conversation

broughtong
Copy link
Contributor

A brief description of what happens in the event that skins are influenced by more than 4 joints

A brief description of what happens in the event that skins are influenced by more than 4 joints
@pjcozzi
Copy link
Member

pjcozzi commented Jul 7, 2018

Cool, thanks @broughtong!

@lexaknyazev or @bghgary?

Copy link
Member

@lexaknyazev lexaknyazev left a comment

Choose a reason for hiding this comment

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

Somewhat related to #1352.
/cc @donmccurdy

@donmccurdy
Copy link
Contributor

Looks good! OK with me to merge this, #1352 does not conflict.

@bghgary
Copy link
Contributor

bghgary commented Jul 17, 2018

Should we reiterate that only one set must be supported by implementations as stated in the mesh attributes section?

Client implementations must support at least two UV texture coordinate sets, one vertex color, and one joints/weights set

Other than that, looks good to me.

@broughtong
Copy link
Contributor Author

Sure I could just append something to the end:

In the event that of any of the vertices are influenced by more than four joints, the additional joint and weight information will be found in subsequent sets. For example JOINTS_1 and WEIGHTS_1 if present will reference the accessor for up to 4 additional joints that influence the vertices. Note that client implementations are only required to support a single set of up to four weights and joints.

Do you think that this then in turn requires mentioning that doing so may have an affect on the final animation (as mentioned by McNopper in #1352)? Something like:

Note that client implementations are only required to support a single set of up to four weights and joints, however not supporting all weight and joint sets present in the file may have an impact on the model's animation.

I guess this might be a mute point though as it's already self evident that if the client doesn't support the additional data sets present in a file then the animation necessarily wont be exactly the same, but it can be left to the client to handle this (ignore, warn etc). What's your opinion on this?

@lexaknyazev
Copy link
Member

The note above looks good to me. The spec cannot require any specific runtime behavior for unsupported features.

@broughtong
Copy link
Contributor Author

Ok I've added the note and ready for another review.

@donmccurdy donmccurdy mentioned this pull request Jul 24, 2018
11 tasks
@donmccurdy
Copy link
Contributor

donmccurdy commented Jul 24, 2018

"may have an impact" is vague enough to be safe, I am OK with merging this.

Whether we can require runtime behavior for unsupported features or not, would we have a preference? E.g. ignoring the skin with a warning seems preferable to a distorted animation, yes? It is at least easier to debug. Asking as an implementor. 😅

@broughtong
Copy link
Contributor Author

There's no way to quantify just how much impact it will have visually on the final animation, so I think what the implementations then do depends on what their use case is.

Additional joint influences could have (close to) zero impact particularly for small translations, so it may not be worth ignoring the entire skinning information.

On the other hand a rotation that's not quite right near the root could seriously throw off the entire animation sequence.

I would say generally from what I've seen they tend to be the former, but certainly that may not be the case for everyone.

@bghgary
Copy link
Contributor

bghgary commented Nov 14, 2018

We discussed this during the WG call today and agree to merge this.

@bghgary bghgary merged commit 43f6909 into KhronosGroup:master Nov 14, 2018
@ziriax
Copy link

ziriax commented Nov 14, 2018

Is it a requirement for implementations that only support 4 joint weights to pick the 3 with largest weight, and re-normalize? Is this is already in the doc?

@bghgary
Copy link
Contributor

bghgary commented Nov 14, 2018

There is no such requirement in the spec. We said during the call that there is no easy way to pick the largest 4 and still have a meaningful result. (Someone correct me if I misspoke)

@broughtong broughtong deleted the many-joints branch November 15, 2018 07:58
@ziriax
Copy link

ziriax commented Nov 15, 2018

Yeah that might indeed be the case. One would have to find a new set of weights and indices that minimises the error across all animation clips I guess... I do think this error would be smaller when sorting by weight, but it might not be the best solution. Needs some math ;-)

@lexaknyazev
Copy link
Member

@ziriax We've discussed the idea of pre-sorting joints by weight (per-vertex) but even in that case visuals will likely be broken anyway.

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.

6 participants