search

donmccurdy / glTF-Transform

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

GLB Blob misidentified as GLTF #916

Closed elalish closed 1 year ago

elalish commented 1 year ago

Describe the bug During debug of https://github.com/google/model-viewer/discussions/4227, it looks like gltf-transform fails to load the attached file when its URL has been converted into a Blob, due to detectFormat returning that it's a GLTF, followed by a failure to parse the binary as JSON.

To Reproduce Steps to reproduce the behavior:

  1. Go to the mv editor
  2. Load the 3DModel_LowPoly.glb from the link above.
  3. Note the SpecGloss conversion never completes.
  4. If you set it to break on caught errors you can see where the problem is in gltf-transform.

Expected behavior A Blob URL should parse like any other.

Versions:

elalish commented 1 year ago

Okay, actually I think the problem is that this file is Draco compressed and I failed to register the dependencies after installing the extensions. So we can either close this or maybe write a clearer error message 😄. I found your answer here, though I'm struggling to convert that into TS-style - ideally I would just lazy-load the decoder somehow, though I'm not even sure their package is a module.

One related question: If I read a draco-compressed file and then write it out again, will it write it with the same compression settings as it came in with?

elalish commented 1 year ago

Okay, well considering your fix, I guess it was two things. Thanks!

donmccurdy commented 1 year ago

I do wish Draco were packaged differently. You'll need to modify it by hand to import as a module, I think.

If I read a draco-compressed file and then write it out again, will it write it with the same compression settings as it came in with?

I don't think I have access to the original compression settings, and the original bitstream cannot be reused here, so it'll export with default compression. To avoid that, on the CLI I'll log a warning like...

Decoded {extensionName}. Further compression will be lossy.

... and then output the model uncompressed, unless the user re-applies compression themselves. Trying to avoid repeatedly applying lossy compression to the same file.

elalish commented 1 year ago

Ah, that's good to know - I probably shouldn't bother supporting auto-converting Draco-compressed files anyway then. I'll find some way to log a warning instead.