search

donmccurdy / glTF-Transform

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

CLI optimize command takes very long but reports it ran in < 200ms #1435

Open marwie opened 2 weeks ago

marwie commented 2 weeks ago

Describe the bug Running optimize with the DamagedHelmet GLB from the gltf sample assets repository takes much longer than expected and the CLI output logs that it just took 200 ms.

To Reproduce Steps to reproduce the behavior:

  1. Run gltf-transform optimize --compress draco --texture-compress ktx2 "source/DamagedHelmet/DamagedHelmet.glb" "output/DamagedHelmet/DamagedHelmet.gltf-transform.glb
  2. Notice that the process hangs at uastc for an unexpected long time

Expected behavior Compression doesnt take a minute / the CLI reports the correct duration it took.

Versions:

Additional context

The same happens without --verbose I can see multiple KTX processes running that take up 100% of my CPU (which is fine) - it just takes a lot longer than expected.

Output

✔ dedup                3ms
✔ instance             1ms
✔ palette              2ms
✔ flatten              1ms
✔ join                 8ms
✔ weld                 9ms
✔ simplify             21ms
✔ resample             0ms
✔ prune                81ms
✔ sparse               6ms
✔ uastc                133,822ms
✔ etc1s                2,219ms
✔ draco                15ms

https://github.com/donmccurdy/glTF-Transform/assets/5083203/98ea6ef6-0d5c-4b89-aee7-ee07d16316cf

donmccurdy commented 1 week ago

About the time — I imagine this comes from the UASTC settings used in optimize, which are conservative and based on the KTX2 Artist Guide. It's very possible these defaults are too conservative, they are definitely on the slowest end of what the KTX Software UASTC encoder allows.

About the reporting of the time — It's not clear to me what in the output is incorrect, as the snippet above shows 133822ms spent on the UASTC step. Is this a . vs , discrepancy? The CLI is not localized now, but it shouldn't be difficult to use Number.prototype.toLocaleString(), if that's the issue.

marwie commented 1 week ago

Oh you're right that's it - my bad, that's because of the , where i expected a .

donmccurdy commented 1 week ago

I see! Does 1e6.toLocaleString() give a .-separated integer for you, executed in Node.js? I'm not so familiar with localization outside of a browser environment.

marwie commented 1 week ago

It prints 1,000,000 but perhaps because my laptop right now (as well as my workstation) is also set to english