Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GLTFExporter grows files 5x from import #23587

Closed
elalish opened this issue Feb 24, 2022 · 15 comments · Fixed by #23592
Closed

GLTFExporter grows files 5x from import #23587

elalish opened this issue Feb 24, 2022 · 15 comments · Fixed by #23592

Comments

@elalish
Copy link
Contributor

elalish commented Feb 24, 2022

Describe the bug

JPEG textures in an input GLB get exported as PNG, causing such models to grow enormously in size.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://threejs.org/editor/
  2. Import DamagedHelment.glb
  3. Export as GLB
  4. Note the original was 3.8 Mb, while the new is 17.9 Mb

Expected behavior

The exported model should be pretty close to the same size as the original. I understand it won't be identical, which is fine, but this is pretty unusable.

Cause

#23385 is sort of the culprit, but really it was #23228 where the RGBFormat was removed and this was the only way we had to tell if a texture had been a JPG or a PNG. @Mugen87 @donmccurdy This is blocking model-viewer's update to r137/r138 because it affects our editor the same as yours. Can anyone think of a quick work-around?

@donmccurdy
Copy link
Collaborator

donmccurdy commented Feb 25, 2022

A couple thoughts here, choose your own adventure —


I feel like the behavior of the three.js editor — exporting all images as PNG — is probably 'right' for its use case. It's the only way to export losslessly, and the only way to avoid compounding compression artifacts with repeated import/edit/export steps. Compression can be applied at the end of the edit process, with a variety of other tools.

Are you hoping for a fix that allows model-viewer to keep the previous behavior, re-compressing any image that was originally a JPEG? Or something else? I'm afraid I don't see a quick workaround that doesn't feel pretty hacky, at the moment.


Perhaps this is a more complicated answer than you're looking for, but — there are a lot of similar but more subtle issues caused by import/edit/export with GLTFExporter as well:

  • Changes in node hierarchy
  • Possible duplication of previously shared materials and textures
  • Repeated import/export accumulates precision loss in vertex data with draco and meshopt
  • Repeated import/export accumulates precision loss in JPEG textures
  • No way to re-export KTX2 textures

Because of all of those issues, I'm working on a different approach with https://gltf.report/

The idea here is that edits can still be displayed instantly, but repeated import/edit/export steps don't have any side effects. The glTF-Transform-Render code still needs quite a bit more testing, though.


I'm pretty close to having @squoosh/lib hooked into glTF Transform, for optimizing PNG and JPEG textures in a glTF file. If you happen to know anyone who could nudge GoogleChromeLabs/squoosh#1084 across the finish line this will work on the web as well, and could be a great option for model-viewer. 😉

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 25, 2022

Another option is to add the following code back and store the result in Texture.userData.hasAlpha.

https://github.com/mrdoob/three.js/pull/22551/files#diff-868e3d9b3219d163d7d53939fc3f7a6e65b7664ad5e3da03cd31fa23ea1a6df6

Texture.userData.hasAlpha could then be evaluated elsewhere like GLTFExporter.

I'm not sure hasAlpha should be a property of Texture. It could be easily misunderstood on app level.

@RemusMar
Copy link
Contributor

Are you hoping for a fix that allows model-viewer to keep the previous behavior, re-compressing any image that was originally a JPEG?

That's wrong anyway.
The real solution is to pack the exisiting JPG/PNG files into the new GLB exported file.
And for future keep PNG for transparency and JPG for the rest.
It will make your life much better ...

@donmccurdy
Copy link
Collaborator

Another option is to add the following code back and store the result in Texture.userData.hasAlpha...

This doesn't feel great to have in GLTFLoader, to me – it's more bookkeeping that needs to be maintained for a narrow use case. Similar to the 'associations' mappings we're maintaining (e.g. #22750); it would be really nice if GLTFLoader could be more focused on just efficiently loading glTF files into "normal" three.js objects.

Also, knowing the texture has alpha isn't enough — if the texture is a normal map you probably want to export it as PNG rather than JPEG, even without alpha. So this starts to turn into custom logic in GLTFLoader and GLTFExporter that is only relevant for that specific workflow. And if it was a JPEG before, re-compressing as JPEG (rather than using the original data) degrades the quality.

The real solution is to pack the existing JPG/PNG files into the new GLB exported file.

Definitely this is the right way to build a model editor. 👍🏻 But since THREE.Texture does not contain enough data to perfectly restore the original PNG/JPG file, there's no simple way to do this. And I'm not sure that GLTFLoader / GLTFExporter are the right place for those solutions. Maybe hooking into the extension APIs would work? But there's a reason I'm writing a separate loader for this use case – a model editor should hold a lossless representation of the data it is editing.

@elalish
Copy link
Contributor Author

elalish commented Feb 25, 2022

@donmccurdy Thanks for the detail; agreed the behavior before was hacky and the right solution is a library like glTF Transform. However, what we had before was usable (in my experience there's actually very little loss from recompressing a JPEG to a JPEG, especially if the quality setting is the same, since the DCT ends up being pretty close to reversible). I have an idea for a small change to the exporter that would infer mime type from the file extension (if present). I realize it's still a hack, but it's vastly better than 5x expansion without recourse. I'll put up a PR for discussion.

I'd love to switch over to a proper method, but that will take time. I need some way to work around this in the meanwhile, and at least the file extension will be a bit more reliable than RGB vs RGBA.

@hybridherbst
Copy link
Contributor

Also recently thought a lot about this, and generally how to get at least "something that looks right" and at best "something that also has the right size" back from an import/export loop.

Similar to what @donmccurdy is proposing, my main line of thinking was to cache the original buffers (if memory permits) on import and then, if possible, re-using them on export. I would have probably built that as import and export extensions to three. This would be contstrained to the current glTF Import/Export flow (e.g. no other formats would immediately do this and for other formats it might be harder), but it solves for a lot of cases of "I'm re-arranging a number of files that have already been nicely compressed and want to have a new file".

In any way, this kind of clashes with the ideal of removing things from CPU memory once they're uploaded to the GPU, but at least from my end I think that hit would be preferrable over "expanding" data on export (readback from GPU and turn everything to PNG).

(And a side note, in UnityGLTF I also added a heuristic on export to determine if files should be PNG or JPEG, but that's easier there since there's import-time metadata from the editor to guide that decision.)

@donmccurdy
Copy link
Collaborator

...the file extension will be a bit more reliable than RGB vs RGBA.

@elalish is the file extension accessible, e.g. after loading from ImageBitmap? That does sound more reliable if so.

@hybridherbst - if that workflow is possible through extensions then it's likely a decent workaround as well! I'm hesitant to preserve that extra data directly in GLTFLoader though.

@RemusMar
Copy link
Contributor

if the texture is a normal map you probably want to export it as PNG rather than JPEG, even without alpha

That's a myth!
I use JPEG (quality factor 100) for all the normal/bump maps for over 15 years in various game engines.
I did never have any artefact and the file size is still much smaller than PNG.
Guys, if you're talking about online and browser based things, the assets filesize is CRITICAL!

@elalish
Copy link
Contributor Author

elalish commented Feb 25, 2022

@elalish is the file extension accessible, e.g. after loading from ImageBitmap? That does sound more reliable if so.

Indeed it's not. Would anyone be tremendously opposed to adding a mimeType member to Texture? I know it's a touch of extra bookkeeping, but it would allow you to inspect what the source image type was and you'd be free to change it to control what the export image type should be. I suppose I could stuff into userData instead as @Mugen87 recommended, at least until we're comfortable with the idea?

@phillip-mcnallen-tfs
Copy link

I think Texture seems the wrong place to add a mimeType field. If there are special behavior requirements, to avoid the complexity tracking that @donmccurdy mentioned, I think the best approach really is hooking the loader to store original textures for later along with anything else application specific. Then add an option to GLTFExporter that allows granular selection of how you want each texture exported. Flags could be set for each texture: useOriginal, or type as png, jpg, etc. This isn't to difficult to add and it would give you all the flexibility you wanted.

I've extended these classes for other reasons and the technique has been stable through many versions of three.js releases.

@elalish
Copy link
Contributor Author

elalish commented Feb 26, 2022

@donmccurdy I had thought I could use image.src because that's what the exporter uses for non-binary glTF. After testing, it turns out non-binary glTF export is just broken, since src is undefined.

@LeviPesin
Copy link
Contributor

Maybe it is better to always export textures as WebP? WebP is now supported in 92-96% of browsers.

@donmccurdy
Copy link
Collaborator

Unfortunately I don't think we'd want WebP as the default — it's not an official Khronos extension in glTF, and will prevent people from loading the model in Blender. Plus we'd still have to decide whether to use WebP's lossless mode or not, similar to the PNG vs. JPEG decision here. If you want to convert glTF files to use WebP that'll be possible with this PR though.

@makc
Copy link
Contributor

makc commented Nov 6, 2022

@donmccurdy

Definitely this is the right way to build a model editor. 👍🏻 But since THREE.Texture does not contain enough data to perfectly restore the original PNG/JPG file, there's no simple way to do this.

dont they all just have blob urls? so you could just fetch() them from browser memory again?

@donmccurdy
Copy link
Collaborator

The Blob URLs are revoked after loading, but yes -- if you were to disable that and use them in GLTFExporter (to access the ArrayBuffer) I think it'd work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants