php-imagine / Imagine

PHP Object Oriented image manipulation library
https://imagine.readthedocs.io
Other
4.42k stars 528 forks source link

Type declaration incompatibility with Imagick 3.5 #766

Closed brandonkelly closed 3 years ago

brandonkelly commented 3 years ago

Issue description

Imagine\Imagick\Imagick::thumbnailImage() has been semi-incompatible with Imagick::thumbnailImage() since Imagick 3.4, which added a new $legacy argument, which conflicts with the Imagine-added $filter argument.

Now with Imagick 3.5 (released 3 days ago with PHP 8 support), Imagine’s thumbnailImage() is fully incompatible with the core extension, which now has type declarations for each of its methods, leading to this PHP error:

Declaration of Imagine\Imagick\Imagick::thumbnailImage($columns, $rows, $bestfit = false, $fill = false, $filter = Imagick::FILTER_TRIANGLE, $keepImageProfiles = false, $keepExifData = false) must be compatible with Imagick::thumbnailImage(?int $columns, ?int $rows, bool $bestfit = false, bool $fill = false, bool $legacy = false):

What version of Imagine are you using?

1.2.4

What's the PHP version you are using?

8.0.3

What's the imaging library you are using [gd/imagick/gmagick/any]?

Imagick 3.5.0

What's the imaging library configuration

imagick
imagick module => enabled
imagick module version => 3.5.0
imagick classes => Imagick, ImagickDraw, ImagickPixel, ImagickPixelIterator, ImagickKernel
Imagick compiled with ImageMagick version => ImageMagick 7.0.11-13 Q16 x86_64 2021-05-16 https://imagemagick.org
Imagick using ImageMagick library version => ImageMagick 7.0.11-13 Q16 x86_64 2021-05-16 https://imagemagick.org
ImageMagick copyright => (C) 1999-2021 ImageMagick Studio LLC
ImageMagick release date => 2021-05-16
ImageMagick number of supported formats:  => 246
ImageMagick supported formats => 3FR, 3G2, 3GP, AAI, AI, APNG, ART, ARW, ASHLAR, AVI, AVIF, AVS, BGR, BGRA, BGRO, BMP, BMP2, BMP3, BRF, CAL, CALS, CANVAS, CAPTION, CIN, CIP, CLIP, CMYK, CMYKA, CR2, CR3, CRW, CUBE, CUR, CUT, DATA, DCM, DCR, DCRAW, DCX, DDS, DFONT, DNG, DOT, DPX, DXT1, DXT5, EPDF, EPI, EPS, EPS2, EPS3, EPSF, EPSI, EPT, EPT2, EPT3, ERF, FARBFELD, FAX, FF, FILE, FITS, FL32, FLV, FRACTAL, FTP, FTS, G3, G4, GIF, GIF87, GRADIENT, GRAY, GRAYA, GROUP4, GV, HALD, HDR, HEIC, HEIF, HISTOGRAM, HRZ, HTM, HTML, HTTP, HTTPS, ICB, ICO, ICON, IIQ, INFO, INLINE, IPL, ISOBRL, ISOBRL6, JNG, JNX, JPE, JPEG, JPG, JPS, JSON, K25, KDC, KERNEL, LABEL, M2V, M4V, MAC, MAP, MASK, MAT, MATTE, MEF, MIFF, MKV, MNG, MONO, MOV, MP4, MPC, MPEG, MPG, MRW, MSL, MSVG, MTV, MVG, NEF, NRW, NULL, ORA, ORF, OTB, OTF, PAL, PALM, PAM, PANGO, PATTERN, PBM, PCD, PCDS, PCL, PCT, PCX, PDB, PDF, PDFA, PEF, PES, PFA, PFB, PFM, PGM, PGX, PHM, PICON, PICT, PIX, PJPEG, PLASMA, PNG, PNG00, PNG24, PNG32, PNG48, PNG64, PNG8, PNM, POCKETMOD, PPM, PS, PS2, PS3, PSB, PSD, PTIF, PWP, RADIAL-GRADIENT, RAF, RAS, RAW, RGB, RGB565, RGBA, RGBO, RGF, RLA, RLE, RMF, RSVG, RW2, SCR, SCT, SFW, SGI, SHTML, SIX, SIXEL, SPARSE-COLOR, SR2, SRF, STEGANO, SUN, SVG, SVGZ, TEXT, TGA, THUMBNAIL, TIFF, TIFF64, TILE, TIM, TM2, TTC, TTF, TXT, UBRL, UBRL6, UIL, UYVY, VDA, VICAR, VID, VIFF, VIPS, VST, WBMP, WEBM, WEBP, WMV, WPG, X, X3F, XBM, XC, XCF, XPM, XPS, XV, XWD, YAML, YCbCr, YCbCrA, YUV
Directive => Local Value => Master Value
imagick.locale_fix => 0 => 0
imagick.skip_version_check => 0 => 0
imagick.progress_monitor => 0 => 0
imagick.set_single_thread => 1 => 1
imagick.shutdown_sleep_count => 10 => 10
imagick.allow_zero_dimension_images => 0 => 0

Minimal PHP code to reproduce the error:

class_exists('Imagine\Imagick\Imagick');
brandonkelly commented 3 years ago

Simply adding type declarations to Imagine\Imagick\Imagick::thumbnailImage() won’t work as that would cause a similar error for environments running Imagick < 3.5; plus you’d still have the conflict between the $legacy and $filter argument.

Is there a good reason why Imagine’s thumbnailImage() must extend the base method, as opposed to being declared as a completely new method name?

ausi commented 3 years ago
class_exists('Imagine\Imagick\Imagick');

Unable to reproduce this, class_exists('Imagine\Imagick\Imagick') returns false in my setup.

Declaration of Imagine\Imagick\Imagick::thumbnailImage(…) must be compatible with Imagick::thumbnailImage(…):

In which file on which line exactly does this error occur?

There seems to be no Imagine\Imagick\Imagick class in this library.

ausi commented 3 years ago

There seems to be no Imagine\Imagick\Imagick class in this library.

But it looks like your fork of the library does have that class: https://github.com/pixelandtonic/Imagine/blob/develop/src/Imagick/Imagick.php

brandonkelly commented 3 years ago

Doh! Sorry about that. It’s been a while and I didn’t realize that was something we had done on our end 🤦‍♂️