draco icon indicating copy to clipboard operation
draco copied to clipboard

Expectations for mesh indices: optional? auto-generated if not set?

Open donmccurdy opened this issue 4 years ago • 7 comments

I've run into a user-reported issue in the glTF-Transform project, which supports both encoding and decoding glTF files using Draco compression: https://github.com/donmccurdy/glTF-Transform/issues/280. The provided glTF file does not specify that any indices exist, which is the first time I've encountered that in a Draco-compressed glTF file. Indices are generally optional in glTF, but this case is ambiguous in the KHR_draco_mesh_compression spec. Despite the glTF file not referencing any indices, calling decoder.GetTrianglesUInt32Array does indeed return an array of ascending integers, i.e. the same result I'd get if we generated indices for an existing un-indexed mesh without merging vertices.

I'm hoping to understand the Draco library's expected behavior around indices. Reading the Encoding API documentation I'd previously assumed that calling AddFacesToMesh was required, and that you needed to generate an index list if you didn't already have one. Furthermore, compression (except sequential) may reorder and merge vertices — which would lead me to assume that even an originally unindexed mesh may be indexed after compression?

donmccurdy avatar Jun 28 '21 17:06 donmccurdy

/cc @UX3D-nopper the glTF file mentioned in the issue above was generated by Gestaltor, do you know whether this is expected behavior?

donmccurdy avatar Jun 28 '21 17:06 donmccurdy

/cc @lilleyse @emackey ... it looks like gltf-pipeline also expects Draco-compressed primitives to have indices, so just FYI on this question.

https://github.com/CesiumGS/gltf-pipeline/blob/490f8941db9e25affeaa60738a7ea7e07f5ca99a/lib/replaceWithDecompressedPrimitive.js#L32-L34

donmccurdy avatar Jun 28 '21 17:06 donmccurdy

We will have a look at it.

UX3D-nopper avatar Jun 28 '21 17:06 UX3D-nopper

Reading the Encoding API documentation I'd previously assumed that calling AddFacesToMesh was required, and that you needed to generate an index list if you didn't already have one

That was my assumption as well, at least with the JS API.

gltf-pipeline will generate indices if they don't exist for that reason. replaceWithDecompressedPrimitive is called after the compression step so the indices will be guaranteed to exist. But this could be a concern for https://github.com/CesiumGS/gltf-pipeline/pull/572 which handles glTFs wth Draco more generically.

lilleyse avatar Jun 28 '21 17:06 lilleyse

In Gestaltor, we use sequential indices for encoding if no indices accessor is given to call draco::Mesh::AddFace(...) as the specification does not enforce an indices accessor and the indices are optimized by draco anyway. In the decoding case, if no indices accessor is given, resulting Draco indices are ignored. With this, vertices are rendered sequentially, again. As the meshes still look fine, I suppose, the Draco API is designed for that.

UX3D-kanzler avatar Jun 29 '21 15:06 UX3D-kanzler

Thanks @UX3D-kanzler! That explanation makes sense to me. My encoding implementation takes a different approach, but it seems like a decoding implementation can easily support both, there isn't much difference from the client's perspective. I'd suggest we put some clarification in the KHR_draco_mesh_compression extension though, such as:

The Draco library offers two compression methods: "edgebreaker" and "sequential". The "edgebreaker" method may change the order and number of vertices, and primitive indices MUST be specified when using this method. The "sequential" method preserves order and number of vertices, and primitive indices are optional in this case. Client implementations may follow the same decoding approach (see: Conformance), regardless of the compression method, processing indices only if they are specified in the glTF primitive definition.

^Does this sound reasonable to you @lilleyse? I'd also be interested in a sanity-check from someone on the Draco team. :)

donmccurdy avatar Jun 29 '21 18:06 donmccurdy

In Gestaltor, we use sequential indices for encoding if no indices accessor is given to call...

I'm hitting a user-reported error with this method, see https://github.com/donmccurdy/glTF-Transform/issues/289. Briefly, I'd assumed that Draco would not change the vertex count when using the sequential encoding method. However, sometimes it does, for unknown reasons, when I've generated sequential indices. I can prevent that (in the sample I have) by welding vertices with some non-zero tolerance and non-sequential indices, but since I don't know exactly when Draco chooses to merge vertices, it's difficult to be sure the count will never change. When encoding glTF files using morph targets, it's critical that the vertex count is not changed, so I avoid the edgebreaker method for these models and was surprised to find the same problem with the sequential method. This is using the draco3dgltf package.

donmccurdy avatar Jul 25 '21 01:07 donmccurdy