Skip to content

Use Imagick methods instead of mogrify() for repo theme images #2375

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.

Sign up for GitHub

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

Open
sbreker wants to merge 3 commits into
base: qa/2.x
Choose a base branch
from
Open

Use Imagick methods instead of mogrify() for repo theme images #2375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a736da
Use Imagick methods for resizing repo theme images
sbreker Jun 19, 2026
5a7e19f
Update repository logo and banner crop logic
sbreker Jun 27, 2026
3869a0a
Add Imagick resource limit theme img processing
sbreker Jun 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions apps/qubit/modules/repository/actions/editThemeAction.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ protected function addField($name)
sfContext::getInstance()->getConfiguration()->loadHelpers('Url');

$this->form->setValidator($name, new sfValidatorFile([
'max_size' => '262144', // 256K
'max_size' => '512000', // 500K
'mime_types' => ['image/png'],
// Crop image, it is synchronous but it should be fast
'validated_file_class' => 'arRepositoryThemeCropValidatedFile',
Expand All @@ -129,7 +129,7 @@ protected function addField($name)
$this->form->setWidget($name, new arB5WidgetFormInputFileEditable([
'label' => $this->context->i18n->__('Banner'),
'help' => $this->context->i18n->__(
'Requirements: PNG format, 256K max. size.<br />Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.',
'Requirements: PNG format, 500K max. size.<br />Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.',
[
'%1%' => arRepositoryThemeCropValidatedFile::BANNER_MAX_WIDTH,
'%2%' => arRepositoryThemeCropValidatedFile::BANNER_MAX_HEIGHT,
Expand All @@ -153,7 +153,7 @@ protected function addField($name)
sfContext::getInstance()->getConfiguration()->loadHelpers('Url');

$this->form->setValidator($name, new sfValidatorFile([
'max_size' => '262144', // 256K
'max_size' => '512000', // 500K
'mime_types' => ['image/png'],
// Crop image, it is synchronous but it should be fast
'validated_file_class' => 'arRepositoryThemeCropValidatedFile',
Expand All @@ -164,7 +164,7 @@ protected function addField($name)
$this->form->setWidget($name, new arB5WidgetFormInputFileEditable([
'label' => $this->context->i18n->__('Logo'),
'help' => $this->context->i18n->__(
'Requirements: PNG format, 256K max. size.<br />Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.',
'Requirements: PNG format, 500K max. size.<br />Recommended dimensions of %1%x%2%px, it will be cropped if ImageMagick is installed.',
[
'%1%' => arRepositoryThemeCropValidatedFile::LOGO_MAX_WIDTH,
'%2%' => arRepositoryThemeCropValidatedFile::LOGO_MAX_HEIGHT,
Expand Down
115 changes: 90 additions & 25 deletions lib/form/arRepositoryThemeCropValidatedFile.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,48 +29,113 @@ class arRepositoryThemeCropValidatedFile extends sfValidatedFile
public const LOGO_MAX_HEIGHT = 270;
public const BANNER_MAX_WIDTH = 800;
public const BANNER_MAX_HEIGHT = 300;
public const IMAGICK_MEMORY_LIMIT_MB = 64;
public const IMAGICK_MAP_LIMIT_MB = 64;
public const IMAGICK_AREA_LIMIT_PIXELS = 16000000;

public function save($file = null, $fileMode = 0666, $create = true, $dirMode = 0777)
{
$file = parent::save($file, $fileMode, $create, $dirMode);

// Check if mogrify is available in the system
exec('which mogrify', $output, $status);
if (0 < $status) {
if (!$this->shouldCropImages()) {
return $file;
}

// Figure out necessary dimensions from the filename
$pathInfo = pathinfo($this->savedName);
if (null === $dimensions = self::getTargetDimensionsFromPath($this->savedName)) {
return $file;
}

$this->resizeAndCropImage($dimensions['width'], $dimensions['height']);

return $file;
}

public static function getTargetDimensionsFromPath($path)
{
$pathInfo = pathinfo($path);

switch ($pathInfo['filename']) {
case 'logo':
$width = self::LOGO_MAX_WIDTH;
$height = self::LOGO_MAX_HEIGHT;

break;
return [
'width' => self::LOGO_MAX_WIDTH,
'height' => self::LOGO_MAX_HEIGHT,
];

case 'banner':
$width = self::BANNER_MAX_WIDTH;
$height = self::BANNER_MAX_HEIGHT;

break;
return [
'width' => self::BANNER_MAX_WIDTH,
'height' => self::BANNER_MAX_HEIGHT,
];
}
}

// Stop execution if dimensions were not set
if (!isset($width, $height)) {
return $file;
public static function getResizeGeometry($sourceWidth, $sourceHeight, $targetWidth, $targetHeight)
{
if (
0 >= $sourceWidth
|| 0 >= $sourceHeight
|| 0 >= $targetWidth
|| 0 >= $targetHeight
) {
return null;
}

// mogrify overwrites the original image file
$command = sprintf(
'mogrify -crop %sx%s+0+0 %s',
$width,
$height,
$this->savedName
);
exec($command, $output, $status);
// Scale to fully cover the target box while preserving aspect ratio.
$scale = max($targetWidth / $sourceWidth, $targetHeight / $sourceHeight);
$resizeWidth = (int) ceil($sourceWidth * $scale);
$resizeHeight = (int) ceil($sourceHeight * $scale);

return $file;
return [
// Crop back to the exact target size from the centered overflow.
'resizeWidth' => $resizeWidth,
'resizeHeight' => $resizeHeight,
'cropX' => max(0, (int) floor(($resizeWidth - $targetWidth) / 2)),
'cropY' => max(0, (int) floor(($resizeHeight - $targetHeight) / 2)),
];
}

protected function shouldCropImages()
{
return extension_loaded('imagick');
}

protected function resizeAndCropImage($width, $height)
{
try {
$image = new Imagick();
$image->setResourceLimit(Imagick::RESOURCETYPE_MEMORY, self::IMAGICK_MEMORY_LIMIT_MB);
$image->setResourceLimit(Imagick::RESOURCETYPE_MAP, self::IMAGICK_MAP_LIMIT_MB);
$image->setResourceLimit(Imagick::RESOURCETYPE_AREA, self::IMAGICK_AREA_LIMIT_PIXELS);
$image->readImage($this->savedName);
$geometry = self::getResizeGeometry(
$image->getImageWidth(),
$image->getImageHeight(),
$width,
$height
);

if (null === $geometry) {
$image->clear();
$image->destroy();

return;
}

$image->resizeImage(
$geometry['resizeWidth'],
$geometry['resizeHeight'],
Imagick::FILTER_LANCZOS,
1
);
$image->cropImage($width, $height, $geometry['cropX'], $geometry['cropY']);
$image->setImagePage(0, 0, 0, 0);
$image->stripImage();
$image->setImageCompressionQuality(90);
$image->writeImage($this->savedName);
$image->clear();
$image->destroy();
} catch (Exception $e) {
// Leave the uploaded file untouched if Imagick cannot process it.
}
}
}
99 changes: 99 additions & 0 deletions test/phpunit/lib/form/arRepositoryThemeCropValidatedFileTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php

use PHPUnit\Framework\TestCase;

require_once __DIR__.'/../../../../lib/form/arRepositoryThemeCropValidatedFile.class.php';

/**
* @internal
*
* @covers \arRepositoryThemeCropValidatedFile
*/
class arRepositoryThemeCropValidatedFileTest extends TestCase
{
public function testLogoUploadsUseExpectedTargetDimensions()
{
$this->assertSame(
[
'width' => arRepositoryThemeCropValidatedFile::LOGO_MAX_WIDTH,
'height' => arRepositoryThemeCropValidatedFile::LOGO_MAX_HEIGHT,
],
arRepositoryThemeCropValidatedFile::getTargetDimensionsFromPath('/tmp/logo.png')
);
}

public function testBannerUploadsUseExpectedTargetDimensions()
{
$this->assertSame(
[
'width' => arRepositoryThemeCropValidatedFile::BANNER_MAX_WIDTH,
'height' => arRepositoryThemeCropValidatedFile::BANNER_MAX_HEIGHT,
],
arRepositoryThemeCropValidatedFile::getTargetDimensionsFromPath('/tmp/banner.png')
);
}

public function testUnknownUploadNamesAreNotCropped()
{
$this->assertNull(arRepositoryThemeCropValidatedFile::getTargetDimensionsFromPath('/tmp/other.png'));
}

public function testResizeGeometryForLandscapeLogoCentersHorizontalCrop()
{
$this->assertSame(
[
'resizeWidth' => 540,
'resizeHeight' => 270,
'cropX' => 135,
'cropY' => 0,
],
arRepositoryThemeCropValidatedFile::getResizeGeometry(1200, 600, 270, 270)
);
}

public function testResizeGeometryForPortraitLogoCentersVerticalCrop()
{
$this->assertSame(
[
'resizeWidth' => 270,
'resizeHeight' => 540,
'cropX' => 0,
'cropY' => 135,
],
arRepositoryThemeCropValidatedFile::getResizeGeometry(600, 1200, 270, 270)
);
}

public function testResizeGeometryForBannerCentersVerticalCrop()
{
$this->assertSame(
[
'resizeWidth' => 800,
'resizeHeight' => 600,
'cropX' => 0,
'cropY' => 150,
],
arRepositoryThemeCropValidatedFile::getResizeGeometry(1600, 1200, 800, 300)
);
}

public function testResizeGeometryRejectsInvalidDimensions()
{
$this->assertNull(arRepositoryThemeCropValidatedFile::getResizeGeometry(0, 1200, 270, 270));
}

public function testCroppingRequiresImagickExtension()
{
$file = new TestRepositoryThemeCropValidatedFile('logo.png', 'image/png', '/tmp/logo.png', 0);

$this->assertSame(extension_loaded('imagick'), $file->shouldCropImagesForTest());
}
}

class TestRepositoryThemeCropValidatedFile extends arRepositoryThemeCropValidatedFile
{
public function shouldCropImagesForTest()
{
return $this->shouldCropImages();
}
}
Loading