-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
Webgl_morphtargets: switched to buffergeometry and simplify #13847
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
Conversation
|
Morph normals are supported. The max 8 influences are split -- 4 for position and 4 for normal. Check out the shader code. Morphing normals are important if you want the lighting to remain consistent as vertices are morphed. Probably the original box example would be better for demonstrating that. Also, sharing vertices, implemented via indexed buffer geometry, prevent tearing when vertices are morphed. That is likely a better approach than using non-indexed buffer geometry as you have here. |
|
Agreed that indexing makes more sense though - I've switched to an indexed cube here, although still only morphing the 4 front face corners for simplicity.
Yes, that's why I switched to a MeshBasicMaterial and removed lights here. I wanted to make this example as simple as possible, adding normals to an indexed cube means that I need 24 rather than 8 vertices. So I'd rather skip them here even though it doesn't look as nice. (EDITED for clarity) Live link (Indexed cube) |
I really do not think rendering a 3D object with Why not use scene.add( camera );
camera.add( pointLight );
var material = new THREE.MeshPhongMaterial( { flatShading: true, morphTargets: true } ); |
|
That looks much better 😄 Live link (Indexed cube with flat shading) |
examples/webgl_morphtargets.html
Outdated
|
|
||
| } | ||
| var ambientLight = new THREE.AmbientLight( 0x333333 ); | ||
| scene.add( ambientLight ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You really don't need ambient lighting with the light source at the camera location. I'd remove the ambient light, as the scene is a bit over-bright. (I recognize this is subjective.)
examples/webgl_morphtargets.html
Outdated
| 1, 5, 3, 3, 5, 7 // right | ||
|
|
||
| ]; | ||
| geometry.setIndex( new THREE.Uint16BufferAttribute( indices, 1 ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be sufficient:
geometry.setIndex( indices );
examples/webgl_morphtargets.html
Outdated
| 0, 6, 4, 6, 0, 2, // left | ||
| 1, 5, 3, 3, 5, 7 // right | ||
|
|
||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor formatting issue.
|
Thanks for feedback! Removed ambient light and fixed formatting. |
|
Here is how you can create a link that does not go stale -- at least not until the branch is deleted. |
|
I would say it's okay like that 👍 |
|
@WestLangley I'm intentionally creating the live links to track individual commits. That way people can see different versions as I update it. |
I wouldn't say that adding lines is simplifying... 🤔 Also, in the old example we were able to manipulate all vertices... Maybe this would be more suitable as a new example in the |
Yeah, the simplification part kind of got lost in the revisions.
Previously the morph targets were set up in a loop, now I'm writing them out explicitly since this is an introductory example and I think that's clearer. That's also the reason for reducing the number of morph targets from 8 to 4. If you guys prefer I can back to morphing all 8 corners, either by writing the targets explicitly like it is now, or in a loop.
I thought we were trying to move away from using Geometry in the examples? |
|
Maybe something along these lines would be more visually interesting: // morph squat
var morphTargetPositions0 = [
-1.25, -1, 1.25,
1.25, -1, 1.25,
-1.25, 0.5, 1.25,
1.25, 0.5, 1.25,
-1.25, -1, -1.25,
1.25, -1, -1.25,
-1.25, 0.5, -1.25,
1.25, 0.5, -1.25,
];
// morph lean
var morphTargetPositions1 = [
-1, -1, 1,
1, -1, 1,
-1.5, 1, 1,
0.5, 1, 1,
-1, -1, -1,
1, -1, -1,
-1.5, 1, -1,
0.5, 1, -1,
]; |
|
Well, I kinda prefer morphing the edges. But really I'm OK with whatever. The main thing is that we get an example that shows morph targets with BufferGeometry, because currently there is nothing. @mrdoob if you want to make some executive decisions here I'll update the PR:
|
Yes, but we also lose an example of morph targets with Geometry, which was simpler. I think it would be better as an additional example. |
|
Oops, forgot about this one! Closing in favour of #15148 |
None of the morph target examples currently use BufferGeometry. Updated and simplified the webgl_morphtargets example so that it serves as a basic introduction to using morphtargets with a buffergeometry.
Now the example uses a square rather than a cube, and the morph positions are created manually rather than in a loop so that the change in position is as obvious as possible.
Old version
New version (non-indexed square)
I've added a note to say that morphing of positions and normals is supported, can somebody confirm that this is correct?