Resize and compress images with imagick

Question

I am building a PHP function to manipulate and generate a series of images with Imagick. The aim is to make responsive images with the <picture> element.

Basically this is what it does:

• Checks if the original image exist.
• Check if the thumbnails already exist and generates the <picture> element.
  Otherwise:
  • Resize images; from the original image it creates 10 thumbnails
  • Compress the thumbnails
  • Saves the thumbnails
  • Generate the <picture> element with the thumbnails

The function uses an array as parameters that looks like this:

$params = array(
  'thumbWidth'  => array( 500,    750,    1000,     1600,     2000),
  'thumbHeight' => array( 333,    500,    667,     1067,     1500),
  'breakPoints' => array(     500,    750,    1000,     1600),
  'quality'     => 65
);

The thumbHeight array can be null so the function only resizes the tumbnails according to the widths. Keeping the aspect ratio.

This is the function:

<?php
public static function picturebuilder($imgUrl, $imageAlt, $params) {
    // check if the image exists
    if (!file_exists($imgUrl)) {
      echo "<script>console.log( 'Cannot find : " . $imgUrl . "' );</script>";
      return false;
    }


    // Get the original image info
    $imgInfo = pathinfo($imgUrl);
    $imgName = $imgInfo['filename'];
    $imgExtension = '.' . $imgInfo['extension'];
    $thumbDir = 'image_thumbs/' . $imgInfo['dirname'];

    // set HD upscale ratio
    $upscale_HD_images = 1.5;
    $thumbQuality = $params['quality'];
    $breakPoints = $params['breakPoints'];

    // Build the Thumb width array
    $paramWidths = $params['thumbWidth'];
    $thumbWidths = array();
    foreach ($paramWidths as $paramWidth) {
      array_push($thumbWidths, $paramWidth);
      array_push($thumbWidths, floor($paramWidth * $upscale_HD_images));
    }

    // Build the thumb height array
    $paramHeights = $params['thumbHeight'];
    $thumbHeights = array();
    list($XLwidth, $XLheight) = getimagesize($imgUrl);
    if ($paramHeights) {
      $crop = true;
      // If height is specified get the width and height of image from params
      foreach ($paramHeights as $paramHeight) {
        array_push($thumbHeights, $paramHeight);
        array_push($thumbHeights, floor($paramHeight * $upscale_HD_images));
      }
    } else {
      // If no height is specified get the width and height of image form original file
      foreach ($thumbWidths as $thumbWidth) {
        $thumbHeight = floor($thumbWidth*$XLheight/$XLwidth);
        array_push($thumbHeights, $thumbHeight);
      }
    }

    // Create the thumb paths array
    $i = 0;
    $thumbPaths = array();
    foreach ($thumbWidths as $thumbWidth) {
      $thumbHeight = $thumbHeights[$i];
      $thumbPath = $thumbDir . '/' . $imgName . '-' . $thumbWidth . 'x' . $thumbHeight . 'q' . $thumbQuality . $imgExtension;
      array_push($thumbPaths,$thumbPath);
      $i++;
    }


    // Generate thumbnails if the number 8 doesn't exist
    if(!file_exists($thumbPaths[8])){
      // Create folders if they don't exixt
      if (!file_exists($thumbDir)) {
        mkdir($thumbDir, 0777, true);
      }

      $i = 0;
      if($crop) {
        foreach ($thumbWidths as $thumbWidth) {
          $imagick = new imagick($imgUrl);
          $thumbHeight = $thumbHeights[$i];
          $imagick->cropThumbnailImage($thumbWidth,$thumbHeight);
          $imagick->setImageCompressionQuality($thumbQuality);
          $imagick->writeImage($thumbPaths[$i]);
          $imagick->clear();
          $imagick->destroy();
          $i++;
        }
      } else {
        foreach ($thumbWidths as $thumbWidth) {
          $imagick = new imagick($imgUrl);
          $thumbHeight = $thumbHeights[$i];
          $imagick->resizeImage($thumbWidth,$thumbHeight,imagick::FILTER_LANCZOS, 1, true);
          $imagick->setImageCompressionQuality($thumbQuality);
          $imagick->writeImage($thumbPaths[$i]);
          $imagick->clear();
          $imagick->destroy();
          $i++;
        }
      }
    }
    ?>

    <picture>
      <source  srcset="<?php echo $thumbPaths[9] . ' 2x,' . $thumbPaths[8]; ?>"  media="(min-width: <?php echo $breakPoints[3]; ?>px)"/>
      <source  srcset="<?php echo $thumbPaths[7] . ' 2x,' . $thumbPaths[6]; ?>"  media="(min-width: <?php echo $breakPoints[2]; ?>px) and (max-width: <?php echo $breakPoints[3] - 1; ?>px)"/>
      <source  srcset="<?php echo $thumbPaths[5] . ' 2x,' . $thumbPaths[4]; ?>"  media="(min-width: <?php echo $breakPoints[1]; ?>px) and (max-width: <?php echo $breakPoints[2] - 1; ?>px)"/>
      <source  srcset="<?php echo $thumbPaths[3] . ' 2x,' . $thumbPaths[2]; ?>"  media="(min-width: <?php echo $breakPoints[0]; ?>px) and (max-width: <?php echo $breakPoints[1] - 1; ?>px)"/>
      <source  srcset="<?php echo $thumbPaths[1] . ' 2x,' . $thumbPaths[0]; ?>"  media="(max-width: <?php echo $breakPoints[0] - 1; ?>px)"/>
      <img src="<?php echo $thumbPaths[8]; ?>"
        srcset="<?php echo $thumbPaths[9] . ' 2x,' . $thumbPaths[8]; ?>"
        alt="<?php echo $imageAlt; ?>"
        width="<?php echo $thumbWidths[8]; ?>"
        height="<?php echo $thumbHeights[8]; ?>"
      />
    </picture>
    <?php
  }
  ?>

This is what the function call looks like (I am using this in a CMS):

$params = array(
  'thumbWidth'  => array( 500,    750,    1000,     1600,     2000),
  'thumbHeight' => null,
  'breakPoints' => array(     500,    750,    1000,     1600),
  'quality'     => 65
);
$imgUrl = 'images/my-big-image.jpg';
$alt= 'Alternative text for my big image';
echo plgContentPicturebuilder::picturebuilder($imgUrl,$alt,$params);

I have successfully tested this function but I am concerned about the performance. Depending on the number of images per page, the function can be called over 20 times on first page load and be asked to generate over 200 thumbnails.
This is when it gets very slow and generates 504 Gateway Time Out errors on my server.

Answer 1

The feedback below won't likely prevent time out errors, but hopefully will clean up the code.

Description-code mismatch

The narrative in your post states:

Basically this is what it does :

• checks if the images exist. If it does it generates the <picture> element.

But the first few lines of the function are:

public static function picturebuilder($imgUrl, $imageAlt, $params) {
    // check if the image exists
    if (!file_exists($imgUrl)) {
      echo "<script>console.log( 'Cannot find : " . $imgUrl . "' );</script>";
      return false;
    }

So the code appears to actually add a JavaScript console.log() call on the client side, instead of adding a <picture> element...

Outputting JavaScript with echo

If you are going to add JavaScript to the client side and echo the image path (i.e. $imgUrl) it would be wise to sanitize that value so it won't break the JavaScript. One technique for that is to use json_encode() or htmlspecialchars().

echo "<script>console.log( 'Cannot find : " . htmlspecialchars($imgUrl) . "' );</script>";

You didn't specify which file system the files are coming from, but some systems allow single-quotes and semi-colons in file names (e.g. the MS NT file system - refer to this answer for a summary).

So it may seem unlikely but possibly that a filename like the following would be passed:

$imgUrl = "photo1';compressed.png";

See how well that works with the echo statement from your code in this playground example. In the browser console, there should be an error:

Uncaught SyntaxError: missing ) after argument list

incrementing a counter variable inside a foreach

The foreach statement has "two syntaxes"¹ and because the array indexes of $thumbWidths are numeric, the second one can be used to have the value of $i updated automatically (i.e. to have each numeric index assigned , starting at 0), reducing the initial assignment of the counter variable (i.e. $i = 0;) and updating the counter value at the end of each block (i.e. $i++;).

foreach (array_expression as $key => $value)

So blocks like this:

$i = 0;
$thumbPaths = array();
foreach ($thumbWidths as $thumbWidth) {
  $thumbHeight = $thumbHeights[$i];
  $thumbPath = $thumbDir . '/' . $imgName . '-' . $thumbWidth . 'x' . $thumbHeight . 'q' . $thumbQuality . $imgExtension;
  array_push($thumbPaths,$thumbPath);
  $i++;
}

Can be simplified like so:

$thumbPaths = array();
foreach ($thumbWidths as $i => $thumbWidth) {
  $thumbHeight = $thumbHeights[$i];
  $thumbPath = $thumbDir . '/' . $imgName . '-' . $thumbWidth . 'x' . $thumbHeight . 'q' . $thumbQuality . $imgExtension;
  array_push($thumbPaths,$thumbPath);
}

And array_map() could also allow it to be simplified even further, though the variables outside the foreach loop would have to be imported into the callback function with the use statement, which might seem a bit wonky:

$prefix = $thumbDir . '/' . $imgName . '-';
$suffix = 'q' . $thumbQuality . $imgExtension
$thumbPaths = array_map(function($thumbWidth, $i) use ($thumbHeights, $prefix, $suffix) {
    $thumbHeight = $thumbHeights[$i];
    return $prefix . $thumbWidth . 'x' . $thumbHeight . $suffix;
}, $thumbWidths, array_keys($thumbWidths));

Repeated code when writing images

The code in the if-else blocks near the end is very identical... the only thing that changes is if $crop evaluates to true then there is a call to cropThumbnailImage(), otherwise resizeImage() is called. Outside of that, the rest of the code appears identical:

if($crop) {
    foreach ($thumbWidths as $thumbWidth) {
      $imagick = new imagick($imgUrl);
      $thumbHeight = $thumbHeights[$i];
      $imagick->cropThumbnailImage($thumbWidth,$thumbHeight);
      $imagick->setImageCompressionQuality($thumbQuality);
      $imagick->writeImage($thumbPaths[$i]);
      $imagick->clear();
      $imagick->destroy();
      $i++;
    }
  } else {
    foreach ($thumbWidths as $thumbWidth) {
      $imagick = new imagick($imgUrl);
      $thumbHeight = $thumbHeights[$i];
      $imagick->resizeImage($thumbWidth,$thumbHeight,imagick::FILTER_LANCZOS, 1, true);
      $imagick->setImageCompressionQuality($thumbQuality);
      $imagick->writeImage($thumbPaths[$i]);
      $imagick->clear();
      $imagick->destroy();
      $i++;
    }
  }

So that could be simplified to the following (also uses the numerical-index array syntax - see above):

foreach ($thumbWidths as $i => $thumbWidth) {
    $imagick = new imagick($imgUrl);
    $thumbHeight = $thumbHeights[$i];
    if ($crop) {
        $imagick->cropThumbnailImage($thumbWidth,$thumbHeight);
    }
    else {
        $imagick->resizeImage($thumbWidth,$thumbHeight,imagick::FILTER_LANCZOS, 1, true);
    }
    $imagick->setImageCompressionQuality($thumbQuality);
    $imagick->writeImage($thumbPaths[$i]);
    $imagick->clear();
    $imagick->destroy();
}