From cad88cae9ab7f83f95cfee7624fcd42f3e6e32be Mon Sep 17 00:00:00 2001 From: Brion Vibber Date: Tue, 20 Sep 2011 22:13:34 +0000 Subject: [PATCH] * (bug 6672, 31024) Fixes for handling of images with an EXIF orientation - sets an image's reported width/height to the logical form (portait image reports itself as portait) - everything works in logical coordinates when sizing -- we don't touch the physical pre-rotation dimensions again until it's actual low-level resize time. This fixes several problems with incorrect thumb sizing (eg getting a 600x800 image when we asked for something that fits in 800x600 box) - fixes unit test cases in ExifRotationTest that were reporting that the width/height were coming back with the physical form which we don't want - removes some test cases on ExifRotationTest that tested dimension swapping in a place where we don't want it - ensures that only logical width/height need be exposed to API etc, making exif-rotated images work via ForeignAPIRepo Note that this may actually cause file metadata to get loaded twice during File::getPropsFromPath, as the $image parameter it passes in to the handler's getImageSize function is bogus and can't be used to fetch an already-loaded metadata blob. This should not generally be too expensive though; it's not a fast path. Rotated files that were uploaded under previous versions may still have their width/height reversed; an action=purge on the file page will refresh it and cause thumbs to be regenerated. Follows up on r79845, r90016, r92246, r92279, r96687, r97651, r97656, r97659. Needs merge to 1.18. --- includes/filerepo/File.php | 6 +- includes/media/Bitmap.php | 53 +++++--------- includes/media/ExifBitmap.php | 72 +++++++++++++++++++ .../includes/media/ExifRotationTest.php | 22 ------ 4 files changed, 94 insertions(+), 59 deletions(-) diff --git a/includes/filerepo/File.php b/includes/filerepo/File.php index 5b48fae87b..93ca6c4f7f 100644 --- a/includes/filerepo/File.php +++ b/includes/filerepo/File.php @@ -1342,7 +1342,11 @@ abstract class File { * @return string */ function getDescriptionUrl() { - return $this->repo->getDescriptionUrl( $this->getName() ); + if ( $this->repo ) { + return $this->repo->getDescriptionUrl( $this->getName() ); + } else { + return false; + } } /** diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index c90e6eea20..24f0dbd0c5 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -43,20 +43,6 @@ class BitmapHandler extends ImageHandler { } } - - if ( self::canRotate() ) { - $rotation = $this->getRotation( $image ); - if ( $rotation == 90 || $rotation == 270 ) { - wfDebug( __METHOD__ . ": Swapping width and height because the file will be rotated $rotation degrees\n" ); - - list( $params['width'], $params['height'] ) = - array( $params['height'], $params['width'] ); - list( $params['physicalWidth'], $params['physicalHeight'] ) = - array( $params['physicalHeight'], $params['physicalWidth'] ); - } - } - - # Don't thumbnail an image so big that it will fill hard drives and send servers into swap # JPEG has the handy property of allowing thumbnailing without full decompression, so we make # an exception for it. @@ -73,6 +59,11 @@ class BitmapHandler extends ImageHandler { /** * Extracts the width/height if the image will be scaled before rotating * + * This will match the physical size/aspect ratio of the original image + * prior to application of the rotation -- so for a portrait image that's + * stored as raw landscape with 90-degress rotation, the resulting size + * will be wider than it is tall. + * * @param $params array Parameters as returned by normaliseParams * @param $rotation int The rotation angle that will be applied * @return array ($width, $height) array @@ -249,6 +240,8 @@ class BitmapHandler extends ImageHandler { * @param $image File File associated with this thumbnail * @param $params array Array with scaler params * @return ThumbnailImage + * + * @fixme no rotation support */ protected function getClientScalingThumbnailImage( $image, $params ) { return new ThumbnailImage( $image, $image->getURL(), @@ -685,33 +678,21 @@ class BitmapHandler extends ImageHandler { } /** - * Try to read out the orientation of the file and return the angle that - * the file needs to be rotated to be viewed + * On supporting image formats, try to read out the low-level orientation + * of the file and return the angle that the file needs to be rotated to + * be viewed. + * + * This information is only useful when manipulating the original file; + * the width and height we normally work with is logical, and will match + * any produced output views. + * + * The base BitmapHandler doesn't understand any metadata formats, so this + * is left up to child classes to implement. * * @param $file File * @return int 0, 90, 180 or 270 */ public function getRotation( $file ) { - $data = $file->getMetadata(); - if ( !$data ) { - return 0; - } - wfSuppressWarnings(); - $data = unserialize( $data ); - wfRestoreWarnings(); - if ( isset( $data['Orientation'] ) ) { - # See http://sylvana.net/jpegcrop/exif_orientation.html - switch ( $data['Orientation'] ) { - case 8: - return 90; - case 3: - return 180; - case 6: - return 270; - default: - return 0; - } - } return 0; } diff --git a/includes/media/ExifBitmap.php b/includes/media/ExifBitmap.php index e58ac5cb89..6b4bf7a0de 100644 --- a/includes/media/ExifBitmap.php +++ b/includes/media/ExifBitmap.php @@ -124,5 +124,77 @@ class ExifBitmapHandler extends BitmapHandler { function getMetadataType( $image ) { return 'exif'; } + + /** + * Wrapper for base classes ImageHandler::getImageSize() that checks for + * rotation reported from metadata and swaps the sizes to match. + * + * @param File $image + * @param string $path + * @return array + */ + function getImageSize( $image, $path ) { + $gis = parent::getImageSize( $image, $path ); + + // Don't just call $image->getMetadata(); File::getPropsFromPath() calls us with a bogus object. + // This may mean we read EXIF data twice on initial upload. + $meta = $this->getMetadata( $image, $path ); + $rotation = $this->getRotationForExif( $meta ); + + if ($rotation == 90 || $rotation == 270) { + $width = $gis[0]; + $gis[0] = $gis[1]; + $gis[1] = $width; + } + return $gis; + } + + /** + * On supporting image formats, try to read out the low-level orientation + * of the file and return the angle that the file needs to be rotated to + * be viewed. + * + * This information is only useful when manipulating the original file; + * the width and height we normally work with is logical, and will match + * any produced output views. + * + * @param $file File + * @return int 0, 90, 180 or 270 + */ + public function getRotation( $file ) { + $data = $file->getMetadata(); + return $this->getRotationForExif( $data ); + } + + /** + * Given a chunk of serialized Exif metadata, return the orientation as + * degrees of rotation. + * + * @param string $data + * @return int 0, 90, 180 or 270 + * @fixme orientation can include flipping as well; see if this is an issue! + */ + protected function getRotationForExif( $data ) { + if ( !$data ) { + return 0; + } + wfSuppressWarnings(); + $data = unserialize( $data ); + wfRestoreWarnings(); + if ( isset( $data['Orientation'] ) ) { + # See http://sylvana.net/jpegcrop/exif_orientation.html + switch ( $data['Orientation'] ) { + case 8: + return 90; + case 3: + return 180; + case 6: + return 270; + default: + return 0; + } + } + return 0; + } } diff --git a/tests/phpunit/includes/media/ExifRotationTest.php b/tests/phpunit/includes/media/ExifRotationTest.php index aab4e53b6a..87aed2941f 100644 --- a/tests/phpunit/includes/media/ExifRotationTest.php +++ b/tests/phpunit/includes/media/ExifRotationTest.php @@ -145,27 +145,5 @@ class ExifRotationTest extends MediaWikiTestCase { ), ); } - - function testWidthFlipping() { - # Any further test require a scaler that can rotate - if ( !BitmapHandler::canRotate() ) { - $this->markTestSkipped( 'Scaler does not support rotation' ); - return; - } - $file = UnregisteredLocalFile::newFromPath( $this->filePath . 'portrait-rotated.jpg', 'image/jpeg' ); - $params = array( 'width' => '50' ); - $this->assertTrue( $this->handler->normaliseParams( $file, $params ) ); - - $this->assertEquals( 50, $params['height'] ); - $this->assertEquals( round( (768/1024)*50 ), $params['width'], '', 0.1 ); - } - function testWidthNotFlipping() { - $file = UnregisteredLocalFile::newFromPath( $this->filePath . 'landscape-plain.jpg', 'image/jpeg' ); - $params = array( 'width' => '50' ); - $this->assertTrue( $this->handler->normaliseParams( $file, $params ) ); - - $this->assertEquals( 50, $params['width'] ); - $this->assertEquals( round( (768/1024)*50 ), $params['height'], '', 0.1 ); - } } -- 2.20.1