-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Draft: MESHOPT_compression #1702
Conversation
@@ -0,0 +1,132 @@ | |||
# MESHOPT\_compression |
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.
Consider MESHOPT_mesh_compression
. 😇
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.
Note that this extension is more general and allows compressing animation data as well. Maybe view_compression
would be appropriate?
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.
Ah, good point. In that case view_compression
sounds reasonable, or I would be open to considering this an exception to the <scope>_<feature>
guideline too.
The JSON structure looks good to me, given the context and longer discussion in #1684. I'll probably have comments on various details later, but nothing that seems important yet.
I would suggest an appendix in README.md for now, but a separate document would also be fine.
A larger comparison table would be very helpful — would it be possible to include decode times as well? |
Yeah, sure thing. Brief non-scientific experiment (will include decode numbers in the future) on Buggy.gltf (500K triangles, 400K vertices):
On models of a massive scale the difference can be more pronounced especially with the SIMD variant, here's Thai Buddha statue (https://sketchfab.com/3d-models/thai-buddha-cba029e262bd4f22a7ee4fcf064e22ee, 6M triangles / 3M vertices):
"simd" above refers to WASM SIMD variant that is supported on Chrome Canary when experimental SIMD support is enabled. |
Incorporate details about filters and address the review feedback regarding resampling.
I've updated the extension spec to the latest version. Will work on bitstream specification in the next few days; comments welcome otherwise. We'll probably need a better extension name but that can wait. The new changes added since last update make the extension more competitive from the transmission size perspective. Here's a comparison table in terms of encoded size (see performance numbers above, the ballpark is about the same) for models from glTF-Sample-Models, excluding texture data:
These were generated as follows:
The use of From the numbers above it seems like this extension is an excellent all-rounder alternative to Draco. With recent additions it supports all glTF features (e.g. now supports efficient encoding of line lists and other topologies), and is future-proof - for example, the parts of this extension that allow compressing animation data (notably the quaternion/exponential filters) apply in the exact same way to EXT_mesh_gpu_instancing without any schema changes because of the orthogonal extension design. My goal over the next week is to document the bitstream and filters, correct any inconsistencies that arise, and possibly add a color filter which I haven't done in the specification yet. Any and all feedback is welcome. |
The spec has been updated with detailed documentation on encoding for all modes and filters. |
Thanks for the detailed numbers here! For easier review I've formatted them in a sheet. three.js will very likely support this extension: the improved decoding speed certainly fills a gap that our users have repeatedly reported. By that I mean, in addition to it being possible to implement the extension externally with the upcoming extension API, I think we'd support including the extension in the repo and npm package if that's of interest. A couple minor spec comments. So far, I think all enum values in the glTF spec fall into one of two cases:
From that perspective, the |
Primitive mode is a small integer, which was part of the motivation here, another part being a slight reduction in glTF size. It's not particularly meaningful so this could be changed to a string. edit ah I just realized after writing this that primitive mode happens to match GL values that instead of using larger 2-byte values use small integers there.
Yeah that's correct. |
Is upper case or lower case more consistent with recent additions? It seems that upper case is perhaps a touch more prevalent, but e.g. camera types and node paths use lower case. |
Light types are lowercase as well; I think that's the only extension to define an enum-like property since the core spec. @lexaknyazev you'd mentioned some opinions on this before (#1560 (comment)), do you have a preference moving forward? |
Separate question: I'd like to rename this to EXT-prefixed extension as it makes more sense to do so - the encoding format/support comes from meshoptimizer/gltfpack but with the spec and existing implementations for three.js and Babylon.JS (both are currently within meshoptimizer repo but can be contributed upstream once this extension lands) this seems to be a cross-vendor extension. With that in mind, what's a good name? Options OTOH
Other variants? |
If the implementations will land upstream — i.e. be accepted by the three.js and/or babylon.js projects — then I agree that the On names, I'd be tempted to use
1 I say "arguably" because it somewhat resembles the double prefixes used in WebGL, like |
Right, that's what I meant, although this kinda goes both ways - if this extension is EXT_ it's more likely to be upstreamable, and if this isn't it's less likely. My main motivation for EXT_ is that this extension fills large gaps in functionality. Without it, entire classes of valid glTF models don't have a valid compression solution in glTF ecosystem. (already mentioned earlier, but this includes point clouds, lines, morph targets for geometry, and animation data) The multi-vendor implementation is, I suppose, more of a consequence. I like |
+1 to It might be just me, but I have an aversion to |
The open questions are:
Unsure how to answer these atm. With some other extensions, I've seen renaming done in-place by editing the PR, or renaming lead to opening a new PR. Which one makes more sense in this case, given that I'd like to move this from "draft" to an actual proposal? |
I think this is up to the preference of the author. When a PR is created in GitHub's web UI it might be easier to restart than to rename folders. It's fine to edit the PR, though.
I think my preference is #1702 (comment) but I'm hoping others may have thoughts on this... I think it's less a question of a "right" answer than of having a rough guideline for the future. @emackey @bghgary @lexaknyazev ? |
I don't think I have a strong opinion. The core enums are all legacy from when glTF 1.0 and prior were targeting the WebGL API directly. Was there ever a reason to prevent non-WebGL enums once that link was broken in 2.0? I'm not sure. In general, consistency is good for the ecosystem. So that would suggest either following the pattern of previous extensions, or explicitly establishing a better pattern for future extensions. |
The existing suboptimal situation reflects three possible enum sources (it's all glTF 1.0 legacy):
To follow the existing design, this extension's JSON should use uppercase strings. Bitstream is not affected by any of this, of course. I'd refrain from "establishing a better pattern for future extensions" unless we're talking about glTF Next. |
@lexaknyazev Thanks! The distinction is clear, I'm going to amend this to use upper-case strings then. |
Since the numbers in the initial post for this PR are out of date and the extension name is going to change, I'm going to close this PR and open a new PR with up-to-date information and new name/spec. |
This is a draft of MESHOPT_compression extension. I'm sharing this early to get feedback that might drastically change the JSON structure here or get extra input on the rest of the proposal.
Rendered version (Updated 4/15/2020)
This extension is used by gltfpack to implement a substantially different type of compressor vs existing offering. Some of this is covered in the overview, but briefly:
Quick examples: (please note that this just two files, I will need to build a larger comparison table later)
BrainStem.glb, original: 3.1 MB
BrainStem.glb, quantized: 1.1 MB (this also does other types of optimization on the file)
BrainStem.glb, quantized + deflate: 560 KB
BrainStem.glb, with this extension: 440 KB
BrainStem.glb, with this extension + deflate: 330 KB
BrainStem.glb, with Draco: 1.4 MB (animations aren't compressed, unfair example)
Corset.glb, original: 660 KB
Corset.glb, quantized: 340 KB
Corset.glb, quantized + deflate: 180 KB
Corset.glb, with this extension: 145 KB
Corset.glb, with this extension + deflate: 103 KB
Corset.glb, Draco: 112 KB
The BrainStem model above has animation data; the Corset.glb is just a static mesh but it has PBR-ready data set. In my experience, Draco tends to win noticeably on very large meshes with just position + normal data - I'm working on making my codec stronger in this case.
However, all comparisons above are unfair in one way or another to Draco - for example I am not matching quantization settings here and using whatever Draco model comes with glTF-Sample-Models - and in general Draco will win vs this extension in terms of pure size comparison when Draco fully supports the models. So the examples above are intended to highlight the wins that come from this extension, and Draco numbers are just ballpark because I know the question will come up. This extension is designed to complement Draco by providing a full compression solution aimed at high decoding speeds and specification simplicity, not to replace Draco fully in cases where mesh compression ratio is the only important criteria.
Note that the details of the bitstream are still in flux. It is my intention to update this extension with the bitstream encoding as it gets finalized; alternatively, the bitstream can be hosted in a separate document. It's much simpler than Draco's but a formal definition may still take significant space? Unsure.
I am hoping to be able to finalize the bitstream details either by the end of the year, or early next year. Any and all feedback welcome.