http://www.mediawiki.org/wiki/Special:Code/MediaWiki/96687
Revision: 96687
Author: bawolff
Date: 2011-09-09 20:13:09 +0000 (Fri, 09 Sep 2011)
Log Message:
-----------
(bug 30640; follow-up r92279) Rotating images was making skewed images
This is Bryan's patch from bug 30640 with a couple minor related changes, plus
some unit tests.
This also addresses an issue with preventing too-big images from being scaled
from r92279.
I also noticed that image magick's rotation support is broken in 6.3.7 (the
version I had installed locally. I've since upgraded) I'm not sure if we should
be doing something about that...
I did test this without both image magick, and gd (although only very briefly
with gd) both seemed to work well. I didn't test any other image scalars.
Modified Paths:
--------------
trunk/phase3/includes/media/Bitmap.php
trunk/phase3/tests/phpunit/includes/media/BitmapScalingTest.php
trunk/phase3/tests/phpunit/includes/media/ExifRotationTest.php
Modified: trunk/phase3/includes/media/Bitmap.php
===================================================================
--- trunk/phase3/includes/media/Bitmap.php 2011-09-09 20:10:01 UTC (rev
96686)
+++ trunk/phase3/includes/media/Bitmap.php 2011-09-09 20:13:09 UTC (rev
96687)
@@ -15,7 +15,9 @@
/**
* @param $image File
- * @param $params
+ * @param $params array Transform parameters. Entries with the keys
'width'
+ * and 'height' are the respective screen width and height, while the
keys
+ * 'physicalWidth' and 'physicalHeight' indicate the thumbnail
dimensions.
* @return bool
*/
function normaliseParams( $image, &$params ) {
@@ -25,38 +27,36 @@
}
$mimeType = $image->getMimeType();
+ # Obtain the source, pre-rotation dimensions
$srcWidth = $image->getWidth( $params['page'] );
$srcHeight = $image->getHeight( $params['page'] );
- $swapDimensions = false;
+ # Don't make an image bigger than the source
+ if ( $params['physicalWidth'] >= $srcWidth ) {
+ $params['physicalWidth'] = $srcWidth;
+ $params['physicalHeight'] = $srcHeight;
+
+ # Skip scaling limit checks if no scaling is required
+ # due to requested size being bigger than source.
+ if ( !$image->mustRender() ) {
+ return true;
+ }
+ }
+
+
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;
list( $params['width'], $params['height'] ) =
- array( $params['width'],
$params['height'] );
+ array( $params['height'],
$params['width'] );
list( $params['physicalWidth'],
$params['physicalHeight'] ) =
- array( $params['physicalWidth'],
$params['physicalHeight'] );
+ array( $params['physicalHeight'],
$params['physicalWidth'] );
}
}
- # Don't make an image bigger than the source
- if ( $params['physicalWidth'] >= $srcWidth ) {
- 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;
-
# 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.
Modified: trunk/phase3/tests/phpunit/includes/media/BitmapScalingTest.php
===================================================================
--- trunk/phase3/tests/phpunit/includes/media/BitmapScalingTest.php
2011-09-09 20:10:01 UTC (rev 96686)
+++ trunk/phase3/tests/phpunit/includes/media/BitmapScalingTest.php
2011-09-09 20:13:09 UTC (rev 96687)
@@ -1,13 +1,24 @@
<?php
class BitmapScalingTest extends MediaWikiTestCase {
+
+ function setUp() {
+ global $wgMaxImageArea;
+ $this->oldMaxImageArea = $wgMaxImageArea;
+ $wgMaxImageArea = 1.25e7; // 3500x3500
+ }
+ function tearDown() {
+ global $wgMaxImageArea;
+ $wgMaxImageArea = $this->oldMaxImageArea;
+ }
/**
* @dataProvider provideNormaliseParams
*/
function testNormaliseParams( $fileDimensions, $expectedParams,
$params, $msg ) {
$file = new FakeDimensionFile( $fileDimensions );
$handler = new BitmapHandler;
- $handler->normaliseParams( $file, $params );
+ $valid = $handler->normaliseParams( $file, $params );
+ $this->assertTrue( $valid );
$this->assertEquals( $expectedParams, $params, $msg );
}
@@ -77,11 +88,37 @@
array( 'width' => 10, 'height' => 5 ),
'Very high image with height set',
),
+ /* Max image area */
+ array(
+ array( 4000, 4000 ),
+ array(
+ 'width' => 5000, 'height' => 5000,
+ 'physicalWidth' => 4000,
'physicalHeight' => 4000,
+ 'page' => 1,
+ ),
+ array( 'width' => 5000 ),
+ 'Bigger than max image size but doesn\'t need
scaling',
+ ),
);
}
+ function testTooBigImage() {
+ $file = new FakeDimensionFile( array( 4000, 4000 ) );
+ $handler = new BitmapHandler;
+ $params = array( 'width' => '3700' ); // Still bigger than max
size.
+ $this->assertFalse( $handler->normaliseParams( $file, $params )
);
+ }
+ function testTooBigMustRenderImage() {
+ $file = new FakeDimensionFile( array( 4000, 4000 ) );
+ $file->mustRender = true;
+ $handler = new BitmapHandler;
+ $params = array( 'width' => '5000' ); // Still bigger than max
size.
+ $this->assertFalse( $handler->normaliseParams( $file, $params )
);
+ }
}
class FakeDimensionFile extends File {
+ public $mustRender = false;
+
public function __construct( $dimensions ) {
parent::__construct( Title::makeTitle( NS_FILE, 'Test' ), null
);
@@ -93,4 +130,7 @@
public function getHeight( $page = 1 ) {
return $this->dimensions[1];
}
-}
\ No newline at end of file
+ public function mustRender() {
+ return $this->mustRender;
+ }
+}
Modified: trunk/phase3/tests/phpunit/includes/media/ExifRotationTest.php
===================================================================
--- trunk/phase3/tests/phpunit/includes/media/ExifRotationTest.php
2011-09-09 20:10:01 UTC (rev 96686)
+++ trunk/phase3/tests/phpunit/includes/media/ExifRotationTest.php
2011-09-09 20:13:09 UTC (rev 96687)
@@ -8,6 +8,7 @@
function setUp() {
parent::setUp();
$this->filePath = dirname( __FILE__ ) . '/../../data/media/';
+ $this->handler = new BitmapHandler();
}
/**
@@ -15,17 +16,16 @@
* @dataProvider providerFiles
*/
function testMetadata( $name, $type, $info ) {
- $handler = new BitmapHandler();
# Force client side resizing
$params = array( 'width' => 10000, 'height' => 10000 );
$file = UnregisteredLocalFile::newFromPath( $this->filePath .
$name, $type );
# Normalize parameters
- $this->assertTrue( $handler->normaliseParams( $file, $params )
);
- $rotation = $handler->getRotation( $file );
+ $this->assertTrue( $this->handler->normaliseParams( $file,
$params ) );
+ $rotation = $this->handler->getRotation( $file );
# Check if pre-rotation dimensions are still good
- list( $width, $height ) =
$handler->extractPreRotationDimensions( $params, $rotation );
+ list( $width, $height ) =
$this->handler->extractPreRotationDimensions( $params, $rotation );
$this->assertEquals( $file->getWidth(), $width,
"$name: pre-rotation width check, $rotation:$width" );
$this->assertEquals( $file->getHeight(), $height,
@@ -73,8 +73,7 @@
* @dataProvider provideBitmapExtractPreRotationDimensions
*/
function testBitmapExtractPreRotationDimensions( $rotation, $expected )
{
- $handler = new BitmapHandler;
- $result = $handler->extractPreRotationDimensions( array(
+ $result = $this->handler->extractPreRotationDimensions( array(
'physicalWidth' => self::TEST_WIDTH,
'physicalHeight' => self::TEST_HEIGHT,
), $rotation );
@@ -101,5 +100,22 @@
),
);
}
+
+ function testWidthFlipping() {
+ $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 );
+ }
}
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
