Imagick adapter, handles color profiles better

[woutersamaey]

Description (*)

Proposal PR for an Imagick adapter.
This one has better image quality and can handle embedded color profiles.
I had a store where the image colors changed because GD could not properly process the ICC profile.
This adapter is better.

I'm just contributing my code for this 1 file and don't want to make a big deal out of it. If someone wants to help complete and integrate further, feel free.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All automated tests passed successfully (all builds are green)

[tmotyl]

Nice idea!
Please also provide which IM version you have tested it with (different IM versions have different quircks). Maybe a check in the code would also be useful?
Do I get it right that to use this adapter, some additional changes/configuration is required? Can you describe it?

[woutersamaey]

@tmotyl unfortunately I don't have the time, but you can look at Varien_Image_Adapter::factory(). This needs to change + some core files + maybe create a config setting somewhere.

I'm not aware of any Imagick quirks to be honest.

[woutersamaey]

This adapter now also supports "exif:Orientation" which can cause an image to appear rotated in Magento when it is not rotated on your computer.

[fballiano]

I always use https://github.com/colinmollenhour/Perfect_Watermarks cause the GD adapter never works for me

[fballiano]

I've added the PerfectWatermarks module to https://openmage.readthedocs.io/en/latest/modules/more.html#images-optimization

[fballiano]

@colinmollenhour what do you think about this PR? Asking you cause you're the author of the PerfectWatermarks extension that I normally use in my projects.

[colinmollenhour]

@fballiano At a quick glance it looks like a huge improvement over the stock code and based on the commit log and Wouter's comments it sounds like it has been through a lot of production usage and tweaking so it is probably a good thing to merge as-is but I just don't have time to test it..

@woutersamaey can you provide some background on all of the testing this has been through and your confidence level that this would not cause any regressions for any user's of the stock Imagemagic adapter (which I think everyone agrees is pretty bad)?

Is anyone else in addition to Wouter using this in production? Just checking to see how many data points there are.

[woutersamaey]

I have 1 huge volume site using this in production. They have a lot of custom code, but that should not affect the image handling. I'm very confident is it stable.

[woutersamaey]

I just noticed that my class is still called Storefront_MediaStorage_Model_Image_Adapter_Imagick. Guess I need to change this to Varien_Image_Adapter_Imagick? It's been a while...

[colinmollenhour]

@woutersamaey Yes the class name will need to match the pattern in the file path. :)

[colinmollenhour]

Seems that either the old class Varien_Image_Adapter_Imagemagic should be deleted or a new constant added for it such as ADAPTER_IM_LEGACY.

[fballiano]

@colinmollenhour Varien_Image_Adapter_Imagemagic doesn't even exist in our repo :-\

[fballiano]

• needs to implement gif, jpg, png, xbm, bmp, webp
• needs to implement the watermark method