-
Notifications
You must be signed in to change notification settings - Fork 834
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
Switch from glTF-Sample-Models to the newer glTF-Sample-Assets #4534
Conversation
It looks like this change to the underlying asset makes the different renders appear to render more similar results. Thus I suggest we update the goldens for model-viewer & filament and go with this. This is based on this analysis - that Babylon and gltf-model-viewer do not change their results, but the end results for everyone is much closer. I suspect that the way the normals were done before were exposing a bug in model-viewer and filament that was not handled but now that the asset is a bit more standard, that bug is no longer exposed. BabylonJS's results on this test do not differ: And filament results differ like model-viewer/three.js: But gltf-model-viewer does not differ: |
I agree with your analysis and suggestion - this is a pretty common type of update we need to do as sample bugs get fixed. |
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.
Upon looking closer at the replaced images, I'm pretty sure there's still some kind of bug, either in the asset or in some of the renderers. Still, that's independent of this change, but worth filing a bug on probably both the sample asset repo and three.js to get to the bottom of it. We should also remove the no-longer-used goldens, but that can happen at a later cleanup stage.
…e#4534) * init submodule for glTF-Sample-Assets. * fix left over popd in fetch khronos gltf-smaples. * remove tests from config.json that no longer have corresponding assets in glTF-Sample-Assets * add more time for tests to run. * using checkout with submodule support. * update goldens for model-viewer + filament for khronos-TextureTransformMultiTest.
Per a previous discussion here, I have switched from the obsolete glTF-Sample-Models git repository for the various models used in the fidelity test to the newer glTF-Sample-Assets repository. I have also switched from a custom git checkout script to just using git submodules.
I had to remove a few tests from config.json unfortunately as they were relying on models that are not available in glTF-Sample-Assets because of licensing issues.