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

Support InterleavedBufferAttributes in VRMUtils #1532

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

mrxz
Copy link
Contributor

@mrxz mrxz commented Nov 15, 2024

Loading a VRM file with interleaved vertex attributes works for the most part, but the VRMUtils removeUnnecessaryVertices and removeUnnecessaryJoints did not support it.

For removeUnnecessaryJoints the code is changed to use setComponent and getComponent which handle indexing into the array internally. The function now works with both BufferAttribute and InterleavedBufferAttribute

For removeUnnecessaryVertices an additional step is added at the start. It iterates over the index attribute and keeps track of which vertices are used by the geometry. In case all vertices are used, further processing is skipped as there are no unnecessary vertices. This can save quite a bit of processing time :-)

In case not all vertices are used, it builds the index mappings (originalIndexNewIndexMap/newIndexOriginalIndexMap) based on the order the vertices appear in the vertex buffers. This should preserve any optimizations done to the vertex buffers for cache/fetching efficiency.

Interleaved buffers with unnecessary vertices is NOT supported by this PR. I have a working implementation, but it comes at a performance cost (~30-40% slower), so decided to not include it.

@0b5vr
Copy link
Contributor

0b5vr commented Nov 18, 2024

If I understand correctly, this PR includes:

  • removeUnnecessaryJoints: support of InterleavedBufferAttributes
  • removeUnnecessaryVertices: improved performance

Is this correct?

When do you need InterleavedBufferAttributes to work over three-vrm? I heard that gltf-transform might export models with interleaved accessors.

Does the change in removeUnnecessaryJoints affect the performance?

Also, if you find a part that is unclear or lacking descriptions inside these codes, feel free to comment.

@mrxz
Copy link
Contributor Author

mrxz commented Nov 18, 2024

Is this correct?

That is correct.

When do you need InterleavedBufferAttributes to work over three-vrm? I heard that gltf-transform might export models with interleaved accessors.

I started working on extensions for gltf-transform to support vrm 1.0 files (https://github.com/mrxz/vrm-transform). By default gltf-transform does indeed interleave vertex data. My goal is to optimize it for WebXR running on devices like the Meta Quest 2/3. Interleaved vertex data is recommended for these mobile GPUs.

Does the change in removeUnnecessaryJoints affect the performance?

Yes, the performance is impacted slightly. Here are some test results I gathered on my machine comparing the time each function takes before and after the changes:

removeUnnecessaryVertices removeUnnecessaryJoints
Avatar Before After Before After
VRM1_Constraint_Twist_Sample.vrm 22.2ms 2.1ms 4.3ms 4.5ms
AvatarSample_C.vrm 21.8ms 22.9ms 4.4ms 6.6ms
Zonko_VRM_221128_ps.vrm 53.3ms 2.7ms 6.2ms 5.7ms
AKAI Original VRM by Shugan.vrm 47.1ms 3.6ms 6.8ms 7.5ms

The last two vrm models were converted to VRM 1.0 using UniVRM.

As can be seen, the removeUnnecessaryJoints does become a bit slower. It is a relatively small part of the total loading time (>300ms without any network latency).

As a heads-up: I have found several places in three-vrm that can be optimized quite considerably (incl. springbones). So there will be more PRs coming once I've cleaned up my changes. One of these changes focusses on sharing a THREE.Skeleton between skinned meshes, as three.js computes and uploads bone matrices once per frame per skeleton. This mostly removes the need for removeUnnecessaryJoints as, for example, one Skeleton with 150 bones outperforms three Skeletons with 60 bones each (150 bone matrices + 1 texture upload vs 180 bone matrices + 3 texture uploads).

@0b5vr
Copy link
Contributor

0b5vr commented Nov 19, 2024

I started working on extensions for gltf-transform to support vrm 1.0 files (https://github.com/mrxz/vrm-transform). By default gltf-transform does indeed interleave vertex data. My goal is to optimize it for WebXR running on devices like the Meta Quest 2/3. Interleaved vertex data is recommended for these mobile GPUs.

True. Thank you for the explanation.

Yes, the performance is impacted slightly. Here are some test results I gathered on my machine comparing the time each function takes before and after the changes:

I really appreciate the result table. it seems it's not well impacted about removeUnnecessaryJoints, and the improvement of removeUnnecessaryVertices is very impressive.

This mostly removes the need for removeUnnecessaryJoints as, for example, one Skeleton with 150 bones outperforms three Skeletons with 60 bones each (150 bone matrices + 1 texture upload vs 180 bone matrices + 3 texture uploads).

The original motivation for why we needed to removeUnnecessaryJoints was several mobile GPUs that don't support many shader uniforms required to transfer bone matrices to the shader. We had to accept the backfire that the number of skeletons the model needs increases.
However, Three.js no longer uses a uniform mat4 array for bone matrices; it now uses bone texture.
https://github.com/mrdoob/three.js/blob/9f05e61074b4b642e7933285e5265653c320885f/src/renderers/shaders/ShaderChunk/skinning_pars_vertex.glsl.js

So it might be the high time to get rid of the removeUnnecessaryJoints at all, honestly.
Let me think about it in a separated PR.

Copy link
Contributor

@0b5vr 0b5vr left a comment

Choose a reason for hiding this comment

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

I really appreciate the contribution. Thank you very much! 😍

@0b5vr 0b5vr merged commit 2ed4eec into pixiv:dev Nov 19, 2024
@0b5vr 0b5vr added this to the next milestone Nov 19, 2024
@0b5vr 0b5vr added the performance Performance issue label Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants