Create thumbnail in the original ratio without empty space

I need to make thumbnails without empty space and in the original ratio. Please help me check this algorithm to improve it.

public function createThumbnail($imagePath, $thumbnailPath, $targetWidth, $targetHeight)
{
    list( $originalWidth, $originalHeight, $originalType ) = getimagesize($imagePath);
    $targetRatio = $targetWidth / $targetHeight;
    $originalRatio = $originalWidth / $originalHeight;

    if ( $originalRatio >= $targetRatio ) {
        if ( $originalRatio >= 1 ) {
            $sourceWidth = $originalHeight * $targetRatio;
            $sourceHeight = $originalHeight;
            $sourceX = ( $originalWidth - $sourceWidth ) / 2;
            $sourceY = 0;
        } else {
            $sourceWidth = $originalWidth / $originalRatio * $targetRatio;
            $sourceHeight = $originalHeight;
            $sourceX = ( $originalWidth - $sourceWidth ) / 2;
            $sourceY = 0;
        }
    } else {
        if ( $originalRatio >= 1 ) {
            $sourceWidth = $originalWidth * $originalRatio / $targetRatio;
            $sourceHeight = $originalHeight;
            $sourceX = ( $originalWidth - $sourceWidth ) / 2;
            $sourceY = 0;
        } else {
            $sourceWidth = $originalWidth;
            $sourceHeight = $originalHeight * $originalRatio / $targetRatio;
            $sourceX = 0;
            $sourceY = ( $originalHeight - $sourceHeight ) / 2;
        }
    }

    $originalImage = $this->imageCreateFromType( $originalType, $imagePath );
    $thumbnailImage = imagecreatetruecolor( $targetWidth, $targetHeight );

    imagecopyresampled( $thumbnailImage, $originalImage, 0, 0, $sourceX, $sourceY, $targetWidth, $targetHeight, $sourceWidth, $sourceHeight );

    imagepng( $thumbnailImage, $thumbnailPath );
}

The first thing that's really needed are some comments describing the different conditions. Sure, you can work them out every time you read the code, but that's error-prone busy-work that you can avoid for future maintainers. You don't need to go crazy with ASCII graphics, though this is one case that might actually deserve them! :)

Here's an example:

if ( $originalRatio >= $targetRatio )
    // original is more landscape
    if ( $originalRatio >= 1 )
        // original is landscape; shrink horizontally
    else
        // both are portrait; shrink horizontally
else
    // original is more portrait
    if ( $originalRatio >= 1 )
        // both are landscape; shrink vertically
    else
        // original is portrait; shrink vertically

Note: Assuming those comments are correct, your calculations in the third case are incorrect.

As for the calculations themselves, it may be more intuitive to calculate the source width/height in each block and move the origin calculations below. It's certainly less code since you can easily calculate the origin from the size. And if you start the source width/height equal to the original values, you only need to set one dimension in each if block.

$sourceWidth = $originalWidth;
$sourceHeight = $originalHeight;

... shrink $sourceWidth or $sourceHeight ...

$sourceX = ($originalWidth - $sourceWidth) / 2;
$sourceY = ($originalHeight - $sourceHeight) / 2;

This relatively-small function (by procedural coding standards) is very difficult to test. Refactor it into several small functions so that each function does one thing:

• Read the original image size and type.

• Calculate the trimmed original size.

• Calculate the trimmed original origin.

• Read the original image from disk. (imageCreateFromType)

• Resize to a new image.

• Write new image to disk.

1, 5, and 6 are single function calls to the GD library already, but I can see 4-6 making a nice "scale disk image" function together.

public function createThumbnail($imagePath, $thumbnailPath, $targetWidth, $targetHeight) {
    list ($originalWidth, $originalHeight, $originalType) = getimagesize($imagePath);
    $trimmedSize = $this->calculateTrimmedSize(
        $originalWidth, $originalHeight, $targetWidth, $targetHeight
    );
    $trimmedOrigin = $this->calculateTrimmedOrigin(
        $originalWidth, $originalHeight, $trimmedSize
    );
    $this->scaleDiskImage(
        $imagePath, $trimmedOrigin, $trimmedSize, 
        $thumbnailPath, $targetWidth, $targetHeight
    );
}

I really don't like dealing with separate width, height, x, and y values and passing them around and would prefer to define Size and Point classes. If this function is the extent of the image manipulation, it's probably not worth the effort, small as it would be. But in larger applications they would clean up the code quite a bit.

Here's the same code as if we had those classes plus a custom ImageInfo that encapsulates the path, size, and type.

public function createThumbnail($imagePath, $thumbnailPath, $targetSize) {
    $original = $this->getImageInfo($imagePath);
    $trimmedSize = $this->calculateTrimmedSize(original->getSize(), $targetSize);
    $trimmedOrigin = $this->calculateTrimmedOrigin(original->getSize(), $trimmedSize);
    $this->scaleDiskImage($imagePath, $trimmedOrigin, $trimmedSize, $thumbnailPath, $targetSize);
}

StackExchange Code Review Q#58169, answer score: 5