phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.79k stars 1.96k forks source link

[NFR] Image scaling #13469

Closed trymkb closed 5 years ago

trymkb commented 6 years ago

Image scaling done using the default settings in GD is pretty horrible for pictures, but might be nice for line art. It would be nice to be able to choose the mode from the adapter, instead of needing to get the image resources and do the scaling using native php commands.

Currently the mode is set to "IMG_BILINEAR_FIXED" by default, and you are not able to alter this resource. http://php.net/manual/en/function.imagescale.php

Example of code today

$avatarFile='myReallyLargeAvatar.png';
$image = new \Phalcon\Image\Adapter\Gd($avatarFile);
$image->resize(300, null, \Phalcon\Image::WIDTH);
$image->save('NotSoLargeAvatar.png');

Work around code

$avatarFile='myReallyLargeAvatar.png';
$image = new \Phalcon\Image\Adapter\Gd($avatarFile);
$myImage=$image->getImage();
$myImage=imagescale($myImage,300,-1,IMG_BICUBIC);
imagepng($myImage,'NotSoLargeAvatar.png');

What I would like

$avatarFile='myReallyLargeAvatar.png';
$image = new \Phalcon\Image\Adapter\Gd($avatarFile);
// Either
**$image->resizeMode(IMG_BICUBIC);**
$image->resize(300, null, \Phalcon\Image::WIDTH,**IMG_BICUBIC**);
$image->save('NotSoLargeAvatar.png');

Nice to have, so you can get the GD resource and do any special outside GD work on it.

$avatarFile='myReallyLargeAvatar.png';
$image = new \Phalcon\Image\Adapter\Gd($avatarFile);
$myImage=$image->getImage();
$myImage=imagescale($myImage,300,-1,IMG_BICUBIC);
$image->setImage($myImage);
$image->save('NotSoLargeAvatar.png');

Details

Affects current version of Phalcon using PHP 7 or greater

Jeckerson commented 6 years ago

I used several times Phalcon Image library because it's very easy and elegant wrapper to handle image manipulation, but nothing more... It won't give any extra performance, nor latest feature of adapters...

But, when it comes to something non-default or extraordinary, it start to make sense to use native PHP GD extension, or even Imagick if you want many custom stuff...

If Phalcon core dev team will respond positively, then I'll consider to make PR for current issue.

trymkb commented 6 years ago

Now I don't disagree with your argument of the scope of the Phalcon Image library, and the point of the wrapper is to give the most used functions an easy to use interface.

But if the point is to get higher adoption of the framework, functions that handle tasks like resizing images should be focused on both ease of use and quality.

Now there is a huge difference in the method used for Phalcon on PHP 5 and PHP 7 when it comes to resizing images, and if you have not witnessed the quality loss you can find an example from another user here: https://forum.phalconphp.com/discussion/18627/image-quality-loss-resize-phalcon-gd-vs-php-gd

The bare minimum solution requires adding 1 more option to pass through to the imagescale routine, and it would solve my problem, and most likely many more.

I am well aware that there are most likely tons more pressing issues that are of higher priority, but I would hope that the suggestion is not tossed and if possible considered something that is looked into in the future.

Tanami commented 6 years ago

@trymkb it sounds like you're more interested in "clean code" than practicality here. What is the true advantage of stuffing more stuff into the phalcon namespace? I say keep it clean.

trymkb commented 6 years ago

@Tanami are you aware that imagescale does not alter the original resource, but returns a new resource? So any and all tasks done after running imagescale on the outside would not transfer to the resource available in the Phalcon Image Library.

This is not about having clean code, but having the options available exposed so you can do the tasks needed without the need to escape or rewrite the framework.

I am well aware of how to solve this, and i merely put the suggestion on how to improve the library so it can suit more needs, and I feel it was not an extreme suggestion either.

And searching for the issue I found numerous references to this exact problem, but most did not exactly know the specific reason for it.

Like these: https://github.com/phalcon/cphalcon/issues/11773 https://forum.phalconphp.com/discussion/18627/image-quality-loss-resize-phalcon-gd-vs-php-gd

Both seem to have the same issue from the default scaling routine, but one of them might also be an issue of scaling non truecolor images.

IMG_BILINEAR_FIXED is horrible, it lightens the image and almost looks like you run a sharpen. IMG_BICUBIC seems to be the softest, and is well suited for thumbnails from pictures.

I hope that this will be looked into, I will certainly rewrite this when i start looking into doing some more work with Zephyr code (BTW. Love that!)

I have nothing but gratitude for the work done 👍

I only hope that this will be addressed in the future, as it is a annoyance, and I will guess this is not the last time someone will request it :-)

stale[bot] commented 5 years ago

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

stale[bot] commented 5 years ago

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues