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

Add example with high number of morph targets #22514

Merged
merged 8 commits into from
Sep 13, 2021
Merged

Add example with high number of morph targets #22514

merged 8 commits into from
Sep 13, 2021

Conversation

looeee
Copy link
Collaborator

@looeee looeee commented Sep 9, 2021

Added a new example to showcase simultaneous animation of a large number of morph targets.

Live link (note: normals incorrect pending build with #22516)

main1

The model was kindly provided by the creator of the iPhone Face Cap app.

Possibly this example can replace webgl_morphtargets_horse as they serve a similar function.

@looeee
Copy link
Collaborator Author

looeee commented Sep 9, 2021

I've created this example as draft because the model is currently quite large (3.9mb). I was able to shrink it down using gltfpack to just 325kb.

facecap-gltfpack.zip

However when I try to load it this is the result (animation plays correctly, lighting or normals are incorrect):

mess

This only occurs when I set scene.environment = texture. If I remove that line and instead add a light, it looks fine.

scene.add(new THREE.HemisphereLight());
// scene.environment = texture;

light only

However if I drop back to r131 and re-able the environment it looks fine (except now the animation can only use 8 morphtargets)

r131

So it seems like the new morph target implementation is interacting badly with environment maps - but only when the model is compressed 😕

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 9, 2021

I would remove the environment map from the scene and use the more neutral RoomEnvironment instead. Then it looks more like the screenshot from #22293 (comment).

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 9, 2021

If I remove that line and instead add a light, it looks fine.

Have you just tested with a hemisphere light? If so, try it again with an additional directional or point light. I assume the normals are corrupted in some way so these lights shouldn't work.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 9, 2021

Okay, the issue happens because WebGLMorphtargets assumes that the morph attribute data are represented as Float32. When using gltf-pack, the normal data are compressed to (normalized) Int8Array. Reading these data to setup the data texture does not work yet.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 9, 2021

Since the data texture needs to be of type FloatType, applying the attribute compression is not only limited useful in the first place because the data needs to be converted back to float.

Is it possible to configure gltf-pack such that morph data are not compressed?

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 9, 2021

@mrdoob I can fix this if you want by doing this in WebGLMorphtargets for each morph normal:

if ( morphNormal.normalized === true ) {

    const denominator = getMaxIntegerValue( morphNormal );

    morph.x /= denominator;
    morph.y /= denominator;
    morph.z /= denominator;

}

getMaxIntegerValue() would be a new function that checks the attribute array for Int8Array, Int16Array or Int32Array and then determines the respective max value. This value will be used to denormlize the morph normal.

If we don't apply the fix, users are not allowed to compress morph target data (this normally means representing morph normals as normalized signed integers)

@donmccurdy
Copy link
Collaborator

This model could be an option as well:

https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/MorphStressTest

I think it would be helpful to support normalized morph attributes, but am happy to help create a version of either model that doesn't include them if needed.

@spiraloid
Copy link

spiraloid commented Sep 9, 2021

plus 1 for using this model. The shapes it has are based on the Animoji FACS (based on my Gollum FACS from LOTR)

I'd suggest skinning to some bones (w 4 bone linear skinning) so that the skinning, morph targets and normals are all on the same gpu loop.

all three are required for character animation. doing just one in isolation makes no sense.

Protip: the vertex positions and normal precision can be done as integers (i.e. pixel color) as long as they are scaled by the magnitude of largest vertex delta found in all the morph targets (and then rescaled before they are skinned). The visual fidelity loss us un-noticeable ;)

this buys you the headroom to do 32 characters at once.

my existence proofs:
https://youtu.be/-cSFPIwMEq4?t=11
https://www.youtube.com/watch?v=36lSzUMBJnc&t=94s
https://www.youtube.com/watch?v=C4cfo0f88Ug

happy to skin up a neck to that mesh if it helps keep the test case legit. lemme know.

@looeee
Copy link
Collaborator Author

looeee commented Sep 10, 2021

I would remove the environment map from the scene and use the more neutral RoomEnvironment instead. Then it looks more like the screenshot from #22293 (comment).

Done, I've updated the live link.

@looeee
Copy link
Collaborator Author

looeee commented Sep 10, 2021

Is it possible to configure gltf-pack such that morph data are not compressed?

I don't think so. I tried making an uncompressed file by running gltfpack -i facecap.glb -o out.glb but the result is the same (filesize around 2.5mb).

@donmccurdy I did also try gltf-transform, both draco and meshopt. Draco barely reduced the filesize and meshopt completed messed up the model. I ran both commands with default settings, but didn't investigate any further yet.

@mrdoob
Copy link
Owner

mrdoob commented Sep 10, 2021

@spiraloid

happy to skin up a neck to that mesh if it helps keep the test case legit. lemme know.

That'd be super useful!

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 10, 2021

I don't think so. I tried making an uncompressed file by running gltfpack -i facecap.glb -o out.glb but the result is the same (filesize around 2.5mb).

@looeee Never mind! With #22516 it's possible to use the model from gltf-pack.

@donmccurdy
Copy link
Collaborator

I did also try gltf-transform, both draco and meshopt. Draco barely reduced the filesize and meshopt completed messed up the model. I ran both commands with default settings, but didn't investigate any further yet.

Ok thanks! Draco won't do anything with morph targets, but I would've expected meshopt to work. Filed a bug at donmccurdy/glTF-Transform#352.

Glad to see #22516!

@spiraloid
Copy link

draco not compressing the targets could be heavy depending how many verts are moving. storing the morph target/normals as a png could be a smart option. each 3 pixels being the unitized position of each vert in an index that gets expanded back to float on draw. that way draco image compression could compress the morph targets…. an idea.

@donmccurdy
Copy link
Collaborator

Draco cannot compress images, though. I think meshopt compression is the right choice here (this is what gltfpack uses). Custom compression schemes (and custom data pipelines to produce them...) can be very effective for particular applications, but I don't think we should be pointing people that way in our examples.

@looeee looeee marked this pull request as ready for review September 11, 2021 03:28
@looeee
Copy link
Collaborator Author

looeee commented Sep 11, 2021

OK, I've switched to the gltfpack compressed model and marked this PR as ready for review. The normals will be incorrect until the builds are updated with #22516.

@looeee
Copy link
Collaborator Author

looeee commented Sep 11, 2021

@spiraloid giving the model a neck would be great 👍 I think I have the original FBX if that's easier for you to work from.

@mrdoob mrdoob added this to the r133 milestone Sep 13, 2021
@mrdoob mrdoob merged commit 777a4be into dev Sep 13, 2021
@mrdoob
Copy link
Owner

mrdoob commented Sep 13, 2021

Thanks!

@mrdoob mrdoob deleted the feat/morph_example branch September 13, 2021 21:01
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.

5 participants