5
\$\begingroup\$

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 );
}
\$\endgroup\$
1
  • \$\begingroup\$ This blog post shows some alternate usage of the API, specifically loading the image and inspecting its width/height and allowing you to choose either fit or fill operation. \$\endgroup\$ – David Harkness Jul 27 '14 at 19:41
5
\$\begingroup\$

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:

  1. Read the original image size and type.
  2. Calculate the trimmed original size.
  3. Calculate the trimmed original origin.
  4. Read the original image from disk. (imageCreateFromType)
  5. Resize to a new image.
  6. 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);
}
\$\endgroup\$
2
  • \$\begingroup\$ I created a github repository and commit code written by following most of your suggestions. github.com/n4nn31355/phpSimpleThumbs Maybe it will help someone. If I must add you in authors, please mail me. \$\endgroup\$ – n4nn31355 Jul 29 '14 at 8:40
  • \$\begingroup\$ @n4nn31355 You are free to use any code I post to SE sites as you please without attribution. \$\endgroup\$ – David Harkness Jul 29 '14 at 19:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.