From: Bryan Tong Minh Date: Fri, 15 Jul 2011 15:13:18 +0000 (+0000) Subject: Follow-up r79845: Fix rotation. Turns out that we need to pass the pre-rotation dimen... X-Git-Tag: 1.31.0-rc.0~28824 X-Git-Url: http://git.cyclocoop.org/ecrire?a=commitdiff_plain;h=64ef056784181e9ca7d5e8e2d72e5c09ce90e114;p=lhc%2Fweb%2Fwiklou.git Follow-up r79845: Fix rotation. Turns out that we need to pass the pre-rotation dimensions to convert.exe. Also added tests. --- diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index be4fb2d709..665aca5d05 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -28,11 +28,13 @@ class BitmapHandler extends ImageHandler { $srcWidth = $image->getWidth( $params['page'] ); $srcHeight = $image->getHeight( $params['page'] ); + $swapDimensions = false; 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" ); + $swapDimensions = true; $width = $params['width']; $params['width'] = $params['height']; $params['height'] = $width; @@ -44,8 +46,13 @@ class BitmapHandler extends ImageHandler { $params['physicalHeight'] = $params['height']; if ( $params['physicalWidth'] >= $srcWidth ) { - $params['physicalWidth'] = $srcWidth; - $params['physicalHeight'] = $srcHeight; + if ( $swapDimensions ) { + $params['physicalWidth'] = $srcHeight; + $params['physicalHeight'] = $srcWidth; + } else { + $params['physicalWidth'] = $srcWidth; + $params['physicalHeight'] = $srcHeight; + } # Skip scaling limit checks if no scaling is required if ( !$image->mustRender() ) return true; @@ -63,6 +70,25 @@ class BitmapHandler extends ImageHandler { return true; } + + /** + * Extracts the width/height if the image will be scaled before rotating + * + * @param $params array Parameters as returned by normaliseParams + * @param $rotation int The rotation angle that will be applied + * @return array ($width, $height) array + */ + public function extractPreRotationDimensions( $params, $rotation ) { + if ( $rotation == 90 || $rotation == 270 ) { + # We'll resize before rotation, so swap the dimensions again + $width = $params['physicalHeight']; + $height = $params['physicalWidth']; + } else { + $width = $params['physicalWidth']; + $height = $params['physicalHeight']; + } + return array( $width, $height ); + } // Function that returns the number of pixels to be thumbnailed. @@ -287,6 +313,9 @@ class BitmapHandler extends ImageHandler { if ( strval( $wgImageMagickTempDir ) !== '' ) { $env['MAGICK_TMPDIR'] = $wgImageMagickTempDir; } + + $rotation = $this->getRotation( $image ); + list( $width, $height ) = $this->extractPreRotationDimensions( $params, $rotation ); $cmd = wfEscapeShellArg( $wgImageMagickConvertCommand ) . @@ -299,7 +328,7 @@ class BitmapHandler extends ImageHandler { // For the -thumbnail option a "!" is needed to force exact size, // or ImageMagick may decide your ratio is wrong and slice off // a pixel. - " -thumbnail " . wfEscapeShellArg( "{$params['physicalDimensions']}!" ) . + " -thumbnail " . wfEscapeShellArg( "{$width}x{$height}!" ) . // Add the source url as a comment to the thumb, but don't add the flag if there's no comment ( $params['comment'] !== '' ? " -set comment " . wfEscapeShellArg( $this->escapeMagickProperty( $params['comment'] ) ) @@ -362,14 +391,7 @@ class BitmapHandler extends ImageHandler { } $rotation = $this->getRotation( $image ); - if ( $rotation == 90 || $rotation == 270 ) { - // We'll resize before rotation, so swap the dimensions again - $width = $params['physicalHeight']; - $height = $params['physicalWidth']; - } else { - $width = $params['physicalWidth']; - $height = $params['physicalHeight']; - } + list( $width, $height ) = $this->extractPreRotationDimensions( $params, $rotation ); $im->setImageBackgroundColor( new ImagickPixel( 'white' ) ); @@ -509,14 +531,7 @@ class BitmapHandler extends ImageHandler { $src_image = call_user_func( $loader, $params['srcPath'] ); $rotation = function_exists( 'imagerotate' ) ? $this->getRotation( $image ) : 0; - if ( $rotation == 90 || $rotation == 270 ) { - # We'll resize before rotation, so swap the dimensions again - $width = $params['physicalHeight']; - $height = $params['physicalWidth']; - } else { - $width = $params['physicalWidth']; - $height = $params['physicalHeight']; - } + list( $width, $height ) = $this->extractPreRotationDimensions( $params, $rotation ); $dst_image = imagecreatetruecolor( $width, $height ); // Initialise the destination image to transparent instead of diff --git a/tests/phpunit/includes/media/ExifRotationTest.php b/tests/phpunit/includes/media/ExifRotationTest.php index b3e2aaece6..1d4b7c513c 100644 --- a/tests/phpunit/includes/media/ExifRotationTest.php +++ b/tests/phpunit/includes/media/ExifRotationTest.php @@ -1,11 +1,12 @@ 10000, 'height' => 10000 ); $file = UnregisteredLocalFile::newFromPath( dirname( __FILE__ ) . '/' . $name, $type ); - $this->assertEquals( $file->getWidth(), $info['width'], "$name: width check" ); - $this->assertEquals( $file->getHeight(), $info['height'], "$name: height check" ); + + # Normalize parameters + $this->assertTrue( $handler->normaliseParams( $file, $params ) ); + $rotation = $handler->getRotation( $file ); + + # Check if pre-rotation dimensions are still good + list( $width, $height ) = $handler->extractPreRotationDimensions( $params, $rotation ); + $this->assertEquals( $file->getWidth(), $width, + "$name: pre-rotation width check, $rotation:$width" ); + $this->assertEquals( $file->getHeight(), $height, + "$name: pre-rotation height check, $rotation" ); + + # Any further test require a scaler that can rotate + if ( !BitmapHandler::canRotate() ) { + $this->markTestIncomplete( 'Scaler does not support rotation' ); + return; + } + + # Check post-rotation width + $this->assertEquals( $params['physicalWidth'], $info['width'], + "$name: post-rotation width check" ); + $this->assertEquals( $params['physicalHeight'], $info['height'], + "$name: post-rotation height check" ); } function providerFiles() { @@ -40,5 +63,42 @@ class ExifRotationTest extends MediaWikiTestCase { ) ); } + + + const TEST_WIDTH = 100; + const TEST_HEIGHT = 200; + + /** + * @dataProvider provideBitmapExtractPreRotationDimensions + */ + function testBitmapExtractPreRotationDimensions( $rotation, $expected ) { + $handler = new BitmapHandler; + $result = $handler->extractPreRotationDimensions( array( + 'physicalWidth' => self::TEST_WIDTH, + 'physicalHeight' => self::TEST_HEIGHT, + ), $rotation ); + $this->assertEquals( $expected, $result ); + } + + function provideBitmapExtractPreRotationDimensions() { + return array( + array( + 0, + array( self::TEST_WIDTH, self::TEST_HEIGHT ) + ), + array( + 90, + array( self::TEST_HEIGHT, self::TEST_WIDTH ) + ), + array( + 180, + array( self::TEST_WIDTH, self::TEST_HEIGHT ) + ), + array( + 270, + array( self::TEST_HEIGHT, self::TEST_WIDTH ) + ), + ); + } }