mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.01k stars 35.33k forks source link

glTF Loader animation bug #10754

Closed leerichard42 closed 6 years ago

leerichard42 commented 7 years ago
Description of the problem

When loading the animated handsWithPlane.gltf file found here: https://www.dropbox.com/sh/dvprkyj2ly782jn/AABLjwQLiOSWfr2VDdtlzjYCa?dl=0

The plane, which should not be animated, seems to be taking animation values from the hands, even though the plane is not attached to any skeleton. tiltedplane

There are two additional test models in the folder, which are isolated versions of the animated hand and non-animated plane.

Three.js version
Browser
OS
mrdoob commented 7 years ago

/cc @takahirox

takahirox commented 7 years ago

I've confirmed this problem. I found hand_L bone transformation affects plane. Maybe SkinnedMesh/Skeleton building issue? I'll take a look closer....

/cc @donmccurdy

takahirox commented 7 years ago

BTW does this animation work correctly on other libraries?

leerichard42 commented 7 years ago

Do you mean other than 3js? I couldn't find anywhere else to load and test glTF files, but I exported the animation as an FBX from Maya and it works when reimporting into Maya - I then converted it to glTF with assimp.

cx20 commented 7 years ago

According to acquaintance Maya user Maya -> FBX -> assimp -> glTF conversion path, I think that there is not much achievement. Please try Maya -> OpenMaya -> dae -> COLLADA2GLTF -> glTF I am sorry, I am not a Maya user, so I do not know how to use it.

leerichard42 commented 7 years ago

I was able to export my Maya file using OpenCOLLADA, but I get a segfault when trying to convert that .dae file using COLLADA2GLTF. I've added my .ma and .dae files to the dropbox link, if that might help someone who is more familiar with the pipeline.

mrdoob commented 7 years ago

/cc @pjcozzi

takahirox commented 7 years ago

This problem seems being caused by sharing a material between SkinnedMesh and Mesh. The material has skinning=true property for SkinnedMesh but this makes plane mesh (non SkinnedMesh) animation go wrong.

An easy workaround so far is separating materials in glTF file. I confirmed this problem disappeared by removing the following line from "mesh" (for SkinnedMesh) of "meshes" in glTF file.

"material": "lambert1",

I'm thinking how to solve this problem on Three.js...

mrdoob commented 7 years ago

Sounds like a bug in WebGLRenderer. It's there a working link I can use for testing?

takahirox commented 7 years ago

I'll make soon.

I speculate skinning related uniforms and attributes for SkinnedMesh are unexpectedly reused also for Mesh in WebGLRenderer. Or skinned related attributes are disabled tho they're defined in GLSL for Mesh?

takahirox commented 7 years ago

I made an example.

https://takahirox.github.io/gltf-animation-bug/index.html

See here for workarounds.

https://github.com/takahirox/takahirox.github.io/blob/master/gltf-animation-bug/index.html#L83-L130

The root issue seems how to share a material between Mesh and SkinnedMesh.

This's my speculation. It seems like this problem is caused by a shared material. Only one material is shared in a scene, its skinning is set true for SkinnedMesh, and the corresponding GLSL code has skinning related lines. But the plane is just Mesh (non SkinnedMesh) and it doesn't have bindMatrix, bindMatrixInverse, skeleton, skeleton.boneMatrices, geometry.attributes.skinIndex, geometry.attributes,skinWeight.

The corresponding attributes and uniforms to these properties are disabled and aren't updated respectively when rendering the plane.

https://github.com/mrdoob/three.js/blob/0aaf85c5fd93f72a5022f46ab8632b6193069981/src/renderers/webgl/WebGLState.js#L449-L462

https://github.com/mrdoob/three.js/blob/8afa08db625c5979bbc77c56ff86e5e525681564/src/renderers/WebGLRenderer.js#L1846-L1869

Then the attribute and uniform values used to previously render another object seem being reused for rendering the plane. So the plane unexpectedly does skinning animation.

To solve this, I have three ideas in my mind.

  1. Separate materials for Mesh and SkinnedMesh in GLTFLoader. Easy but it won't strictly follow what glTF file specifies.
  2. Upload dummy attributes and uniforms for skinning not to do unexpected skin animation if material.skinning is true and object is not SkinnedMesh. Similar approarch to https://github.com/takahirox/takahirox.github.io/blob/master/gltf-animation-bug/index.html#L110-L130
  3. Update GLSL code. Currently branching with #define USE_SKINNING for skinning. If we replace it with uniform flag, we can dynamically turn on/off skinning depending on object.
takahirox commented 7 years ago

BTW, mesh_0 ~ mesh_10, mesh_12 ~ mesh_15 in this glTF file look weird. They're used by nodes which don't have skin/skeleton but have JOINT/WEIGHT (skinIndex/skinWeight) attributes. I'm wondering if it's correctly made/converted.

(This isn't root issue. Even if we fix this, the reported animation bug probably remains.)

leerichard42 commented 7 years ago

Good catch, I can file an assimp issue. Just to clarify, are the other hands most likely being animated because they're reusing the skinning from the first hand then?

takahirox commented 7 years ago

Yep, I think so.

donmccurdy commented 7 years ago

@takahirox Thanks for wading through all of this! 👏

  1. Separate materials for Mesh and SkinnedMesh in GLTFLoader. Easy but it won't strictly follow what glTF file specifies.

If I understand correctly, this option won't help when there are two SkinnedMesh instances using the same material, but one has animation playing and the other does not?

takahirox commented 7 years ago

Probably it won't be problem.

This problem occurs if an object doesn't have skinning properties (bindMatrix, bindMatrixInverse, skeleton, and so on). The two SkinnedMesh instances should have them even if they don't play animation.

mrdoob commented 7 years ago

How about we remove material.skinning and inside WebGLProgram we check for object.isSkinnedMesh?

I'll give a it a try.

mrdoob commented 7 years ago

I think it should be fixed now?

takahirox commented 7 years ago

Not yet.

https://takahirox.github.io/gltf-animation-bug/index.html

(This example uses the latest dev three.js via rawgit)

takahirox commented 7 years ago

As I wrote the above, the problem is occurred by non-updating skinning attribute/uniform values.

https://github.com/mrdoob/three.js/issues/10754#issuecomment-279193969

The skinning GLSL code is kinda hardcoded. It's turned on by setting #define USE_SKINNING.

https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLProgram.js#L348

If it sets, the GLSL code expects that skinning attribute/uniform values are uploaded for rendered object. If we don't upload, animation goes weird because the attribute/uniform values of previously rendered object are re-used.

Even though we see isSkinnedMesh instead of material.skinning, this issue remains.

mrdoob commented 7 years ago

I'll keep trying! 😁

takahirox commented 7 years ago

Cool!

The key is how we can share skinning material (GLSL code with #define USE_SKINNING) between normal Mesh and SkinnedMesh.

The problem is, in current implementation, if #define USE_SKINNING is set the skinning attribute/uniform values must be uploaded because skinning code is hardcoded, we can't directly switch on/off.

For example, if we replace #define USE_SKINNING with uniform book use_skinning we could dynamically turn on/off, like this

from

#ifdef USE_SKINNING

#else

#endif

to

uniform bool use_skinning;  // object.isSkinnedMesh === true

if (use_skinning) {

} else {

}
takahirox commented 7 years ago

BTW, do you keep deprecating material.skinning even it didn't solve this issue? If so, I need to remove material.skinning from some modules (mmd, outline, deferred...)

mrdoob commented 7 years ago

BTW, do you keep deprecating material.skinning even it didn't solve this issue?

Yes, I can't see any reason for keeping it.

mrdoob commented 7 years ago

Interestingly, there is no shader crash when using http://threejs.org/build/three.js...

mrdoob commented 7 years ago

Okay, so material.skinning did indeed fix the issue, but there is another issue going on... GLTFLoader doesn't set Skeleton.useVertexTexture to true and we're running out of uniforms.

WebGLRenderer: too many bones - 336, this GPU supports just 251 (try OpenGL instead of ANGLE)

I've hacked this code in the test:

loader.load( './handsWithPlane.gltf', function ( gltf ) {

    var object = gltf.scene !== undefined ? gltf.scene : gltf.scenes[ 0 ];

    object.traverse( function ( child ) {

        if ( child.isSkinnedMesh ) {

            var skeleton = child.skeleton;

            var size = Math.sqrt( skeleton.bones.length * 4 );
            size = THREE.Math.nextPowerOfTwo( Math.ceil( size ) );
            size = Math.max( size, 4 );

            skeleton.boneTextureWidth = size;
            skeleton.boneTextureHeight = size;

            skeleton.boneMatrices = new Float32Array( skeleton.boneTextureWidth * skeleton.boneTextureHeight * 4 ); // 4 floats per RGBA pixel
            skeleton.boneTexture = new THREE.DataTexture( skeleton.boneMatrices, skeleton.boneTextureWidth, skeleton.boneTextureHeight, THREE.RGBAFormat, THREE.FloatType );

            skeleton.useVertexTexture = true;

        }

    } );

    var animations = gltf.animations;
    ...

And that gives me this:

screen shot 2017-02-15 at 08 41 35

Note that this is what I get with the builds pre-material.skinning removal:

screen shot 2017-02-15 at 08 43 04

mrdoob commented 7 years ago

The solution for now will be to move the useVertexTexture logic to the WebGLRenderer.

takahirox commented 7 years ago

Okay, so material.skinning did indeed fix the issue

You meant, on your environment the issue was solved by removing material.skinning and the animation on this example works correctly (the plane doesn't play animation) now?

https://takahirox.github.io/gltf-animation-bug/index.html

But on my environment (Windows10 + Chrome 56.0.2924.87 (64-bit)) still animation goes wrong, the plane plays animation (rotation).

image image

This issue would be platform specific? That can be because the behavior if we don't upload attribute/uniform could be platform specific.

mrdoob commented 7 years ago

Ok, now WebGLRenderer uses float vertex textures by default for skinning. If the GPU doesn't pass gl.getParameter( gl.MAX_VERTEX_TEXTURE_IMAGE_UNITS ) > 0 && !! extensions.get( 'OES_texture_float' ) then it'll try using uniforms. If the total uniforms is more than the GPU can handle it will log an error without crashing.

mrdoob commented 7 years ago

I guess everything I've done so far is fixing issues that are only popping on my limited GPU (MacBook Air). Now I can finally see the plane rotation issue you guys are talking about.

mrdoob commented 7 years ago

Have we confirmed that the animation plays correctly with other libraries?

takahirox commented 7 years ago

I've tried some other libraries but they couldn't load. (LMK if anyone succeeded)

As I wrote, this glTF file has some problems. I think we should make a simpler glTF file which can reproduce this issue. I'm trying.

takahirox commented 7 years ago

This issue isn't glTF specific. Sharing material between Mesh and SkinnedMesh happens this issue.

I made a simpler example.

(Demo) https://takahirox.github.io/gltf-animation-bug/index2.html (Code) https://github.com/takahirox/takahirox.github.io/blob/master/gltf-animation-bug/index2.html

The left one is normal Mesh and the right one is SkinnedMesh. They share a material.

Rotating a Bone of the SkinnedMesh unexpectedly affects the normal Mesh rotation.

https://github.com/takahirox/takahirox.github.io/blob/master/gltf-animation-bug/index2.html#L138

image

If we separate material by replacing this line

plane = new THREE.Mesh( createPlaneGeometry(), material );

https://github.com/takahirox/takahirox.github.io/blob/master/gltf-animation-bug/index2.html#L111

with

plane = new THREE.Mesh( createPlaneGeometry(), material.clone() );

then we get an expected result, non-rotation normal Mesh.

Refer to my previous posts why sharing material causes this issue.

takahirox commented 7 years ago

I just noticed that webgl_animation_skinning_morph example doesn't display 2nd model. It seems it has the same root issue.

mrdoob commented 7 years ago

I just noticed that webgl_animation_skinning_morph example doesn't display 2nd model. It seems it has the same root issue.

Yep. Noticed that too... Reverting!

mrdoob commented 7 years ago

@leerichard42 any chance you could export to GLTF2? I had a quick look again and I'm getting this:

screen shot 2017-02-24 at 11 44 26

donmccurdy commented 7 years ago

@mrdoob the warning can be ignored there; GLTF2Loader does still accept multiple nodes per mesh. Not sure if that result is related to gltf 2.0 or not.

mrdoob commented 7 years ago

Basically the problem (as @takahirox has explained) is that GLTF*Loader is creating Meshes and SkinnedMeshes that share the same material. Unfortunatelly, WebGLRenderer doesn't support that at the moment.

I've also noticed that GLTF*Loader also produce SkinnedMeshes with materials that do not have the material.skinning set to true.

I hope to clean this up eventually, but it's a hard problem that needs more clean up first.

bhaux commented 7 years ago

@mrdoob we're hoping to get character animation through gltf but this is one of our roadblocks - what timeline are you looking at for this?

mrdoob commented 7 years ago

@bhaux I think we need more samples.

bhaux commented 7 years ago

I'm happy to produce some - what specifications/properties do you need for these samples?

mrdoob commented 7 years ago

We basically need samples that do not load correctly. So basically any roadblock you find.

enesser commented 6 years ago

Ran into this problem. Could not rig this box as follows --

With even just the top flap rigged, the entire box rotates: https://codepen.io/enesser/pen/ad750cee253066e8e192951bb07c362f

Rigging the side flaps results in distorted geometry.

Workaround appears to setting different materials on anything that is rigged with, say, a "skinned_" prefix and traversing through the model and setting any material without that prefix to material.skinning = false;. Works OK for something as simple as the top flap animation but more complex animations are very difficult to do with this problem.

Blender file here: https://s3-us-west-2.amazonaws.com/s.cdpn.io/168282/rigged-gift-simple.blend

donmccurdy commented 6 years ago

^Same file: https://gltf-viewer.donmccurdy.com/#model=https://s3-us-west-2.amazonaws.com/s.cdpn.io/168282/rigged-gift-simple-v2.gltf

... the structure is interesting:

screen shot 2017-10-16 at 8 38 18 am

Not sure why Blender writes two meshes here. Does there need to be a bone anchoring the box or something? Could be broken skin weights/indices or how we're loading them...

enesser commented 6 years ago

@donmccurdy Not sure, here's what the outliner looks like for convenience --

screen shot 2017-10-16 at 12 01 22 pm 1

I can stabilize the box by using code to turn off material's skinning (material.skinning) on all materials that aren't attached to bones. Of course I have to duplicate materials in order to successfully to do that. That fixes the back flap but getting the side flaps not to distort seems tricky.

Interested in learning all I can about this, PBR & animations working on the web is really exciting.

donmccurdy commented 6 years ago

@enesser what are the names of the nodes in the three.js scene that you are disabling skinning on? I'm trying on the Cube_0 and Cube_1 meshes, with https://gltf-viewer.donmccurdy.com/#model=https://s3-us-west-2.amazonaws.com/s.cdpn.io/168282/rigged-gift-simple-v2.gltf, and not seeing the right results. Were there any other changes?

donmccurdy commented 6 years ago

@enesser I've found the issue. There seems to be an expectation somewhere that the skin weights are normalized, or at least that there be a bone assigned to each vertex. In the .blend model you include, the vertices around the box's base are not assigned to any bones. That's not necessarily invalid (clearly it works in Blender) but it's causing issues in three.js for sure...

Anyway it's pretty easily fixed by adding a bone for the base, and assigning the remaining vertices.

Result: rigged-simple-edit.zip

91ace8b8-eed0-485e-9772-cccebf52716a-50400-0001d3ad823cdf37

I believe we've fixed the original bug (via #12791), so I'll close this issue.