wp-media / imagify-plugin

Speed up your website with lighter images without losing quality.
https://imagify.io
69 stars 24 forks source link

Closes #796: Auto AVIF Convertion on settings save #811

Closed Miraeld closed 3 months ago

Miraeld commented 3 months ago

Description

Fixes #796

Documentation

User documentation

When uploading images to wordpress, when the latest is over a certain dimension, WordPress will resize it and named it -scaled. Originally, we were doing a backup of the Original image, while on the bulk optimization it's trying to reach the -scaled version as WordPress takes it as default. So we've added the -scaled version to be backed-up.

Technical documentation

Before trying to save the -scaled version of the image in backup, we check if it exists. Same with the deletion of the image.

Type of change

Checklists

Feature validation

Code style

Observability

Miraeld commented 3 months ago

@Mai-Saad , The problem we encountered with big images like this was the fact that WordPress is resizing the image and name it -scaled, so for example, for an image bigimage.jpg it would become bigimage-scaled.jpg. In the DB, the -scaled version is saved.

So while trying to get back the IDs of unoptimized medias while running the bulk optimization, it wouldn't keep it as the backup file does not exists.

To counter that issue, I've added the scaled version to the backup, as that's the main one Wordpress is using, and the bulk optimization will work without trouble.

I've also taken care of the deletion of the image, when we delete the image, it will delete both backup, the original and the scaled version.

Mai-Saad commented 3 months ago

@Miraeld Thanks for the PR, now AVIF is created for large image with the following notes. 1- if the image have webp before enable avif, we are deleting the webp version before generating the avif (this is not happening with small image, here we add avif and keep webp there). @piotrbak shouldn't it be same as with small image? 2- if we uploaded image(s) before activating imagify. Those images won't be automatically processed after activate imagify and enable AVIF. @piotrbak are we ok here or we need separate GH? 3- if internet went down while optimization is in-progress then once back, it won't be resumed automatically. (probably same on trunk, needs further investigation and may need separate GH)

piotrbak commented 3 months ago

@Mai-Saad @Miraeld 1st and 2nd point should be fixed.

Miraeld commented 3 months ago

Hello hello,

@Mai-Saad @piotrbak , for the 1st point, it's been fixed in the before last commit. 2nd point is fixed by the last commit.

Mai-Saad commented 3 months ago

@Miraeld Thanks for the update, compared to trunk and feature/avif branch, we will have regression here: 1- Create avif is enable 2- upload image => image have avif created 3- disable avif 4- permanently delete the image from media library => avif won't be removed from upload folder (same will occur if webp image was created while avif is enabled and we try to permanently delete the image) can you please check?

piotrbak commented 3 months ago

@Miraeld Should it be moved to the QA again? Or it needs another CR?

Miraeld commented 3 months ago

Umm, I think it doesn't need another CR as I just applied what @jeawhanlee advised. I can move it back to QA

Mai-Saad commented 3 months ago

@Miraeld Thanks for the update.