Add ImageMagick support

+1 for ImageMagick support!

Here is a little patch to add ImageMagick support. It detects which one is the toolkit that is been used and based on that follows the GD path or the new IM path. Reviews welcome

I have rerolled the patch provided by @alexmoreno to apply correctly with latest dev version. Also changed a patch a bit to use quality settings while converting the image.

Please review
Thanks

needs reroll again @guptahemant. However I'd like to bring back the security issue we discussed about using exec

I ran into a problem enabling the WebP module, where it complained "The GD library must be compiled with WebP support on your server." even though I had the ImageMagick installed. Turns out webp.install's webp_requirements() function needed to be modified to also check for ImageMagick. I did that using Imagick::getVersion() but maybe there is some better way to handle it. In any case I have attached a patch that takes care of this for webp.install.

I am currently running WebP 1.x-dev with this patch as well as add-imagemagick-support-3032318-6.patch, and it works as expected.

Thanks @caspervoogt for looking into this

+ $imagemagickversion = Imagick::getVersion();

I think that would not necessarily mean that webp is supported by imagemagick. We may need a function that calls webp or specifically returns what we are looking for

Also, what happens if it’s not installed at all, would not throw an exception?

@alexmoreno good point. I just threw this in there to solve my immediate need, which was to be able to enable WebP, knowing that much particular installation does support WebP. Not sure how to detect supported image formats with WebP. I did a brief search and saw no obvious way.

I've been having a look as well, yes, there is not much documented, but there is sons coder here and there couldn’t give some ideas.

For exampme, they use a constant in Wordpress to detect that. See

https://plugins.trac.wordpress.org/browser/webp-express/tags/0.17.2/vend...

Although this uses exec, from which I think we should move from for security reasons.

This as well could give us some ideas

https://github.com/Orbitale/ImageMagickPHP/blob/master/Command.php

I’ll try to do tears as well in the next days, I see some libraries that wet could leverage further not just for this but for other parts of the module.

Thanks :-)

@alexmoreno I think https://www.php.net/manual/en/imagick.queryformats.php may be what we need. Trying it out.

OK, this patches the install file and uses that Imagemagick command to retrieve a list of supported formats. The formats are in all-caps and this code assumes that is always so. Might need additional checks in case of server differences in that regard.

+++ b/src/Webp.php
@@ -71,71 +71,79 @@ class Webp {
+      $webp = $this->createImageMagicImage($uri, $quality);

typo, should be createImageMagickImage

+++ b/modules/contrib/webp/webp.install
@@ -15,10 +15,11 @@ function webp_requirements($phase) {
+  if (!$gd_info['WebP Support'] && !in_array("WEBP",Imagick::queryFormats())) {

we can probably use strtoupper here, so we ensure it never fails because it does not returns caps.

I'll get this, I'm rerolling and creating another patch

--- a/modules/contrib/webp/webp.install
+++ b/modules/contrib/webp/webp.install

error: modules/contrib/webp/webp.install: No such file or directory

patch should be done against the root of the module, not the docroot of your site. I'll reroll this

rerolled patch

@szeidler could you give more details to reproduce it?

@alexmoreno maybe Imagick::queryFormats() needs to be in an IF statement that first checks if Imagick is even a thing. Then inside that IF do an additional check for WebP support.

I think this should solve the issue:

  $supported_formats = array();
  if (!extension_loaded('imagick')) {
    $supported_formats = array_map('strtoupper', Imagick::queryFormats());
  }

looks good to me @alexmoreno

@alexmoreno @caspervoogt
I get an error on a system without imagick, I think it should be

  $supported_formats = array();
  if (extension_loaded('imagick')) {
    $supported_formats = array_map('strtoupper', Imagick::queryFormats());
  }

yes, corrected @elex. I need to test it on that scenario though