Add UV2 morph support and load UV and UV2 morph targets from glTF #15602
Conversation
|
To reviewer: I'm not sure how to test the code changes. I only know it works because I have a downstream application that uses node material. Haven't tested other materials at all. |
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
chubei-urus
force-pushed
the
uv2-morph
branch
from
ae8fe37 to
d000b83
Compare
|
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/15602/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/15602/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/15602/merge#BCU1XR#0 |
There was a problem hiding this comment.
The code looks really solid at a first glance, I ll let @deltakosh validate and @bghgary for the gltf part
|
@chubei-urus can you add a gltf visualization test ? so it would not experience nme but already a big part of the code. |
Sure @sebavan! Do you have any pointers on where to add that? |
|
Visualization tests for WebGPU (Experimental) |
you can add a test model in https://github.com/BabylonJS/Assets and then follow the contributing doc here to add the new test https://doc.babylonjs.com/contribute/toBabylon/HowToContribute#visualization-tests Do not hesitate to provide doc feedback so we can improve it if needed :-) |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
|
Seems like some WGSL shader updates are missing, is this PR still a draft? |
I didn't know how to run the vis tests locally. I'll convert to draft and fix the errors before requesting review again. |
Looks good from the glTF perspective. The spec says this FYI:
|
@chubei-urus |
|
Hey there! any update? |
|
Hi! I haven't found time to work on this PR yet. |
|
This pull request has been marked as stale because it has been inactive for more than 14 days. Please update to "unstale". |
chubei-urus
force-pushed
the
uv2-morph
branch
from
d000b83 to
1c4778d
Compare
|
Visualization tests for WebGPU (Experimental) |
There was a problem hiding this comment.
LGTM!
You will also have to update:
https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Meshes/mesh.ts#L3962-L4026
|
Yes, we can keep the PR in draft until you add a proper visu test. |
|
@chubei-urus Do you think you can add the visualization test, as we are waiting for this PR to be merged so we can make another one related to morph targets? Thanks in advance. |
|
Thanks for the heads up. I'll try to do that within this week. |
|
I think I hit an existing bug that's blocking me from adding the vis test. See https://forum.babylonjs.com/t/uv-morphing-results-in-two-textures-of-different-types-use-the-same-sampler-location/55346 |
I'm opening this PR following the suggestion in https://forum.babylonjs.com/t/morph-uv2-and-import-the-morph-target-from-gltf/53558/2.
The PR adds uv2 morphing to the built-in materials as well as the morph target block in the node material. It also modifies the glTF loader to load the TEXCOORD_0 and TEXCOORD_1 morph target attributes.