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

BufferGeometry with morphAttributes does not create mesh with morphTargetInfluences #11277

Closed
looeee opened this issue May 4, 2017 · 6 comments
Labels

Comments

@looeee
Copy link
Collaborator

looeee commented May 4, 2017

If I create a buffer geometry and add a position morph attribute, then create a mesh from it, the mesh doesn't have a morphTargetInfluences property so I can't animate the morph.

Workaround is to add a fake morphTarget to the geometry:

    bufferGeometry.morphTargets = [];
    bufferGeometry.morphTargets.push( 0 );

See this pen:
http://codepen.io/looeee/pen/YVxaKL?editors=0010

@Mugen87
Copy link
Collaborator

Mugen87 commented May 4, 2017

The following version of Mesh .updateMorphTargets() should solve the problem. Unfortunately, at least MMDLoader won't work with this change. MMDLoader assigns to its BufferGeometry output also .morphTargets which is actually a property of Geometry. I think MMDLoader does this because the name of a morph target animation sequence can't be saved in morphAttributes. But i'm not 100% sure about this. Didn't have time to debug MMDLoader so far. The only thing i can say is that the following example has problems with this change: https://threejs.org/examples/webgl_loader_mmd_pose.html

updateMorphTargets: function () {

	var geometry = this.geometry;

	if ( geometry.isBufferGeometry ) {

		var morphAttributes = geometry.morphAttributes;
		var keys = Object.keys( morphAttributes );

		if ( keys.length > 0 ) {

			var morphAttribute = morphAttributes[ keys[ 0 ] ];

			if ( morphAttribute !== undefined ) {

				this.morphTargetInfluences = [];

				for ( var i = 0, l = morphAttribute.length; i < l; i ++ ) {

					this.morphTargetInfluences.push( 0 );

				}

			}

		}

	} else {

		var morphTargets = geometry.morphTargets;

		if ( morphTargets !== undefined && morphTargets.length > 0 ) {

			this.morphTargetInfluences = [];
			this.morphTargetDictionary = {};

			for ( var m = 0, ml = morphTargets.length; m < ml; m ++ ) {

				this.morphTargetInfluences.push( 0 );
				this.morphTargetDictionary[ morphTargets[ m ].name ] = m;

			}

		}

	}

},

@Mugen87 Mugen87 added the Bug label May 4, 2017
@looeee
Copy link
Collaborator Author

looeee commented May 5, 2017

It looks to me as if MMDLoader is using a similar hack - it's essentially doing

 bufferGeometry.morphTargets = [];
 bufferGeometry.morphTargets.push( morph.name );

https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/MMDLoader.js#L861, where params is morph.name

The problem (or at least one problem!) is that these names are then used to create the mesh.morphTargetDictionary, which webgl_loader_mmd_pose then loops over to create it's morph controls. So you example code above would need to be updated to create the mesh.morphTargetDictionary as well.

Which would be easily solved if BufferAttribute had a .name property, but at the moment it doesn't. Should we add the property?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 5, 2017

So you example code above would need to be updated to create the mesh.morphTargetDictionary as well.

Correct!

Which would be easily solved if BufferAttribute had a .name property, but at the moment it doesn't. Should we add the property?

I like the idea because we won't break stuff and it's easy to implement.

@mrdoob, @WestLangley What's your opinion about this?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 5, 2017

I've created a PR so we can better see the changes...

@Mugen87
Copy link
Collaborator

Mugen87 commented May 5, 2017

@looeee With the new PR the creation of a morph attribute looks like this:

const morphAttribute = new THREE.BufferAttribute( morphPositions, 3 );
morphAttribute.name = 'base'; // optional, .morphTargetDictionary will always be created with default names
geometry.morphAttributes.position.push( morphAttribute );

@looeee
Copy link
Collaborator Author

looeee commented May 6, 2017

Sweet! 😁

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

No branches or pull requests

2 participants