It seems that when optimization happens (tried with reSmush.it) the module overwrites the original (large) image with its optimized (smaller) version -- correct?

This can be a problem for those users who happen to have their original images in the files folder and they rely on them not being tampered with.

It can also be problematic for those sites that reuse their images for other purposes. It is always wiser to resize, crop, or otherwise manipulate the original images, and only then optimize them.

Or am I being too fussy? :-)

CommentFileSizeAuthor
#10 optimize_target_image-2893046-10.patch1.41 KBSpaghettiBolognese
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Vacilando created an issue. See original summary.

Steven Jones’s picture

Steven Jones CreditAttribution: Steven Jones at ComputerMinds commented
Status: Active » Fixed

It's possible that the 7.x-1.x branch is optimizing your uploaded images.

7.x-2.x implements this functionality in a different way so that it only optimizes generated images, which is as you suggest best practice.

I'm aiming to clean the branch up and get an upgrade path in place in the next few weeks!

Hope that helps.

Vacilando’s picture

Vacilando CreditAttribution: Vacilando as a volunteer commented

Well in fact I looked at 7.x-2.x -- there too the sources are being overwritten.

See file_unmanaged_save_data($result->data, $dst, FILE_EXISTS_REPLACE); in http://cgit.drupalcode.org/imageapi_optimize/tree/plugins/imageapi_optim...

There, $dst is just drupal_realpath() of the original file's location in the files folder.

Steven Jones’s picture

Steven Jones CreditAttribution: Steven Jones at ComputerMinds commented

Ah yeah, absolutely.

The file is generated by Drupal from the original image, applying an image style or whatever, and then we optimise and replace that generated file.
We avoid replacing the very original source image.

jcisio’s picture

jcisio CreditAttribution: jcisio at Axess Open Web Services commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Fixed » Active

Then it's a problem with the 2.x branch. In 1.x branch, we only optimize when the image is save after all effects being applied, i.e. it's the derivative.

jcisio’s picture

jcisio CreditAttribution: jcisio at Axess Open Web Services commented
Category: Support request » Bug report
Priority: Normal » Critical

PS: I reopen because I think it's problematic optimizing (even lossless) the original image. Besides, we are going to introduce some lossy optimizer (e.g. Google's guetzli).

Critical because there is data loss.

Steven Jones’s picture

Steven Jones CreditAttribution: Steven Jones at ComputerMinds commented

@jcisio I'm struggling to see where the data loss is here? Especially in the 7.x-2.x branch.
The 7.x-2.x branch is even more conservative with the images that it will 'overwrite' than 7.x-1.x.

7.x-2.x provides an API for optimising an image 'in place', overwriting the existing image. It then uses that API when generating image styles to optimise the derivative image. I don't see this is problematic data loss? Drupal core provides a UI for deleting all these images, so the data loss of these images can't be a problem.

Fundamentally there's a misunderstanding here, imageapi_optimize has always replaced the image it passes to the optimisers:
http://cgit.drupalcode.org/imageapi_optimize/tree/services/resmushit.inc...
It's just that most of the time in 7.x-1.x and all of the time in 7.x-2.x that image passed to the optimisers is already generated and not the same as the very originally uploaded source image.

jcisio’s picture

jcisio CreditAttribution: jcisio at Axess Open Web Services commented
Priority: Critical » Normal

You're correct. I've checked the 2.x code and it seems that it still does in the same way (all the stuffs done in image_imageapi_optimize_save() when it is invoked using image_toolkit_invoke('save', ...)). No optimization happens for uploaded files. Maybe the OP misconfigured the image field to resize the uploaded image and optimization happened? In that case I'd say that it is expected behavior.

I don't close this issue because we need some confirmation about that. I've also open #2893186: Update README because README file has not been updated for 5 years.

Vacilando’s picture

Vacilando CreditAttribution: Vacilando as a volunteer commented

Indeed, the 7.x-1.x was always overwriting the source images.

But doesn't 7.x-2.x do the same? See http://cgit.drupalcode.org/imageapi_optimize/tree/plugins/imageapi_optim...

file_unmanaged_save_data($result->data, $dst, FILE_EXISTS_REPLACE);
SpaghettiBolognese’s picture

SpaghettiBolognese CreditAttribution: SpaghettiBolognese at Emble commented
FileSize
optimize_target_image-2893046-10.patch1.41 KB

For whoever is interested: here is a patch for the 7.x-1.x branch which adds the option to only optimize imagestyles.

garbo’s picture

garbo CreditAttribution: garbo commented

Hi, I'm reading this thread and I'm getting worried.
With Image Optimize v7.1.3, is it the original uploaded image that is being optimized or is it the derivative image style that is being optimized?

If it is the first, than this module is an absolute no-go for me. I want to be able to keep the original unoptimized images since you never know if a new and better optimization toolkit will become available in the future.

Thanks in advance for any reply on my question.

dddbbb’s picture

dddbbb CreditAttribution: dddbbb as a volunteer commented

Thanks for the patch in #10.

dddbbb’s picture

dddbbb CreditAttribution: dddbbb as a volunteer commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: optimize_target_image-2893046-10.patch, failed testing. View results

guardiola86’s picture

guardiola86 CreditAttribution: guardiola86 commented

Patch works but I don't see any massive improvement in file size reduction.