-
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
Export KHR extensions: IOR, Transmission and Volume #12389
Export KHR extensions: IOR, Transmission and Volume #12389
Conversation
The package.json and package.lock should not be in the PR |
Please make sure to tag your PR with "bug", "new feature" or "breaking change" tags. |
Reviewer - this PR has made changes to one or more package.json files. |
This reverts commit 0b87226.
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/12389/merge/index.html#WGZLGJ#4600 To test the snapshot in the playground itself use (for example): https://playground.babylonjs.com/?snapshot=refs/pull/12389/merge#BCU1XR#0 |
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/12389/merge/index.html#WGZLGJ#4600 To test the snapshot in the playground itself use (for example): https://playground.babylonjs.com/?snapshot=refs/pull/12389/merge#BCU1XR#0 |
Visualization tests for webgl1 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. |
Visualization tests for webgl2 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. |
@bghgary I guess the visualisation test should not pass because adding these extensions will definitively change the final webgl result.. |
I don't think any of these tests use the extensions you added, so something is going on. |
packages/dev/serializers/src/glTF/2.0/Extensions/KHR_materials_transmission.ts
Outdated
Show resolved
Hide resolved
packages/dev/serializers/src/glTF/2.0/Extensions/KHR_materials_transmission.ts
Show resolved
Hide resolved
Co-authored-by: Gary Hsu <[email protected]>
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/12389/merge/index.html#WGZLGJ#4600 To test the snapshot in the playground itself use (for example): https://playground.babylonjs.com/?snapshot=refs/pull/12389/merge#BCU1XR#0 |
Visualization tests for webgl1 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. |
Visualization tests for webgl2 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. |
@bghgary may i have the model used for the test (which is related to the GLTF serializer) ?. The only reason i imagine it's it may export an extension (which was not possible before) because of the material, or because of a wrong selecting test into the new extensions. |
You can figure this out by looking at https://github.com/BabylonJS/Babylon.js/blob/579cb2ee652b179b18dbb0605664a99712e803e1/legacy/tests/validation/config.json and search for the title of the test. This will give you a playground to test. |
…ume/Babylon.js into export_KHR_materials_ior
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/12389/merge/index.html#WGZLGJ#4600 To test the snapshot in the playground itself use (for example): https://playground.babylonjs.com/?snapshot=refs/pull/12389/merge#BCU1XR#0 |
Visualization tests for webgl1 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. |
Visualization tests for webgl2 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. |
Already solve some visualisation tests fails, but still stoked on somes such this one where no serializer involved. So far the result in my local playground is the same as the online playground. |
@RaananW Can you take a look when you get back and help us figure out why this is failing? Thanks. |
Seems like a Netlify issue, as some tests use assets from the main page and not the cdn. You can see the error in the logs: ERR Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://www.babylonjs.com/Assets/NeonPipe/glTF/NeonPipe.gltf. (Reason: CORS request did not succeed). Status code: (null). The solution would probably be to move all tests to use the assets cdn. But that's not related to this PR |
Tests are green. Good to go @bghgary ? |
GLTF 2.0 Serializer is missing the
extensions
This PR add support for these extensions to the GLTF2.0 serializer.
Also, some minor modification has been made into the GLTF Loader extensions to be consistant with the GLTF definitions.
Test has been conducted using this standard sample model
Materials has been loaded and serialized succesfully without any diff.