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 2.0 morph animation support #11350

Merged
merged 7 commits into from
May 20, 2017
Merged

glTF 2.0 morph animation support #11350

merged 7 commits into from
May 20, 2017

Conversation

takahirox
Copy link
Collaborator

@takahirox takahirox commented May 17, 2017

This PR lets GLTF2Loader support glTF 2.0 morph animation, excluding TANGENT. #11326

I'm not familiar with tangent yet.
How can we support tangent?
Converting tangent to normal?
Let's make another PR for TANGENT support.

// + weight1 * morphTarget1
// ...
// then adding position to morphTarget.
// motphTarget should depend on mesh, so cloning attribute.
Copy link
Collaborator

@donmccurdy donmccurdy May 17, 2017

Choose a reason for hiding this comment

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

*morphTarget

These BufferAttributes aren't mutated while playing animation, are they? If not, what's the reason for cloning them? I'm not sure I follow the "should depend on mesh" part. Hmm never mind I see.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I've updated the comment to clarify more.

@donmccurdy
Copy link
Collaborator

Nice! Does this work with both the sphere and cube examples?

@takahirox
Copy link
Collaborator Author

Yup! But we also need to merge #11351 for them.

@donmccurdy
Copy link
Collaborator

Ok cool! May also want to wait on this PR until #11285 is in.

@@ -2111,6 +2112,93 @@ THREE.GLTF2Loader = ( function () {
meshNode = new THREE.Mesh( geometry, material );
meshNode.castShadow = true;

if ( primitive.targets !== undefined ) {

geometry.morphTargets = [];
Copy link
Collaborator

@Mugen87 Mugen87 May 17, 2017

Choose a reason for hiding this comment

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

If #11285 is merged, creating geometry.morphTargets = []; should not be necessary anymore. Instead, you need to set the name of a morph target directly to a morphAttribute, see https://github.com/Mugen87/three.js/blob/f375cf243b9214266e9d40d3bf0e5da7862b47bc/examples/js/loaders/MMDLoader.js#L789

Copy link
Owner

Choose a reason for hiding this comment

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

#11285 has been merged 😊

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yay 🎉 ! 😊

@takahirox
Copy link
Collaborator Author

Updated!

@takahirox
Copy link
Collaborator Author

Updated for the case if some target in targets don't have all attributes, like this

"targets": [
    {
        "POSITION": 0
    },
    {
        "NORMAL": 1
    }
] 

@mrdoob
Copy link
Owner

mrdoob commented May 18, 2017

Updated for the case if some target in targets don't have all attributes

@donmccurdy looks good?

@donmccurdy
Copy link
Collaborator

Looks good to me!

@mrdoob mrdoob merged commit 0c5e2de into mrdoob:dev May 20, 2017
@mrdoob
Copy link
Owner

mrdoob commented May 20, 2017

Thanks!

@takahirox takahirox deleted the GLTF2Morph branch June 23, 2017 02:02
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

Successfully merging this pull request may close these issues.

4 participants