From: Bryan Tong Minh Date: Fri, 15 Jul 2011 19:03:40 +0000 (+0000) Subject: (bug 28762) Resizing to specified height broken for very thin images X-Git-Tag: 1.31.0-rc.0~28811 X-Git-Url: http://git.cyclocoop.org/%22.%24image2.%22?a=commitdiff_plain;h=0222e58b9f2daf12c5fe7eaca2d6b8e0f0d11833;p=lhc%2Fweb%2Fwiklou.git (bug 28762) Resizing to specified height broken for very thin images Restructure and comment of ImageHandler::normaliseParams(). ImageHandler::normaliseParams() will now output both width/height as physicalWidth/Height, where width/height are the dimensions used by the browser, and physicalWidth/Height are the dimensions of the thumbnail that will be created. This allows keeping the use of unique dimensions for a specified thumbnail width, by forcing client side resizing of 1px-wide images with non corresponding height. Also fixed a bug where the check for mustRender() was in an if statement where it should not have been. --- diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index 665aca5d05..a1cbbc5bdc 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -35,16 +35,14 @@ class BitmapHandler extends ImageHandler { 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; + list( $params['width'], $params['height'] ) = + array( $params['width'], $params['height'] ); + list( $params['physicalWidth'], $params['physicalHeight'] ) = + array( $params['physicalWidth'], $params['physicalHeight'] ); } } # Don't make an image bigger than the source - $params['physicalWidth'] = $params['width']; - $params['physicalHeight'] = $params['height']; - if ( $params['physicalWidth'] >= $srcWidth ) { if ( $swapDimensions ) { $params['physicalWidth'] = $srcHeight; @@ -53,10 +51,11 @@ class BitmapHandler extends ImageHandler { $params['physicalWidth'] = $srcWidth; $params['physicalHeight'] = $srcHeight; } - # Skip scaling limit checks if no scaling is required - if ( !$image->mustRender() ) - return true; } + + # 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 diff --git a/includes/media/DjVu.php b/includes/media/DjVu.php index 43d72aa9c7..2833f68323 100644 --- a/includes/media/DjVu.php +++ b/includes/media/DjVu.php @@ -147,7 +147,8 @@ class DjVuHandler extends ImageHandler { # Use a subshell (brackets) to aggregate stderr from both pipeline commands # before redirecting it to the overall stdout. This works in both Linux and Windows XP. - $cmd = '(' . wfEscapeShellArg( $wgDjvuRenderer ) . " -format=ppm -page={$page} -size={$width}x{$height} " . + $cmd = '(' . wfEscapeShellArg( $wgDjvuRenderer ) . " -format=ppm -page={$page}" . + " -size={$params['physicalWidth']}x{$params['physicalHeight']} " . wfEscapeShellArg( $srcPath ); if ( $wgDjvuPostProcessor ) { $cmd .= " | {$wgDjvuPostProcessor}"; diff --git a/includes/media/Generic.php b/includes/media/Generic.php index 2a526835d3..48735ebf7d 100644 --- a/includes/media/Generic.php +++ b/includes/media/Generic.php @@ -570,13 +570,44 @@ abstract class ImageHandler extends MediaHandler { $srcWidth = $image->getWidth( $params['page'] ); $srcHeight = $image->getHeight( $params['page'] ); + if ( isset( $params['height'] ) && $params['height'] != -1 ) { + # Height & width were both set if ( $params['width'] * $srcHeight > $params['height'] * $srcWidth ) { + # Height is the relative smaller dimension, so scale width accordingly $params['width'] = wfFitBoxWidth( $srcWidth, $srcHeight, $params['height'] ); + + if ( $params['width'] == 0 ) { + # Very small image, so we need to rely on client side scaling :( + $params['width'] = 1; + } + + $params['physicalWidth'] = $params['width']; + } else { + # Height was crap, unset it so that it will be calculated later + unset( $params['height'] ); } } - $params['height'] = File::scaleHeight( $srcWidth, $srcHeight, $params['width'] ); - if ( !$this->validateThumbParams( $params['width'], $params['height'], $srcWidth, $srcHeight, $mimeType ) ) { + + if ( !isset( $params['physicalWidth'] ) ) { + # Passed all validations, so set the physicalWidth + $params['physicalWidth'] = $params['width']; + } + + # Because thumbs are only referred to by width, the height always needs + # to be scaled by the width to keep the thumbnail sizes consistent, + # even if it was set inside the if block above + $params['physicalHeight'] = File::scaleHeight( $srcWidth, $srcHeight, + $params['physicalWidth'] ); + + # Set the height if it was not validated in the if block higher up + if ( !isset( $params['height'] ) || $params['height'] == -1 ) { + $params['height'] = $params['physicalHeight']; + } + + + if ( !$this->validateThumbParams( $params['physicalWidth'], + $params['physicalHeight'], $srcWidth, $srcHeight, $mimeType ) ) { return false; } return true; @@ -613,6 +644,10 @@ abstract class ImageHandler extends MediaHandler { } $height = File::scaleHeight( $srcWidth, $srcHeight, $width ); + if ( $height == 0 ) { + # Force height to be at least 1 pixel + $height = 1; + } return true; } diff --git a/includes/media/SVG.php b/includes/media/SVG.php index a7565be56a..17fb94693e 100644 --- a/includes/media/SVG.php +++ b/includes/media/SVG.php @@ -59,8 +59,6 @@ class SvgHandler extends ImageHandler { return false; } # Don't make an image bigger than wgMaxSVGSize - $params['physicalWidth'] = $params['width']; - $params['physicalHeight'] = $params['height']; if ( $params['physicalWidth'] > $wgSVGMaxSize ) { $srcWidth = $image->getWidth( $params['page'] ); $srcHeight = $image->getHeight( $params['page'] ); diff --git a/tests/phpunit/includes/media/BitmapScalingTest.php b/tests/phpunit/includes/media/BitmapScalingTest.php new file mode 100644 index 0000000000..bca4b55a5f --- /dev/null +++ b/tests/phpunit/includes/media/BitmapScalingTest.php @@ -0,0 +1,96 @@ +normaliseParams( $file, $params ); + $this->assertEquals( $expectedParams, $params, $msg ); + } + + function provideNormaliseParams() { + return array( + /* Regular resize operations */ + array( + array( 1024, 768 ), + array( + 'width' => 512, 'height' => 384, + 'physicalWidth' => 512, 'physicalHeight' => 384, + 'page' => 1, + ), + array( 'width' => 512 ), + 'Resizing with width set', + ), + array( + array( 1024, 768 ), + array( + 'width' => 512, 'height' => 384, + 'physicalWidth' => 512, 'physicalHeight' => 384, + 'page' => 1, + ), + array( 'width' => 512, 'height' => 768 ), + 'Resizing with height set too high', + ), + array( + array( 1024, 768 ), + array( + 'width' => 512, 'height' => 384, + 'physicalWidth' => 512, 'physicalHeight' => 384, + 'page' => 1, + ), + array( 'width' => 1024, 'height' => 384 ), + 'Resizing with height set', + ), + + /* Very tall images */ + array( + array( 1000, 100 ), + array( + 'width' => 5, 'height' => 1, + 'physicalWidth' => 5, 'physicalHeight' => 1, + 'page' => 1, + ), + array( 'width' => 5 ), + 'Very wide image', + ), + + array( + array( 100, 1000 ), + array( + 'width' => 1, 'height' => 10, + 'physicalWidth' => 1, 'physicalHeight' => 10, + 'page' => 1, + ), + array( 'width' => 1 ), + 'Very high image', + ), + array( + array( 100, 1000 ), + array( + 'width' => 1, 'height' => 5, + 'physicalWidth' => 1, 'physicalHeight' => 10, + 'page' => 1, + ), + array( 'width' => 10, 'height' => 5 ), + 'Very high image with height set', + ), + ); + } +} + +class FakeDimensionFile extends File { + public function __construct( $dimensions ) { + parent::__construct( Title::makeTitle( NS_FILE, 'Test' ), null ); + + $this->dimensions = $dimensions; + } + public function getWidth( $page = 1 ) { + return $this->dimensions[0]; + } + public function getHeight( $page = 1 ) { + return $this->dimensions[1]; + } +} \ No newline at end of file