donmccurdy / glTF-Transform

glTF 2.0 SDK for JavaScript and TypeScript, on Web and Node.js.
https://gltf-transform.dev
MIT License
1.42k stars 150 forks source link

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

Closed allcreater closed 6 months ago

allcreater commented 7 months ago

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:

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

donmccurdy commented 7 months ago

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.

allcreater commented 7 months ago

@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 6 months ago

Thanks @allcreater! This should be fixed shortly with: