Commit 844f024a authored by Helmut Hummel's avatar Helmut Hummel Committed by Benni Mack
Browse files

[BUGFIX] Process cropped images only once

To avoid loss of quality and spawning unnecessary imagemagick
processes, cropping and scaling of images is now done
with a single imagemagick process.

By doing so, the code for SVG processing is streamlined.
SVG processing code can further be improved later,
by putting it into a dedicated file processing task processor.

Releases: master, 10.4
Resolves: #91855
Change-Id: I3bf735e74dd46dec73431405f37616506747ccdf
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65187


Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Markus Klein's avatarMarkus Klein <markus.klein@typo3.org>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Markus Klein's avatarMarkus Klein <markus.klein@typo3.org>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
parent c577de8b
......@@ -18,6 +18,7 @@ namespace TYPO3\CMS\Core\Imaging;
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Charset\CharsetConverter;
use TYPO3\CMS\Core\Core\Environment;
use TYPO3\CMS\Core\Imaging\ImageManipulation\Area;
use TYPO3\CMS\Core\Type\File\ImageInfo;
use TYPO3\CMS\Core\Utility\ArrayUtility;
use TYPO3\CMS\Core\Utility\CommandUtility;
......@@ -339,8 +340,6 @@ class GraphicalFunctions
// ... but if 'processor_effects' is set, enable effects
if ($gfxConf['processor_effects']) {
$this->processorEffectsEnabled = true;
$this->cmds['jpg'] .= $this->v5_sharpen(10);
$this->cmds['jpeg'] .= $this->v5_sharpen(10);
}
// Secures that images are not scaled up.
$this->mayScaleUp = (bool)$gfxConf['processor_allowUpscaling'];
......@@ -2097,9 +2096,6 @@ class GraphicalFunctions
$newExt = $info[2];
} else {
$newExt = $this->gif_or_jpg($info[2], $info[0], $info[1]);
if (!$params) {
$params = $this->cmds[$newExt];
}
}
}
if (!in_array($newExt, $this->imageFileExt, true)) {
......@@ -2115,7 +2111,7 @@ class GraphicalFunctions
// given or if the destination w/h matches the original image
// dimensions or if the option to not scale the image is set)
$noScale = !$w && !$h || $data[0] == $info[0] && $data[1] == $info[1] || !empty($options['noScale']);
if ($noScale && !$data['crs'] && !$params && !$frame && $newExt == $info[2] && !$mustCreate) {
if ($noScale && !$data['crs'] && !$params && !$frame && $newExt === $info[2] && !$mustCreate) {
// Set the new width and height before returning,
// if the noScale option is set
if (!empty($options['noScale'])) {
......@@ -2125,13 +2121,33 @@ class GraphicalFunctions
$info[3] = $imagefile;
return $info;
}
$info[0] = $data[0];
$info[1] = $data[1];
$frame = $this->addFrameSelection ? (int)$frame : 0;
if (!$params) {
$params = $this->cmds[$newExt];
$params = ($this->cmds[$newExt] ?? '') . ($params ? ' ' . $params : '');
$command = '';
// Cropping
if (($options['crop'] ?? null) instanceof Area) {
/** @var Area $cropArea */
$cropArea = $options['crop'];
$command .= sprintf(
$this->scalecmd . ' %dx%d! -crop %dx%d+%d+%d +repage ' . $params . ' ',
$info[0],
$info[1],
$cropArea->getWidth(),
$cropArea->getHeight(),
$cropArea->getOffsetLeft(),
$cropArea->getOffsetTop()
);
}
// Scaling when required
if ($info[0] !== $data[0] || $info[1] !== $data[1]) {
$effectsParams = ' ';
if ($this->processorEffectsEnabled && in_array($newExt, ['jpg', 'jpeg'], true)) {
$effectsParams .= $this->v5_sharpen(10) . ' ';
}
// Don't scale if original size equals target size
$command .= $this->scalecmd . ' ' . $data[0] . 'x' . $data[1] . '! ' . $params . $effectsParams;
}
// Cropscaling:
// Crop scaling:
if ($data['crs']) {
if (!$data['origW']) {
$data['origW'] = $data[0];
......@@ -2141,11 +2157,8 @@ class GraphicalFunctions
}
$offsetX = (int)(($data[0] - $data['origW']) * ($data['cropH'] + 100) / 200);
$offsetY = (int)(($data[1] - $data['origH']) * ($data['cropV'] + 100) / 200);
$params .= ' -crop ' . $data['origW'] . 'x' . $data['origH'] . '+' . $offsetX . '+' . $offsetY . '! +repage';
$command .= ' -crop ' . $data['origW'] . 'x' . $data['origH'] . '+' . $offsetX . '+' . $offsetY . '! +repage ' . $params . ' ';
}
$command = $this->scalecmd . ' ' . $info[0] . 'x' . $info[1] . '! ' . $params . ' ';
// re-apply colorspace-setting for the resulting image so colors don't appear to dark (sRGB instead of RGB)
$command .= ' -colorspace ' . $this->colorspace;
$cropscale = $data['crs'] ? 'crs-V' . $data['cropV'] . 'H' . $data['cropH'] : '';
if ($this->alternativeOutputKey) {
$theOutputName = GeneralUtility::shortMD5($command . $cropscale . PathUtility::basename($imagefile) . $this->alternativeOutputKey . '[' . $frame . ']');
......@@ -2163,13 +2176,15 @@ class GraphicalFunctions
$this->imageMagickExec($imagefile, $output, $command, $frame);
}
if (file_exists($output)) {
$info[3] = $output;
$info[0] = $data[0];
$info[1] = $data[1];
$info[2] = $newExt;
$info[3] = $output;
// params might change some image data!
if ($params) {
$info = $this->getImageDimensions($info[3]);
}
if ($info[2] == $this->gifExtension && !$this->dontCompress) {
if (!$this->dontCompress && $info[2] === $this->gifExtension) {
// Compress with IM (lzw) or GD (rle) (Workaround for the absence of lzw-compression in GD)
self::gifCompress($info[3], '');
}
......@@ -2370,28 +2385,28 @@ class GraphicalFunctions
// If scaling should be performed. Check that input "info" array will not cause division-by-zero
if (($w || $h) && $info[0] && $info[1]) {
if ($w && !$h) {
$info[1] = ceil($info[1] * ($w / $info[0]));
$info[1] = (int)ceil($info[1] * ($w / $info[0]));
$info[0] = $w;
}
if (!$w && $h) {
$info[0] = ceil($info[0] * ($h / $info[1]));
$info[0] = (int)ceil($info[0] * ($h / $info[1]));
$info[1] = $h;
}
if ($w && $h) {
if ($max) {
$ratio = $info[0] / $info[1];
if ($h * $ratio > $w) {
$h = round($w / $ratio);
$h = (int)round($w / $ratio);
} else {
$w = round($h * $ratio);
$w = (int)round($h * $ratio);
}
}
if ($crs) {
$ratio = $info[0] / $info[1];
if ($h * $ratio < $w) {
$h = round($w / $ratio);
$h = (int)round($w / $ratio);
} else {
$w = round($h * $ratio);
$w = (int)round($h * $ratio);
}
}
$info[0] = $w;
......@@ -2403,13 +2418,13 @@ class GraphicalFunctions
// Set minimum-measures!
if (isset($options['minW']) && $out[0] < $options['minW']) {
if (($max || $crs) && $out[0]) {
$out[1] = round($out[1] * $options['minW'] / $out[0]);
$out[1] = (int)round($out[1] * $options['minW'] / $out[0]);
}
$out[0] = $options['minW'];
}
if (isset($options['minH']) && $out[1] < $options['minH']) {
if (($max || $crs) && $out[1]) {
$out[0] = round($out[0] * $options['minH'] / $out[1]);
$out[0] = (int)round($out[0] * $options['minH'] / $out[1]);
}
$out[1] = $options['minH'];
}
......
......@@ -16,11 +16,11 @@
namespace TYPO3\CMS\Core\Resource\Processing;
use TYPO3\CMS\Core\Core\Environment;
use TYPO3\CMS\Core\Imaging\ImageManipulation\Area;
use TYPO3\CMS\Core\Resource;
use TYPO3\CMS\Core\Resource\FileInterface;
use TYPO3\CMS\Core\Resource\ProcessedFile;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Frontend\Imaging\GifBuilder;
/**
......@@ -62,6 +62,10 @@ class LocalCropScaleMaskHelper
*/
public function processWithLocalFile(TaskInterface $task, string $originalFileName): ?array
{
if (empty($GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_enabled'])) {
return null;
}
$result = null;
$targetFile = $task->getTargetFile();
......@@ -75,53 +79,16 @@ class LocalCropScaleMaskHelper
}
$options = $this->getConfigurationForImageCropScaleMask($targetFile, $gifBuilder);
$croppedImage = null;
if (!empty($configuration['crop'])) {
// check if it is a json object
$cropData = json_decode($configuration['crop']);
if ($cropData) {
$crop = implode(',', [(int)$cropData->x, (int)$cropData->y, (int)$cropData->width, (int)$cropData->height]);
} else {
$crop = $configuration['crop'];
}
[$offsetLeft, $offsetTop, $newWidth, $newHeight] = explode(',', $crop, 4);
$backupPrefix = $gifBuilder->filenamePrefix;
$gifBuilder->filenamePrefix = 'crop_';
$jpegQuality = MathUtility::forceIntegerInRange($GLOBALS['TYPO3_CONF_VARS']['GFX']['jpg_quality'], 10, 100, 85);
// the result info is an array with 0=width,1=height,2=extension,3=filename
$result = $gifBuilder->imageMagickConvert(
$originalFileName,
$configuration['fileExtension'],
'',
'',
sprintf('-crop %dx%d+%d+%d +repage -quality %d', $newWidth, $newHeight, $offsetLeft, $offsetTop, $jpegQuality),
'',
['noScale' => true],
true
);
$gifBuilder->filenamePrefix = $backupPrefix;
if ($result !== null) {
$originalFileName = $croppedImage = $result[3];
}
}
// Normal situation (no masking)
if (!(is_array($configuration['maskImages']) && $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_enabled'])) {
if (empty($configuration['maskImages'])) {
// the result info is an array with 0=width,1=height,2=extension,3=filename
$result = $gifBuilder->imageMagickConvert(
$originalFileName,
$configuration['fileExtension'],
$configuration['width'],
$configuration['height'],
$configuration['additionalParameters'],
$configuration['frame'],
$configuration['fileExtension'] ?? null,
$configuration['width'] ?? null,
$configuration['height'] ?? null,
$configuration['additionalParameters'] ?? null,
$configuration['frame'] ?? null,
$options
);
} else {
......@@ -138,16 +105,16 @@ class LocalCropScaleMaskHelper
$tempFileInfo = $gifBuilder->imageMagickConvert(
$originalFileName,
$temporaryExtension,
$configuration['width'],
$configuration['height'],
$configuration['additionalParameters'],
$configuration['frame'],
$configuration['width'] ?? null,
$configuration['height'] ?? null,
$configuration['additionalParameters'] ?? null,
$configuration['frame'] ?? null,
$options
);
if (is_array($tempFileInfo)) {
$maskBottomImage = $configuration['maskImages']['maskBottomImage'];
$maskBottomImage = $configuration['maskImages']['maskBottomImage'] ?? null;
if ($maskBottomImage instanceof FileInterface) {
$maskBottomImageMask = $configuration['maskImages']['maskBottomImageMask'];
$maskBottomImageMask = $configuration['maskImages']['maskBottomImageMask'] ?? null;
} else {
$maskBottomImageMask = null;
}
......@@ -189,7 +156,7 @@ class LocalCropScaleMaskHelper
// check if the processing really generated a new file (scaled and/or cropped)
if ($result !== null) {
if ($result[3] !== $originalFileName || $originalFileName === $croppedImage) {
if ($result[3] !== $originalFileName) {
$result = [
'width' => $result[0],
'height' => $result[1],
......@@ -201,11 +168,6 @@ class LocalCropScaleMaskHelper
}
}
// Cleanup temp file if it isn't used as result
if ($croppedImage && ($result === null || $croppedImage !== $result['filePath'])) {
GeneralUtility::unlink_tempfile($croppedImage);
}
return $result;
}
......@@ -219,22 +181,38 @@ class LocalCropScaleMaskHelper
{
$configuration = $processedFile->getProcessingConfiguration();
if ($configuration['useSample']) {
if ($configuration['useSample'] ?? null) {
$gifBuilder->scalecmd = '-sample';
}
$options = [];
if ($configuration['maxWidth']) {
if ($configuration['maxWidth'] ?? null) {
$options['maxW'] = $configuration['maxWidth'];
}
if ($configuration['maxHeight']) {
if ($configuration['maxHeight'] ?? null) {
$options['maxH'] = $configuration['maxHeight'];
}
if ($configuration['minWidth']) {
if ($configuration['minWidth'] ?? null) {
$options['minW'] = $configuration['minWidth'];
}
if ($configuration['minHeight']) {
if ($configuration['minHeight'] ?? null) {
$options['minH'] = $configuration['minHeight'];
}
if ($configuration['crop'] ?? null) {
$options['crop'] = $configuration['crop'];
// This normalisation is required to preserve backwards compatibility
// In the future the whole processing configuration should be a plain array or json serializable object for straightforward serialisation
// The crop configuration currently can be a json string, string with comma separated values or an Area object
if (is_string($configuration['crop'])) {
// check if it is a json object
$cropData = json_decode($configuration['crop']);
if ($cropData) {
$options['crop'] = new Area($cropData->x, $cropData->y, $cropData->width, $cropData->height);
} else {
[$offsetLeft, $offsetTop, $newWidth, $newHeight] = explode(',', $configuration['crop'], 4);
$options['crop'] = new Area($offsetLeft, $offsetTop, $newWidth, $newHeight);
}
}
}
$options['noScale'] = $configuration['noScale'];
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment