2
\$\begingroup\$

I am calling show_thumb to create thumbnails , but I am realizing that it is making the site load a little bit slower.

Below are two functions: show_thumb function that shows the thumbnail and make_thumb_gd that takes care of the thumbnail. Any suggestion on making it faster? The current approach makes the site slow, because those thumbnails are shown in the moving Jcarousel that contains the links returned by show_thumb.

Show thumb function

function show_thumb($file_org, $width, $height,$folder_version,$ratio_type = 'width')
{

    $file_name = str_replace(SITE_WS_PATH."/", "", $file_org);
    $file_name = str_replace("/", "^", $file_name);
    $file_name = preg_replace("/[ ]+/", "_", $file_name);
    $cache_file = $width."x".$height.'__'.$ratio_type .'__'.$file_name;
    $folder_version = trim($folder_version); 

    if($folder_version =="english"){

        $version ="";
    }
    else {

        $version = $folder_version;
    }

    $file_fs_path = str_replace(SITE_WS_PATH, ROOT, $file_org); 

    if(!is_file(ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file)) {

       $d = make_thumb_gd($file_fs_path, ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file, $width, $height, $ratio_type );



    }else{
        @unlink(ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file);
        make_thumb_gd_1($file_fs_path, ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file, $width, $height, $ratio_type );
    }

    if($folder_version =="english"){

        $link = SITE_WS_PATH."/".THUMB_CACHE_DIR."/".$cache_file;
    }
    else {

        $link = SITE_WS_PATH."/".$folder_version."/".THUMB_CACHE_DIR."/".$cache_file;
    }   

    return $link;
}

Make thumb function

function make_thumb_gd($imgPath, $destPath, $newWidth, $newHeight, $ratio_type = 'width', $quality = 60, $verbose = false)
{ 
    // options for ratio type = width|height|distort|crop
    // get image info (0 width and 1 height, 2 is (1 = GIF, 2 = JPG, 3 = PNG)
    $size = @getimagesize($imgPath); 
    // break and return false if failed to read image infos
    if (!$size) {
        if ($verbose) {
            echo "Unable to read image info.";
        }
        return false;
    } 
    $curWidth   = $size[0];
    $curHeight  = $size[1];
    $fileType   = $size[2];

    // width/height ratio
    $ratio =  $curWidth/$curHeight;

    $srcX = 0;
    $srcY = 0;
    $srcWidth = $curWidth;
    $srcHeight = $curHeight;

    if($ratio_type=='width') {
        // If the dimensions for thumbnails are greater than original image do not enlarge
        if($newWidth > $curWidth) {
            $newWidth = $curWidth;
        }
        $newHeight  = $newWidth / $ratio;
    }else if($ratio_type=='crop') {
        $thumbRatio = $newWidth / $newHeight;
        if($ratio < $thumbRatio) {
            $srcHeight = round($curHeight*$ratio/$thumbRatio);
            $srcY = round(($curHeight-$srcHeight)/2);
        } else {
            $srcWidth = round($curWidth*$thumbRatio/$ratio);
            $srcX = round(($curWidth-$srcWidth)/2);
        }
        /*echo "<br>curWidth: $curWidth";
        echo "<br>curHeight: $curHeight";
        echo "<br>newWidth: $newWidth";
        echo "<br>newHeight: $newHeight";
        echo "<br>ratio: $ratio";
        echo "<br>thumbRatio: $thumbRatio";
        echo "<br>srcWidth: $srcWidth";
        echo "<br>srcX: $srcX";
        echo "<br>srcHeight: $srcHeight";
        echo "<br>srcY: $srcY";*/
    } else if($ratio_type=='height') {
        // If the dimensions for thumbnails are greater than original image do not enlarge
        if($newHeight > $curHeight) {
            $newHeight = $curHeight;
        }
        $newWidth   = $newHeight * $ratio;
    } else if($ratio_type=='distort') {
    }

    // create image
    switch ($fileType) {
        case 1:
            if (function_exists("imagecreatefromgif")) {
                $originalImage = imagecreatefromgif($imgPath);
            } else {
                if ($verbose) {
                    echo "GIF images are not support in this php installation.";
                    return false;
                }
            } 
            $fileExt = 'gif';
            break;
        case 2: 
            $originalImage = imagecreatefromjpeg($imgPath);
            $fileExt = 'jpg';
            break;
        case 3: 
            $quality = 9;
            $originalImage = imagecreatefrompng($imgPath);
            $fileExt = 'png';
            break;
        default:
            if ($verbose) {
                echo "Not a valid image type.";
            }
            return false;
    } 
    // create new image

    $resizedImage = imagecreatetruecolor($newWidth, $newHeight);
    //echo "$srcX, $srcY, $newWidth, $newHeight, $curWidth, $curHeight";
    //echo "<br>$srcX, $srcY, $newWidth, $newHeight, $srcWidth, $srcHeight<br>";
    imagecopyresampled($resizedImage, $originalImage, 0, 0, $srcX, $srcY, $newWidth, $newHeight, $srcWidth, $srcHeight);
    switch ($fileExt) {
        case 'gif':
            imagegif($resizedImage, $destPath, $quality);
            break;
        case 'jpg': 
            imagejpeg($resizedImage, $destPath, $quality);
            break;
        case 'png':
            $quality = 9;
            imagepng($resizedImage, $destPath, $quality);
            break;
    } 
    // return true if successfull
    return true;
} 
\$\endgroup\$
5
  • \$\begingroup\$ Well unlike most PHP functionality, image processing is processor intensive rather than RAM intensive. So a lower grade processor could be a factor. Beyond that, make this script faster is no different than optimizing other PHP scripts; just prevent redundant actions and free any resources as soon as you can. \$\endgroup\$
    – Nick J
    Aug 29, 2014 at 22:02
  • \$\begingroup\$ So is there anything I can improve on the script or is it written just fine ? \$\endgroup\$
    – Stevie Nix
    Aug 29, 2014 at 22:05
  • 2
    \$\begingroup\$ @StevieNix I am not gonna sit here and do your job for you. You want your code faster, look for ways to make it faster, and then you apply them. If you want tips that can generally make your code faster then your in the right place. For example, I can tell you that the @ silencer will always slow your application down. I can also tell you, that if your using @ you have made your code wrong, and need to back and fix it so that no @s are required. \$\endgroup\$
    – Nick J
    Aug 29, 2014 at 22:14
  • 2
    \$\begingroup\$ Only call make_thumb_gd when absolutely required. If the thumbnail has already been generated and cached then you don't need to regenerate it. No need to unlink and recreate the work that has already been done, assuming the image hasn't changed since it's last thumbnail generation. \$\endgroup\$
    – MrYellow
    Aug 29, 2014 at 23:04
  • \$\begingroup\$ @MrYellow .. THANK YOU !! make_thumb_gd was called each time the images are displayed , that's why the site it slow .. I should only use it when uploading images , then if they exist , just provide the link to it stored in the database \$\endgroup\$
    – Stevie Nix
    Aug 30, 2014 at 23:03

1 Answer 1

3
\$\begingroup\$

Your code looks nice, but it could be improved in places:


show_thumb():

  1. Whitespace is missing in many places, and should be added in between operators, or between commas.
$height,$folder_version,$ratio_type = 'width'
$cache_file = $width."x".$height.'__'.$ratio_type .'__'.$file_name;
if(!is_file(ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file)) {
  1. The method you use to run over the $folder_version feels really backwards and should be changed.

    if($folder_version != "english"){
        $version = $folder_version;
    } else {
        $version = "";
    }
    

That is much clearer, however if you're into ternaries, you could use the following to shorten it:

$version = ($folder_version == "english" ? "" : $folder_version);
  1. You shouldn't build a long string instead an if statement like the following as it is bad practice.
if(!is_file(ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file)) {
   $d = make_thumb_gd($file_fs_path, ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file, $width, $height, $ratio_type );
}else{
    @unlink(ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file);
    make_thumb_gd_1($file_fs_path, ROOT."/".$version."/".THUMB_CACHE_DIR."/".$cache_file, $width, $height, $ratio_type );
}

Instead, you should:

$url = ROOT . "/$version/". THUMB_CACHE_DIR . "/$cache_file";
if(!is_file($url)){
   $d = make_thumb_gd($file_fs_path, $url, $width, $height, $ratio_type);
} else {
    @unlink($url);
    make_thumb_gd_1($file_fs_path, $url, $width, $height, $ratio_type);
}
  1. Is make_thumb_gd_1() really a thing, or did you misspell? (Why use two seperate functions?)

  2. The following code seems again, backwards, and could be shortened with some refactoring, or a ternary. I'd suggest the latter.

if($folder_version =="english"){

   $link = SITE_WS_PATH."/".THUMB_CACHE_DIR."/".$cache_file;
}
else {

   $link = SITE_WS_PATH."/".$folder_version."/".THUMB_CACHE_DIR."/".$cache_file;
}   

return $link;

into:

return = SITE_WS_PATH . "/" . ($version == "english" ? "" : $version) . THUMB_CACHE_DIR . "/$cache_file";

make_thumb_gd():

The code in this function seems a lot better formatted, and even has comments!

  1. You space the operator out here, so that it's formatted better, but they can be brought in another space:
$curWidth   = $size[0];
$curHeight  = $size[1];
$fileType   = $size[2];

into:

$curWidth  = $size[0];
$curHeight = $size[1];
$fileType  = $size[2];
  1. you shouldn't skip a few characters at the expense of readability:
    Instead of cur, use current (assuming that's what the abbreviation is for)

  2. You should add whitespace around your operators to improve readability:

round(($curWidth-$srcWidth)/2);
  1. Your comment openers/closers (/*, */) should always be on a different line than the comment content. Also, misspell.
// return true if successfull
                            ^
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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