Imagick HHVM compatibility

[DuckDagobert]

Hey,

I am using

Array
(
    [versionNumber] => 1655
    [versionString] => ImageMagick 6.7.7-10 2014-03-08 Q16 http://www.imagemagick.org
)

in /wp-includes/class-wp-image-editor-imagick.php line 57-75 you list the required methods WP needs in imagick to later check using array_diff.
The problem is that you use old capitalisation e.g. getimage but Imagick lately (since at least 1 year already) uses getImage, writeImage,... so this will cause that imagick is never used.

Array
(
    [3] => getimage
    [4] => writeimage
    [5] => getimageblob
    [6] => getimagegeometry
    [7] => getimageformat
    [8] => setimageformat
    [9] => setimagecompression
    [10] => setimagecompressionquality
    [11] => setimagepage
    [12] => scaleimage
    [13] => cropimage
    [14] => rotateimage
    [15] => flipimage
    [16] => flopimage
)

As I saw activity in here: #21668 only a couple hours ago by an admin after years of inactivity but dont know how to get into the slack conversation, I opened a ticket here. Also it would be nice if you would finally support progressive jpegs as stated in that ticket.

What is your version of Imagick? With version 3.3.0 everything is still lowercase for me.

I am on HHVM and just updated to the latest version of Imagick yesterday (but capitalization was oon the version I had before too, which was around 7 months old) (​https://github.com/facebook/hhvm/tree/master/hphp/runtime/ext/imagick)

I also checked on stackoverflow, phpimagick,... and they all use Capitalized commands too.

That makes a lot of sense. Thats not the original Imagick but adjusted version for HHVM. I guess this is the risk of using HHVM. The commands are capitalized as getImage but get_class_methods( 'Imagick' ) returns it as getimage.

Btw, I added a patch and was wondering if you could test this one?

It looks like 35973.patch​ is ~19ms (on shared) slower than current trunk, so we probably want to see if there's an alternate way to do this comparison.

Thats not that bad for resizing images ;). 2nd patch would fix that.

@DuckDagobert Renamed the ticket so that the primary issue you found (with HHVM) is clear, and this doesn't get missed in 4.5.

I would encourage continued discussion on progressive JPEGs on #21668.

This is on my list today for deeper testing, and commit if it looks good.

In 36916:

Media: Support Imagick in HHVM.

Removes case-sensitivity from Imagick feature detection within WP_Image_Editor_Imagick::test().
This allows correct detection of Imagick support within HHVM.

Props markoheijnen, DuckDagobert.
Fixes #35973.

Hmm, while I saw it working during testing, this introduces two failing Imagick unit tests for HHVM on Travis:
​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/114952704#L1184

Going to see what I can do to get unit tests running on HHVM on this side, but any digging into this is appreciated.

Don't use HHVM when loading image file from URL.

Imagick in HHVM fails when trying to load a file from a URL.

35973.3.patch​ works around this by failing test() when a URL is being loaded, causing WordPress to choose the "next best" editor (which is usually GD, which does load URLs in HHVM).

This fixes the test failure for Tests_Image_Functions::test_wp_crop_image_url().

Looks good

Reported upstream here: ​https://github.com/facebook/hhvm/issues/6908

In 36996:

Media: Fall back to GD when loading URL in HHVM Imagick.

HHVM does not currently support loading URLs in its port of Imagick.
Fixes test_wp_crop_image_url() failure introduced in [36916].

See #35973.

What's left: Still trying to reproduce the test_image_preserves_alpha_on_rotate() failure.

When testing with a local HHVM 3.6.6 (with the same build hash), it does not fail.

Opened #36311 to track test_image_preserves_alpha_on_rotate() test failure.

Closing this out, since HHVM Imagick detection is fixed.