Avatar

i got this code running on my old webserver. on the new one (better hardware), this script is pretty slow. dont know why. maybe also an apache problem. but maybe someone can altough improve this code. thanks.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
<?
    #dl("php_gd2.dll");

    // requiered argument
    $pic = $_REQUEST["pic"];
    
    // optional
    $width = $_REQUEST["width"];
    $height = $_REQUEST["height"];
    
    if (($width == "") || ($height == "")) {
        $maxwidth = "160";
        $maxheight = "120";
    }
    else {
        $maxwidth = $width;
        $maxheight = $height;
    }
    
    $getimg = getimagesize($pic);
    // 0. width
    // 1. height
    // 2. Typ: 1=GIF, 2=JPEG, 3=PNG
    // 3. size html tags (e.g. 'width="111" height="24"')
    
    // preventing from accessing non gfx files.
    if ((($getimg[2] < 1) && ($getimg[2] > 3)) || ($getimg[2] == "")) {
        die("No valid gfx-source.");
    }
    
    $oldwidth = $getimg[0];
    $oldheight = $getimg[1];
    
    // If original is smaller then new image
    if (($oldwidth <= $maxwidth) && ($oldheight <= $maxheight)) {
        // JPEG
        if($getimg[2]==2){
            $img = ImageCreateFromJPEG($pic);
            header("Content-Type: image/jpeg");
            imagejpeg($img);
        }
        
        // Gif
        if($getimg[2]==1){
            $img = ImageCreateFromGIF($pic);
            header("Content-Type: image/gif");
            imagegif($img);
        }
    
        // PNG
        if($getimg[2]==3){
            $img = ImageCreateFromPNG($pic);
            header("Content-Type: image/png");
            imagepng($img);
        }
    }
    else {    
        if ($oldwidth >= $oldheight) {
            $relation = $oldwidth / $oldheight;
            
            $newwidth = $maxwidth;
            $newheight = round($newwidth / $relation);
            
            while (($newwidth > $maxwidth) || ($newheight > $maxheight)) {
                $newwidth--;
                $newheight = round($newwidth / $relation);                    
            }
        }
        else {
            $relation = $oldheight / $oldwidth;
            
            $newheight = $maxheight;
            $newwidth = round($newheight / $relation);
            
            while (($newheight > $maxheight) || ($newwidth > $maxwidth)) {
                $newheight--;
                $newwidth = round($newheight / $relation);                    
            }
        }

        $imgA = imagecreatetruecolor($newwidth,$newheight);    //alte gdlib --> imagecreate()
    
        // JPEG
        if($getimg[2]==2){
                $imgB = ImageCreateFromJPEG($pic);
                imagecopyresized($imgA, $imgB, 0,0, 0,0, $newwidth, $newheight, $oldwidth, $oldheight);
                header("Content-Type: image/jpeg");
                imagejpeg($imgA);
        }
    
        // Gif
        if($getimg[2]==1){
            $imgB = ImageCreateFromGIF($pic);    
            imagecopyresized($imgA, $imgB, 0,0, 0,0, $newwidth, $newheight, $oldwidth, $oldheight);        
            header("Content-Type: image/gif");
            imagegif($imgA);
        }
    
        // PNG
        if($getimg[2]==3){
            $imgB = ImageCreateFromPNG($pic);
            imagecopyresized($imgA, $imgB, 0,0, 0,0, $newwidth, $newheight, $oldwidth, $oldheight);
            header("Content-Type: image/png");
            imagepng($imgA);
        }
    }
?>

Refactorings

No refactoring yet !

A2c8fecfd1fb707dd0a8f292ade77e1e

typefreak

November 1, 2007, November 01, 2007 12:17, permalink

1 rating. Login to rate!

Personally, I would rather use $_POST or $_GET then $_REQUEST

Either way, you're not even trying to prevent notices. Bad idea. Especially if you're trying to send headers after that.

The statement ($getimg[2] < 1 && $getimg[2] > 3) alway returns false, as a number can't be smaller then 1, and bigger then 3 at the same time.

The use of the while-loop doensn't make any sense to me.

Function names are supposed to be case-sensitive, better use lowercase where appropriate.

$imgA and $imgB aren't really describing names. Rather use $smallimg and $bigimg instead.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
<?
// requiered argument
if ( empty($_REQUEST["pic"]) ) {
    die('You haven\'t specified a picture');
}
$pic = $_REQUEST["pic"];

// optional parameters
$maxwidth = (isset($_REQUEST['width']) ? $_REQUEST['width'] : 160);
$maxheight = (isset($_REQUEST['height']) ? $_REQUEST['height'] : 120);

$getimg = getimagesize($pic);
// 0. width
// 1. height
// 2. Type: 1=GIF, 2=JPEG, 3=PNG
// 3. size html tags (e.g. 'width="111" height="24"')

// preventing from accessing non gfx files.
if ( $getimg[2] < 1 || $getimg[2] > 3 || $getimg[2] == "" ) {
    die("No valid gfx-source.");
}

$oldwidth = $getimg[0];
$oldheight = $getimg[1];

// If original is smaller then or equal to new image
if ( $oldwidth <= $maxwidth && $oldheight <= $maxheight ) {
    switch ($getimg[2]) {
        case 1:
            // Gif image
            $img = imagecreatefromgif($pic);
            header("Content-Type: image/gif");
            imagegif($img);
            break;
        case 2:
            // Jpeg image
            $img = imagecreatefromjpeg($pic);
            header("Content-Type: image/jpeg");
            imagejpeg($img);
            break;
        case 3:
            // Png image
            $img = imagecreatefrompng($pic);
            header("Content-Type: image/png");
            imagepng($img);
            break;
    }
}
else {
    // Image should be rezised
    $widthdif = $oldwidth / $maxwidth;
    $heightdif = $oldheight / $maxheight;
    $maxdif = max($widthdif, $heightdif);
    $newwidth = floor($oldwidth / $maxdif);
    $newheight = floor($oldheight / $maxdif);

    $smallimg = imagecreatetruecolor($newwidth,$newheight);    //alte gdlib --> imagecreate()
    switch ($getimg[2]) {
        case 1:
            // Gif image
            $bigimg = imagecreatefromgif($pic);    
            imagecopyresized($smallimg, $bigimg, 0,0, 0,0, $newwidth, $newheight, $oldwidth, $oldheight);        
            header("Content-Type: image/gif");
            imagegif($smallimg);
            break;
        case 2:
            // Jpeg image
            $bigimg = imagecreatefromjpeg($pic);
            imagecopyresized($smallimg, $bigimg, 0,0, 0,0, $newwidth, $newheight, $oldwidth, $oldheight);
            header("Content-Type: image/jpeg");
            imagejpeg($smallimg);
            break;
        case 3:
            // Png image
            $imgB = imagecreatefrompng($pic);
            imagecopyresized($smallimg, $bigimg, 0,0, 0,0, $newwidth, $newheight, $oldwidth, $oldheight);
            header("Content-Type: image/png");
            imagepng($smallimg);
            break;
    }
}
?>
Avatar

tempouser.myopenid.com

November 2, 2007, November 02, 2007 09:04, permalink

No rating. Login to rate!

Hello,

thanks for the refactoring. I still got the performance issues, but that must not implicitly topic to the php code.

Great code improvment! Especialy the resize part. Dont know what i was doing in complex stuff, the "while-thing etc" ;)

What is this kind of php syntax postet below? Is there an manual entry?

1
$maxwidth = (isset($_REQUEST['width']) ? $_REQUEST['width'] : 160);
A2c8fecfd1fb707dd0a8f292ade77e1e

typefreak

November 2, 2007, November 02, 2007 09:47, permalink

No rating. Login to rate!

It is called a ternary operator: (scroll down to the 'ternary operator' heading.)
http://nl2.php.net/operators.comparison

In short:
if the part before the ? is true, then the part between ? and : is executed. otherwise, the part after : is executed.

The line you copied is functional equivilant to: 

1
2
3
4
5
6
if ( isset($_REQUEST['width']) ) {
    $maxwidth = $_REQUEST['width'];
}
else {
    $maxwidth = 160;
}
Avatar

tempouser.myopenid.com

November 2, 2007, November 02, 2007 10:03, permalink

No rating. Login to rate!

Thanks a lot!

08b49e3d543f221ed51cdbe118448275

zaadjis

November 2, 2007, November 02, 2007 13:53, permalink

No rating. Login to rate!

using Imagick http://valokuva.org/?p=33

Avatar

tempouser.myopenid.com

November 5, 2007, November 05, 2007 08:32, permalink

No rating. Login to rate!

I noticed, that the imagecreatetruecolor() function can be very slow for images larger than 2000x2000.
So I added a cache-mode. Maybe there is something to refactor ;)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
<?
    // cache mode settings
    $cache = true;                    // true/false --> enable/disable cache mode
    $cache_dir = "./cache";                // server directory -> must be writeable
    $cache_web_dir = "/fotos/cache";    // web directory

    // requiered argument
    $pic = $_REQUEST["pic"];
    
    // optional parameters
    $maxwidth = (isset($_REQUEST['width']) ? floor($_REQUEST['width']) : 160);    // default width 160
    $maxheight = (isset($_REQUEST['height']) ? floor($_REQUEST['height']) : 120);    // default height 120

    $getimg = getimagesize($pic);
    // 0. width
    // 1. height
    // 2. typ: 1=GIF, 2=JPEG, 3=PNG
    // 3. html tag (e.g. 'width="111" height="24"')
    
    // preventing from accessing non gfx files.
    if ((($getimg[2] < 1) || ($getimg[2] > 3)) || ($getimg[2] == "")) {
        die("No valid gfx-source.");
    }
    
    $oldwidth = $getimg[0];
    $oldheight = $getimg[1];
    
    // if original is smaller then or equal to new image
    if ( $oldwidth <= $maxwidth && $oldheight <= $maxheight ) {
        header("HTTP/1.1 302 Found");
        header("Location: " . $pic);
    }
    else {
        // image should be resized
        $widthdif = $oldwidth / $maxwidth;
        $heightdif = $oldheight / $maxheight;
        $maxdif = max($widthdif, $heightdif);
        $newwidth = floor($oldwidth / $maxdif);
        $newheight = floor($oldheight / $maxdif);

        // caching enabled
        if ($cache) {
            $char_replace = array(".", "/");
            $cache_dir = $cache_dir . "/" . str_replace($char_replace, "", dirname($pic)) . "w" . $newwidth . "h" . $newheight . basename($pic);
            $cache_web_dir = $cache_web_dir . "/" . str_replace($char_replace, "", dirname($pic)) . "w" . $newwidth . "h" . $newheight . basename($pic);
            if (file_exists($cache_dir)) {
                header("HTTP/1.1 302 Found");
                header("Location: " . $cache_web_dir);
                die;
            }
        }

        $smallimg = imagecreatetruecolor($newwidth,$newheight);

        switch ($getimg[2]) {
            case 1:
                // gif image
                $bigimg = imagecreatefromgif($pic);
                imagecopyresized($smallimg, $bigimg, 0, 0, 0, 0, $newwidth, $newheight, $oldwidth, $oldheight);

                if ($cache) {
                    imagegif($smallimg, $cache_dir);
                    header("HTTP/1.1 302 Found");
                    header("Location: " . $cache_web_dir);
                }
                else {    
                    header("Content-Type: image/gif");
                    imagegif($smallimg);
                }
                break;
            
            case 2:
                // jpg image
                $bigimg = imagecreatefromjpeg($pic);
                imagecopyresized($smallimg, $bigimg, 0, 0, 0, 0, $newwidth, $newheight, $oldwidth, $oldheight);
                
                if ($cache) {
                    imagejpeg($smallimg, $cache_dir);
                    header("HTTP/1.1 302 Found");
                    header("Location: " . $cache_web_dir);
                }
                else {    
                    header("Content-Type: image/jpeg");
                    imagejpeg($smallimg);
                }
                break;
                
            case 3:
                // png image
                $bigimg = imagecreatefrompng($pic);
                imagecopyresized($smallimg, $bigimg, 0, 0, 0, 0, $newwidth, $newheight, $oldwidth, $oldheight);

                if ($cache) {
                    imagepng($smallimg, $cache_dir);
                    header("HTTP/1.1 302 Found");
                    header("Location: " . $cache_web_dir);
                }
                else {    
                    header("Content-Type: image/png");
                    imagepng($smallimg);
                }
                break;
        }
    }
?>
Ae0689b76b0fa370800323a71742c24f

Hubert Roksor

November 6, 2007, November 06, 2007 17:16, permalink

No rating. Login to rate!

You should bump the cache-hit thing way up. The beginning of the script should look like the snippet below.

Also, is that code run often? I mean, is it used to display thumbnails on a live page? Because as I understand it, if $_REQUEST["pic"] is a URL, you get a round trip to the external server everytime someone displays the image if that image is not cached (if it's, ironically, not deemed worth caching because of its small size). Checking external resources is very costly and you'd actually be better off hosting a replica of the image on your own server.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
// cache mode settings
$cache = true;                    // true/false --> enable/disable cache mode
$cache_dir = "./cache";                // server directory -> must be writeable
$cache_web_dir = "/fotos/cache";    // web directory

// requiered argument
$pic = $_REQUEST["pic"];

// caching enabled
if ($cache) {
	$char_replace = array(".", "/");
	$cache_dir = $cache_dir . "/" . str_replace($char_replace, "", dirname($pic)) . "w" . $newwidth . "h" . $newheight . basename($pic);
	$cache_web_dir = $cache_web_dir . "/" . str_replace($char_replace, "", dirname($pic)) . "w" . $newwidth . "h" . $newheight . basename($pic);

	if (file_exists($cache_dir)) {
		header("HTTP/1.1 302 Found");
		header("Location: " . $cache_web_dir);
		die;
	}
}
Ae0689b76b0fa370800323a71742c24f

Hubert Roksor

November 6, 2007, November 06, 2007 17:18, permalink

No rating. Login to rate!

Oh, also, you should take a look at imagecopyresampled(). It's the same as imagecopyresized() except that it gives better results, but it is also a little less performant than imagecopyresized(). But since you're caching the results, it wouldn't really matter.

584799d026024e108d87aeceb51804d3

JWvdV

November 6, 2007, November 06, 2007 17:43, permalink

No rating. Login to rate!

Perhaps you can use this...
Good luck.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
<?php
function image_createfromany($file){
	if(is_file($file) && is_array($image_info = getimagesize($file))){
		switch(strtolower($image_info['mime'])){
			case 'image/jpeg':
				if(function_exists('imagecreatefromjpeg')) $src = @imagecreatefromjpeg($file);
			break;
			case 'image/gif':
				if(function_exists('imagecreatefromgif')) $src = @imagecreatefromgif($file);
			break;
			case 'image/png':
				if(function_exists('imagecreatefrompng')) $src = @imagecreatefrompng($file);
			break;
			case 'image/xbm':
			case 'image/x-xbitmap':
				if(function_exists('imagecreatefromxbm')) $src = @imagecreatefromxbm($file);
			break;
			case 'image/vnd.wap.wbmp':
				if(function_exists('imagecreatefromwbmp')) $src = @imagecreatefromwbmp($file);
			break;
			case 'image/x-xpixmap':
				if(function_exists('imagecreatefromxpm')) $src = @imagecreatefromxpm($file);
			break;
			default:
				return false;
		}
		
		if(isset($src) && !empty($src)) return($src);
	}
	return false;
}

function image_thumb($img, $maxwidth, $maxheight, $scale=true){
	if(include_functions('validate') && is_int($maxwidth) && is_int($maxheight)){
		if(is_file($img)) $img = image_createfromany($img);
		if($img && function_exists('imagesx') && function_exists('imagesy') && is_numeric($width = @imagesx($img)) && is_numeric($height = @imagesy($img))){
			if($scale && (($maxwidth < $width) || ($maxheight < $height))){
				$ratio = ((($width / $maxwidth) > ($height / $maxheight))? ($maxwidth / $width) : ($maxheight / $height));
				$width = round($ratio * $width);
				$height = round($ratio * $height);
			}
			
			if(function_exists('imagecreate')){
				$src = (function_exists('imagecreatetruecolor')? imagecreatetruecolor($width, $height) : imagecreate($width, $height));
				if(function_exists('imagecopyresampled')) imagecopyresampled($src, $img, 0, 0, 0, 0, $width, $height, imagesx($img), imagesy($img));
				else if(function_exists('imagecopyresized')) imagecopyresized($src, $img, 0, 0, 0, 0, $width, $height, imagesx($img), imagesy($img));
				else return(false);
				return $src;
			}
		}
	}
	return false;
}
?>
Avatar

tempouser.myopenid.com

November 7, 2007, November 07, 2007 09:33, permalink

No rating. Login to rate!

@ Huber Roksor: its all local stuff, no remote ressources.
ive tested the imagecopyresampled() function. i will use imagecopyresized, because in my tests, the quality results are not better than expected. in my opinion the images are looking a bit washy.

Your refactoring





Format Copy from initial code

or Cancel