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

Draco compression probably breaks CESIUM_primitive_outline extension #630

Closed
javagl opened this issue Mar 14, 2023 · 4 comments · Fixed by #631
Closed

Draco compression probably breaks CESIUM_primitive_outline extension #630

javagl opened this issue Mar 14, 2023 · 4 comments · Fixed by #631

Comments

@javagl
Copy link
Contributor

javagl commented Mar 14, 2023

This is strongly related to #629 , and may just be another "instance" of the same problem:

The Draco compression restructures the meshes in a non-obvious way. Every extension that is added to a glTF asset for a given mesh structure will likely break when Draco is applied to this asset. Specifically, the CESIUM_primitive_outline extension defines a reference to some indices, but when the topology of the mesh is changed by Draco, then these indices will become invalid.

I don't know the necessary technical details of Draco to say how much effort it is (or whether it is even possible) to avoid this. In any case, each extension that should "survive" the Draco compression will have to be taken into account by the gltf-pipeline (and CESIUM_primitive_outline does not seem to be taken into account right now.


EDIT: This is in fact a duplicate of #558 , and all these issues might be specific cases of #579 . One suggestion in a comment there was to use --draco.compressionLevel=0 to still have some compression, but keep the indices intact. Maybe that helps (I haven't tried it out yet).

@kring
Copy link
Member

kring commented Mar 15, 2023

each extension that should "survive" the Draco compression will have to be taken into account by the gltf-pipeline

Well... yes and no. gltf-pipeline's Draco compression needs to know that a particular index buffer refers to vertices so that it can rewrite the index buffer when the vertex order changes. But it doesn't need to know about CESIUM_primitive_outline specifically. For example, I can imagine annotating the extension's JSON schema with enough information so that gltf-pipeline knows how to do this (as long as it can find the JSON schema for any given extension). Or a user could even supply a command-line argument that says "the indices property of CESIUM_primitive_outline identifies an index buffer that should be updated according to the renumbered vertices." I don't mean to make it sound trivial, cause it's not, but it would be nice to not need to code each extension into gltf-pipeline explicitly.

@javagl
Copy link
Contributor Author

javagl commented Mar 15, 2023

How exactly it will determine which indices have to be updated is one question. Certain extensions could be supported by default, and one could think deeply about how to offer further options beyond that (e.g. at the command line, ranging from --updateAcessors=[3,14,59] to some JSON-path-regexes like --updateAccessors=["*.CESIUM_primitive_outline.indices"] or so...).

The more challenging point is how that rewriting itself should take place. Iff the number of indices remains the same, one could try to build some map<int,int> oldToNewIndices there, but whether that mapping can trivially be reconstructed by comparing the Draco input and output, whether that can be used to update other index sets without breaking possible "semantics" that these other indices might have is not so obvious. And... when the number of vertices or indices changes, rewriting other index buffers might not even be possible....

@lilleyse
Copy link
Contributor

Possibly related issue: google/draco#584

@javagl
Copy link
Contributor Author

javagl commented Mar 15, 2023

There are some related issues, e.g. google/draco#326 or google/draco#363

But I gave this a stab.

Disclaimer:

I have no clue how Draco works, on any level, and how it is integrated into (and used in) the gltf-pipeline. I compressed the "Duck.glb" sample model with gltf-pipeline and compressionLevel=10, and had a look at the result. From what I've seen, the KHR_draco_mesh_compression extension object is only covering the attributes. The indices seem to remain unaffected. The number of vertices (i.e. e.g. the size of the POSITIONS accessor) remained the same.

Specifically: I don't know whether this is generally true, or whether gltf-pipeline "intentionally" does not modify the indices.


What I've done now:

  • created a simple unit box model, added the CESIUM_primitive_outline extension (and yes, I manually fiddled with the indices there)
  • compressed it with gltf-pipeline, once with compressionLevel=0 and once with compressionLevel=10
  • tried to load the uncompressed and compressed results in CesiumJS

The uncompressed one worked fine. The compressed ones caused the expected

An error occurred while rendering. Rendering has stopped.
TypeError: Cannot read properties of undefined (reading 'count')
at loadAccessor (https://sandcastle.cesium.com/CesiumUnminified/Cesium.js:108653:36)
...

Looking at the GLBs, I noticed that the accessor that the CESIUM_primitive_outline object referred to had simply been removed. A possible fix for this is in #631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants