Exif orientation tag related issues and how to solve them

[adamkiss]

Describe the bug

If an image includes EXIF orientation code, both GD and ImageMagick behave wildly inconsistently: It might be anything from resizing with dimensions switched, but correct orientation (i.e. thumb(400, 300) creates a 300x400 image), through stretched images, etc.

To Reproduce
Steps to reproduce the behavior:

1. Go to any image field
2. Test any of the images available here: https://github.com/recurser/exif-orientation-examples
3. See the bug

Screenshots

These are screenshots from one of the many seemingly unrelated problems, in this case it's from flokosiol/kirby-focus#50

Additional context

Few of the solutions found on the internet deal with this by basically "burning the orientation in" and removing the exif orientation tag on upload.

Some examples are in this forum thread, my own looks like this:

<?php

use Kirby\Cms\App;
use Kirby\Toolkit\Str;

App::plugin('adamkiss/fix-orientation', [
	'hooks' => [
		'file.create:after' => function ($file) {
			if (in_array(Str::lower($file->extension()), ['jpg', 'jpeg'])) {
				$filename = $file->root();
				$image = imagecreatefromjpeg($filename);
				$exif = exif_read_data($filename);

				if (!empty($exif['Orientation'])) {
					switch ($exif['Orientation']) {
						case 3:
							$image = imagerotate($image, 180, 0);
							break;

						case 6:
							$image = imagerotate($image, -90, 0);
							break;

						case 8:
							$image = imagerotate($image, 90, 0);
							break;
					}
				}
				imagejpeg($image, $filename);
			}
		}
	]
]);

One problem with this is that the "fix" changes picture files, which might not be always applicable, so maybe configurable option "burn in exif orientation", with default state either backwards compatible (off) or forward problem solving (on) :)

[lukasbestle]

Several users have reported this issue in multiple different variations (for example #1926, I can't find the other issues on the fly).

A new idea: Given that images that are served to the visitor are basically always run through the thumb engine (using the original images in full resolution doesn't make sense for most use-cases), the only place that really needs fixing is the thumb engine itself. It could apply the rotation on a temporary image file before and then pass that rotated image through the actual thumb library. Additionally, we would need to ensure that our other thumb-related classes (e.g. the Image\Dimensions class) all base their calculations on the EXIF data instead of on the actual image size.

Advantage: The original image would be unchanged, but thumbs would be generated correctly.

[adamkiss]

@lukasbestle Do the "resize" and "crop" methods use the thumbs class? Because it would be bad if the redults were incosistent, IMO.

[lukasbestle]

Yes, they do!

[distantnative]

Would be good to check if GD and ImageMagick fixed their false behaviour in the meantime.

[lukasbestle]

Don't think it's false behavior in the drivers actually. They just ignore EXIF information and resize/crop exactly like we tell them to on a pixel level.

[silllli]

Since I changed from the Gd to the ImageMagick driver in my latest project, I have problems with this. The image orientation is read correctly (due to the -auto-orient option) from the EXIF data, but the resize() function of the ImageMagick driver uses the file’s original dimensions, squeezing rotated images into wrong dimensions. Shouldn’t width and height be flipped according to rotation information in the EXIF data?

kirby/src/Image/Darkroom/ImageMagick.php

Line 186 in 0412716

return '-thumbnail ' . escapeshellarg(sprintf('%sx%s!', $options['width'], $options['height']));

Something like this:

$image = new Image($file);

$flip = false;
$problematicOrientations = [6, 8];

// check if orientation was set in EXIF data
if (array_key_exists('Orientation', $image->exif()->data())) {
  $fileOrientation = $image->exif()->data()['Orientation'];
  
  // check if a rotated orientation was set
  if (in_array($fileOrientation, $problematicOrientations)) $flip = true;
}

$width = $flip ? $options['height'] : $options['width'];
$height = $flip ? $options['width'] : $options['height'];

return '-thumbnail ' . escapeshellarg(sprintf('%dx%d!', $width, $height)); 

[lukasbestle]

@silllli Thanks for sharing. So you're saying that you didn't have these problems when using GD?

[silllli]

@silllli Thanks for sharing. So you're saying that you didn't have these problems when using GD?

Exactly. I noticed it with one image, which was fine before I started using the ImageMagick driver. I switched back to Gd to be sure and the thumbnail was created correctly again.

[Jayshua]

@silllli Thanks for sharing. So you're saying that you didn't have these problems when using GD?

Exactly. I noticed it with one image, which was fine before I started using the ImageMagick driver. I switched back to Gd to be sure and the thumbnail was created correctly again.

I had the opposite issue - the ImageMagick driver worked but the GD driver swapped the width/height of the crop. I looked at the source for the GD driver in GdLib.php and found that the image is resized before it is oriented. Presumably resize doesn't consider the EXIF orientation. The crop is applied correctly if I swap the ->autoOrient() and ->resize() calls so that the image is oriented before resizing.

// kirby/src/Image/Darkroom/GdLib.php:35

- $image = $this->resize($image, $options);
- $image = $this->autoOrient($image, $options);

+ $image = $this->autoOrient($image, $options);
+ $image = $this->resize($image, $options);

To confirm, I checked the ImageMagick driver and found it behaves the same way, except the order of operations is already correct.

// kirby/src/Image/Darkroom/ImageMagick.php:136

$command[] = $this->autoOrient($file, $options);
$command[] = $this->resize($file, $options);

If I swap those two lines so that the resize is applied before the orientation, ImageMagick crops incorrectly just like the GD driver.

Here's the image I used. Assuming Github didn't strip EXIF data from it, it should have an Orientation tag set to Rotate 90 CW.

[lukasbestle]

Thanks for sharing. Reading the diff from the GD driver it makes a lot of sense. But now I'm confused why GD has worked for @silllli but not ImageMagick. Maybe we should build a collection of different test images that fail in one or the other driver. Ideal of course would be rather small files that we can then include in our unit tests as test fixtures.

[fabianmichael]

Here is another example image where Kirby/GD/ImageMagic does not recognize EXIF rotation correctly, provided by one of my clients:

[rasteiner]

Has someone tried fixing the Dimensions class first? In the end the drivers do what the Dimensions class says.

A fix would be something like:

	/**
	 * Detect the dimensions for an image file
	 */
	public static function forImage(string $root): static
	{
		if (file_exists($root) === false) {
			return new static(0, 0);
		}

		// create Image to read exif Orientation data
		$image = new Image($root);
		$orientation = $image->exif()->data()['Orientation'] ?? 1;

		// orientation 1 is normal, 2 - 4 are flipped, 5 - 8 are rotated
		if($orientation < 5) {
			list($width, $height) = getimagesize($root);
		} else {
			list($height, $width) = getimagesize($root);
		}

		return new static($width ?? 0, $height ?? 1);
	}

If you want test images, here are images in all possible orientations (actually also a 0 orientation which technically doesn't exist, afaik): https://github.com/recurser/exif-orientation-examples

[rasteiner]

I've also made these: https://github.com/rasteiner/exif-orientation-test-images
The idea was to have images that are simple to automatically test, hence the 4 colored rects...

You can test the test images by putting them into a Kirby installation and see that for images 5, 6, 7 and 8 the panel lies to you:

[distantnative]

With @rasteiner's test images, what seems to work for me is a combination:

// Kirby\Image\Darkroom\GdLib::process()

+ $image = $this->autoOrient($image, $options);
+ $image = $this->resize($image, $options);
- $image = $this->resize($image, $options);
- $image = $this->autoOrient($image, $options);

// Kirby\Image\Dimensions

public static function forImage(string $root): static
{
	if (file_exists($root) === false) {
		return new static(0, 0);
	}

	// create Image to read exif Orientation data
	$image 		 = new Image($root);
	$orientation = $image->exif()->data()['Orientation'] ?? 1;

	// orientation 1 is normal, 2 - 4 are flipped, 5 - 8 are rotated
	if($orientation < 5) {
		[$width, $height] = getimagesize($root);
	} else {
		[$height, $width] = getimagesize($root);
	}

	return new static($width ?? 0, $height ?? 1);
}

[distantnative]

I've started a PR. Would appreciate help to add unit tests with proper small test images for both libraries #5071