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

Crashes when rendering gltf model with skeleton #202

Open
afonso360 opened this issue May 1, 2018 · 12 comments
Open

Crashes when rendering gltf model with skeleton #202

afonso360 opened this issue May 1, 2018 · 12 comments

Comments

@afonso360
Copy link

afonso360 commented May 1, 2018

Hi,

Three-rs crashes when I try to render a gltf model with a skeleton. The crash happens at src/render/mod.rs:719. This happens because we get a bone before a skeleton.

I've temporaraly fixed it by iterating twice, once for the skeletons and again for the bones.

Thanks.

@kvark
Copy link
Collaborator

kvark commented May 1, 2018

@afonso360 thank you for submitting the issue! Could you share the model for us to repro?

@afonso360
Copy link
Author

Sure,

You should need these two files:
soldier.gltf
buffer_soldier.bin

I exported them from blender using the latest version of blendergltf

@kvark
Copy link
Collaborator

kvark commented May 2, 2018

Thanks for the files! I'm able to repro now.
@alteous do you know if this is expected from gltf? we might need to stash those bones somewhere then, until the skeleton comes.

@kvark
Copy link
Collaborator

kvark commented May 2, 2018

I wonder if this will be affected (positively?) by #203. cc @randomPoison

@randomPoison
Copy link
Contributor

I'll certainly test it out and make sure it doesn't cause a problem with the new implementation. @afonso360 what is the licensing on the glTF files you provided? Could they be added to the three repo (or rather the example-data repo to be used for examples and regression testing?

@afonso360
Copy link
Author

afonso360 commented May 2, 2018

Unfortunately, I think that might be a more complicated issue. I got the model from here, changed a few things, did the animation and exported. I'm not sure that the original license covers this use case.

However, i've quickly created another example animation which shows the same issues. I've uploaded it here:
2_twisty_cube.gltf
buffer_2_twisty_cube.bin

This one is completely mine, so feel free to use it. If you guys want me to explicitly license it, let me know which one would be preferable.

@randomPoison
Copy link
Contributor

I can confirm that the changes in #203 avoid this issue when loading glTF files, but the underlying bug is easy to reproduce by making the first child of a Group a Bone and then the next child the Skeleton it belongs to (note that you do this by adding the Skeleton first then the Bone, because internally the children are listed in reverse of the order in which they are added).

Internally, the issue has to do with how the renderer handles skeletons and bones. When the renderer walks the node hierarchy, it pushes any skeletons it finds onto a stack. When it finds a bone, it assumes the bone belongs to the skeleton at the top of the stack (i.e. the skeleton it found most recently). This assumption doesn't match how the three-rs API allows users to put Skeleton objects anywhere in the scene's hierarchy.

Personally, I think the best solution is to somehow limit bones/skeletons such that the bones in a skeleton must be "children" of that skeleton. In practice, any other setup doesn't make sense, and allowing skeletons/bones to be put anywhere only allows users to inadvertently create nonsensical hierarchies. If that's not desirable, I think it would be possible to rework the renderer to fully adhere to the public API's semantics.

@kvark
Copy link
Collaborator

kvark commented May 3, 2018

This assumption doesn't match how the three-rs API allows users to put Skeleton objects anywhere in the scene's hierarchy.

Three supports having the skeleton separately from the meshes it affects. But not the bones - I don't see the point of having them outside of the skeleton. So we are in agreement here, and just need to adjust the code to enforce it (earlier and more politely - a panic is also sorta enforcement).

@randomPoison
Copy link
Contributor

I dunno if this is the best place to discuss it, but my suggestion would be to limit skeleton/bone hierarchies to only be composed of Bone objects, with a single Skeleton at the root. The user would be able to add bones (and only bones) as children of a Skeleton, and they could add bones (and only bones) as children of other bones. There are some fiddly details that would have to be worked out, but this approach would enforce the relationship between bones and skeletons.

@randomPoison
Copy link
Contributor

A problem with the solution I described above: How would we handle the case where we want to attach an object to a joint in a skeleton? For example, say we have a skinned character model and we want to have it hold an object in its hand. Under the current model, we could easily add the model to the Group that represents the character's hand. But if skeleton hierarchies can only be made up of bones, how do we have another object follow a joint in the skeleton?

I could imagine adding some kind Follows constraint that allows you to specify that an object should always be moved to the world position of another object, which would effectively allow an object to be attached to another without having an actual parent/child relationship. I don't know that's actually a better solution, though.

@kvark
Copy link
Collaborator

kvark commented May 16, 2018

@randomPoison right, we can't have bones only in there.

I could imagine adding some kind Follows constraint that allows you to specify that an object should always be moved to the world position of another object, which would effectively allow an object to be attached to another without having an actual parent/child relationship. I don't know that's actually a better solution, though.

I've been thinking around this idea for a long time, albeit not so deeply. The conclusion I came up with is that parent/child relationships are bad in general and should be gone. Instead, all objects should only have the world space transformation tracked. And there should be a set of physical constraints between them. Some of the constraints would then simulate the old-style parent-child relationships.

However, this is too futuristic and experimental. Three-rs is a fine place to try out things internally, but it's not meant to expose unconventional APIs to the users. So we are pretty much settled on having the traditional parent-child hierarchies here, for convenience reasons.

@donmccurdy
Copy link

donmccurdy commented Jul 25, 2018

Note that glTF does not currently require a particular skeleton structure — the bones do not have to form a tree, and (if they are a tree) there may also be other nodes in that tree. The skin.skeleton property is optional, and not used at all in some glTF skinning implementations including three.js. There has been discussion of removing it from the spec entirely.

See KhronosGroup/glTF#1403 for details and links to related issues, and feel free to comment there. 🙂

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

No branches or pull requests

4 participants