KHR_draco_mesh_compression: Restrictions on accessor IDs

[donmccurdy]

There was a proposed restriction from @JeremieA in PR #874, which I think we mostly agreed with but that didn't make it into the spec before release:

Restrictions on accessors ids

To facilitate converting between compressed and uncompressed versions, and allow loaders to implement decoding as a preprocess step, the accessor ids used for attributes and targets within the primitive must be used for a unique compressed mesh and attribute. Multiple primitives can refer to the same accessor only if it is associated with the same draco compressed geometry and draco attribute id within it. This is similar to the standard case where the data for a given accessor is unique.

(struck the portion about morph targets, which have been left for a future version or new extension)

I'm fine with adding this requirement, although the three.js implementation does not depend on it. We could also wait and see if the issue is raised by others implementing the extension.

[pjcozzi]

CC @FrankGalligan @fanzhanggoogle

[JeremieA]

Thanks for considering my proposed change for inclusion.

One issue with this change that I did not mention, is that while this restriction would allow for greater flexibility for loaders implementation, it would make it a bit more complex for developers of "optimiser" tools such as gltf-pipeline, as it would need to be considered when de-duplicating accessors.

With this restriction in place, two accessors that have exactly the same json content would not be allowed to be replaced by a single instance if both were referenced from different Draco-compressed meshes. This is not an issue for "normal" accessors, as their json content refer to the actual data of the accessor (so their json can only be identical if they refer to the same data). This is definitely fixable in the optimiser implementation (by adding a reference to the Draco bufferView and attribute id within "internal" json attributes, such as extras._pipeline already used within gltf-pipeline, of any accessors that are referenced from this extension, such that this information is considered when looking for duplicate accessors), but it can have an unintended side-effect of requiring special support for this extension on generic optimisation passes that would otherwise not need to be aware of it.

For the proposed restriction, this is only an issue if the Draco compression extension is used as extensionRequired, not when only used as extensionUsed, as in this case the accessors still have a reference to the uncompressed data, so they can only be identical if this data is also identical. This is consistent with the mechanism that a loader (or optimiser) should bail out of processing an asset if it does not support all extensions in the extensionRequired list.

Given the choice, I would still prefer to have the restriction in place, as I guess more developers will implement loaders, which should be as easy to do as possible, than advanced optimiser tools.

Note that similar issues for optimisers are already present for de-duplicating or re-indexing other arrays such as bufferViews for instance. If the list of views is changed and ids have to be updated, the optimiser does need to know that the Draco extension includes a reference to a bufferView that also need to be updated. For this issue one possible solution would be to make bufferView a reserved json property name, that can (or must) only be used by all extensions (and extras?) for referring to ids in the bufferViews array. This way an optimiser can update all references from all extensions without having to understand them. In my current simplistic implementation I used this as an assumption (see here). If this is something that is worth considering to add in the standard, and that is not already covered somewhere else, we could create a separate ticket.

[FrankGalligan]

Hi @JeremieA,
I'm trying to clarify the issue.

For example you have two primitives that have the exact same compressed data and they are referencing the same bufferView. Then you want to make sure that the "attributes" and "indices" in the two primitives must contain the same accessor ids?

I attached two gltf files. One that I think would be valid according to this restriction and one that would be invalid according to the restriction.

clarify_issue_1267.zip

[donmccurdy]

Then you want to make sure that the "attributes" and "indices" in the two primitives must contain the same accessor ids?

^If so, my proposed wording in the original comment should mention indices. That was brought up in #1333, and to my understanding this restriction would be pointless if we are not also restricting index accessor IDs.

[donmccurdy]

I think we are going to need to decide on this and (one way or another) add further detail to the extension specification. Another case that is not handled well by my proposed wording above would be this: suppose two primitives have the same POSITION/NORMAL data and share the same Draco buffer for these attributes. Now suppose each primitive also has a COLOR attribute stored in an uncompressed accessor. I think that would be allowed, but it isn't clear. If so we should amend the restriction to:

Restrictions on accessors reuse

To facilitate converting between compressed and uncompressed versions, and to allow loaders to implement decoding as a preprocess step, a restriction is imposed on accessor reuse. If an attribute or index accessor's data is stored in a compressed bufferView (either solely or with uncompressed fallback), that accessor cannot be referenced anywhere else in the asset, except by another primitive that also uses the same compressed bufferView.

[MiiBond]

I'm unclear on what this restriction means when the Draco extension is required. Currently, when Adobe Dimension outputs Draco, I assign dummy data to the "regular" attributes and indices to satisfy the validator but I expect that this data is never used. When multiple, different meshes are present, the bufferViews are, of course, different but the dummy data is the same.

This works fine in Babylon but three.js thinks the meshes are the same and just uses a copy of the same mesh rather than loading a new one. My assumption was that this was a bug in the Three.js loader but, if this thread implies that regular attributes must be different for different meshes, even if they aren't used, I should change the exporter.

[donmccurdy]

My assumption was that this was a bug in the Three.js loader but, if this thread implies that regular attributes must be different for different meshes, even if they aren't used, I should change the exporter.

That is what the thread implies, yes — if the Draco extension is required and two meshes have dummy POSITION attributes pointing at accessor 0, but the meshes' compressed data is in different bufferviews, it would be invalid with this restriction. In other words, loaders should be able to pre-process an asset replacing dummy accessors with decompressed Draco data, without needing to create any new accessors in the process.

But, I thought we'd managed to avoid relying on this restriction in the three.js loader — do you mind sharing an example that reproduces the issue so I can debug? I'm still open to merging the restriction or omitting it, either way.

[MiiBond]

Sure. I'll send you a sample file.

[donmccurdy]

Thanks, looked into this and it's a bug in the threejs loader. We cache on the accessor indices to determine when a mesh is reused (because glTF meshes cannot be instantiated apart independently of their materials), and didn't account for extensions in that caching. We'll fix that, but I'm still undecided on this restriction..