Add proper support for animated images #2910
Conversation
Why does all of this look so arbitrary?
This is the best part of CI: when stuff is so broken you can't even test things on the repo itself, use someone else's tooling!
|
Bad news! If you want this PR to be less spaghetti-y, this will rely on image-rs/image#2360 to be acted on some form; we can, however, perhaps make it cleaner than it is right now Honestly? My worry isn't more about the quality of the code but really, about how well the code functions; so I'll perhaps continue testing and see if I can see something bad be spit out by this thing |
Does it matter? No, Modrinth does not support AVIF images as an input yet, but we are already updating the dependency anyway so we might as well do it
|
Update: I have failed on the mission of getting a test animated image that would start as a smaller resolution and would have a bigger resolution frame on the middle; I did manage to successfully test the "Big Resolution Start, Small Resolution End" case, but it appears that WebP's encoders makes sure that any frame cannot exceed the initial frame's resolution? Not being able to guarantee said edge-case is covered concerns me a bit |
|
#2526 is probably relevant to this |
You are right, it is; however, this piece of pipeline isn't specific to project logos (and profile pics) though, and also affects images uploaded through Modrinth itself iirc (ex: gallery and project images) If that's correct, then this PR is still relevant for those cases Update: Can confirm it is indeed necessary for those cases to be animated! So yeah, animated icons should be considered a separate issue from animated images (although to be fair? railguards to have all animated images only animate on hover would be appreciated) |
|
Animated webps have v limited browser support iirc |
Are you sure about that? Can I Use points towards all recent browsers supporting animated WebPs (with green being full support), and I can confirm that at least Safari has said support for animated WebPs (I've tested on an Apple computer); something that I should note, however, is that all browsers have this tendency to struggle with decoding heavy multi-megabyte animated WebPs If support is unsatisfactory, we could maybe go with APNGs, which have been supported by everyone except Chromium-based browsers until 2017 (therefore they ironically have better support than static WebPs!), although the need to be lossless plus extra information compared to GIF can be detrimental to it I believe that animated WebPs are the way to go though, after all, y'all already made the decision to adopt WebP as the main image format on MR; this means y'all are already comfortable with its current level of support anyway |
|
After some experiences involving animated WebPs, I believe I have some extra experience to talk about this; but also, I'll have to shift the focus from "process all animated images to WebP" to "process supported animated images into their respective formats" I will post a mini-rant that will make this clearer later, but basically: you should not worry about animated WebP's compatibility, because if you have full support of it and its extended features (lossless, alpha, etc.)? you also have support for animated WebP! This means that all new non-Chromium browsers (Firefox, Safari) that have finally decided to adopt WebP after dismissing it as a silly redundant Google thing should have no problems with handling animation at all That being said, I have realized that while on a context of icons capped to 256 KBs, a conversion to WebP would theoretically work great with no major problems at all, the issue begins if you decide to embed <1 MB animated images on your project page or add an animated gallery image to your project. Then you stumble upon animated WebP's biggest weakness so far which can be a massive annoyance: it's goddamn slow! It turns out animated WebPs are slow to decode, despite its tricks to make loading smoother, and while this wouldn't be a problem for icons? this will be a massive problem for other images, so while it may be a sacrifice for saving file sizes for some? converting a GIF to it might be a big problem for others Since I believe support for uploading animated WebPs and APNGs are still important regardless of these issues, I might consider changing the processing of GIFs so that they are still processed into GIFs, but still allow them to be resized and such by the now-built animated image pipeline (extra testing will be needed there!); APNGs and animated WebPs (which were formerly squashed into PNGs and WebPs) will continue being processed into WebPs so that said support remains This is a shame though, WebPs aren't going to be the uniting format we needed; |
|
I have went ahead and reintroduced the GIF skip for when no modifications are requested (a.k.a. when an embedded image is uploaded); Despite the wall of text I've written (and that I will continue to write) about animated WebPs? I still believe that if kept on a low file size, they won't be an issue at all, and that they'll be more beneficial for icons and such than GIFs, a pretty inefficient (and obsolete) format, can ever be |
|
Update: the Firefox throttling issue (and slowdowns on Chrome's side) affects all animated images! Therefore, universal WebP conversion can maybe go ahead unless there are any objections |
An issue that Modrinth's image conversion had was: when it converted to WebP, it never cared if said image had extra frames or not, it is going to be squashed into a static image and that's it! And this is bad! This makes GIFs require skipping said processing in order to preserve their animations while APNGs and animated WebPs were impossible to upload, preventing some assets from being uploaded thanks to the file size limit.
This PR fixes it! It adds animated image processing and makes sure that everything animated (including GIFs) is going to be converted to animated WebPs for lighter file sizes (albeit at the cost of a bit of lossy quality in parity with other images)
I've tested with a few APNGs, GIFs and animated WebPs that I had around, and I can confirm that it's functional! it can handle resizing and cropping well; although I cannot confirm if it's going to work well at the edge-case of a frame having a smaller dimension than the other, since this assumes the first frame is the entire image's resolution;
Do note that uh, this code is extremely messy and I haven't been able to test it on Labrinth itself (I tested it by extracting the code and providing an input to it), so if any of the maintainers could take this over? I'd really appreciate it; really, I just want animated WebPs a lot :p
TODO:
Craft an animated image with a frame bigger than another and feed it to this machine