SkeletonUtils: Add clone() helper for skinned meshes. #14494

donmccurdy · 2018-07-18T04:56:07Z

Adds a helper method, SkeletonUtils.clone( object ) that preserves SkinnedMesh/Bone relationships. Ideally this would "just work" with the usual object.clone(), but I can't see any clean way of doing that — bones are not necessarily children of the SkinnedMesh(es?) they're associated with, and trying to connect them requires logic that would need to live somewhere outside the recursive callback series. This helper method provides an easy way to work around the problem.

Based on https://gist.github.com/zellski/be4e9207ab8e70c4e89062d48ce345ba#file-gltf_utils-js-L19 by @zellski and https://gist.github.com/cdata/f2d7a6ccdec071839bc1954c32595e87 by @cdata.

Mugen87 · 2018-07-18T09:20:18Z

I think it's good to have such a function as a work around until we solve the real problem and have a consistent approach for skeleton animation in three.js.

takahirox · 2018-07-18T09:38:54Z

I haven't followed up the discussion yet, but why is cloning method for SkinnedMesh in AnimationUtils?

takahirox · 2018-07-18T09:41:42Z

src/animation/AnimationUtils.js

+
+		source.traverse( function ( node ) {
+
+			sourceNodes[ node.name ] = node;


Do you assume node has an unique name?

Unique within the subtree being cloned, yes.

IMO it may be expecting too much... As you know, name is optional in Three.js and two or more nodes have the same name. In many cases, most of non-bone nodes have an empty strings name.

If I were you, I'd write like the following by making the use of that source and clone have the same tree structure.

var clone = source.clone(); var sourceToClone = new Map(); function makeMapping( object1, object2 ) { sourceToClone( object1, object2 ); for ( var i = 0, il = object1.chilldren.length; i < il; i ++ ) { traverse( object1.children[ i ], object2.children[ i ] ); } } makeMapping( source, clone ); source.traverse( function ( node ) { if ( ! node.isSkinnedMesh ) return; var sourceMesh = node; var clonedMesh = sourceToClone.get( node ); var sourceBones = sourceMesh.skeleton.bones; clonedMesh.skeleton = sourceMesh.skeleton.clone(); clonedMesh.skeleton.bones = sourceBones.map( function ( bone ) { return sourceToClone.get( bone ); } ); clonedMesh.bind( clonedMesh.skeleton, sourceMesh.bindMatrix ); } ); return clone;

EDIT: Updated the code.

donmccurdy · 2018-07-18T13:54:52Z

I haven't followed up the discussion yet, but why is cloning method for SkinnedMesh in AnimationUtils?

I'd rather it just work via mesh.clone() of course, but bones may not be children of the mesh. Consider this subtree:

object
object
object
- bone
  - bone
    - bone
object
- skinned mesh

There's no guarantee the new bones will exist at the time the skinned mesh is recursively cloned. It seems like we need somewhere else to put that logic, and so a Utils class seems as good as any. This solution is another, but relies on cloning happening at the Scene level, and not all loaders return Scene objects.

takahirox · 2018-07-18T22:05:34Z

Sorry I wrote poorly. I think it's good to have a workaround in Utils for now. My question is why is it in "Animation"Utils tho cloning SkinnedMesh isn't animation special.

takahirox · 2018-07-18T22:31:49Z

src/animation/AnimationUtils.js

+
+			clonedMesh.skeleton.bones = sourceBones.map( function ( bone ) {
+
+				return clonedNodes[ bone.name ];


Similarly do you assume bones are in a subtree?

This assumes the bones are within the subtree of the object being cloned, but not necessarily descendants of the SkinnedMesh they're associated with.

I think we need to check if bones are in a subtree and halt with warn/error log if they aren't. Because it isn't guaranteed, for example users might call Utils.clone( object2 ) for the following graph. Sometimes even bones wouldn't be in the scene graph.

object1

bone1

bone2

object2

skinned mesh

donmccurdy · 2018-07-18T22:32:44Z

My question is why is it in "Animation"Utils tho cloning SkinnedMesh isn't animation special.

Because I can't think of a better place to put it, and skinning is at least somewhat related to animation. Suggestions welcome. 😉

EDIT: Maybe SceneUtils.cloneWithSkinning( ... )?

takahirox · 2018-07-19T02:41:16Z

Hm, SceneUtils or (Skinned)MeshUtils?

takahirox · 2018-07-19T03:27:27Z

examples/js/loaders/GLTFLoader.js


+			} else {
+
+				node.name = 'gltfNode' + ( nextNodeIndex++ );


Is this for naming an unique name as much as possible?

Right. 👍

takahirox · 2018-07-19T05:42:48Z

src/animation/AnimationUtils.js

+			var sourceBones = sourceMesh.skeleton.bones;
+
+			clonedMesh.skeleton = sourceMesh.skeleton.clone();
+			clonedMesh.bindMatrix = sourceMesh.bindMatrix;


We should copy the matrix, rather than we share?

donmccurdy · 2018-07-21T23:14:52Z

Updated to ensure cloning does not depend on names. I removed the part adding unique names to glTF nodes for now, but will probably try another PR for that in the future since animation tracks still require it.

takahirox · 2018-07-23T12:44:13Z

src/animation/AnimationUtils.js

+
+			} );
+
+			clonedMesh.bind( clonedMesh.skeleton, clonedMesh.bindMatrix );


Copying matrix to .bindMatrix is done in SkinnedMesh.bind().

https://github.com/mrdoob/three.js/blob/dev/src/objects/SkinnedMesh.js#L112

So I think you can replace this line with

clonedMesh.bind( clonedMesh.skeleton, sourceMesh.bindMatrix );

and remove the line above clonedMesh.bindMatrix.copy( sourceMesh.bindMatrix );.

You need to pass "source"Mesh.bindMatrix.

takahirox · 2018-07-25T06:44:07Z

src/animation/AnimationUtils.js

+		} );
+
+		return clone;
+


Can't we remove sourceLookup?

source.traverse( function ( node ) { if ( ! node.isSkinnedMesh ) return; var sourceMesh = node; var sourceBones = sourceMesh.skeleton.bones; var clonedMesh = cloneLookup.get( node ); clonedMesh.skeleton = sourceMesh.skeleton.clone(); clonedMesh.skeleton.bones = sourceBones.map( function ( bone ) { return cloneLookup.get( bone ); } ); clonedMesh.bind( clonedMesh.skeleton, sourceMesh.bindMatrix ); } ); return clone;

takahirox · 2018-07-26T04:45:16Z

Thanks for the update.

So, you still want this method in AnimationUtils? Or do you move it to SceneUtils or somewhere?

takahirox · 2018-07-26T04:47:15Z

src/animation/AnimationUtils.js

+
+			clonedMesh.skeleton.bones = sourceBones.map( function ( bone ) {
+
+				return cloneLookup.get( bone );


Do you wanna check the existence of bones in the subtree as I mentioned?

donmccurdy · 2018-07-27T00:23:49Z

I'm OK with whatever namespace for this, but I don't think I have a better idea than the current name... maybe cloneWithBones or cloneWithSkinning? It is all a less-preferable solution than fixing .clone() directly but I don't see any straightforward way to do that. /cc @mrdoob

takahirox · 2018-07-27T09:20:13Z

src/animation/AnimationUtils.js

+
+					throw new Error( 'THREE.AnimationUtils: Required bones are not descendants of the given object.' );
+
+				}


nit: Using .has() may look more natural?

if ( ! cloneLookup.has( sourceBone ) ) { throw new Error( 'THREE.AnimationUtils: Required bones are not descendants of the given object.' ); } var clonedBone = cloneLookup.get( sourceBone );

takahirox · 2018-07-28T03:40:22Z

Voting for SceneUtils.cloneWithSkinning.

donmccurdy · 2018-07-29T04:16:00Z

If #14574 goes in, SkeletonUtils seems like a good place to put this. 🤔

takahirox · 2018-07-29T04:32:59Z

Agreed.

mrdoob · 2018-08-29T07:30:44Z

If #14574 goes in, SkeletonUtils seems like a good place to put this. 🤔

Merged.

donmccurdy · 2018-08-29T18:39:23Z

@takahirox @mrdoob Does SkeletonUtils.clone() seem OK as a name? Or something like more explicit like .cloneWithSkinning()?

takahirox · 2018-08-30T10:25:51Z

I prefer the explicit one.

Mugen87 · 2019-02-19T13:06:07Z

I have lately used this code in a project and it works great. Would love to see it in the code base 😊

Eketol · 2019-02-19T15:03:31Z

Me too. I've also tested it and it works fine for me. Just one question: the use of Map() is "es2015" (ES6), is that ok?
Also, in case it helps other noobs like me: Materials are not cloned, but referenced. In my case I think it is the right behaviour. If you want to customise some of the cloned object's materials, bear that in mind.

Mugen87 · 2019-02-19T21:32:16Z

I think using Map in this case is okay since SkeletonUtils is no part of the core. SkeletonUtils.cloneWithSkinning() seems to be a good place for this code.

@donmccurdy Any chances to update the PR? 😇

AnimationUtils: Clean up lint errors. AnimationUtils: Ensure .clone() does not depend on node names. GLTFLoader: Revert automatic node naming.

donmccurdy · 2019-02-20T05:09:15Z

Thanks!

I've moved the clone() method from AnimationUtils to SkeletonUtils, and added documentation for SkeletonUtils. I think this is ready to go. 👍

donmccurdy · 2019-03-23T07:12:24Z

@mrdoob could this be included with r103?

titansoftime · 2019-03-23T07:18:23Z

This just solved a huge issue I was having. Thank you.

Mugen87 · 2019-03-23T10:38:40Z

This just solved a huge issue I was having.

https://discourse.threejs.org/t/solved-issue-with-gltfloader-and-reusing-geometry/6697

mrdoob · 2019-03-26T02:11:01Z

@mrdoob could this be included with r103?

Done! 😇

mrdoob · 2019-03-26T02:11:07Z

Thanks!

canadaduane · 2020-04-02T23:50:31Z

Also, in case it helps other noobs like me: Materials are not cloned, but referenced. In my case I think it is the right behaviour. If you want to customise some of the cloned object's materials, bear that in mind.

Is there an option to clone the materials as well? In my case, I'd like some of my skinnedmesh objects to be transparent (but not all), even though they've been cloned from the same original.

Mugen87 · 2020-04-03T08:55:01Z

SkeletonUtils.clone() does not touch materials. You have to apply the material changes in a subsequent step.

enricocantori · 2022-02-26T18:23:29Z

Three.is is frequently used in the world of character animations. I can’t find no functions, no working examples related to the replace of parts of characters. Almost all characters are composed by skinnedMesh children and a lot of time is necessary change a part of the character like clothes, hair, face within characters that have same skeleton without broke animation (ex. Readyplayes characters). Clone and SkeletonUtils.Clone doesn’t solve this problem. Exist one more specific function?

This was referenced Jul 18, 2018

geometry.clone() doesn't copy reference to bones/skinweights/animations etc. #5878

Closed

No way of cloning a mesh so it works with a skinning material #11574

Closed

takahirox reviewed Jul 18, 2018

View reviewed changes

takahirox reviewed Jul 19, 2018

View reviewed changes

takahirox reviewed Jul 23, 2018

View reviewed changes

donmccurdy force-pushed the feat-animationutils-clone branch from 3a0e2b8 to a9cde37 Compare July 23, 2018 14:33

takahirox reviewed Jul 25, 2018

View reviewed changes

takahirox reviewed Jul 26, 2018

View reviewed changes

takahirox reviewed Jul 27, 2018

View reviewed changes

looeee mentioned this pull request Aug 5, 2018

FBX Object .clone() method not working properly #14647

Closed

13 tasks

mrdoob added this to the r97 milestone Aug 29, 2018

mrdoob modified the milestones: r97, r98 Sep 25, 2018

donmccurdy mentioned this pull request Nov 28, 2018

Object3D.clone has a bug #15332

Closed

mrdoob modified the milestones: r99, r100 Nov 29, 2018

mrdoob modified the milestones: r100, r101 Dec 31, 2018

mrdoob modified the milestones: r101, r102 Jan 31, 2019

Don McCurdy and others added 2 commits February 19, 2019 20:42

AnimationUtils: Add clone() helper for skinned meshes.

b5a16a5

AnimationUtils: Clean up lint errors. AnimationUtils: Ensure .clone() does not depend on node names. GLTFLoader: Revert automatic node naming.

Move 'clone' from AnimationUtils to SkeletonUtils.

427863f

donmccurdy force-pushed the feat-animationutils-clone branch from 6d20dc2 to 427863f Compare February 20, 2019 05:07

Mugen87 approved these changes Feb 20, 2019

View reviewed changes

donmccurdy changed the title ~~AnimationUtils: Add clone() helper for skinned meshes.~~ SkeletonUtils: Add clone() helper for skinned meshes. Feb 20, 2019

mrdoob modified the milestones: r102, r103 Feb 28, 2019

strazdinsg mentioned this pull request Mar 17, 2019

Add an example of multiple animated GLTF 3D objects in a single scene. #16006

Merged

mrdoob merged commit 7946bd5 into mrdoob:dev Mar 26, 2019

donmccurdy deleted the feat-animationutils-clone branch March 26, 2019 03:05

Mugen87 mentioned this pull request May 22, 2020

SkinnedMesh.clone doesn't work properly, results in undefined skeleton property #19419

Open

4 tasks


		source.traverse( function ( node ) {

		sourceNodes[ node.name ] = node;


		clonedMesh.skeleton.bones = sourceBones.map( function ( bone ) {

		return clonedNodes[ bone.name ];


		} );

		clonedMesh.bind( clonedMesh.skeleton, clonedMesh.bindMatrix );


		clonedMesh.skeleton.bones = sourceBones.map( function ( bone ) {

		return cloneLookup.get( bone );


		throw new Error( 'THREE.AnimationUtils: Required bones are not descendants of the given object.' );

		}

Uh oh!

SkeletonUtils: Add clone() helper for skinned meshes. #14494

SkeletonUtils: Add clone() helper for skinned meshes. #14494

Uh oh!

Conversation

donmccurdy commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

takahirox Jul 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donmccurdy commented Jul 18, 2018

Uh oh!

takahirox commented Jul 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donmccurdy commented Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Jul 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donmccurdy commented Jul 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

takahirox commented Jul 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donmccurdy commented Jul 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

takahirox commented Jul 28, 2018

Uh oh!

donmccurdy commented Jul 29, 2018

Uh oh!

takahirox commented Jul 29, 2018

Uh oh!

mrdoob commented Aug 29, 2018

Uh oh!

donmccurdy commented Aug 29, 2018

donmccurdy commented Jul 18, 2018 •

edited

Loading

Mugen87 commented Jul 18, 2018 •

edited

Loading

takahirox commented Jul 18, 2018 •

edited

Loading

takahirox Jul 19, 2018 •

edited

Loading

donmccurdy commented Jul 18, 2018 •

edited

Loading

Mugen87 commented Feb 19, 2019 •

edited

Loading