-
Notifications
You must be signed in to change notification settings - Fork 79
Add MeshoptCubeTest model #257
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
base: main
Are you sure you want to change the base?
Conversation
|
Please add The PNG files with label textures use 16-bit precision and contain extra metadata. The screenshot image has unneeded alpha channel and extra metadata. Please optimize them by running the following command and force-push the image updates so that only the optimized files reach the main branch. |
|
Cleaned up the images and JSON whitespace and added README.body.md with a full description of the encoding as well as the differences between two model folders. |
|
I had seen this PR, and quickly scrolled over the linked gist, but not yet done a real "review". The first part of the review would be to check whether there is important information in the Gist that is not part of the README (I've seen that there is a lot of information in the README, but haven't cross-checked it). Depending on the intended depth of the review, I may have to allocate a bit more time for that. From a first click on the "Files Changed": The files are called |
|
Good point, I've renamed scene.* to MeshoptCubeTest.* (and scene-fallback.bin to MeshoptCubeTestFallback.bin) |
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 looked at the information from the README, and the information from the Gist, and it seems to be complete.
I tested the glTF variant in https://gltf-viewer.donmccurdy.com/ and https://sandbox.babylonjs.com/ , and it worked properly in both.
I also tested the glTF-Meshopt variant in both viewers, and they failed as expected.
A slight point of confusion: The animations of the cubes in the last column are indeed separate animations. Viewers are usually playing the first animation by default. I wondered whether there might have been an issue with the encoded animations, until I realized that there are multiple of them.
It might be better to combine these animations into one. That's not a very strong opinion, and I don't know whether it's easier to adjust the Gist or the glTFs directly.
Do others have thoughts about this?
Another thing, iff something is changed: I don't see a reason for the KHR_materials_unlit extension. I think it could (and therefore: should) simply be omitted. (Also no strong request, but may be worth considering).
There is a pending discussion about the directory names at #258 (comment) . The directory here could/should probably be called glTF-Meshopt-KHR, but that should be decided in the linked issue.
|
I've updated the scripts in the Gist to merge animations and use normals on the label quads instead of the unlit extension, and regenerated the files in this PR. I've commented in the other PR on names, will wait for resolution to make any changes to folder naming (if the suggestion in that PR is okay then this PR would not need further changes) |
Specification: KhronosGroup/glTF#2517
This model is generated by the script from the following Gist repository using
node generate.jsfor glTF-Meshopt version, which has no fallback data and requires the extension, andnode generate.js truefor glTF version, which has a fallback binary buffer and can be loaded by any viewer, but viewers that support KHR_meshopt_compression will not need to read the fallback buffer.The asset contains a grid of cubes; each column is using the same source data and vertex layout (in order: interleaved with quantized 8-bit normals/colors; separate streams with quantized 8-bit normals/colors; separate streams with quantized 16-bit normals/colors; and the last column is using uncompressed geometry with a 16-bit quantized quaternion animation track). Each row is using a different set of modes and filters with KHR_meshopt_compression extension (in order: no compression; v0 vertex compression and INDICES index compression; v0 vertex compression and TRIANGLES index compression; v0 vertex compression with exponential, octahedral, color and quaternion filters; v1 vertex compression with the same set of filters).
The leftmost column is using interleaved data and as such is not using filters even when the other elements of the same row do use it, because filters only work for a specific set of byte strides.
All colored cubes are expected to look the same. All animated cubes in the rightmost column are expected to look the same (no colors on this column) and rotate in sync.
To load glTF-Meshopt variant you need a viewer that supports KHR_meshopt_compression; it loads and displays correctly in three.js after mrdoob/three.js#32163 (assuming MeshoptDecoder object passed to the GLTFLoader supports KHR functionality, which is true as of meshoptimizer 0.25).