* (bug 6672, 31024) Fixes for handling of images with an EXIF orientation
authorBrion Vibber <brion@users.mediawiki.org>
Tue, 20 Sep 2011 22:13:34 +0000 (22:13 +0000)
committerBrion Vibber <brion@users.mediawiki.org>
Tue, 20 Sep 2011 22:13:34 +0000 (22:13 +0000)
- 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
includes/media/Bitmap.php
includes/media/ExifBitmap.php
tests/phpunit/includes/media/ExifRotationTest.php

index 5b48fae..93ca6c4 100644 (file)
@@ -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;
+               }
        }
 
        /**
index c90e6ee..24f0dbd 100644 (file)
@@ -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;
        }
 
index e58ac5c..6b4bf7a 100644 (file)
@@ -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;
+       }
 }
 
index aab4e53..87aed29 100644 (file)
@@ -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 );
-       }
 }