-
Notifications
You must be signed in to change notification settings - Fork 79
Update BrainStem and DragonAttenuation to KHR_meshopt_compression #258
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
Conversation
|
Having these models somewhere is good, and I see the reasoning from the context of the discussion at KhronosGroup/glTF#2517 (comment) (~"there must be sample models for the ratification") But... (probably mainly @lexaknyazev , but also the other "usual suspects") : Should the original EXT meshopt versions be retained? I'm leaning towards keeping the original ones, and adding the new ones. This mainly rooted in ... certain aspects that may not have been thought through back when the structures here have been established:
|
|
Agreed that original EXT variants should be preserved (even if renamed); deferring to @DRx3D on the directory structure. |
|
I think we could rename the old directory to |
|
That sounds reasonable. Iff this is done (whoever is responsible for the final 👍 or 👎 on that...), then the directory from #257 should also be called |
|
One possible alternative I was considering yesterday but didn't suggest is |
|
I'm not sure why this is not visible in the diff, but it seems like the URI to the checkerboard image in the DragonAttenuation asset changed from .png to .jpg
But the image itself is still a .png file. Therefore, I get an error while loading this in sample viewer. |
|
@javagl :
Does this request mean all assets that use extensions need to include the extension name and prefix or would this only apply when the asset comes in different flavors by using different extension(s)? |
|
@DRx3D IIRC, we decided to use |
|
@UX3D-haertl Thanks! This is because my branch was based on an older version that used JPG files but they were later changed to PNG. The resulting conflict is silent as the relevant portions of the file didn't change. I will update this PR to rectify that and also rename the old files to glTF-Meshopt-EXT. |
The updates are performed by converting the source models with gltfpack, using -cz for DragonAttenuation and -cz -vpf for BrainStem (consistent with the original conversion settings), and post-processing the glTF files using `jq` as a pretty-printer.
These are using original files before the KHR update
Specification: KhronosGroup/glTF#2517
The updates are performed by converting the source models with gltfpack 1.0, using -cz for DragonAttenuation and -cz -vpf for BrainStem (consistent with the original conversion settings), and post-processing the glTF files using
jqas a pretty-printer.The resulting files render the same, but require the viewer to support the KHR variant of the extension. While this is typically very easy to do (see mrdoob/three.js#32163 for example), as of this PR no viewer supports it yet, so we may want to wait to merge this until a couple viewers get the KHR support.
The updated files here are not using fallback buffers so the extension is required; see MeshoptCubeTest model for a more complex and varied set of tests that exercise the JSON structure more broadly, this PR is focused on updating the more real-world models so that we have both a synthetic test version and a more realistic one.