Skip to content
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

Merged
merged 50 commits into from
Feb 14, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
acfd558
Initial commit
fanzhanggoogle Mar 10, 2017
6472e25
Finished most of the Readme
fanzhanggoogle Mar 10, 2017
41432dc
Added json schema file
Mar 10, 2017
66dfde1
Finished the first version
Mar 10, 2017
eae186d
Fix style issue
Mar 10, 2017
3dff72b
Changed extension name
fanzhanggoogle Mar 13, 2017
54db399
Removed support for multiple compression libraries
fanzhanggoogle Mar 13, 2017
65e79d3
Added support for point cloud in the spec
fanzhanggoogle Mar 13, 2017
b132fa0
Fix
fanzhanggoogle Mar 13, 2017
7c48fb7
Refined alternative option
fanzhanggoogle Mar 13, 2017
007e570
Removed unused file
fanzhanggoogle Mar 13, 2017
7e7ff5d
Changes on lexaknyazev's comment
fanzhanggoogle Mar 14, 2017
ca7c8ee
Fixes
fanzhanggoogle Mar 14, 2017
ddf534f
Renaming
fanzhanggoogle Mar 14, 2017
1d4ab96
Fix
fanzhanggoogle Mar 14, 2017
251452d
Address Patrick's comments
fanzhanggoogle Mar 14, 2017
260534e
Added version property and description
fanzhanggoogle Mar 14, 2017
ea273be
Changed indicesCount to indexCount
fanzhanggoogle Mar 15, 2017
198792f
grammar fix
fanzhanggoogle Mar 15, 2017
5956b53
minor fix
fanzhanggoogle Mar 19, 2017
f60ff63
Removed indexCount and vertexCount
fanzhanggoogle Apr 19, 2017
61bd686
Removed attributes in the extension
fanzhanggoogle Apr 20, 2017
2c3a051
Define order of attributes
fanzhanggoogle Apr 24, 2017
3c14401
Fix
fanzhanggoogle Apr 24, 2017
a502855
minor fix
fanzhanggoogle Apr 24, 2017
fba95f4
Merge branch 'master' into KHR_mesh_compression
fanzhanggoogle Jul 27, 2017
c765c2f
Added draco extension to extension README
fanzhanggoogle Jul 27, 2017
0cd8f60
Fix for comments
fanzhanggoogle Aug 8, 2017
6102695
Add fallback for loaders not support the extension.
fanzhanggoogle Aug 9, 2017
63f07ee
Changed last paragraph about the fallback
fanzhanggoogle Aug 9, 2017
a8f4e06
Added support for gltf attribute types that are not defined in Draco
fanzhanggoogle Aug 10, 2017
d0d5fa8
Fixed comma
fanzhanggoogle Aug 10, 2017
1da7ae2
Fix duplicate description
fanzhanggoogle Aug 10, 2017
85673d4
fix nit and added 1) attributes in extension must be subset of attrib…
fanzhanggoogle Aug 10, 2017
99b131c
Removed version property, specify draco bitstream version for the ext…
fanzhanggoogle Aug 10, 2017
306e5bc
Refine conformance
fanzhanggoogle Aug 10, 2017
ae9c8d6
removed version in schema; removed alternative approach
fanzhanggoogle Aug 16, 2017
9039f52
Force to consistently use attribute id to get data
fanzhanggoogle Aug 16, 2017
0cdbd3c
Fix example
fanzhanggoogle Aug 29, 2017
d643d96
minor fix
fanzhanggoogle Sep 12, 2017
ae30e83
Removed point cloud.
fanzhanggoogle Oct 4, 2017
2cba5c2
typo
fanzhanggoogle Oct 9, 2017
e71f0d8
Removed unused image
fanzhanggoogle Oct 9, 2017
e3cce40
Wording fix
fanzhanggoogle Nov 1, 2017
66bfc6d
more wording fixes
fanzhanggoogle Nov 1, 2017
734b3cb
more fixes
fanzhanggoogle Nov 1, 2017
e42d251
Change Draft to Complete
fanzhanggoogle Nov 1, 2017
8e9a344
Change Conformance to Recommended Loader Process; change normative
fanzhanggoogle Nov 2, 2017
1eba79a
Make conformance normative
fanzhanggoogle Nov 2, 2017
cb478d9
Merge branch 'master' into KHR_mesh_compression
pjcozzi Feb 8, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 171 additions & 0 deletions extensions/Khronos/KHR_draco_mesh_compression/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
# KHR_draco_mesh_compression

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

## 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 an schema to use Draco geometry compression libraries in glTF format. This allows glTF to support streaming compressed geometry data instead of the raw data.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a schema

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


The [conformance](#conformance) section specifies what an implementation must to do when encountering this extension, and how the extension interacts with the attributes defined in the base specification.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


## 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a primitive contains an extension property and the extension property defines it's KHR_draco_mesh_compression property, then the Draco geometry compression must be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 `accessor` in extended `primitive` no
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested tweak: "accessor in an extended primitive no longer points to a bufferView"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

longer point 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 or the id is 0, it will be used with `sparse` field to act as a sparse accessor. In this extension, we will ignore the `bufferView` property.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was discussed offline, but to confirm - does this create a scenario for bottom-up loaders where they might fill an accessor with zeros thinking it is for a sparse accessor, but then later it is used for compressed data? Probably not, right? Because a loader will probably only access buffer and bufferView bottom, then access primitive before touching an accessor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. For bottom-up loading, even if the accessor is filled with zeros, it will be accessed through primitive --> attributes where the primitive is extended, so the decompressed data will be used instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

points

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


Usage of the extension must be listed in the `extensionUsed` and `extensionsRequired`.
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the input!
For question 1: Yes. The loader has to have support for Draco decoding to load the asset with Draco compression.
For question 2: That's a good question. It's kinda conflict with the idea that making the assets smaller since it will have more data even than uncompressed, right? We thought about people could add a link of the uncompressed asset to the extension. So if the loader doesn't support decoding, it could download the uncompressed asset. But most exported glTF files might not have that.
May I ask what are the cases you are thinking regarding this question?

Copy link
Contributor

@bghgary bghgary Aug 8, 2017

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be possible to define a fallback efficiently: include primitive.attributes pointing to real accessors, and also define the Draco extension defining those same accessors, and the loader can choose which to use. In this case, an efficient exporter would put the standard attributes and the Draco data into different buffers, so that the loader can choose which it will download, and only download one.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 extensionUsed, if not then download the binary source with uncompressed assets, otherwise download the one with compressed assets. And throughout the loading, it should process the extension consistently, either use it or omit it.

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 accessor since the bufferView should be different because they point to different binary file. It could not use data in compressed binary source at all, but it needs to when the extension is available.

Copy link
Contributor

@donmccurdy donmccurdy Aug 8, 2017

Choose a reason for hiding this comment

The 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:

"attributes": {
    "POSITION" : 11,
    "NORMAL" : 12,
    "TEXCOORD_0" : 13,
    "TEXCOORD_1" : 14
}

And let's assume the Draco extension defines compressed versions of the first three, but not TEXCOORD_1. Then, the exporter should put attributes 1-3 into one buffer, TEXCOORD_1 into another, and the Draco data into a third buffer. Loaders supporting Draco will request buffers 2+3, and loaders not supporting Draco will load buffers 1+2.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 TEXCOORD_1 might start from different byteOffset of the file which will result to different bufferView or buffer. The buffer 2 in your example will actually be different depends on which binary source it belongs to.

Copy link
Contributor

@donmccurdy donmccurdy Aug 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The buffer and byte offset for TEXCOORD_1 should be the same, whether the loader supports Draco or not.

In an efficient case, it would be:

"attributes": {
    "POSITION" : 11,  // accessor located in buffer1.bin
    "NORMAL" : 12, // accessor located in buffer1.bin
    "TEXCOORD_0" : 13, // accessor located in buffer1.bin
    "TEXCOORD_1" : 14 // accessor located in buffer2.bin
}
// extension
"KHR_draco_mesh_compression" : {
  "bufferView" : 5,  // bufferView located in buffer3.bin
  "attributes": ["POSITION", "NORMAL", "TEXCOORD_0"]
}

And then, a loader supporting Draco would load only buffer3.bin and buffer2.bin.

It would also be valid to have all of the above in the same .bin file, just very inefficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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, not that all other nodes stay the same
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not following this sentence. Is the intention to say that only the primitive object is extended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Changed description.

except `primitives`:

```javascript

"mesh" : {
"primitives" : [
{
"attributes" : {
"POSITION" : 11,
"NORMAL" : 12,
"TEXCOORD_0" : 13,
},
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 accessors since there are other properties. For example, WebGL might require these properties to render the assets correctly.
Another reason we don't explicitly omit the attributes is that we want to minimize the change in the spec when Draco extension is presented. In our design, the accessors of attributes could even have other extensions as long as they use the decompressed data instead of from bufferView.

Copy link
Contributor

@donmccurdy donmccurdy Aug 7, 2017

Choose a reason for hiding this comment

The 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 attributes to avoid duplication:

"attributes" : {
    "TEXCOORD_1" : 11
}

... 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 attributes, even if it's already in the Draco buffer? This would enable loaders to treat the Draco extension as optional, and to either load the compressed Draco data, or uncompressed data from another buffer, depending on runtime needs/capabilities. Or we could disallow this case, to keep things simple for the initial version, and say that attributes must be omitted when the Draco extension is used.

In previous comments, attributes was removed, and later re-added to specify Draco attribute order. With the observance that object properties aren't ordered, we then added attributeOrder as well. I'm not sure that we need both. Perhaps we don't need either one?

"indices" : 10,
"mode" : 4
"extensions" : {
"KHR_draco_mesh_compression" : {
"bufferView" : 5,
"indicesCount" : 1000,
"vertexCount" : 500,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: vertexCount and indicesCount are inconsistent. Perhaps consider using either vertexCount and indexCount, or verticesCount and indicesCount? The first seems more natural to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm indice isn't familiar to me: indices is the plural of index. Unless there is some reason that vertex should be singular and indices should be plural, my vote would be:

"indexCount" : 1000,
"vertexCount" : 500,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"attributes" : [
{
"semantic" : "POSITION",
"componentType" : 5126,
"type" : "VEC3",
},
{
"semantic" : "NORMAL",
"componentType" : 5126,
"type" : "VEC3",
},
{
"semantic" : "TEXCOORD_0",
"componentType" : 5126,
"type" : "VEC2",
},
]
}
}
},
]
}

"bufferViews" : [
// ...
// bufferView of Id 5
{
"buffer" : 10,
"byteOffset" : 1024,
"byteLength" : 10000
}
// ...
}

```
We will explain each of the property in the following sections.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

properties

Should we just remove this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bufferView property points to the buffer containing compressed data. The data must be passed to a Draco decoder and decompressed to a mesh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

mesh.

#### indicesCount, vertexCount
`indicesCount` and `vertexCount` are references for verifying the decompression of
mesh.

#### attributes
The decompressed mesh needs to be write to the memory for uploading to GPU,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to the GPU"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

including indices and attributes data. `attributes` is used to define the
order of the attributes. For example, if we have the above `attributes`, then we need to write the attributes to vertex buffer like
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to a vertex buffer"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


--------------------------------------------------
| POSITION | NORMAL | TEXCOORD_0 |
--------------------------------------------------

`componentType` and `type` are duplicated properties in
`accessor` of `attributes`. The purpose of having these in the extension is to
define how to write the decompressed mesh to buffer for uploading to GPU without
refering to the sibling `attributes` node.

The extension currently don't support morph targets, e.g. `targets` is used in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"don't" -> "doesn't"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

`primitive`.

#### Restrictions on geometry type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using this extension, the mode of primitive must be either TRIANGLES or TRIANGLE_STRIP and the mesh data will be decoded accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. Sounds good to us.
cc @ondys

`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:
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"loading a glTF asset"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

glTF.
* When encounter a `primitive` with the extension the first time, must process the extension first. Get the data from the pointed `buffer` in the extension and decompress the data to a geometry of a specific format, e.g. Draco geometry.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"encountering"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"for the first time"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"you must process"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pointed buffer" or "pointed bufferView?" The extension references the bufferView, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decompress the data to a geometry of a specific format, e.g. Draco geometry

I'm not following this; isn't the data already in the Draco format and now we want to decompress it into a raw format?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The extension refers to a bufferView. Changed.

Yes. The purpose is eventually decompress to a raw format. I just separated it into 1) decompressing data using the library, 2) writing the mesh to raw format. I figured it will be a little cleaner.

* Write the indices and vertex data of the mesh to separate buffers. This allows for uploading data with different target including ARRAY_BUFFER and ELEMENT_ARRAY_BUFFER. When writing attributes to a buffer, the order should follow property `attributesOrder` as discribed above.
* Then, process `attributes` and `indices` properties of the `primitive`. When handling `accessor`, insteading of getting data from the regular `bufferView --> buffer`, get the data from the buffers created in previous step.

It is pretty straigtforward for top-down loading of glTF file, e.g. only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"a glTF asset"

We generally don't say file since, for example, binary glTF (.glb) may be part of a byte stream in memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

decompress the geometry data when a `primitive` is met 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 enought to change the loader to ignore `accessor` without `bufferView` in glTF 2.0. But we are
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we are definitely open to change this if there actually is some use cases that require loading accessor independently.

Probably remove this from the spec, but put it at the top of the PR so folks in the community reviewing this will know you are open to feedback. I'll help spread the word on twitter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to the description of PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enough

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

definitely open to change this if there actually is some use cases that require
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are some

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

loading `accessor` independently.
Copy link
Contributor

@donmccurdy donmccurdy Aug 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per comment above, could I suggest that the attributes list be omitted, or that we clarify how it must be used in conjunction with Draco?


## Alternative Approach
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps put this in an appendix (still in this README.md).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually perhaps this does belong in a separate .md file, then this file can link to it as an alternative that was considered just like the OpenGL extensions have the FAQ sections.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we are definitely open to discussions about the choice we made.

This is also probably better to put in the PR than the spec.



**Figure 2**: Structure of extension.
![](figures/decompressed.png)
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"$schema" : "http://json-schema.org/draft-04/schema",
"title" : "attribute",
"type" : "object",
"properties" : {
"semantic" : {
"type" : "string",
"description" : "Mesh attribute semantic. e.g. POSITION, NORMAL, TEXCOORD, COLOR, JOINT, WEIGHT and the form [sementic]_[set_index] for TEXCOORD and COLOR.",
},
"componentType" : {
"type" : "integer",
"description" : "The datatype of components in the attribute.",
"enum" : [5120, 5121, 5122, 5123, 5125, 5126],
"gltf_enumNames" : ["BYTE", "UNSIGNED_BYTE", "SHORT", "UNSIGNED_SHORT", "FLOAT"],
"gltf_detailedDescription" : "The datatype of components in the attribute. All valid values correspond to WebGL enums. The corresponding typed arrays are `Int8Array`, `Uint8Array`, `Int16Array`, `Uint16Array` and `Float32Array`, respectively.",
"gltf_webgl" : "`vertexAttribPointer()` type parameter"
},
"type" : {
"type" : "string",
"description" : "Specifies if the attribute is a scalar, vector, or matrix.",
"enum" : ["SCALAR", "VEC2", "VEC3", "VEC4", "MAT2", "MAT3", "MAT4"],
"gltf_detailedDescription" : "Specifies if the attribute is a scalar, vector, or matrix, and the number of elements in the vector or matrix."
},
}

},

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{
"$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."
},
"indicesCount" : {
"type" : "integer",
"description" : "The number of indices of this primitive.",
"minimum" : 0
},
"vertexCount" : {
"type" : "integer",
"description" : "The number of vertex of this primitive.",
"minimum" : 0
},
"attributes" : {
"type" : "array",
"description" : "The information for correctly writing attributes of decompressed geometry to a buffer.",
"items" : {
"allOf" : [ { "$ref" : "attribute.KHR_draco_mesh_compression.schema.json" } ],
"description" : "Information of each attribute that is in the compressed geometry.",
}
},
},
"additionalProperties" : false,
"required" : ["bufferView", "attributes"]
}