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

Joints that are defined as joints becomes a transform #93

Closed
fire opened this issue Oct 1, 2019 · 29 comments
Closed

Joints that are defined as joints becomes a transform #93

fire opened this issue Oct 1, 2019 · 29 comments

Comments

@fire
Copy link
Contributor

fire commented Oct 1, 2019

If a joint is not in a mesh skin, it becomes a transform.

A peer fixed this by making a new skin with a existing joint and joints without a mesh attached, but it caused validation warnings about empty objects.

Not sure how to deal with this.

@fire fire changed the title Joints that are defined as joints disappears Joints that are defined as joints becomes a transform Oct 1, 2019
@ziriax
Copy link
Contributor

ziriax commented Oct 1, 2019

Yes that is by design, a joint that is not attached to a skin is just a transform, it would be inefficient to perform skin deformation for such a dummy joint no?

Or did I miss some use case? It would be easy to add a flag for this I guess, since it is an optimisation.

@fire
Copy link
Contributor Author

fire commented Oct 1, 2019

Given that artist defined the node as a joint, we want the skeleton to match when some scenes use the common skeleton versus other scenes that don't.

Currently some animated scenes lose bones in the export, so the animations don't match exactly with a common skeleton.

@ziriax
Copy link
Contributor

ziriax commented Oct 1, 2019

But if the joint is not attached to a skin, how can I know what GLTF skin to assign it to? This might be tricky, I need to investigate this.

Or do you mean that if a joint all has zero weights, the corresponding GLTF node will have no skin property?

Or maybe I am misunderstanding your request?

@fire
Copy link
Contributor Author

fire commented Oct 1, 2019

Use case there is a standard asset.

For example:
image

Other people create a new assets.

In this example we want to give the tiger a rigged cape using the common skeleton, but perhaps in the scene the cape doesn't reference certain bones and thus your optimization makes them transforms and the bone is lost.

This means when we combine the animated cape with the standard asset the skeletons are different and animations are different.

A peer fixed this by making a new skin with a existing joint and joints without a mesh attached, but it caused validation warnings about empty objects.

Not sure how to fix this.

Or do you mean that if a joint all has zero weights, the corresponding GLTF node will have no skin property?

This is the other issue. #91

@ziriax
Copy link
Contributor

ziriax commented Oct 1, 2019

Why would the skeletons be different? The GLTF nodes are still all present in the scene no?

Could you maybe export two scenes so I can really see the problem? I would like to understand this better, since we will certainly need a solution for this when we start building a character customizer.

GLTF is a bit ambiguous about this. In the spec, it says that a node has a skin property. But it seems valid that the same node is used by multiple skins, as in this file

So really, a node shouldn't have a skin property, the skin object itself already specifies the nodes...

Discussions about this:

KhronosGroup/glTF#1403

KhronosGroup/glTF#1454

@marstaik
Copy link

marstaik commented Oct 1, 2019

If a skin does not define a node, you will never know that it is a joint.

Take for example any skeleton that uses a root bone at (0,0,0). It's a critical part of the skeleton for game animators. But the fact is, no mesh actually binds to that node. Thus, no skin ever defines that node. Following that, any game engine using a skeleton based system cannot figure out that the root "node" is actually a joint that's a part of the skeleton on import.

@ziriax
Copy link
Contributor

ziriax commented Oct 2, 2019

Yes, I guess that is why the glTF spec defined the skeleton property, but after many discussions, I think the consensus was that this skeleton property is redundant.

IMHO the only difference between a node and a joint, is that one must pass information from a joint to the skin deformer (typically a vertex or compute shader).

Since the skin object in GLTF defines those nodes, an engine should have enough information. No other information should be needed, e.g. the skin property of a node is redundant.

I think what @fire is trying to say, is that it is annoying for an engine to deal with two skins that use a different set of joints, the JOINT_ indices in the meshes will be different, and the buffer that is filled with joint matrix information cannot be reused. This is not good for performance, although a glTF compliant engine should be able to handle this.

So what @fire is proposing, is that all joints of a mesh are regarded as potential skin joints, so that a single buffer with all these joint matrices can be filled and shared, and all JOINT_ elements will have the same indices, in every glTF file that is exported from the same rig.

I now get it, and that should be easy to add I guess...

Another solution would be to modify my tool that merges multiple glTF files into a single one, and have that remap indices, but that will not work when downloading extra glTF files on the fly.

@fire good catch!

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

Hang on, my bad, I misinterpreted the spec here (although I did correctly implement it in Maya2glTF).

Well the spec isn't really clear on this, but it seems a skin property is only present on a mesh node, not on nodes that are joints. Makes sense, since one joint node can be used in multiple skins.

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

I'm making test models right now, and it seems although your code in PR #100 will "fix" your issue, it seems that in the current released version, when I export a scene where a bone has zero weights, the node is still considered as a skin node, so I don't understand what the issue actually is?

For example, in the attached test scene with 3 joints, joint2 has zero weights, but when exporting, the skins section is:

"skins": [
        {
            "name": "pCubeShape1",
            "inverseBindMatrices": 7,
            "joints": [
                1,
                4,
                0
            ]
        }
    ],

So the 3 nodes are skin nodes, even though joint2 has all zero weights.

So your PR is not going to change anything really? It will add zero weights to the WEIGHTS_n accessors for the joint indices in the JOINTS_n accessor, but why would you want to do this? The WEIGHTS_n and JOINTS_n accessors hold the weights and joint indices that are used per vertex. If a joint a zero weight, it doesn't matter if it present or not. Maya2glTF outputs a (0,0) joint/weight pair, and that is perfectly fine, the index of the joint doesn't matter if the weight is 0. It could certainly add (j,0) instead, but it would change the output of any shader.

So could you please explain in more detail what issue you are actually trying to solve? Do you have a practical example of an engine that doesn't handle a specific GLTF file?

I understood the example you gave with the cape, and I was assuming that Maya2glTF generated incorrect output for the joints property of the skin, but that doesn't seem to be the case.

Thanks for giving me more feedback!

ThreeJoints_Joint2_HasZeroWeights.zip

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

For example, this a dump of the skin related attribute values of the first 8 vertices in the test scene:

WEIGHTS_0:
0.99588483      0.00001686      0               0
0.99588483      0.00001686      0               0
0.00408163      0.00408163      0               0
0.00408163      0.00408163      0               0
0.99588483      0.00001686      0               0
0.99588483      0.00001686      0               0
0.99588483      0.00001686      0               0
0.99588483      0.00001686      0               0

JOINTS_0:
0               2               0               0
0               2               0               0
0               2               0               0
0               2               0               0
2               0               0               0
2               0               0               0
2               0               0               0
2               0               0               0

After your PR, this will become:

JOINTS_0:
0               2               1               0
0               2               1               0
0               2               1               0
0               2               1               0
2               0               1               0
2               0               1               0
2               0               1               0
2               0               1               0

If this is really needed, that is fine to me, but I'm not sure what this would change.

@fire
Copy link
Contributor Author

fire commented Oct 3, 2019

#100 is not the one I'm worried about.

It was just the easiest.

The important one is when the root joints aren't considered part of the skeleton because it's not in a skin.

This causes the game engine code to not know it's a skeleton bone, so it'll not put in the skeleton bone list and animations will refer to the node location rather than the skeleton bone location.

[Edited]

I'll try to find an example, but I had a hard time.

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

I know it was the easiest, but is it needed? What would it solve? It don't think anything needs to change really.

Yes, the other one, regarding the root, I agree, we need to keep all joints by default. That will be a bit harder to fix, I see what I can do here. The question is, if other nodes (like locators) are between joints in the hierarchy, do we promote these as joints? I guess not.

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

No here's an interesting question. If you export a rig without any mesh, but you select joints, then one also would want a skeleton to be exported no? GLTF will give a warning about such a file, since a skin will be exported that is not used, but I guess that's okay...

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

Mmm, a skin object must contain the inverse bind matrices of that skin. The formula of an inverse bind matrix is:

MMatrix inverseBindMatrix = meshWorldMatrix * inverseJointWorldMatrix;

So if we want to export a skin that can be applied to other meshes, or export a skin for a rig that has no skinned mesh at all, then the artist must follow a convention that every mesh that ever needs to be deformed by this skin must have the same world transform as the root joint. In your example, the cape and tiger must have the same world transform as the root joint, otherwise the skin cannot be used for these meshes.

So clearly this can't be the default exporter setting, this must become a special flag.

The idea would be that for every skeleton that you want to export, you select the root joint of that skeleton, and optionally select the meshes, and pass the special flag. This will then export a skin for each root joint, and use the corresponding skin for the selected meshes. If the selected meshes have a different world transform than the root joint, an error will be printed.

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

Well, this requires considerable refactoring, but it is worth it I guess, it clearly separates the skin from the mesh. Working on it, not obvious

@ziriax
Copy link
Contributor

ziriax commented Oct 3, 2019

Actually, my proposal above is overly complicated.

When the flag -jointHierarchySkeleton or -jhs is passed (and a checkbox must be added to the UI), the following should happen:

  • when a mesh is selected that has a skin, a MeshSkeleton instance will be created or re-used if the skeleton and the mesh have the same root transform and share a joint. If the root transforms are different, a warning will be printed, and a new skeleton is created (might become an error when another flag like -requireSharedSkeletons or so is passed?)

  • a skeleton instance is created for the full joint hierarchy, upto the root joint.

  • when a joint is selected, a skeleton will also be created or re-used if a skeleton exists that contains that joint.

Like that it is fully automatic, and I guess more artist friendly.

@marstaik
Copy link

marstaik commented Oct 4, 2019

@ziriax

Mmm, a skin object must contain the inverse bind matrices of that skin. The formula of an inverse bind matrix is:

That statement is false, you can have a skin without IBM's.

https://github.com/KhronosGroup/glTF/blob/master/specification/2.0/schema/skin.schema.json

The only required field is "joints".

@ziriax
Copy link
Contributor

ziriax commented Oct 4, 2019

Oh, I see. The documentation itself is not that clear about this, do you have a reference that explains what needs to be done in that case?

This issue seems to imply that the vertices of the meshes to be skinned should be premultiplied with the inverse bind matrices?

KhronosGroup/glTF#461

But how many engines can deal with that? I guess from a performance point of view this is actually better.

@marstaik
Copy link

marstaik commented Oct 4, 2019

I believe you are just supposed to compute the IBMs from the scene pose.

I remember reading this in some comment chain in glTF a while ago.

@marstaik
Copy link

marstaik commented Oct 7, 2019

@ziriax I'm working on a gltf extension to handle skeleton support better, and prevent loss of joints.

https://github.com/marstaik/glTF/blob/staik_skeletons/extensions/2.0/Vendor/STAIK_skeletons/README.md

I was wondering if you had any feedback or would like to try implementing this. It's supposed to be completely optional in that it shouldn't break existing importers.

@ziriax
Copy link
Contributor

ziriax commented Oct 7, 2019

I spent a lot of time getting this "right" in the exporter, mainly by trying the results in existing render engines...

I will need to dig into this again. For me the spec was rather clear and unambiguous (besides the skeleton property), but it seems many people have trouble with it.

I am not sure why a "skeleton" at all should exist, the skin uses a subset of the nodes in the scene, and will deform meshes using that skin using a well know formula (basically each vertex in the mesh must record its coordinates relative the joint world matrix when the skin was bound, and that is achieved at runtime indirectly through the inverse bind matrices using the original vertices, or directly by baking it into the vertices themselves, not requiring the IBMs, something I don't support yet)

Any summary of the problem would be appreciated, the discussions in the references articles are very long ;-)

@marstaik
Copy link

marstaik commented Oct 8, 2019

tldr; The current specification is lossy. As you saw with your optimizations, zero-skin weights and empty skin groups can be removed from the skin on export. However, doing so causes the information about which nodes are joints to be lost. There is a diagram on the linked README that shows this better than I can explain.

The purpose of the above extension is to make skeleton definitions strict, and force skin joints to map to a skeletons joints on export.

It is actually not as difficult as it seems to implement, the skeleton is basically one giant skin with all the possible joints and default bind matrices given if available, regardless of meshes using all the joints.

@ziriax
Copy link
Contributor

ziriax commented Oct 8, 2019

Thanks for your nice work!

I agree that many old school renderers (like Ogre3D) have a special skeleton system, but IMHO these are badly (or just old object-oriented) designed systems.

Personally I really like the GLTF spec, because it doesn't have joint objects. GLTF only has nodes (transforms). The skin object lists the nodes that need to be used. Nice, clean design.

Our own inhouse engine handles this nicely, and orthogonally from the rest of the system.

But I agree that if GLTF needs to be useful for other engines with a fixed skeleton system, then an extension is needed. However, extensions make it even harder of engine developers to support GLTF IMHO.

Anyway, I will look into this after my crazy deadline (October 25th).

@marstaik
Copy link

marstaik commented Oct 8, 2019

I think it's incorrect to call the skeleton system badly designed. It exists for a very important purpose, and that purpose is for high performance skeletal animation.

When game engines try to render 100+ skeletons in a scene, they need all the speed they can get. The purpose of the skeleton is to keep the joint array in a single location, so that the CPU can use the spatial locality to optimize. Also, unlike track based animation, many engines bake the transforms of each joint, for every frame, into another joint array. This is all done because game engines need performance, and to do so they give up memory.

Yes, I agree the skin based rendering is really easy to implement, but it really needs to have the game engine allow joints to be an abstraction. However the abstraction is a very large performance hit for games.

@ziriax
Copy link
Contributor

ziriax commented Oct 8, 2019

Yes that is very valid concern, but I guess it applies to all the nodes really, it would be best to store all of these into a single flat array.
Same for the animation frames, instead of having keyframes of following times next to each other in memory, keyframes of the same time should be close.

We really haven't done much work to make the exporter suitable for this kind of low level optimizations, and indeed glTF itself might not be the best format for this.

I will look into this after my deadline

@ziriax
Copy link
Contributor

ziriax commented Oct 10, 2019

@marstaik Took the time to read your diagram a bit more in detail, and yes, that certainly makes sense.

One question though, does your extension allow non-joint nodes between joints in the skeleton hierarchy?

@marstaik
Copy link

No it does not, and I specifically made it that way because the extension is meant to define skeleton systems vs. just taking nodes as joints.

For export reasons, maybe a simple check/warning to the user that a proper skeleton wont be able to be exported due to non-connected joints would be more informative than just attempting to do some fake-joint creation algorithm.

I wrote a fake-joint/root finder that created whole skeletons from sparse joints in Godot's importer, but it is critically dependent on what specific subset of nodes have been defined.
As an example, if I were to bring in the same scene from maya2gltf into Godot vs the exported gltf straight from blender, I would get different resulting skeletons.

@ziriax
Copy link
Contributor

ziriax commented Oct 11, 2019

Okay, because the exporter could promote such nodes to joints, albeit with zero weights...

@fire
Copy link
Contributor Author

fire commented Aug 11, 2020

Closing due to lack of approach for changing this.

@fire fire closed this as completed Aug 11, 2020
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

3 participants