-
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
[new feature] add vertex color morph target #16179
[new feature] add vertex color morph target #16179
Conversation
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://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16179/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/16179/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/16179/merge#BCU1XR#0 |
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://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16179/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/16179/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/16179/merge#BCU1XR#0 |
WebGL2 visualization test reporter: |
Visualization tests for WebGPU |
WebGL2 visualization test reporter: |
Visualization tests for WebGPU |
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 is very good! thanks a lot for your appreciated contribution
cc @alexchuber for the serializer change |
You have several webgpu tests failing though |
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.
I reviewed only the glTF exporter-specific file. Just requested a quick change to accomodate RGB vs RGBA color attributes, but other than that, LGTM!
Thanks for the contribution!
packages/dev/serializers/src/glTF/2.0/glTFMorphTargetsUtilities.ts
Outdated
Show resolved
Hide resolved
packages/dev/serializers/src/glTF/2.0/glTFMorphTargetsUtilities.ts
Outdated
Show resolved
Hide resolved
WebGL2 visualization test reporter: |
Visualization tests for WebGPU |
packages/dev/core/src/Materials/Node/Blocks/Vertex/morphTargetsBlock.ts
Outdated
Show resolved
Hide resolved
packages/dev/core/src/Materials/Node/Blocks/Vertex/morphTargetsBlock.ts
Outdated
Show resolved
Hide resolved
packages/dev/core/src/ShadersWGSL/ShadersInclude/morphTargetsVertexGlobalDeclaration.fx
Outdated
Show resolved
Hide resolved
WebGL2 visualization test reporter: |
Visualization tests for WebGPU |
@alexchuber , can we merge this? |
Yes, sorry for the delay. Thanks! |
see alpha testing changes: BabylonJS/Babylon.js#16144 see new color morph feature: BabylonJS/Babylon.js#16179
Any interest in having a morph target for vertex color? There are some glTF viewers out there that will actually display animated point clouds if you add morph targets for POSITION and COLOR_0. Two that I found are gestaltar and https://gltf-viewer.donmccurdy.com/. Looks like babylon.js doesn't wire in color as a morph target, so I grepped through the code and put it in.
Test plan
I only tried it with a test glb with animated point clouds defined as morph targets. I'm not sure how to correctly test all of this.