[FIXED] Why does this basic imagejpeg() resizer returns a black image?

Issue

EDIT

Thanks for all your answers, especially @Mailerdaimon who noticed that I wasn't using the computed values in the imagecopyresampled function.

I don't get black images anymore, but i still do get some black part so i figure my ratio formula should be updated : if i upload a landscape image, the height of the new image is smaller than 170px, and then there's some black showing.

How can i make sure the height of the image goes all the way ?

---

Below is a simple script to allow users upload pictures. Once the upload is done, the pictures are displayed as a 170px(h) x 150px(w) thumbnail.

The resize part does work since the output image is 170x150px BUT i still get some black area if

if ($_SERVER["REQUEST_METHOD"] == "POST") 
{

$maxWidth  = 150;
$maxHeight = 170;

$name = $_FILES ['image'] ['name'];
$type = $_FILES ["image"] ["type"];
$size = $_FILES ["image"] ["size"];
$tmp_name = $_FILES ['image'] ['tmp_name'];
list($originalWidth, $originalHeight) = getimagesize($tmp_name);


 if ($originalWidth > $originalHeight) 
 {
   $thumbnail_height = floor(($originalHeight/$originalWidth)*$maxWidth);
   $thumbnail_width  = $maxWidth;
 } else {
   $thumbnail_width  = floor(($originalWidth/$originalHeight)*$maxHeight);
   $thumbnail_height = $maxHeight;
 }

 // Resample  
  $image_p = imagecreatetruecolor($maxWidth, $maxHeight);
  imagecreatefrompng($tmp_name);
  imagecopyresampled($image_p, 0, 0, 0, 0, $thumbnail_width, $thumbnail_height, 
  $originalWidth, $originalHeight);

//start upload process
$RandomNumber = uniqid();
$location = "uploads/$RandomNumber";
imagejpeg($image_p,  $location, 100);      

$sql=query("UPDATE users SET image = '".$location."' WHERE id = '$id'"); 

        } 
  }

Any idea what I'm doing wrong?

Solution

I never used php, so this might not be correct, but i can´t see where you use your computed values for width and height!

if ($originalWidth > $maxWidth || $originalHeight > $maxHeight)
 {
      if ($originalWidth / $maxWidth > $originalHeight / $maxHeight) 
     {
       // width is the limiting factor
       $width = $maxWidth;
       $height = floor($width * $originalHeight / $originalWidth);

     } else { 
       // height is the limiting factor
       $height = $maxHeight;
       $width = floor($height * $originalWidth / $originalHeight);
     }

here you compute the height and width and after that they dont appear anywhere in your code...

EDIT: You use:

$image_p = imagecreatetruecolor($maxWidth, $maxHeight);
imagecreatefrompng($tmp_name);
imagecopyresampled($image_p, 0, 0, 0, 0, $thumbnail_width, $thumbnail_height,$originalWidth, $originalHeight);

But the Documentation of imagecopyresmapled states that the function takes the follow

imagecopyresampled ($dst_image ,$src_image ,$dst_x ,$dst_y ,$src_x ,$src_y ,$dst_w , $dst_h ,$src_w ,$src_h )

Though i think your code should be

imagecopyresampled($image_p,$tmp_name, 0, 0, 0, 0, $thumbnail_width, $thumbnail_height,$originalWidth, $originalHeight);

as already mentioned by @Manolo Salsas (and several others)

EDIT2: another problems lies in how you compute your Heigth and Width Values. if $height gets computed likes this

$height = floor($width * $originalHeight / $originalWidth);

it can be smaller than $maxHeight, though creating a black (unfilled) area in your Resulting image, as you have created this image using the max values

$image_p = imagecreatetruecolor($maxWidth, $maxHeight);

To fix this use your computed $thumbnail_height and $thumbnail_width values to create the image

 $image_p = imagecreatetruecolor($thumbnail_width, $thumbnail_height);

or always resize your image to the max values

imagecopyresampled($image_p,$tmp_name, 0, 0, 0, 0, $maxWidth, $maxHeight,$originalWidth, $originalHeight);

which may distort the image. If you dont want to distort the image think about resizing first such that on size fits in the thumbnail and then crop the longer side.

Answered By - Mailerdaimon

Answer Checked By - Mary Flores (PHPFixing Volunteer)