donmccurdy/glTF-Transform

Draco compression may produce an UNSIGNED_SHORT index buffer with vertex count > 65535

Closed this issue · 3 comments

allcreater commented

Hello, thank you so much for this library!

Describe the bug
Transforming some models with Draco compression may produce glTF with invalid accessors declaration

To Reproduce
Steps to reproduce the behavior:

  1. Download an attached test model

  2. Run gltf-transform draco ../dangerous_model.glb optimize_test/dangerous_model.gltf

  3. Open the result glTF file in text mode and take a look on the index accessor with "componentType": 5123 (should be 5125)

  4. Despite some of the web-based renderers, like Three.js, Babylon.js can load out model without any problems, some renderers like Google Filament yield an artifacts

Expected behavior
Index accessor pointed on vertex array with size more than 65535 should have a componentType === 5125

Versions:

  • Version: 3.10.1
  • Environment: both CLI and library

Additional context
It looks like the user will encounter the bug on a model that meets two requirements:

  • one of its meshes must have a "borderline" number of vertices, near, but not exceeding, the 65535, so that it may be indexed with uint16 values.
  • Draco compression algorithm must produce additional vertices so that the result number will exceed the border value.
donmccurdy commented

Yikes, thanks for reporting this! Likely related to:

A design goal of glTF Transform is that the document being exported shouldn't change during export, but Draco compression seems to throw a few wrinkles into that plan. More investigation needed.

  • ❤️1
allcreater commented

@donmccurdy thanks for your attention!

If I understand correctly, Draco may increase the number of vertices, it's pretty expected behavior, but the model just saves with incorrect componentType. As a workaround I just change the componentType from 5123 to 5125 and the patched model are correctly loaded by all renderers.

Of course, this trick is possible just with such "lazy" accessors whose data is populated after decompression.

donmccurdy commented

Thanks @allcreater! This should be fixed shortly with:

  • ❤️1