Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image flip bug wrong parameter #1

Open
jasonspar opened this issue Dec 31, 2023 · 8 comments
Open

image flip bug wrong parameter #1

jasonspar opened this issue Dec 31, 2023 · 8 comments

Comments

@jasonspar
Copy link

jasonspar commented Dec 31, 2023

There is a bug in this routine: fix_orientation

The call to imageflip returns a boolean, not a flipped image. The fixed routine is like this:

function fix_orientation ($orientation, $fileandpath, $ext, $quality=80) {

// Gets the GD image resource for loaded image
$img_res = get_image_resource ($fileandpath, $ext);

// If it failed to load a resource, give up
if (is_null ($img_res)) {
return false;
}
// auto rotate
switch ($orientation) {

// Correct orientation, but flipped on the horizontal axis (might do it at some point in the future)
case 2:
  $ret = imageflip ($img_res, IMG_FLIP_HORIZONTAL);
break;

// Upside-Down
case 3:
  $ret = imageflip ($img_res, IMG_FLIP_VERTICAL);
break;

// Upside-Down & Flipped along horizontal axis
case 4:
  $ret = imageflip ($img_res, IMG_FLIP_BOTH);
break;

// Turned 90 deg to the left and flipped
case 5:
  $ret = imagerotate($img_res, -90, 0);
  $ret = imageflip ($img_res, IMG_FLIP_HORIZONTAL);
break;

// Turned 90 deg to the left
case 6:
  $ret = imagerotate ($img_res, -90, 0);
break;

// Turned 90 deg to the right and flipped
case 7:
  $ret = imagerotate ($img_res, 90, 0);
  $ret = imageflip ($img_res,IMG_FLIP_HORIZONTAL);
break;

// Turned 90 deg to the right
case 8:
  $ret = imagerotate ($img_res, 90, 0);
break;

}

// If theres no final image resource to output for whatever reason, give up
if (!isset ($ret)) {
return false;
}
// save image
save_image_resource ($img_res, $fileandpath, $ext, $quality);

// Free up memory
imagedestroy ($img_res);
}

@jasonspar
Copy link
Author

You can reproduce the bug in X3.3.2 with the attached image uploaded.
buggyimg

@mjau-mjau
Copy link
Owner

Thanks for reporting this. Strange that imagerotate() returns the resource while imageflip() only returns a boolean. There is now only one problem in the code ... When imagerotate() is used, $img_res will refer to the original non-rotated image, so the image will not get rotated. So, for imagerotate(), we definitely need to use the return value.

I will look into it for next release. Thanks for reporting!

PS! I have a short working version of this in the files.gallery application.

function exif_orientation($orientation, &$image){
  if(empty($orientation) || !is_numeric($orientation) || $orientation < 3 || $orientation > 8) return;
  $image = imagerotate($image, array(6 => 270, 5 => 270, 3 => 180, 4 => 180, 8 => 90, 7 => 90)[$orientation], 0);
  if(in_array($orientation, array(5, 4, 7)) && function_exists('imageflip')) imageflip($image, IMG_FLIP_HORIZONTAL);
  return true;
}

@jasonspar
Copy link
Author

jasonspar commented Jan 1, 2024

I see that. How about something like this code:

function fix_orientation ($orientation, $fileandpath, $ext, $quality=80) {

// Gets the GD image resource for loaded image
$img_res = get_image_resource ($fileandpath, $ext);

// If it failed to load a resource, give up
if (is_null ($img_res)) {
return false;
}
$rotated_img = null;
$ret = false;
// auto rotate
switch($orientation) {
// need just flipping
case 2:
$ret = imageflip ($img_res, IMG_FLIP_HORIZONTAL);
break;

case 4:  // Upside-Down & Flipped along horizontal axis
   $ret = imageflip ($img_res, IMG_FLIP_VERTICAL);
   break;

// need rotation and flip
case 5:  // Turn 90 deg to the right and flip
   $rotated_img = imagerotate ($img_res, -90, 0);
   $ret = imageflip ($rotated_img, IMG_FLIP_HORIZONTAL);
   break;

case 7:  // Turned 90 deg to the left and flip
   $rotated_img = imagerotate ($img_res, 90, 0);
   $ret = imageflip ($rotated_img,IMG_FLIP_HORIZONTAL);
   break;

// rotation is enough for these
case 3: // Upside-Down
   $ret = true;
   $rotated_img = imagerotate ($img_res, 180, 0);
   break;
   
case 6:  // Turn 90 deg to the right
   $ret = true;
   $rotated_img = imagerotate ($img_res, -90, 0);
   break;

case 8:  // Turn 90 deg to the left
   $ret = true;
   $rotated_img = imagerotate ($img_res, 90, 0);
   break;

}

if (!$ret) { // If theres no final image resource to output for whatever reason, give up
return false;
}
// save image
if ($rotated_img != null) {
save_image_resource ($rotated_img, "D:/WEB/ecdrc.net/html/ces/data/hose-fixed.jpg", $ext, $quality);
imagedestroy ($rotated_img);
}
else {
save_image_resource ($img_res, "D:/WEB/ecdrc.net/html/ces/data/hose-fixed.jpg", $ext, $quality);
}
// Free up memory
imagedestroy ($img_res);
}

@jasonspar
Copy link
Author

the last code I posted is tested against a set of images in different orientations. It passes. The code from files.gallery also has a bug with case 2 and 4. Here's a fixed version:

function exif_orientation ($orientation = -1, &$image = null) {

if (empty ($orientation) || !is_numeric ($orientation)) { // bad input
return (false);
}
if ($image == null) { // nothing to work on
return (false);
}
if ($orientation < 2 || $orientation > 8) { // do we need to mess with it
return (true);
}

// lets rotate first
$angles = array (6 => 270, 5 => 270, 3 => 180, 4 => 180, 8 => 90, 7 => 90);
$orval = $angles[$orientation];
$image = imagerotate ($image, $orval, 0);

// do we need to flip it
if (function_exists ('imageflip')) {
$horiz_flip_needed = array(2, 5, 7);
if (in_array ($orientation, $horiz_flip_needed)) {
imageflip ($image, IMG_FLIP_HORIZONTAL);
}
else if ($orientation == 4) {
imageflip ($image, IMG_FLIP_VERTICAL);
}
}
return (true);
}

@mjau-mjau
Copy link
Owner

mjau-mjau commented Jan 2, 2024

The code from files.gallery also has a bug with case 2 and 4.

I can't see any bugs there. As you can see, the &$image is passed as a reference, and used by the caller of the function. For imagerotate() it it correctly applied to return to the reference &$image and for imageflip() it correctly applied directly on the reference resource. It's a replication of the code from the below:
https://github.com/gumlet/php-image-resize/blob/master/lib/ImageResize.php

Anyway, thanks for reporting this and making suggestions. I will definitely have it resolved for next X3 release.

@jasonspar
Copy link
Author

Yes, that code is buggy. Here are the graphics I used to set up the cases.
photo_2024-01-01_18-38-22
photo_2024-01-01_18-59-44

I can send you the image test set as well. After you upload them, you can check which ones failed.

@jasonspar
Copy link
Author

Here's the test set.
fliprotatetestset.zip

@mjau-mjau
Copy link
Owner

I see the images in your test set, are the same images that I have in our Files Gallery demo:
https://demo.files.gallery/?tests/exif-image-orientation

All the resized images (thumbnails) have been oriented from the code posted, and as you can see, it's definitely working as it should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants