-
-
Notifications
You must be signed in to change notification settings - Fork 35.4k
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
Zlib: update external library, introduces 'Deflate' #19748
Conversation
Have you seen Pako? It's supposed to be faster than zlib and creates identical output, at the expense of a few extra kb of JS. It's what JSZip uses internally. There's also Uzip by @photopea which is supposed to have an |
Oh, didn't know about these, looks promising. I will run some tests on these libs and make sure they are actually compatible with the internal format expected by the loaders, and most importantly the exporter, because we need to make sure the compressed data can be correctly inflated by the OpenEXR implementation. |
I've tested out UZIP against JSZip on my own project. It seems to work flawlessly and it's a lot faster. There's a couple of bytes difference at equivalent compression levels but both zips seem to be fine. The main difference is that with UZIP I have to convert files to Results for a folder containing 12 text files and two PNGS. No compression:UZIP: JSZip: Compression Level 6:UZIP: JSZip: Difference in library size:UZIP.min.js: 15kb |
This disadvantage can be neglected since |
In that case, should we rename the exported module to the name of function instead? So that we don't have two different Zlib modules being exported by two different files? So instead of using this: import { Zlib } from "../libs/inflate.module.min.js";
//...
new Zlib.Inflate(); We use this: import { Inflate } from "../libs/inflate.module.min.js";
//...
new Inflate(); // maybe zInflate / zDeflate, to make it obvious it's a zlib implementation This way, if we ever need both Deflate and Inflate modules in the same file, we don't run into problems like the following import { Zlib } from "../libs/inflate.module.min.js";
import { Zlib } from "../libs/deflate.module.min.js";
// although we can always import one of them with
// import * as Zlib2 from "../libs/deflate.module.min.js"; |
Sounds good. |
Just pushed an update with the discussed changes, preliminary tests on EXR, FBX and VTK loaders shows everything is in order. @looeee I haven't discarded using other libs, just for the sake of getting the EXR Exporter PR out there asap, I'll go with this approach for now and we can continue testing zlib.js alternatives. |
Hi guys, I am really glad that you use UZIP.js . I created it because I was not happy with the speed and size of pako.js . To have extremely fast decompression (inflate), give it a target Uint8Array, into which the data should be decompressed (otherwise, the target array must be re-allocated several times during the decompression, as the final size of data is not known). Also, UZIP.js can very easily parse and create .ZIP files (unlike pako.js). |
When inflating EXR pixel data, we have the privilege of knowing exactly the target |
Thanks! |
This PR is intended to update the current Zlib.js external library used, with the purpose of including the "Deflate" option.
The reason for this is to prepare for an incoming EXR Exporter, which makes use of zlib to compress data internally.
The way I see it, we have two options on how to approach this, that's why I have this PR set-up as a draft.
inflate
anddeflate
.The downside of the second approach is that it may cause a namespace conflict either for the module ( Zlib ) or global variable when using the non-modular version. Given the minimal difference in file-size overall, I felt like it was better to go with the former.
The initial draft of EXRExporter is ready, I'm just waiting for this to get resolved. Would be nice if we could settle on this ASAP, so that we can get some proper testing on EXRExporter before the next release.