-
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
KHR_draco_mesh_compression #874
Changes from 24 commits
acfd558
6472e25
41432dc
66dfde1
eae186d
3dff72b
54db399
65e79d3
b132fa0
7c48fb7
007e570
7e7ff5d
ca7c8ee
ddf534f
1d4ab96
251452d
260534e
ea273be
198792f
5956b53
f60ff63
61bd686
2c3a051
3c14401
a502855
fba95f4
c765c2f
0cd8f60
6102695
63f07ee
a8f4e06
d0d5fa8
1da7ae2
85673d4
99b131c
306e5bc
ae9c8d6
9039f52
0cdbd3c
d643d96
ae30e83
2cba5c2
e71f0d8
e3cce40
66bfc6d
734b3cb
e42d251
8e9a344
1eba79a
cb478d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
## Alternative Approach | ||
|
||
We also thought about other designs for adding geometry compression as an extension, but we don't think they are as good as the proposed one. We will describe one here in case someone is interested. | ||
|
||
|
||
The idea is to add an extension object `decompressedBuffer` to `bufferView` instead of `primitive`. See the following picture for the structure of the alternative approach. | ||
`decompressedBuffer` will work like `bufferView` but pointing to compressed data. The extension will basically declare that the data pointed by the `bufferView` is compressed geometry data. Then for bottom-up loading, the process is to decompress the data when the `bufferView` is loaded, and store the data as a normal buffer of `bufferView`. So after decompression `bufferView` should work the same with or without the extension. For top-down loading, when `accessor` requires the data through `bufferView` the first time, the loader needs to look at the decompressed mesh data of `bufferView` instead of regular `bufferView` --> `buffer` approach. | ||
|
||
|
||
The advantage of this alternative approch is that it has less impact on the modularity of layers of standard glTF spec. For example, for bottom-up loading, it only requires to load `decompressedBuffer` between `buffer` and `bufferView` so that it could replace the data used by `bufferView`. However, it will change the `bufferView` that its `buffer` property will not be used any more. Another disadvantage is that it is not good for high level understanding and looks hacky. The proposed approach is mush easier to understand and the structure is clear since the compression is applied on the mesh/primitive objects. But we are definitely open to discussions about the choice we made. | ||
|
||
|
||
**Figure 2**: Structure of extension. | ||
![](figures/decompressed.png) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
# KHR_draco_mesh_compression | ||
|
||
## Contributors | ||
|
||
* Fan Zhang, Google, <mailto:[email protected]> | ||
* Ondrej Stava, Google, <mailto:[email protected]> | ||
* Frank Galligan, Google, <mailto:[email protected]> | ||
* Kai Ninomiya, Google, <mailto:[email protected]> | ||
* Patrick Cozzi, Cesium, [@pjcozzi](https://twitter.com/pjcozzi) | ||
|
||
## Status | ||
|
||
Draft | ||
|
||
## Dependencies | ||
|
||
Written against the glTF 2.0 spec. | ||
|
||
## Overview | ||
|
||
This extension defines a schema to use Draco geometry compression libraries in glTF format. This allows glTF to support streaming compressed geometry data instead of the raw data. | ||
|
||
The [conformance](#conformance) section specifies what an implementation must do when encountering this extension, and how the extension interacts with the attributes defined in the base specification. | ||
|
||
## glTF Schema Updates | ||
|
||
Draco geometry compression library could be used for `primitive` by adding an `extension` property to a primitive, and defining its `KHR_draco_mesh_compression` property. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless the loader doesn't support Draco and use the fallback uncompressed data? |
||
|
||
The following picture shows the structure of the schema update. | ||
|
||
**Figure 1**: Structure of geometry compression extension. | ||
![](figures/structure.png) | ||
|
||
In general, we will use the extension to point to the buffer that contains the compressed data. The major change is that the `accessor` in an extended `primitive` no | ||
longer points to a `bufferView`. Instead, the `attributes` of a primitive will use the decompressed data. This is valid because in glTF 2.0, `bufferView` is not required in `accessor`, although if it is not present, it will be used with `sparse` field to act as a sparse accessor. In this extension, we will ignore the `bufferView` property. | ||
|
||
Usage of the extension must be listed in the `extensionUsed` and `extensionsRequired`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will a glTF asset with Draco compression fail to load with a loader that doesn't support Draco? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to create an asset that has both Draco compressed geometry and core non-compressed geometry? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the input! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The scenario I was imagining is where an asset is hosted on a server. If I want that asset to be the most compatible with all loaders, I would want an asset that can serve both core and Draco compressed. I would have expected that a single asset will be able to do this. Clients that understand Draco will download only the compressed bits. Clients that don't will continue to download the core bits. I am worried using extensionsRequired will cause fragmentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be possible to define a fallback efficiently: include But, IMO this might be unnecessarily complex for v1 of this spec. I would be OK with saying that the Draco extension is always required, and does not provide a fallback, for v1. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks a good solution. Just two thoughts. One thing is that a loader should determine whether or not support the extension when checking I think the fallback has one problem left that if the loader supports the extension but there's some attributes, e.g. user defined generic attributes, that are not compressed in the extension. In this case, that attribute will have conflict There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good on the first point. I don't think I understand the second. Taking this example again, with four uncompressed attributes:
And let's assume the Draco extension defines compressed versions of the first three, but not But, that choice of buffers should probably be an implementation note and not part of the spec. It would be technically valid for an exporter to put all the compressed and redundant uncompressed data into one buffer, even though that is inefficient. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might not completely understand your design. In your example, for the binary source with Draco and without Draco, the buffer for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The buffer and byte offset for In an efficient case, it would be:
And then, a loader supporting Draco would load only It would also be valid to have all of the above in the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. Thanks for the clarification. I was more thinking of the case where all binary data is in one .bin file. But you are right, it's more implementation-wise problem, not the spec. |
||
|
||
```javascript | ||
"extensionsUsed" : [ | ||
"KHR_draco_mesh_compression" | ||
] | ||
|
||
"extensionsRequired" : [ | ||
"KHR_draco_mesh_compression" | ||
] | ||
|
||
``` | ||
|
||
The extension then could be used like the following, note that all other nodes stay the same | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Below is an example of what a part of a glTF file may look like if the Draco extension is set. Note that all other nodes stay the same except There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
except `primitives`: | ||
|
||
```javascript | ||
|
||
"mesh" : { | ||
"primitives" : [ | ||
{ | ||
"attributes" : { | ||
"POSITION" : 11, | ||
"NORMAL" : 12, | ||
"TEXCOORD_0" : 13, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The attributes above are ignored when the extension is present, correct? Or do these accessors have some special meaning to the Draco extension? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In practical, for an implementation of loader like ThreeJS, it might be OK to ignore the attributes. But it is preferred not to ignore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I assume you mean that the primitive might list additional attributes here, beyond those encoded in the Draco buffer? If so, then let's clarify this example by changing
... in which case, a loader would be expected to load POSITION, NORMAL, and TEXCOORD_0 from the Draco buffer, and then to add TEXCOORD_1 from a traditional accessor. Alternatively, do we consider it valid to include POSITION in In previous comments, |
||
"indices" : 10, | ||
"mode" : 4 | ||
"extensions" : { | ||
"KHR_draco_mesh_compression" : { | ||
"bufferView" : 5, | ||
"attributesOrder" : [ | ||
"POSITION", | ||
"NORMAL", | ||
"TEXCOORD_0" | ||
], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the Draco examples, it doesn't seem like the order of attributes is needed to decode a mesh: if that's the case, is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Don, thanks for the input on the extension. The purpose of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I still don't understand the need for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry that I didn't explain well. You are right. ThreeJS doesn't need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As THREE.GLTF2Loader demonstrates, this decoding can be done without
Given the POSITION and NORMAL data decoded from (1), can't a loader just do:
After steps (1) and (2), this should just work.
I don't think that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way you showed pushing attributes to accessors definitely works if an engine could do that. The problem we are considering is that in this way the primitives won't have any
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry to keep picking nits on this one.. Unfortunately I can't join the meeting tomorrow, and would like to give all the feedback I can before then. :)
The accessors of the primitive are not ordered. If the primitive provides:
and in the extension:
.... the order given in If the intention is to minimize impact on the spec, and also support attributes not compressed in the extension, my suggestion would be:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem at all. It's absolutely great to get more input on the proposal before the meeting. |
||
"version" : "0.9.1" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it have any conformance implications? How should that field be used by implementations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added description for properties and conformance. |
||
} | ||
} | ||
}, | ||
] | ||
} | ||
|
||
"bufferViews" : [ | ||
// ... | ||
// bufferView of Id 5 | ||
{ | ||
"buffer" : 10, | ||
"byteOffset" : 1024, | ||
"byteLength" : 10000 | ||
} | ||
// ... | ||
} | ||
|
||
``` | ||
We will explain each of the property in the following sections. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. properties Should we just remove this line? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed. |
||
#### bufferView | ||
The `bufferView` property points to the buffer containing compressed data. The data should be passed to a mesh decoder and decompressed to a | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
mesh. | ||
|
||
#### version | ||
The version of Draco encoder used to compress the mesh. This is used for verifying compatibility of Draco encoder and decoder. With this property, the loader could easily determine if the current decoder supports decoding the data. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is version compatibility defined? Strict There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. We have fixed isVersionSupported() in 1.0 release. I'll send the fix to 3JS shortly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, but, could you also verbally define what compatible versions would be, here in the spec? Or link to a source that explains this, e.g. http://semver.org/? I just worry about saying "This is used for verifying compatibility" without saying how it must be used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the advice. I added a sentence that might help. We don't have explicit version support description right now but will add that shortly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good, thanks. |
||
|
||
### attributesOrder | ||
`attributesOrder` defines the order of attributes stored in the decompressed geometry. E.g, in the example above, `POSITION` will have attribute id 0 in the decoded mesh, and `NORMAL` as 1, `TEXCOORD_0` as 2. The id should be used along with `accessors` to correctly load all attributes from extension. | ||
|
||
#### Restrictions on geometry type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When using this extension, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
When using this extension, the `mode` of `primitive` could only be one of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a future idea for the Draco team outside the scope of this extension, there are use cases for line compression, especially in the geospatial world, e.g., roads, rivers, county borders, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Acknowledged. Sounds good to us. |
||
`POINTS`, `TRIANGLES` and `TRIANGLE_STRIP` and the mesh data will be decoded accordingly. For example, if `mode` is `POINTS`, then the | ||
decompressed geometry will be a point cloud. | ||
|
||
### JSON Schema | ||
|
||
For full details on the `KHR_draco_mesh_compression` extension properties, see the schema: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be cool to try the latest wetzel (CesiumGS/wetzel#4) to generate reference doc to paste in here like the glTF spec has. |
||
|
||
* [extension property](schema/node.KHR_draco_mesh_compression.schema.json) `KHR_draco_mesh_compression` extensions object. | ||
|
||
## Conformance | ||
|
||
To process this extension, there are some changes need to be made in loading a glTF asset. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Below is the process when a loader encounters a glTF asset with the Draco extension set: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
* Check `version` property and verify the version of encoder used for the mesh | ||
is compatible with the current decoder. | ||
* When encountering a `primitive` with the extension the first time, you must process the extension first. Get the data from the pointed `bufferView` in the extension and decompress the data to a geometry of a specific format, e.g. Draco geometry. | ||
* Then, process `attributes` and `indices` properties of the `primitive`. When loading each `accessor`, if there is no `bufferView` then go to the previously decoded geometry in the `primitive` to get indices and attributes data. To load an attriute in extension, it is required to first determine the id of this attribute in using `attributesOrder`, then use the id to get the attribute from decoded mesh. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/attriute/attribute |
||
|
||
It is pretty straigtforward for top-down loading of a glTF asset, e.g. only | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/straigtforward/straightforward |
||
decompress the geometry data when a `primitive` is met for the first time. However, for | ||
bottom-up loading, loading `accessor` before `primitive` will not get the data. It could only be handled when processing its parent `primitive`. This is based on the consideration that it will rarely happen that | ||
loading an `accessor` without knowing its parent `primitive`. And it should be | ||
easy enough to change the loader to ignore `accessor` without `bufferView` in glTF 2.0. But we are | ||
definitely open to change this if there actually are some use cases that require | ||
loading `accessor` independently. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per comment above, could I suggest that the |
||
|
||
## Resources | ||
|
||
* [Draco Open Source Library](https://github.com/google/draco) | ||
* [ThreeJS | ||
Loader](https://github.com/mrdoob/three.js/blob/dev/examples/js/loaders/DRACOLoader.js) | ||
and | ||
[example](https://github.com/mrdoob/three.js/blob/dev/examples/webgl_loader_draco.html) | ||
|
||
|
||
# Appendix: Alternative Approach | ||
|
||
See [ALTERNATIVE.md](ALTERNATIVE.md). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
{ | ||
"$schema" : "http://json-schema.org/draft-04/schema", | ||
"title" : "KHR_draco_mesh_compression extension", | ||
"type" : "object", | ||
"properties" : { | ||
"bufferView" : { | ||
"allOf" : [ { "$ref" : "glTFid.schema.json" } ], | ||
"description" : "The index of the bufferView." | ||
}, | ||
"attributesOrder" : { | ||
"type" : "array", | ||
"description" : "The order of attributes in decompressed geometry.", | ||
"items" : { | ||
"type" : "string", | ||
}, | ||
"minItems" : 1, | ||
"maxItems" : 8, | ||
"gltf_detailedDescription" : "It is used to define the order of attributes stored in the decompressed geometry. The respective attribute id is used to get attribute data." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does "attribute id" mean here? Maybe array's index? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. array's index makes more sense. |
||
}, | ||
"version" : { | ||
"type" : "string", | ||
"description" : "The version of Draco encoder that is used on the compressed geometry data." | ||
}, | ||
}, | ||
"additionalProperties" : false, | ||
"required" : ["bufferView", "attributes"] | ||
} |
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.
All Khronos specifications should contain the standard Copyright Header:
https://www.khronos.org/members/login/groups/Agreements%20and%20Licenses/Open%20Source%20Repository%20Resources/Khronos%20Specification%20Copyright%20License%20Header%20V3%20May17.txt
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.
As the Draco extension is referring to significant external documents – namely the Bitstream specification we should also include, and be consistent with the additional header wording for normative clarity:
https://www.khronos.org/members/login/groups/Agreements%20and%20Licenses/Open%20Source%20Repository%20Resources/Khronos%20Copyright%20License%20Header%20Addition%20for%20NORMATIVE%20CLARITY%20Oct17.txt Note - this has some boiler plate that needs to be tweaked to fit the spec.
The key text in the header is:
Some parts of this Specification are purely informative and do not define requirements
necessary for compliance and so are outside the Scope of this Specification. These
parts of the Specification are marked by the "
Note
" icon or designated "Informative
"<replace/insert specific conventions for the specification here>.
And
Where this Specification includes normative references to external documents, only the
specifically identified sections and functionality of those external documents are in
Scope. Requirements defined by external documents not created by Khronos may contain
contributions from non-members of Khronos not covered by the Khronos Intellectual
Property Rights Policy.
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.
Done