Skip to content

SkinnedMesh/Skeleton serialization #11603

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.

Sign up for GitHub

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

Closed
wants to merge 11 commits into from
Closed

SkinnedMesh/Skeleton serialization #11603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
37954e2
Support SkinnedMesh and Skeleton serialization
takahirox Jun 23, 2017
46d516a
Add comment for skeletonUUID in ObjectLoader
takahirox Jun 23, 2017
5228523
Remove unused getSkeleton() from ObjectLoader
takahirox Jun 24, 2017
daf7399
Fix typo in ObjectLoader
takahirox Jun 29, 2017
ae7b71d
Merge branch 'dev' into SkinnedMeshSerialization
takahirox Sep 21, 2017
a5e418d
Merge branch 'dev' into SkinnedMeshSerialization
takahirox Dec 21, 2017
e9e54cc
Merge branch 'dev' into SkinnedMeshSerialization
takahirox Dec 29, 2017
a78ddb5
Merge branch 'dev' into SkinnedMeshSerialization
takahirox Jan 11, 2018
af6d2aa
Add missing break to ObjectLoader
takahirox May 7, 2018
d5d283f
Merge branch 'dev' into SkinnedMeshSerialization
takahirox May 7, 2018
8a4da31
Clean up ObjectLoader
takahirox Oct 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/core/Object3D.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,8 @@ Object3D.prototype = Object.assign( Object.create( EventDispatcher.prototype ),
materials: {},
textures: {},
images: {},
shapes: {}
shapes: {},
skeletons: {}
};

output.metadata = {
Expand Down Expand Up @@ -663,6 +664,11 @@ Object3D.prototype = Object.assign( Object.create( EventDispatcher.prototype ),

if ( this.matrixAutoUpdate === false ) object.matrixAutoUpdate = false;

// SkinnedMesh specific

if ( this.bindMode !== undefined ) object.bindMode = this.bindMode;
if ( this.bindMatrix !== undefined ) object.bindMatrix = this.bindMatrix.toArray();

Copy link
Collaborator

@Mugen87 Mugen87 Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this.bindMatrixInverse? For the sake of completeness we should also regard this property.

Copy link
Collaborator Author

@takahirox takahirox Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bindMatrixInverse is calculated from bindMatrix in SkinnedMesh.bind() so I didn't think we need to serialize bindMatrixInverse.
Do you think we should do that?

Copy link
Collaborator

@Mugen87 Mugen87 Jun 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just prefer to serialize all public attributes. But your point of view is also valid. The current code is definitely correct. Because i don't have a strong opinion on this, i leave it up to you 😉

Copy link
Collaborator Author

@takahirox takahirox Jun 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so let's keep this code so far.

Copy link
Collaborator Author

@takahirox takahirox Jul 1, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please close change request?

Copy link
Collaborator

@Mugen87 Mugen87 Aug 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@takahirox I'm sorry! I've missed your comment. Approving...

//

function serialize( library, element ) {
Expand Down Expand Up @@ -729,6 +735,20 @@ Object3D.prototype = Object.assign( Object.create( EventDispatcher.prototype ),

}

// SkinnedMesh specific

if ( this.skeleton !== undefined ) {
Copy link
Contributor

@tentone tentone Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi

Why not move this to a toJSON inside SkinnedMesh i think it would make more sense to move mesh serialization specifics to the Mesh and SkinnedMesh objects!

Thanks

Copy link
Contributor

@tentone tentone Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can wait until this gets merged and i can try to propose moving and cleanup these after maybe?

Copy link
Collaborator Author

@takahirox takahirox Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you do that?

Copy link
Contributor

@tentone tentone Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By moving that code to a toJSON method inside SkinnedMesh :)

Copy link
Collaborator Author

@takahirox takahirox Jun 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try to make sample code and share with me?
I tried the same idea before but ended up realizing it'd be simpler if they'd be in Object3D.toJSON().

Copy link
Contributor

@tentone tentone Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this

SkinnedMesh.prototype.toJSON = function(meta)
{
	var data = THREE.Object3D.prototype.toJSON.call(this, meta);

	if(this.bindMode !== undefined)
	{
		data.object.bindMode = this.bindMode;
	}
	if(this.bindMatrix !== undefined)
	{
		data.object.bindMatrix = this.bindMatrix.toArray();
	}

	if(this.skeleton !== undefined)
	{
		if(meta.skeletons[this.skeleton.uuid] === undefined)
		{
			meta.skeletons[this.skeleton.uuid] = this.skeleton.toJSON(meta);
		}

		data.object.skeleton = this.skeleton.uuid;
	}

};

Copy link
Contributor

@jbaicoianu jbaicoianu Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to do this once - it's trickier than it seems, because of how toJSON is written, it makes it hard to do this without copy-pasting the whole toJSON function.

Ideally, since SkinnedMesh extends Object3D, you should just be able to call Object3D.prototype.toJSON.call(this, meta) - but in practice it's trickier, because Object3D.toJSON defines a bunch of internal functions - not sure if we need those internal functions for the skeleton, but I have run into cases in the past where this made it difficult without duplicating a lot of code, or restructuring.

Copy link
Contributor

@tentone tentone Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well im doing a ton in nunuStudio i have pretty much overrided more than half of the threejs serialization process to include resources, external lib data etc.

There is some tricks to it sometimes (specially when working with object relations) but for these cases its pretty straight forward just call the thing on Object3D and add whatever you need to add, its always better and cleaner than filling Object3D serialization with a ton of verifications to decide whether or not to include a resource from a specific class.

Copy link
Contributor

@jbaicoianu jbaicoianu Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, your code looks good, and this is definitely the way to go rather than piling it all into Object3D. Maybe at some point we can move the serialize and extractFromCache local convenience functions out to be either methods of the Object3D class or part of a utility class, to make it easier to do this for all cases.

Copy link
Collaborator Author

@takahirox takahirox Jun 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think your code is good enough.
I think meta initialization and isRootObject related codes are also necessary in SkinnedMesh.toJSON() in addition to extractFromCache.
Imagine what'll happen with

var skin = new THREE.SkinnesMesh( geometry, material );
skin.toJSON();

I don't really like making duplicated codes in SkinnedMesh.
So IMO, as @jbaicoianu mentioned, we need restructuring if we move this into SkinnedMesh.
Let's make another PR if we do.


if ( meta.skeletons[ this.skeleton.uuid ] === undefined ) {

meta.skeletons[ this.skeleton.uuid ] = this.skeleton.toJSON( meta );

}

object.skeleton = this.skeleton.uuid;

}

//

if ( this.children.length > 0 ) {
Expand All @@ -750,12 +770,14 @@ Object3D.prototype = Object.assign( Object.create( EventDispatcher.prototype ),
var textures = extractFromCache( meta.textures );
var images = extractFromCache( meta.images );
var shapes = extractFromCache( meta.shapes );
var skeletons = extractFromCache( meta.skeletons );

if ( geometries.length > 0 ) output.geometries = geometries;
if ( materials.length > 0 ) output.materials = materials;
if ( textures.length > 0 ) output.textures = textures;
if ( images.length > 0 ) output.images = images;
if ( shapes.length > 0 ) output.shapes = shapes;
if ( skeletons.length > 0 ) output.skeletons = skeletons;

}

Expand Down
111 changes: 110 additions & 1 deletion src/loaders/ObjectLoader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
LinearMipMapLinearFilter
} from '../constants.js';
import { Color } from '../math/Color.js';
import { Matrix4 } from '../math/Matrix4.js';
import { Object3D } from '../core/Object3D.js';
import { Group } from '../objects/Group.js';
import { Sprite } from '../objects/Sprite.js';
Expand All @@ -31,6 +32,8 @@ import { LOD } from '../objects/LOD.js';
import { Mesh } from '../objects/Mesh.js';
import { SkinnedMesh } from '../objects/SkinnedMesh.js';
import { Shape } from '../extras/core/Shape.js';
import { Bone } from '../objects/Bone.js';
import { Skeleton } from '../objects/Skeleton.js';
import { Fog } from '../scenes/Fog.js';
import { FogExp2 } from '../scenes/FogExp2.js';
import { HemisphereLight } from '../lights/HemisphereLight.js';
Expand Down Expand Up @@ -139,6 +142,10 @@ Object.assign( ObjectLoader.prototype, {

var object = this.parseObject( json.object, geometries, materials );

var skeletons = this.parseSkeletons( json.skeletons, object );

this.bindSkeletons( object, skeletons );

if ( json.animations ) {

object.animations = this.parseAnimations( json.animations );
Expand Down Expand Up @@ -587,6 +594,48 @@ Object.assign( ObjectLoader.prototype, {

},

parseSkeletons: function ( json, object ) {

var skeletons = {};

if ( json === undefined ) return skeletons;

for ( var i = 0, il = json.length; i < il; i ++ ) {

var skeletonParams = json[ i ];

var uuid = skeletonParams.uuid;
var boneUUIDs = skeletonParams.bones;
var boneInverseArrays = skeletonParams.boneInverses;

var bones = [];
var boneInverses = [];

for ( var j = 0, jl = boneUUIDs.length; j < jl; j ++ ) {

var boneUUID = boneUUIDs[ j ];
var bone = object.getObjectByProperty( 'uuid', boneUUID );

if ( bone === undefined ) {

console.warn( 'THREE.ObjectLoader: Not found Bone whose uuid is ' + boneUUID );
bone = new Bone();

}

bones.push( bone );
boneInverses.push( new Matrix4().fromArray( boneInverseArrays[ j ] ) );

}

skeletons[ uuid ] = new Skeleton( bones, boneInverses );

}

return skeletons;

},

parseObject: function ( data, geometries, materials ) {

var object;
Expand Down Expand Up @@ -730,7 +779,33 @@ Object.assign( ObjectLoader.prototype, {

case 'SkinnedMesh':

console.warn( 'THREE.ObjectLoader.parseObject() does not support SkinnedMesh yet.' );
var geometry = getGeometry( data.geometry );
var material = getMaterial( data.material );

var currentBones;

// If data has skeleton, assuming bones are already in scene graph.
// Then temporarily undefining geometry.bones not to create duplicated
// bones in SkinnedMesh constructor.

if ( data.skeleton !== undefined && geometry.bones !== undefined ) {

currentBones = geometry.bones;
geometry.bones = undefined;

}

object = new SkinnedMesh( geometry, material );

if ( data.bindMode !== undefined ) object.bindMode = data.bindMode;
if ( data.bindMatrix !== undefined ) object.bindMatrix.fromArray( data.bindMatrix );

// .skeletonUUID is temporal. Deleting after binding the skeleton.
// See .bindSkeletons().
if ( data.skeleton !== undefined ) object.skeletonUUID = data.skeleton;
if ( currentBones !== undefined ) geometry.bones = currentBones;

break;

case 'Mesh':

Expand All @@ -749,6 +824,12 @@ Object.assign( ObjectLoader.prototype, {

Copy link

@mjbradford89 mjbradford89 May 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a break; missing here for the Mesh case?

Copy link
Collaborator Author

@takahirox takahirox May 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, thanks. I'll fix later.

Copy link
Collaborator Author

@takahirox takahirox May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks!

break;

case 'Bone':

object = new Bone();

break;

case 'LOD':

object = new LOD();
Expand Down Expand Up @@ -868,6 +949,34 @@ Object.assign( ObjectLoader.prototype, {

return object;

},

bindSkeletons: function ( object, skeletons ) {

if ( Object.keys( skeletons ).length === 0 ) return;

object.traverse( function ( obj ) {

if ( obj.isSkinnedMesh === true && obj.skeletonUUID !== undefined ) {

var skeleton = skeletons[ obj.skeletonUUID ];

if ( skeleton === undefined ) {

console.warn( 'THREE.ObjectLoader: Not found Skeleton whose uuid is ' + obj.skeletonUUID );

} else {

obj.bind( skeleton, obj.bindMatrix );

}

delete obj.skeletonUUID;

}

} );

}

} );
Expand Down
30 changes: 30 additions & 0 deletions src/objects/Skeleton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { _Math } from '../math/Math.js';
import { Matrix4 } from '../math/Matrix4.js';

/**
Expand All @@ -9,6 +10,8 @@ import { Matrix4 } from '../math/Matrix4.js';

function Skeleton( bones, boneInverses ) {

this.uuid = _Math.generateUUID();

// copy the bone array

bones = bones || [];
Expand Down Expand Up @@ -170,6 +173,33 @@ Object.assign( Skeleton.prototype, {

return undefined;

},

toJSON: function ( meta ) {

var data = {};

var bones = [];
var boneInverses = [];

for ( var i = 0, il = this.bones.length; i < il; i ++ ) {

bones.push( this.bones[ i ].uuid );

}

for ( var i = 0, il = this.boneInverses.length; i < il; i ++ ) {

boneInverses.push( this.boneInverses[ i ].toArray() );

}

data.uuid = this.uuid;
data.bones = bones;
data.boneInverses = boneInverses;

return data;

}

} );
Expand Down