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

glTF Loader animation bug #10754

Closed
3 of 12 tasks
leerichard42 opened this issue Feb 7, 2017 · 47 comments
Closed
3 of 12 tasks

glTF Loader animation bug #10754

leerichard42 opened this issue Feb 7, 2017 · 47 comments

Comments

@leerichard42
Copy link

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
  • Dev
  • r84
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • Linux
  • Android
  • IOS
@mrdoob
Copy link
Owner

mrdoob commented Feb 7, 2017

/cc @takahirox

@takahirox
Copy link
Collaborator

takahirox commented Feb 8, 2017

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
Copy link
Collaborator

BTW does this animation work correctly on other libraries?

@leerichard42
Copy link
Author

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
Copy link
Contributor

cx20 commented Feb 8, 2017

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
Copy link
Author

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
Copy link
Owner

mrdoob commented Feb 9, 2017

/cc @pjcozzi

@takahirox
Copy link
Collaborator

takahirox commented Feb 10, 2017

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
Copy link
Owner

mrdoob commented Feb 10, 2017

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

@takahirox
Copy link
Collaborator

takahirox commented Feb 10, 2017

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
Copy link
Collaborator

takahirox commented Feb 12, 2017

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.

function disableUnusedAttributes() {
for ( var i = 0, l = enabledAttributes.length; i !== l; ++ i ) {
if ( enabledAttributes[ i ] !== newAttributes[ i ] ) {
gl.disableVertexAttribArray( i );
enabledAttributes[ i ] = 0;
}
}
}

if ( material.skinning ) {
p_uniforms.setOptional( _gl, object, 'bindMatrix' );
p_uniforms.setOptional( _gl, object, 'bindMatrixInverse' );
var skeleton = object.skeleton;
if ( skeleton ) {
if ( capabilities.floatVertexTextures && skeleton.useVertexTexture ) {
p_uniforms.set( _gl, skeleton, 'boneTexture' );
p_uniforms.set( _gl, skeleton, 'boneTextureWidth' );
p_uniforms.set( _gl, skeleton, 'boneTextureHeight' );
} else {
p_uniforms.setOptional( _gl, skeleton, 'boneMatrices' );
}
}
}

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
Copy link
Collaborator

takahirox commented Feb 12, 2017

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
Copy link
Author

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
Copy link
Collaborator

Yep, I think so.

@donmccurdy
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Owner

mrdoob commented Feb 14, 2017

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

I'll give a it a try.

@mrdoob
Copy link
Owner

mrdoob commented Feb 14, 2017

I think it should be fixed now?

@takahirox
Copy link
Collaborator

Not yet.

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

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

@takahirox
Copy link
Collaborator

takahirox commented Feb 14, 2017

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

#10754 (comment)

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
Copy link
Owner

mrdoob commented Feb 14, 2017

I'll keep trying! 😁

@takahirox
Copy link
Collaborator

takahirox commented Feb 14, 2017

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
Copy link
Collaborator

takahirox commented Feb 15, 2017

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
Copy link
Owner

mrdoob commented Feb 15, 2017

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
Copy link
Owner

mrdoob commented Feb 15, 2017

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

@mrdoob
Copy link
Owner

mrdoob commented Feb 15, 2017

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
Copy link
Owner

mrdoob commented Feb 15, 2017

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
Copy link
Owner

mrdoob commented Feb 15, 2017

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
Copy link
Owner

mrdoob commented Feb 15, 2017

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

@takahirox
Copy link
Collaborator

takahirox commented Feb 16, 2017

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
Copy link
Collaborator

takahirox commented Feb 16, 2017

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 added a commit to takahirox/three.js that referenced this issue Feb 20, 2017
@takahirox takahirox mentioned this issue Feb 20, 2017
@takahirox
Copy link
Collaborator

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

@mrdoob
Copy link
Owner

mrdoob commented Feb 23, 2017

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 added a commit that referenced this issue Feb 23, 2017
mrdoob added a commit that referenced this issue Feb 23, 2017
mrdoob added a commit that referenced this issue Feb 23, 2017
@mrdoob
Copy link
Owner

mrdoob commented Feb 24, 2017

@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
Copy link
Collaborator

@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
Copy link
Owner

mrdoob commented Feb 24, 2017

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
Copy link

bhaux commented Mar 9, 2017

@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
Copy link
Owner

mrdoob commented Mar 9, 2017

@bhaux I think we need more samples.

@bhaux
Copy link

bhaux commented Mar 9, 2017

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

@mrdoob
Copy link
Owner

mrdoob commented Mar 9, 2017

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

@enesser
Copy link

enesser commented Oct 16, 2017

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
Copy link
Collaborator

donmccurdy commented Oct 16, 2017

^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
Copy link

enesser commented Oct 16, 2017

@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
Copy link
Collaborator

@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
Copy link
Collaborator

donmccurdy commented Jan 10, 2018

@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.

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

7 participants