OleksandrWebLab / oc-image-compress-plugin

Simple compression and resize for images
MIT License
6 stars 2 forks source link

Smaller images than the set max height and width are always enlarged #5

Open chocolata opened 6 years ago

chocolata commented 6 years ago

When enabling the resize option in the Image Compress plugin, setting a max height and max width seems to result in that smaller images are enlarged instead of left alone.

Let's say I have an image of 800x600 and I've set the max height and max width to 1600x900, then the original image is enlarged to 1600x900, resulting in a distorted image.

The correct behaviour (I think) should be that an image smaller than the max height and width should not be enlarged but should be left alone. Or maybe this could be a setting enlarge smaller images?

What would be your viewpoint? Again thanks for the great work.

OleksandrWebLab commented 6 years ago

What version of the plugin are you using? 1.0.1 or 1.0.2? If 1.0.2 - tell me which compression mode you have installed?

chocolata commented 6 years ago

At the moment 1.0.2, please find settings below:

knipsel

OleksandrWebLab commented 6 years ago

In the Resizer class, properties and methods for obtaining image sizes are protected

chocolata commented 6 years ago

Thanks for investigating. Something to pick up with the October CMS crew.

chocolata commented 6 years ago

Hi, I was investigating something this further. We could do something like this: In your Plugin.php file, around line 60, could we do something like this?

                        $filePath = $model->getLocalPath();

                        // Get original width & height from filePath
                        list($originalWidth, $originalHeight) = getimagesize($filePath);

                        // Only resize if the width or height of the actual image is bigger than what we need.
                        if($originalWidth > $width || $originalHeight > $height) {                     
                          Resizer::open($filePath)
                              ->resize($width, $height, $options)
                              ->save($filePath);

                          clearstatcache();
                        }

What are your thoughts?

chocolata commented 6 years ago

Also, I think the quality is defined as a parameter in the wrong function.

Docs of the resizer

Usage: Resizer::open(mixed $file) ->resize(int $width , int $height, string 'exact, portrait, landscape, auto or crop') ->save(string 'path/to/file.jpg', int $quality);

So, using your system, the quality would always be 95, the default quality of October CMS. I think we could do:


$quality = ImageCompressSettings::get('quality');

 Resizer::open($filePath)
                              ->resize($width, $height, $options)
                              ->save($filePath,$quality);

What do you think?

OleksandrWebLab commented 6 years ago

Few rewrote the logic. Recent edits added to 'develop' branch. Can you check functionality new settings?

chocolata commented 6 years ago

Hi, thanks so much for your work. I'm a bit swamped right now, but I'll help to check the functionality asap, probably this weekend. Did you see my last remark of the second parameter of the save() method?

OleksandrWebLab commented 6 years ago

Yes I saw. I think this is not a good idea, and the save() method does not have a second parameter, in fact:

https://github.com/octobercms/library/blob/master/src/Database/Attach/Resizer.php#L404

chocolata commented 6 years ago

Good call, I guess the documentation is a bit outdated there in that case.

OleksandrWebLab commented 6 years ago

Can I assume that this problem is solved?

chocolata commented 6 years ago

Hi, haven't had the time to test it yet, but at first glance, it does look like it's fixed. The only thing I can think of is that on upload, a user might not want to upscale smaller images, but does want to compress them.

Let's think about a scenario in which we set the max width and height to 1000px and the quality to 80. When an image is for example 800x600, but 300dpi, with a big filesize, these images would be unaffected? We might want to skip resizing, but still do the compression (quality change)?

chocolata commented 6 years ago

Hi, sorry for the wait. I've been testing in detail today and everything looks good. The "make enlarge" checkbox works perfectly as expected. Enabling and disabling works like a charm. When checking the "change quality" checkbox, the quality does change according to your settings. I've tested with different qualities and it does seem to behave as expected.

One small detail that I cannot get my head around is the following: when unchecking the "change quality" checkbox, the quality still gets downgraded on upload. My guess is that the save() method by October CMS still performs their standard compression (I guess it's 95%).

That's the only thing that doesn't behave as expected. Could you please have a look at it? My guess the solution would be on line 56 of Plugin.php

                        if ($isChangeQuality == true) {
                            $options['quality'] = ImageCompressSettings::get('quality');
                        } else {
                            $options['quality'] = 100;
                        }

What do you think?

Thanks again for all your nice work. Thanks to you I will save a lot of disk space in my project. If there's anything else I can do for you, please don't hesitate to let me know.

OleksandrWebLab commented 6 years ago

Hi, thanks for the feedback about my work, very nice.

Perhaps you should turn on image compression by 100%, instead of changing the default value? Setting the value to 100% does not give a visible improvement in quality, compared to 95% by default, but the file size increases noticeably.

chocolata commented 6 years ago

Hi, maybe in such case the image processing should be skipped in such case? If change quality is not checked and the image is smaller than the maximum height / width, skip the save() method. What do you think?

chocolata commented 6 years ago

Hi, did you have the chance to look at my last comment?

Wachem commented 5 years ago

@maartenmachiels In follow up of your last posts, I would implement a bool to do that. For example (simple):

if ($isChangeQuality == true) {
     $options['quality'] = ImageCompressSettings::get('quality');
     // Add bool (best to define it at the beginning of the function)
    $saveToNewFile = true;
} else {
     $options['quality'] = 100;
     $saveToNewFile = false;
}

And then:

if ($saveToNewFile) { ...

chocolata commented 5 years ago

Thanks for your comment, Wachem! @VoroninWD what are your views on this?