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

Clarify skinning #1784

Closed
jjcasmar opened this issue Mar 26, 2020 · 14 comments
Closed

Clarify skinning #1784

jjcasmar opened this issue Mar 26, 2020 · 14 comments

Comments

@jjcasmar
Copy link

I have read that skeleton property can be ignored by importers (#1403)

I would like to know what happens in a case like the following:

  • A node with a mesh and a skin on the root of the scene. It may have a transform
  • A joints hierarchy with a common ancestor with a transform
  • The common ancestor with a transform has an ancestor with a transform

From the docs it is clear that the transform of the node containing the mesh should be ignored, but how should we interpret the transform for the skeleton itself?

The skeleton has a common ancestor with a transform. Should we apply that transform to the skeleton? If one uses the skeleton property, it can set it to be the common ancestor with the transform but also the one without the transform. Why should we select one or the other?

Also, what happens if there is no common ancestor and the joints come directly from the scenes array?

@donmccurdy
Copy link
Contributor

how should we interpret the transform for the skeleton itself? ... The skeleton has a common ancestor with a transform. Should we apply that transform to the skeleton?

The skeleton property (or lack thereof) has no bearing on the transforms of joints. Each joint's world transform — relative to the scene root — is affected by all of the joint's ancestors.

If one uses the skeleton property, it can set it to be the common ancestor with the transform but also the one without the transform. Why should we select one or the other?

Looking for a transform is probably not a good way to assign the skeleton property. It indicates a semantic root of the skeleton, which may be helpful to software in which nodes and bones are not mixed in the node hierarchy, to preserve object types. In short, it is just a hint. If a skeleton property does not have some meaning in the software exporting the model, you might as well omit it.

Also, what happens if there is no common ancestor and the joints come directly from the scenes array?

Joints must belong to a single scene. If they have no common ancestor other than the scene, the scene itself is necessarily the common ancestor. In that case the skeleton property would be omitted.

@jjcasmar
Copy link
Author

jjcasmar commented Mar 29, 2020

The skeleton property (or lack thereof) has no bearing on the transforms of joints. Each joint's world transform — relative to the scene root — is affected by all of the joint's ancestors.

Client implementations should apply only the transform of the skeleton root node to the skinned mesh while ignoring the transform of the skinned mesh node. In the example below, the translation of node_0 and the scale of node_1 are applied while the translation of node_3 and rotation of node_4 are ignored.

As i understood the spec, the joints form a tree of joints, that tree of joints is transformed by the transform of the node from which it starts, in this case, the node the skeleton property is pointing to. Therefore, the skeleton property is not just a semantic hint, it has a real meaning and having or not having the property can make a scene comlpetely different.

With that in mind, if the skin doesn't have a skeleton property, which should be considered as the "node from which it starts"?

@scurest
Copy link

scurest commented Mar 30, 2020

The spec is, at the very least, exceedingly unclear. It's been like that forever. Is it ever going to have the actual formula to use added?

The formula is in the overview or the tutorials. The world space position of a vert is

mat4 skinMat =
    a_weight.x * u_jointMat[int(a_joint.x)] +
    a_weight.y * u_jointMat[int(a_joint.y)] +
    a_weight.z * u_jointMat[int(a_joint.z)] +
    a_weight.w * u_jointMat[int(a_joint.w)];
vec4 pos = u_modelMatrix * skinMat * vec4(a_position,1.0);

where

jointMatrix(j) =
  globalTransformOfNodeThatTheMeshIsAttachedTo^-1 *
  globalTransformOfJointNode(j) *
  inverseBindMatrixForJoint(j);

modelMatrix =
  globalTransformOfNodeThatTheMeshIsAttachedTo

The use of globalTransformOfNodeThatTheMeshIsAttachedTo is rather pointless because it's canceled by its inverse (supposing it actually has an inverse), but you can see that neither the skin.skeleton nor the node where the skinned mesh is attached plays any role in the formula.

@javagl
Copy link
Contributor

javagl commented Mar 30, 2020

An issue to extend the skinning section in the spec with a (non-normative) section that contains further details has been on my bucket list for quite a while now - see #1504 (comment) .

Before adding something like this to the spec, however, I'd like to create further examples that show the differences of implementing skinning properly and point out possible pitfalls in the implementations. The most recent attempt of creating a representative (but simple) example was at KhronosGroup/glTF-Sample-Models#243 ...

@jjcasmar
Copy link
Author

Thanks @scurest , now is a bit more clear. The spec is very confusing and it looks like the formula should be:

jointMatrix(j) =
  globalTransformOfNodeThatTheMeshIsAttachedTo^-1 *
  globalTransformOfSkeletonNode * // This is the skeleton property node
  globalTransformOfJointNode(j) *
  inverseBindMatrixForJoint(j);

That is why I was completely confused.

@scurest
Copy link

scurest commented Mar 30, 2020

@javagl Thanks a lot for writing the formulas up. You're the only reason I could figure out how it was supposed to work at all.

non-normative

Why non-normative? My whole point is the spec should normatively define how skinning works so everyone agrees how to do it.

@scurest
Copy link

scurest commented Mar 30, 2020

Before adding something like this to the spec, however, I'd like to create further examples that show the differences of implementing skinning properly and point out possible pitfalls in the implementations.

Another interesting test is to set scale to [0,0,0] on the node with the skinned mesh in SimpleSkin. If it's really being ignored there should be no effect. The formula with globalTransformOfNodeThatTheMeshIsAttachedTo^-1 doesn't make sense in this case since the inverse doesn't exist.

The Babylon viewer passes this test, but the Three.js viewer appears to fail it.

@javagl
Copy link
Contributor

javagl commented Mar 30, 2020

Regarding the "non-normative" part: Things that are going into the spec have to undergo a ratification process, so it's not entirely trivial to change the spec "just so". It might be part of the next glTF version, though. (And first having it as a non-normative section would increase the chance that it will undergo a critical review by people who are actually using it as the basis for their implementations).

Regarding the "scaling to 0,0,0" idea, I'd have to check whether this is actually allowed by the spec. Regardless of that: There are certain errors in implementations of skinning that can be observed occasionally. E.g. even for the (intentionally) most simple possible example of skinning, some viewers created wrong results (see KhronosGroup/glTF-Sample-Models#243 (comment) ). Again, there are some tasks on my TODO list regarding skinning (and I'm juggling with the priorities), but these tasks include, among others, to figure out

  • what exactly is the reason for the observed behavior
  • how it can be fixed
  • how this should be addressed via a clarifying "implementation note" in the spec

(some details are mentioned in the PR 242, but I haven't yet allocated (enough) time for that)

@jjcasmar
Copy link
Author

I think I have now completely understood this. The issue comes by comparison with Kuesa engine. In Kuesa, we add the skeleton as a component of an entity, and the skeleton is transformed by the global transform of the entity. Therefore, our joints are relative to the entity.

We can close this issue if you want

@jjcasmar
Copy link
Author

jjcasmar commented Apr 1, 2020

Two more doubts about skins:
Is it possible that a skin doesn't mark all the nodes of a hierarchy as joints? For example, we have the hierarchy N0 -> N1 -> N2 -> N3 but the skin has "joints": [0, 2, 3]

Is it possible that a skin mark as joints nodes that are directly the scene root? For example

"nodes": [
    {"children": [1]},
    {"children": [2]},
    {}
],
"scenes": [ {
    "nodes": [0]
    }
],
"skins": [
    {
        "joints": [0,1,2]
    }
]

or even more complicated case

"nodes": [
    {},
    {},
    {}
],
"scenes": [ {
    "nodes": [0,1,2]
    }
],
"skins": [
    {
        "joints": [0,1,2]
    }
]

@scurest
Copy link

scurest commented Apr 1, 2020

Is it possible that a skin doesn't mark all the nodes of a hierarchy as joints? For example, we have the hierarchy N0 -> N1 -> N2 -> N3 but the skin has "joints": [0, 2, 3]

Yes, and you do see "gaps" in the wild. This is one of the things #1747 tries to restrict more heavily.


Is it possible that a skin mark as joints nodes that are directly the scene root?

The spec says

Each skin's joints must have a common root, which may or may not be a joint node itself.

which appears to forbid that (although it wouldn't matter to how skinning works).

edit: donmccurdy's comment above suggests that the scene itself could function as a common root.

@jjcasmar
Copy link
Author

jjcasmar commented Apr 1, 2020

Thanks @scurest
I think Ive been able to correctly code Kuesa importer to handle all cases, even several joint nodes at the scene itself.
Do you know of any set of assets to test skins?

@scurest
Copy link

scurest commented Apr 1, 2020

For testing the Blender importer, I use the files from all the issues listed at the top of KhronosGroup/glTF-Blender-IO#857 since they all previously presented some kind of difficulty when importing.

@lexaknyazev
Copy link
Member

It looks like all of the OP questions have been resolved. The updated AsciiDoc version of the spec includes the clarified skinning section.

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

5 participants