increase the image-quality #195

Closed
wants to merge 8 commits into
from

4 participants

@mdemirci
  • adding filter-param for resize
  • adding unsharpMask-function for imagick
@romainneutron
Collaborator

Great !

Just one note : IMO the filter value should not be aware of the driver implementation being used.

I mean, you should probably define ImageInterface::FILTER_* constants, so developers could use these constants with any driver implementation.

See ImageInterface::RESOLUTION_* constants that are used in the same logic I described to define resolution units : https://github.com/avalanche123/Imagine/blob/develop/lib/Imagine/Image/ImageInterface.php#L27

@avalanche123
Owner

HI, thanks for the PR, can you explain what you're implementing with it? I'm unsure what these filters do.

@mdemirci

if you thumbnail an image, you can loose the imageQuality. With the filters especially with the Lanchoz-filter you can get better images.
http://www.dylanbeattie.net/magick/filters/result.html

@romainneutron romainneutron commented on an outdated diff Feb 5, 2013
lib/Imagine/Imagick/Image.php
@@ -538,6 +546,23 @@ public function layers()
}
/**
+ * unsharpMask the image
+ *
+ * @param float $radius
+ * @param float $sigma
+ * @param float $amount
+ * @param float $threshold
+ *
+ * @return \Imagine\Imagick\Image
+ */
+ public function unsharpMask($radius, $sigma, $amount, $threshold)
@romainneutron
Collaborator
romainneutron added a line comment Feb 5, 2013

This method should be part of the interface and documented.

GraphicsMagick supports it (http://www.graphicsmagick.org/GraphicsMagick.html#details-unsharp), but it's not yet in \Gmagick driver. Opening a ticket for this support in \Gmagick isue tracker could be great :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@romainneutron
Collaborator

I'm wondering if this unsharpMask method should be moved in the effects API

@digitalkaoz

good catch, sharpen is also in the effects API, so should unsharpen

@romainneutron
Collaborator

Hey, are you still interested in working on this ?

We need some unit tests on your work and a piece of doc before merging. If you can do that, that would be awesome. Thanks for your work and patience :)

@romainneutron
Collaborator

Closing the PR due to lack of activity / feature partially on their way to be merged (see #234 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment