-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GLTFSerializer : Ext mesh gpu instancing #12495
Conversation
Please make sure to tag your PR with "bug", "new feature" or "breaking change" tags. |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12495/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12495/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/12495/merge#BCU1XR#0 |
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.
LTGM overall. I'm confused by the normalized array code.
packages/dev/serializers/src/glTF/2.0/Extensions/EXT_mesh_gpu_instancing.ts
Outdated
Show resolved
Hide resolved
packages/dev/serializers/src/glTF/2.0/Extensions/EXT_mesh_gpu_instancing.ts
Outdated
Show resolved
Hide resolved
packages/dev/serializers/src/glTF/2.0/Extensions/EXT_mesh_gpu_instancing.ts
Outdated
Show resolved
Hide resolved
packages/dev/serializers/src/glTF/2.0/Extensions/EXT_mesh_gpu_instancing.ts
Outdated
Show resolved
Hide resolved
Please make sure to tag your PR with "bug", "new feature" or "breaking change" tags. |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12495/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12495/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/12495/merge#BCU1XR#0 |
the buffer might be always a Float32Array because all the values are coming from matrix decompose
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12495/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12495/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/12495/merge#BCU1XR#0 |
@bghgary I ll let you review and merge this one :-) |
There is at least one outstanding comment that hasn't been addressed. I'll review once it is. :-) |
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12495/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/12495/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/12495/merge#BCU1XR#0 |
Good to go? |
Add support to EXT_mesh_gpu_instancing to Babylon GLTF Serializer.
Thin instances are used to save transformation into GLTF buffer.
This extension is NOT official and not supported yet by many viewer. This may be used with caution while terminology between Local Transformation and World transformation are not consistent beetwen this Extension and the babylon Thin instances.
Test has been conducted sucessfully with playground and sandbox, using respectively coding samples and imported gltf sample.
The main sample used was the "official" gltf sample
Also add Visualization test.
Note - deleted previous PR for configuration issues.