-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DEP Upgrade to intervention/image 3 #621
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,7 @@ | |
|
||
namespace SilverStripe\Assets\Conversion; | ||
|
||
use Imagick; | ||
use Intervention\Image\Exception\ImageException; | ||
use Intervention\Image\Exceptions\RuntimeException; | ||
use SilverStripe\Assets\File; | ||
use SilverStripe\Assets\Image_Backend; | ||
use SilverStripe\Assets\InterventionBackend; | ||
|
@@ -31,7 +30,9 @@ public function supportsConversion(string $fromExtension, string $toExtension, a | |
if (!is_a($backend, InterventionBackend::class)) { | ||
return false; | ||
} | ||
return $this->supportedByIntervention($fromExtension, $backend) && $this->supportedByIntervention($toExtension, $backend); | ||
/** @var InterventionBackend $backend */ | ||
$driver = $backend->getImageManager()->driver(); | ||
return $driver->supports($fromExtension) && $driver->supports($toExtension); | ||
Comment on lines
-34
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is built in now! 🎉 No need for our hardcoded nonsense lol |
||
} | ||
|
||
public function convert(DBFile|File $from, string $toExtension, array $options = []): DBFile | ||
|
@@ -46,7 +47,9 @@ public function convert(DBFile|File $from, string $toExtension, array $options = | |
$actualClass = $originalBackend ? get_class($originalBackend) : 'null'; | ||
throw new FileConverterException("ImageBackend must be an instance of InterventionBackend. Got $actualClass"); | ||
} | ||
if (!$this->supportedByIntervention($toExtension, $originalBackend)) { | ||
/** @var InterventionBackend $originalBackend */ | ||
$driver = $originalBackend->getImageManager()->driver(); | ||
if (!$driver->supports($toExtension)) { | ||
throw new FileConverterException("Convertion to format '$toExtension' is not suported."); | ||
} | ||
|
||
|
@@ -66,7 +69,7 @@ function (AssetStore $store, string $filename, string $hash, string $variant) us | |
return [$tuple, $backend]; | ||
} | ||
); | ||
} catch (ImageException $e) { | ||
} catch (RuntimeException $e) { | ||
Comment on lines
-69
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this is Intervention's special exception, not the base one.... I can add an alias if you want to avoid confusion. |
||
throw new FileConverterException('Failed to convert: ' . $e->getMessage(), $e->getCode(), $e); | ||
} | ||
// This is very unlikely but the API for `manipulateExtension()` allows for it | ||
|
@@ -90,102 +93,4 @@ private function validateOptions(array $options): array | |
} | ||
return $problems; | ||
} | ||
|
||
private function supportedByIntervention(string $format, InterventionBackend $backend): bool | ||
{ | ||
$driver = $backend->getImageManager()->config['driver'] ?? null; | ||
|
||
// Return early for empty values - we obviously can't support that | ||
if ($format === '') { | ||
return false; | ||
} | ||
|
||
$format = strtolower($format); | ||
|
||
// If the driver is somehow not GD or Imagick, we have no way to know what it might support | ||
if ($driver !== 'gd' && $driver !== 'imagick') { | ||
$supported = false; | ||
$this->extend('updateSupportedByIntervention', $supported, $format, $driver); | ||
return $supported; | ||
} | ||
|
||
// GD and Imagick support different things. | ||
// This follows the logic in intervention's AbstractEncoder::process() method | ||
// and the various methods in the Encoder classes for GD and Imagick, | ||
// excluding checking for strings that were obviously mimetypes | ||
switch ($format) { | ||
case 'gif': | ||
// always supported | ||
return true; | ||
case 'png': | ||
// always supported | ||
return true; | ||
case 'jpg': | ||
case 'jpeg': | ||
case 'jfif': | ||
// always supported | ||
return true; | ||
case 'tif': | ||
case 'tiff': | ||
if ($driver === 'gd') { | ||
false; | ||
} | ||
// always supported by imagick | ||
return true; | ||
case 'bmp': | ||
case 'ms-bmp': | ||
case 'x-bitmap': | ||
case 'x-bmp': | ||
case 'x-ms-bmp': | ||
case 'x-win-bitmap': | ||
case 'x-windows-bmp': | ||
case 'x-xbitmap': | ||
if ($driver === 'gd' && !function_exists('imagebmp')) { | ||
return false; | ||
} | ||
// always supported by imagick | ||
return true; | ||
case 'ico': | ||
if ($driver === 'gd') { | ||
return false; | ||
} | ||
// always supported by imagick | ||
return true; | ||
case 'psd': | ||
if ($driver === 'gd') { | ||
return false; | ||
} | ||
// always supported by imagick | ||
return true; | ||
case 'webp': | ||
if ($driver === 'gd' && !function_exists('imagewebp')) { | ||
return false; | ||
} | ||
if ($driver === 'imagick' && !Imagick::queryFormats('WEBP')) { | ||
return false; | ||
} | ||
return true; | ||
case 'avif': | ||
if ($driver === 'gd' && !function_exists('imageavif')) { | ||
return false; | ||
} | ||
if ($driver === 'imagick' && !Imagick::queryFormats('AVIF')) { | ||
return false; | ||
} | ||
return true; | ||
case 'heic': | ||
if ($driver === 'gd') { | ||
return false; | ||
} | ||
if ($driver === 'imagick' && !Imagick::queryFormats('HEIC')) { | ||
return false; | ||
} | ||
return true; | ||
default: | ||
// Anything else is not supported | ||
return false; | ||
} | ||
// This should never be reached, but return false if it is | ||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to go with the latest version in the end because it introduces some API that we need.