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

`Expected positive integer for width but received 0 of type number` when running `optimize` #1500

Closed hybridherbst closed 1 month ago

hybridherbst commented 1 month ago

Describe the bug The attached model causes an error during gltf-transform optimize. My understanding is that the textures use non-standard ASTC6x6 which is understood by three.js, but maybe not yet in gltf-transform.

To Reproduce Steps to reproduce the behavior:

  1. Download d9836c94f70dcebf0bc6.glb.zip
  2. Run gltf-transform optimize d9836c94f70dcebf0bc6.glb d9836c94f70dcebf0bc6.opt.glb
  3. Note error image

Expected behavior Can be optimized and/or meshopt compressed

Versions: 4.0.8

Additional context The model is non-standard in some ways, see https://x.com/hybridherbst/status/1833281592351166901 for a bit of a breakdown

donmccurdy commented 1 month ago

Seems that gltf-transform inspect fails too:

error: Unexpected KTX2 colorModel, "162".

I don't think this is technically a valid file per spec, but agree that glTF Transform should be able to ignore unknown texture types safely, and will look into fixing that!

hybridherbst commented 1 month ago

Thank you! I opened a similar issue for gltf.report as well by the way, which logs that same error message regarding the color model.

donmccurdy commented 1 month ago

Currently in textureCompress we filter out textures by MIME type:

https://github.com/donmccurdy/glTF-Transform/blob/0bf0ecb514b4298eabeae9d13a0d4ae9180adf89/packages/functions/src/texture-compress.ts#L172-L174

I think we may need an additional sanity-check with ImageUtils#getMimeType on the bytes to check that the specified MIME type is actually correct, and perhaps also attempt to read the width/height of the image before we continue to compression.

Similar for inspect.ts. I'm traveling this week but will continue on this when I'm back!

donmccurdy commented 1 month ago

invalid_mime_type

Hm, there are a few issues:

  1. File contains images with .jpg extensions, but which declare image/ktx2 as the MIME type. This breaks texture-compress.ts. At least some of the textures are really KTX2, I didn't check them all.
  2. File fails validation with additional Recognized image format 'image/jpeg' does not match declared image format 'image/png' errors. I can't tell if that affects glTF Transform.
  3. KTX2 files use plain ASTC, as you mention, which isn't allowed by KHR_texture_basisu and isn't supported by glTF Transform. For three.js it might be OK, but would require the viewer to have an ASTC-compatible GPU.

I support fixing (3); glTF Transform should be able to process glTF files containing .ktx2 textures that don't use or conform to KHR_texture_basisu, ignoring the textures as needed. That's necessary to support userland implementation of extensions like KHR_texture_astc. But issues (1) and (2) I'm not so sure about, trying to sniff the correct MIME type and override what the .gltf file claims does not feel great.

I'd be happy to accept a PR for (3), or to try an implementation if we have a .gltf sample containing an ASTC .ktx2 texture without these other issues. But in the meantime, I may leave this open at lower priority. Marking this as "enhancement" since the required changes would involve adding more checks and logic for previously unsupported types of KTX2 data.

donmccurdy commented 1 month ago

Closing for now, not sure this is actionable, but happy to revisit later.