BabylonJS / Babylon.js

Babylon.js is a powerful, beautiful, simple, and open game and rendering engine packed into a friendly JavaScript framework.
http://www.babylonjs.com
Apache License 2.0
22.75k stars 3.39k forks source link

Rewrite morph target gltf export #15229

Closed ryantrem closed 1 week ago

ryantrem commented 1 week ago

glTFExporter had logic to export morph targets, but it did not work correctly with submeshes (see this forum thread: https://forum.babylonjs.com/t/gltfexporter-not-working-with-morph-targets/51337). There were a couple significant problems with the code that basically required a rewrite of this logic:

  1. It was writing separate segments of data to the buffer (and separate buffer views) for each submesh. This is always wrong for indexed geometry, and is broken if there are multiple submeshes.
  2. The logic for determining the range of vertex data to write per submesh was just wrong, and could only work for indexed geometry with one submesh.

For 2, I couldn't even tell what the logic was trying to do:

for (let k = meshPrimitive.verticesStart; k < meshPrimitive.verticesCount; ++k) {
  index = meshPrimitive.indexStart + k * stride;
    const vertexData = Vector3.FromArray(meshAttributeArray, index);

verticesStart is only meaningful for non-indexed geometry, so combining indexStart and verticesStart in any way doesn't make sense. I think this code only worked because it was only ever tested in cases where both indexStart and verticesStart were zero.

Overall, the approach now is to write all the morph target vertex data for the entire mesh and create an associated buffer view for each attribute (where morph data is present), then create one accessor per submesh per morph target that potentially has an offset into the buffer view (for submeshes beyond the first).

This does not fix the pre-existing issues with non-indexed geometry in general, but that should be handled in a separate PR. This approach does not prevent us from making non-indexed geometry work, there are just additional changes needed that can be done in isolation from these changes.

bjsplat commented 1 week ago

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). To prevent this PR from going to the changelog marked it with the "skip changelog" label.

bjsplat commented 1 week ago

Snapshot stored with reference name: refs/pull/15229/merge

Test environment: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15229/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15229/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/15229/merge https://sandbox.babylonjs.com/?snapshot=refs/pull/15229/merge https://gui.babylonjs.com/?snapshot=refs/pull/15229/merge https://nme.babylonjs.com/?snapshot=refs/pull/15229/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/15229/merge#BCU1XR#0

bjsplat commented 1 week ago

WebGL2 visualization test reporter:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15229/merge/testResults/webgl2playwright/index.html

bjsplat commented 1 week ago

Visualization tests for WebGPU (Experimental)

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15229/merge/testResults/webgpuplaywright/index.html