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

Multiple duplicated skins are exported for each mesh child of the armature #566

Closed
dpalacio opened this issue Jun 25, 2019 · 20 comments
Closed
Assignees
Labels
exporter This involves or affects the export process Skinning_&_Rigging

Comments

@dpalacio
Copy link

Description
When an armature in Blender has multiple children meshes, each mesh gets its own skin copy in the glTF export. The result is multiple skeletons, one for each child mesh, in the exported file.

To Reproduce
Steps to reproduce the behavior:

  1. Open the attached .blend file and check the hierarchy
  2. Export scene to .gltf text
  3. Verify that multiple skeletons have been created

Expected behavior
A single skin/skeleton should be made for each Blender armature

Test case
I have attached a blend file to help reproduce the behavior. I'm no 3D modeler so please excuse the content.
blender_export_skin_test.zip

Version

  • OS: Linux
  • Blender Version: blender-2.80-c0c1b4542f39-linux-glibc224-x86_64

Additional context
This behavior has been referenced also in issue #331, which I have also have seen happen.

@marstaik
Copy link

marstaik commented Aug 5, 2019

I recently ran into this behavior on my own, without the test case, while trying to import to Godot.

This is a very annoying problem to deal with, every mesh has a skeleton copy.

@julienduroure
Copy link
Collaborator

@donmccurdy @emackey I have a question about inverseBindMatrices: If multiple mesh objects are linked to an unique armature, I don't know how to create only 1 skin object in glTF, because inverseBindMatrices are differents ... Any thoughts / test cases / info about that ?

@emackey
Copy link
Member

emackey commented Aug 9, 2019

Sorry, I don't know about skins/armature. @donmccurdy?

@donmccurdy
Copy link
Contributor

The child meshes are not just rigidly transformed by the parent bones, I assume, but their vertices are weighted and transformed? In the rigid case, like parts of the "Robot" example, the child meshes do not need skins at all.

Assuming the more complex case where each child mesh's vertices are transformed, I believe one option would be to create a new skin with unique inverseBindMatrices for each mesh, while reusing the same joints (or a subset of the joints) for each skin. Then the skeleton hierarchy is not duplicated, and each mesh has its own matrix.

This test case covers the situation above: https://bghgary.github.io/glTF-Assets-Viewer/?manifest=https://raw.githubusercontent.com/KhronosGroup/glTF-Asset-Generator/v0.6.0/Output/Manifest.json&folder=2&model=8

/cc KhronosGroup/glTF#1454

Is that approach sufficient? We've discussed other alternatives for sharing skins in the future, but this would require an extension and isn't in progress right now: KhronosGroup/glTF#1285

@julienduroure
Copy link
Collaborator

Currently the cache of gather_skin function has 2 parameters : blender_object (armature) and mesh_object (the skinned object). That's why 2 skins are generated, because the mesh_object is different. Mesh object is used to generate/calculate the inversedBindMatrix. So my main issue is that I use a unique IBM for multiple meshes only if mesh objects have same transformation...
@donmccurdy did I miss something? Do you have any link to explain exactly what are IBM?

@donmccurdy
Copy link
Contributor

donmccurdy commented Aug 13, 2019

That's why 2 skins are generated, because the mesh_object is different...

As long as the 2 skins don't result in duplicate joints, I think that's not too bad. If the meshes can use the same IBM, they might as well share a skin of course, but not doing so is not as bad as duplicating the joints.

Does this help, as an IBM explanation? KhronosGroup/glTF#1504 ... afraid i'm not the best person to help with that. 😅

/cc @lexaknyazev just confirming the IBM transforms vertices into the joint's coordinate space, I think sharing joints in several skins is consistent with the spec?

@marstaik
Copy link

marstaik commented Aug 20, 2019

To be honest this is what I dislike about the gltf spec. Joints can be used for multiple skins, but the joints mentioned in those skins could belong to a subset of the "skeleton".

I put skeleton in quotes, because there is no way to know what actually establishes a full armature in the current spec.

You could have an armature, with two arms, and two meshes each bound to one arm. The exporter could export skin bindings separately for each arm, which only include the joints of the arms, and not the entire armature.

This results in two subsets of a skeleton in the two skin definitions, with no easy way to derive what the "entire" skeleton should be. You have to result to some combination of disjoint sets to establish a parent relationship, using either the "optionally" defined "skeleton" parameter, or just join all the sets at the root.

Not to mention I haven't seen anything in the spec that prevents a mesh from also being a joint. In this case, any game engine will have to do some manual "ghost" bone creation to get the actual expected behavior. No game engine off the top of my head allows using a mesh object as a target for animation.

Then, there's another issue with the multiple skins and animation channels, and having to remap the channel targets to your "derived" skeleton to get animation play back to work correctly.

The extra exported skins and lack of proper skeleton definitions make all of this infuriating to work with due to the additional complexity I have to build in order to handle these weird situations, not to mention just the extra parsing time required to go through all the duplicated inverse bind matrices and skin definitions.

@reduz
Copy link

reduz commented Aug 20, 2019

Actually, I don't blame the GLTF specification for this, I think the Blender exporter has misunderstood it.

If you have a skeleton and different meshes, the spec is very clear that all the mesh transform (the transform in the actual nodes) values must be ignored. This means, that you must transform the mesh vertices, normals, tangents, etc. and make them skeleton-local. Always.

This way, multiple meshes will work with a single skeleton. I don' t think there is any way around this, and the exporter does not seem to be doing the right thing.

This is the typical behavior people used to have when writing Collada exporters, trying to outwit the apparent limitations of the format or take the shortest path instead of the right one, and then making life hell for those trying to import the files. Please don't do it, stay close to the intent of the spec as much as possible.

@reduz
Copy link

reduz commented Aug 20, 2019

Please read here:

https://github.com/KhronosGroup/glTF/tree/master/specification/2.0#skins

Implementation Note: 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.

The intention of the spec is that if your mesh has a transform different to the skeleton transform, you apply it to make it skeleton local before exporting it. This makes it much easier for importers. You are trying to work this limitation around by moving the skeleton to where the mesh is, and this goes against the intention of the spec, and forces you to do a pretty bad hack to support multiple skinned meshes with one skeleton.

@marstaik
Copy link

Reading through it again, that makes more sense.

But it also seems unreasonable to expect implementers to ascertain the actual implications of that line. As that note reads, it means that the reader should ignore any transforms on the meshes skinned to the skin, even if they are defined.

It mentions nothing about requiring the exporter to localize/apply the meshes transformations during export to that of the skeleton. But now, after reading that note, we can see that is what they intend.

This seems like a but of ambiguity in the specification that should probably be brought up in the gltf spec...

@reduz
Copy link

reduz commented Aug 21, 2019

@marstaik Yeah, you are right, I think this should be more explicit or, at least, a recommendation in the specification.

@emackey
Copy link
Member

emackey commented Aug 21, 2019

Great discussion. @marstaik Would you be willing to open a PR with that note on https://github.com/KhronosGroup/glTF ?

@marstaik
Copy link

I would love to, but I am currently out of the country on vacation. ETA of return is ~1.5 weeks and all I have is my phone :)

Unless you would like to wait for me to return, I'd be willing to review the PR if someone else has the time to make it.

I do feel that this is quite an important change to get in, it would correct a lot of exporter issues.

@donmccurdy
Copy link
Contributor

It has been difficult to determine all of the use cases that should (or should not) be handled by skinning, and how to implement them consistently in runtime and DCC tools. See KhronosGroup/glTF#1403 for some examples.

That said, I do think tightening glTF's requirements — or just adding solid best practices — would be very welcome additions to the spec and appendices. 👍

I realize this is a big ask, but would someone have time to attempt a PR fixing the exporter per the suggestion above? It has been a very bumpy road to get the addon's skinning export as stable as it is today (thank you to everyone who contributed!) and so we should plan to spend some time regression testing any major changes now.

@julienduroure
Copy link
Collaborator

I started working on it

@marstaik
Copy link

I made a post about potential sticter skinning requirements to make importing less complicated. I would appreciate some feedback.

@julienduroure
Copy link
Collaborator

julienduroure commented Aug 23, 2019

Some notes about the corrective development :
I already be able to export correclty 2 meshes (with different transforms) deformed by only one armature (at a third transform) --> export with only 1 skin.
Still have to test lots of configuration (parented / non parented / etc...), and still have to fix normals & tangents, but this is promising.
I hope to be able to provide a complete PR during next week.

@julienduroure
Copy link
Collaborator

Not sure to export it correctly yet, but with glb file in attachment, I get some different results on few viewers: (generated with branch fix_IBM)

Original file: (skin_posed)
image

Results in different viewers:

skin

In order to understand what happen, I made a case identical to previous, except that pose is the same as rest pose. (Skin_not_posed): Everything seems to be OK in treejs, babylonjs, godot. Only Blender importer is wrong.

I tested with very simple case (skin_simple_not_posed):
Everything seems to be OK everywhere

And with very simple case, but with a pose != rest pose:
Seems that Godot do not take into account the current pose on the glb file when it's different to rest pose

image
image

To conclude:
We have to fix some things in importer, and I will contact @reduz in order to know if my guess is true about godot importer

All files:
test_skin.zip

@julienduroure
Copy link
Collaborator

Doing the same exercice, but with a file with animation.

treejs, babylonjs and blender importer are displaying same result, that is the animation from original file.

image

Godot 3.1.1 is not displaying the same result:

image

See with @reduz --> I am going to open an issue in godot repository to understand where is the issue.

Files :
animated.zip

@julienduroure julienduroure self-assigned this Sep 4, 2019
@julienduroure
Copy link
Collaborator

Should be fixed now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter This involves or affects the export process Skinning_&_Rigging
Projects
None yet
Development

No branches or pull requests

6 participants