From: Brian Wolff Date: Wed, 9 Jul 2014 20:03:31 +0000 (-0300) Subject: Split BitmapHandler into two classes. X-Git-Tag: 1.31.0-rc.0~14005^2 X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_aide%28?a=commitdiff_plain;h=757a70ae0a94f8a25034e9cd28e71edd7087a3b2;p=lhc%2Fweb%2Fwiklou.git Split BitmapHandler into two classes. BitmapHandler has a lot of generic-ish functionality that could be re-usable by extension classes (Such as how it organizes $scalerParams array, or various image magick escaping methods). However it's combined with a lot of very format specific things, such as the shell-out call to image magick. Try to separate out the more generic stuff into TransformationalImageHandler. In order to do this, I also made canRotate, autoRotateEnabled, and getScalerType non-static. No extensions in our repo appeared to be using these methods, and they don't really make sense to be static (imo). In particular, I think code duplication can be reduced in PagedTiffHandler by extending this new class. See comments on I1b9a77a4a56eeb65. Change-Id: Id3a8b25a598942572cb5791a95e86054d7784961 --- diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 5242ec0f65..0f1caff80c 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -753,6 +753,7 @@ $wgAutoloadLocalClasses = array( 'SVGReader' => 'includes/media/SVGMetadataExtractor.php', 'ThumbnailImage' => 'includes/media/MediaTransformOutput.php', 'TiffHandler' => 'includes/media/Tiff.php', + 'TransformationalImageHandler' => 'includes/media/TransformationalImageHandler.php', 'TransformParameterError' => 'includes/media/MediaTransformOutput.php', 'XCFHandler' => 'includes/media/XCF.php', 'XMPInfo' => 'includes/media/XMPInfo.php', diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php index b2802ddb31..e81b37de37 100644 --- a/includes/media/Bitmap.php +++ b/includes/media/Bitmap.php @@ -26,238 +26,7 @@ * * @ingroup Media */ -class BitmapHandler extends ImageHandler { - /** - * @param File $image - * @param array $params 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 ) { - if ( !parent::normaliseParams( $image, $params ) ) { - return false; - } - - # Obtain the source, pre-rotation dimensions - $srcWidth = $image->getWidth( $params['page'] ); - $srcHeight = $image->getHeight( $params['page'] ); - - # 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; - } - } - - # Check if the file is smaller than the maximum image area for thumbnailing - $checkImageAreaHookResult = null; - wfRunHooks( - 'BitmapHandlerCheckImageArea', - array( $image, &$params, &$checkImageAreaHookResult ) - ); - - if ( is_null( $checkImageAreaHookResult ) ) { - global $wgMaxImageArea; - - if ( $srcWidth * $srcHeight > $wgMaxImageArea - && !( $image->getMimeType() == 'image/jpeg' - && self::getScalerType( false, false ) == 'im' ) - ) { - # Only ImageMagick can efficiently downsize jpg images without loading - # the entire file in memory - return false; - } - } else { - return $checkImageAreaHookResult; - } - - return true; - } - - /** - * 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 array $params Parameters as returned by normaliseParams - * @param int $rotation 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 ); - } - - /** - * @param File $image - * @param string $dstPath - * @param string $dstUrl - * @param array $params - * @param int $flags - * @return MediaTransformError|ThumbnailImage|TransformParameterError - */ - function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) { - if ( !$this->normaliseParams( $image, $params ) ) { - return new TransformParameterError( $params ); - } - - # Create a parameter array to pass to the scaler - $scalerParams = array( - # The size to which the image will be resized - 'physicalWidth' => $params['physicalWidth'], - 'physicalHeight' => $params['physicalHeight'], - 'physicalDimensions' => "{$params['physicalWidth']}x{$params['physicalHeight']}", - # The size of the image on the page - 'clientWidth' => $params['width'], - 'clientHeight' => $params['height'], - # Comment as will be added to the Exif of the thumbnail - 'comment' => isset( $params['descriptionUrl'] ) - ? "File source: {$params['descriptionUrl']}" - : '', - # Properties of the original image - 'srcWidth' => $image->getWidth(), - 'srcHeight' => $image->getHeight(), - 'mimeType' => $image->getMimeType(), - 'dstPath' => $dstPath, - 'dstUrl' => $dstUrl, - ); - - if ( isset( $params['quality'] ) && $params['quality'] === 'low' ) { - $scalerParams['quality'] = 30; - } - - # Determine scaler type - $scaler = self::getScalerType( $dstPath ); - - wfDebug( __METHOD__ . ": creating {$scalerParams['physicalDimensions']} " . - "thumbnail at $dstPath using scaler $scaler\n" ); - - if ( !$image->mustRender() && - $scalerParams['physicalWidth'] == $scalerParams['srcWidth'] - && $scalerParams['physicalHeight'] == $scalerParams['srcHeight'] - && !isset( $scalerParams['quality'] ) - ) { - - # normaliseParams (or the user) wants us to return the unscaled image - wfDebug( __METHOD__ . ": returning unscaled image\n" ); - - return $this->getClientScalingThumbnailImage( $image, $scalerParams ); - } - - if ( $scaler == 'client' ) { - # Client-side image scaling, use the source URL - # Using the destination URL in a TRANSFORM_LATER request would be incorrect - return $this->getClientScalingThumbnailImage( $image, $scalerParams ); - } - - if ( $flags & self::TRANSFORM_LATER ) { - wfDebug( __METHOD__ . ": Transforming later per flags.\n" ); - $newParams = array( - 'width' => $scalerParams['clientWidth'], - 'height' => $scalerParams['clientHeight'] - ); - if ( isset( $params['quality'] ) ) { - $newParams['quality'] = $params['quality']; - } - return new ThumbnailImage( $image, $dstUrl, false, $newParams ); - } - - # Try to make a target path for the thumbnail - if ( !wfMkdirParents( dirname( $dstPath ), null, __METHOD__ ) ) { - wfDebug( __METHOD__ . ": Unable to create thumbnail destination " . - "directory, falling back to client scaling\n" ); - - return $this->getClientScalingThumbnailImage( $image, $scalerParams ); - } - - # Transform functions and binaries need a FS source file - $thumbnailSource = $image->getThumbnailSource( $params ); - - $scalerParams['srcPath'] = $thumbnailSource['path']; - $scalerParams['srcWidth'] = $thumbnailSource['width']; - $scalerParams['srcHeight'] = $thumbnailSource['height']; - - if ( $scalerParams['srcPath'] === false ) { // Failed to get local copy - wfDebugLog( 'thumbnail', - sprintf( 'Thumbnail failed on %s: could not get local copy of "%s"', - wfHostname(), $image->getName() ) ); - - return new MediaTransformError( 'thumbnail_error', - $scalerParams['clientWidth'], $scalerParams['clientHeight'], - wfMessage( 'filemissing' )->text() - ); - } - - # Try a hook - $mto = null; - wfRunHooks( 'BitmapHandlerTransform', array( $this, $image, &$scalerParams, &$mto ) ); - if ( !is_null( $mto ) ) { - wfDebug( __METHOD__ . ": Hook to BitmapHandlerTransform created an mto\n" ); - $scaler = 'hookaborted'; - } - - switch ( $scaler ) { - case 'hookaborted': - # Handled by the hook above - /** @var MediaTransformOutput $mto */ - $err = $mto->isError() ? $mto : false; - break; - case 'im': - $err = $this->transformImageMagick( $image, $scalerParams ); - break; - case 'custom': - $err = $this->transformCustom( $image, $scalerParams ); - break; - case 'imext': - $err = $this->transformImageMagickExt( $image, $scalerParams ); - break; - case 'gd': - default: - $err = $this->transformGd( $image, $scalerParams ); - break; - } - - # Remove the file if a zero-byte thumbnail was created, or if there was an error - $removed = $this->removeBadFile( $dstPath, (bool)$err ); - if ( $err ) { - # transform returned MediaTransforError - return $err; - } elseif ( $removed ) { - # Thumbnail was zero-byte and had to be removed - return new MediaTransformError( 'thumbnail_error', - $scalerParams['clientWidth'], $scalerParams['clientHeight'], - wfMessage( 'unknown-error' )->text() - ); - } elseif ( $mto ) { - return $mto; - } else { - $newParams = array( - 'width' => $scalerParams['clientWidth'], - 'height' => $scalerParams['clientHeight'] - ); - if ( isset( $params['quality'] ) ) { - $newParams['quality'] = $params['quality']; - } - return new ThumbnailImage( $image, $dstUrl, $dstPath, $newParams ); - } - } +class BitmapHandler extends TransformationalImageHandler { /** * Returns which scaler type should be used. Creates parent directories @@ -265,9 +34,9 @@ class BitmapHandler extends ImageHandler { * * @param string $dstPath * @param bool $checkDstPath - * @return string One of client, im, custom, gd, imext + * @return string|Callable One of client, im, custom, gd, imext or an array( object, method ) */ - protected static function getScalerType( $dstPath, $checkDstPath = true ) { + protected function getScalerType( $dstPath, $checkDstPath = true ) { global $wgUseImageResize, $wgUseImageMagick, $wgCustomConvertCommand; if ( !$dstPath && $checkDstPath ) { @@ -290,25 +59,6 @@ class BitmapHandler extends ImageHandler { return $scaler; } - /** - * Get a ThumbnailImage that respresents an image that will be scaled - * client side - * - * @param File $image File associated with this thumbnail - * @param array $scalerParams Array with scaler params - * @return ThumbnailImage - * - * @todo FIXME: No rotation support - */ - protected function getClientScalingThumbnailImage( $image, $scalerParams ) { - $params = array( - 'width' => $scalerParams['clientWidth'], - 'height' => $scalerParams['clientHeight'] - ); - - return new ThumbnailImage( $image, $image->getURL(), null, $params ); - } - /** * Transform an image using ImageMagick * @@ -544,18 +294,6 @@ class BitmapHandler extends ImageHandler { return false; # No error } - /** - * Get a MediaTransformError with error 'thumbnail_error' - * - * @param array $params Parameter array as passed to the transform* functions - * @param string $errMsg Error message - * @return MediaTransformError - */ - public function getMediaTransformError( $params, $errMsg ) { - return new MediaTransformError( 'thumbnail_error', $params['clientWidth'], - $params['clientHeight'], $errMsg ); - } - /** * Transform an image using the built in GD library * @@ -651,131 +389,8 @@ class BitmapHandler extends ImageHandler { } /** - * Escape a string for ImageMagick's property input (e.g. -set -comment) - * See InterpretImageProperties() in magick/property.c - * @param string $s - * @return string - */ - function escapeMagickProperty( $s ) { - // Double the backslashes - $s = str_replace( '\\', '\\\\', $s ); - // Double the percents - $s = str_replace( '%', '%%', $s ); - // Escape initial - or @ - if ( strlen( $s ) > 0 && ( $s[0] === '-' || $s[0] === '@' ) ) { - $s = '\\' . $s; - } - - return $s; - } - - /** - * Escape a string for ImageMagick's input filenames. See ExpandFilenames() - * and GetPathComponent() in magick/utility.c. - * - * This won't work with an initial ~ or @, so input files should be prefixed - * with the directory name. - * - * Glob character unescaping is broken in ImageMagick before 6.6.1-5, but - * it's broken in a way that doesn't involve trying to convert every file - * in a directory, so we're better off escaping and waiting for the bugfix - * to filter down to users. - * - * @param string $path The file path - * @param bool|string $scene The scene specification, or false if there is none - * @throws MWException - * @return string - */ - function escapeMagickInput( $path, $scene = false ) { - # Die on initial metacharacters (caller should prepend path) - $firstChar = substr( $path, 0, 1 ); - if ( $firstChar === '~' || $firstChar === '@' ) { - throw new MWException( __METHOD__ . ': cannot escape this path name' ); - } - - # Escape glob chars - $path = preg_replace( '/[*?\[\]{}]/', '\\\\\0', $path ); - - return $this->escapeMagickPath( $path, $scene ); - } - - /** - * Escape a string for ImageMagick's output filename. See - * InterpretImageFilename() in magick/image.c. - * @param string $path The file path - * @param bool|string $scene The scene specification, or false if there is none - * @return string - */ - function escapeMagickOutput( $path, $scene = false ) { - $path = str_replace( '%', '%%', $path ); - - return $this->escapeMagickPath( $path, $scene ); - } - - /** - * Armour a string against ImageMagick's GetPathComponent(). This is a - * helper function for escapeMagickInput() and escapeMagickOutput(). - * - * @param string $path The file path - * @param bool|string $scene The scene specification, or false if there is none - * @throws MWException - * @return string - */ - protected function escapeMagickPath( $path, $scene = false ) { - # Die on format specifiers (other than drive letters). The regex is - # meant to match all the formats you get from "convert -list format" - if ( preg_match( '/^([a-zA-Z0-9-]+):/', $path, $m ) ) { - if ( wfIsWindows() && is_dir( $m[0] ) ) { - // OK, it's a drive letter - // ImageMagick has a similar exception, see IsMagickConflict() - } else { - throw new MWException( __METHOD__ . ': unexpected colon character in path name' ); - } - } - - # If there are square brackets, add a do-nothing scene specification - # to force a literal interpretation - if ( $scene === false ) { - if ( strpos( $path, '[' ) !== false ) { - $path .= '[0--1]'; - } - } else { - $path .= "[$scene]"; - } - - return $path; - } - - /** - * Retrieve the version of the installed ImageMagick - * You can use PHPs version_compare() to use this value - * Value is cached for one hour. - * @return string Representing the IM version. + * Callback for transformGd when transforming jpeg images. */ - protected function getMagickVersion() { - global $wgMemc; - - $cache = $wgMemc->get( "imagemagick-version" ); - if ( !$cache ) { - global $wgImageMagickConvertCommand; - $cmd = wfEscapeShellArg( $wgImageMagickConvertCommand ) . ' -version'; - wfDebug( __METHOD__ . ": Running convert -version\n" ); - $retval = ''; - $return = wfShellExec( $cmd, $retval ); - $x = preg_match( '/Version: ImageMagick ([0-9]*\.[0-9]*\.[0-9]*)/', $return, $matches ); - if ( $x != 1 ) { - wfDebug( __METHOD__ . ": ImageMagick version check failed\n" ); - - return null; - } - $wgMemc->set( "imagemagick-version", $matches[1], 3600 ); - - return $matches[1]; - } - - return $cache; - } - // FIXME: transformImageMagick() & transformImageMagickExt() uses JPEG quality 80, here it's 95? static function imageJpegWrapper( $dst_image, $thumbPath, $quality = 95 ) { imageinterlace( $dst_image ); @@ -787,8 +402,8 @@ class BitmapHandler extends ImageHandler { * * @return bool */ - public static function canRotate() { - $scaler = self::getScalerType( null, false ); + public function canRotate() { + $scaler = $this->getScalerType( null, false ); switch ( $scaler ) { case 'im': # ImageMagick supports autorotation @@ -810,12 +425,12 @@ class BitmapHandler extends ImageHandler { * @see $wgEnableAutoRotation * @return bool Whether auto rotation is enabled */ - public static function autoRotateEnabled() { + public function autoRotateEnabled() { global $wgEnableAutoRotation; if ( $wgEnableAutoRotation === null ) { - // Only enable auto-rotation when the bitmap handler can rotate - $wgEnableAutoRotation = BitmapHandler::canRotate(); + // Only enable auto-rotation when we actually can + return $this->canRotate(); } return $wgEnableAutoRotation; @@ -834,7 +449,7 @@ class BitmapHandler extends ImageHandler { $rotation = ( $params['rotation'] + $this->getRotation( $file ) ) % 360; $scene = false; - $scaler = self::getScalerType( null, false ); + $scaler = $this->getScalerType( null, false ); switch ( $scaler ) { case 'im': $cmd = wfEscapeShellArg( $wgImageMagickConvertCommand ) . " " . @@ -872,15 +487,4 @@ class BitmapHandler extends ImageHandler { "$scaler rotation not implemented" ); } } - - /** - * Returns whether the file needs to be rendered. Returns true if the - * file requires rotation and we are able to rotate it. - * - * @param File $file - * @return bool - */ - public function mustRender( $file ) { - return self::canRotate() && $this->getRotation( $file ) != 0; - } } diff --git a/includes/media/ExifBitmap.php b/includes/media/ExifBitmap.php index ae1ff9d7fe..b7657cb3aa 100644 --- a/includes/media/ExifBitmap.php +++ b/includes/media/ExifBitmap.php @@ -173,7 +173,7 @@ class ExifBitmapHandler extends BitmapHandler { // Don't just call $image->getMetadata(); FSFile::getPropsFromPath() calls us with a bogus object. // This may mean we read EXIF data twice on initial upload. - if ( BitmapHandler::autoRotateEnabled() ) { + if ( $this->autoRotateEnabled() ) { $meta = $this->getMetadata( $image, $path ); $rotation = $this->getRotationForExif( $meta ); } else { @@ -202,7 +202,7 @@ class ExifBitmapHandler extends BitmapHandler { * @return int 0, 90, 180 or 270 */ public function getRotation( $file ) { - if ( !BitmapHandler::autoRotateEnabled() ) { + if ( !$this->autoRotateEnabled() ) { return 0; } diff --git a/includes/media/MediaHandler.php b/includes/media/MediaHandler.php index 13c2a912d7..64ca011560 100644 --- a/includes/media/MediaHandler.php +++ b/includes/media/MediaHandler.php @@ -745,10 +745,10 @@ abstract class MediaHandler { /** * True if the handler can rotate the media - * @since 1.21 + * @since 1.24 non-static. From 1.21-1.23 was static * @return bool */ - public static function canRotate() { + public function canRotate() { return false; } diff --git a/includes/media/TransformationalImageHandler.php b/includes/media/TransformationalImageHandler.php new file mode 100644 index 0000000000..13fb52b6d3 --- /dev/null +++ b/includes/media/TransformationalImageHandler.php @@ -0,0 +1,587 @@ +getWidth( $params['page'] ); + $srcHeight = $image->getHeight( $params['page'] ); + + # 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; + } + } + + # Check if the file is smaller than the maximum image area for thumbnailing + # For historical reasons, hook starts with BitmapHandler + $checkImageAreaHookResult = null; + wfRunHooks( + 'BitmapHandlerCheckImageArea', + array( $image, &$params, &$checkImageAreaHookResult ) + ); + + if ( is_null( $checkImageAreaHookResult ) ) { + global $wgMaxImageArea; + + if ( $srcWidth * $srcHeight > $wgMaxImageArea + && !( $image->getMimeType() == 'image/jpeg' + && $this->getScalerType( false, false ) == 'im' ) + ) { + # Only ImageMagick can efficiently downsize jpg images without loading + # the entire file in memory + return false; + } + } else { + return $checkImageAreaHookResult; + } + + return true; + } + + /** + * 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 array $params Parameters as returned by normaliseParams + * @param int $rotation 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 ); + } + + /** + * Create a thumbnail. + * + * This sets up various parameters, and then calls a helper method + * based on $this->getScalerType in order to scale the image. + * + * @param File $image + * @param string $dstPath + * @param string $dstUrl + * @param array $params + * @param int $flags + * @return MediaTransformError|ThumbnailImage|TransformParameterError + */ + function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) { + if ( !$this->normaliseParams( $image, $params ) ) { + return new TransformParameterError( $params ); + } + + # Create a parameter array to pass to the scaler + $scalerParams = array( + # The size to which the image will be resized + 'physicalWidth' => $params['physicalWidth'], + 'physicalHeight' => $params['physicalHeight'], + 'physicalDimensions' => "{$params['physicalWidth']}x{$params['physicalHeight']}", + # The size of the image on the page + 'clientWidth' => $params['width'], + 'clientHeight' => $params['height'], + # Comment as will be added to the Exif of the thumbnail + 'comment' => isset( $params['descriptionUrl'] ) + ? "File source: {$params['descriptionUrl']}" + : '', + # Properties of the original image + 'srcWidth' => $image->getWidth(), + 'srcHeight' => $image->getHeight(), + 'mimeType' => $image->getMimeType(), + 'dstPath' => $dstPath, + 'dstUrl' => $dstUrl, + ); + + if ( isset( $params['quality'] ) && $params['quality'] === 'low' ) { + $scalerParams['quality'] = 30; + } + + // For subclasses that might be paged. + if ( $image->isMultipage() && isset( $params['page'] ) ) { + $scalerParams['page'] = intval( $params['page'] ); + } + + # Determine scaler type + $scaler = $this->getScalerType( $dstPath ); + + wfDebug( __METHOD__ . ": creating {$scalerParams['physicalDimensions']} " . + "thumbnail at $dstPath using scaler $scaler\n" ); + + if ( !$image->mustRender() && + $scalerParams['physicalWidth'] == $scalerParams['srcWidth'] + && $scalerParams['physicalHeight'] == $scalerParams['srcHeight'] + && !isset( $scalerParams['quality'] ) + ) { + + # normaliseParams (or the user) wants us to return the unscaled image + wfDebug( __METHOD__ . ": returning unscaled image\n" ); + + return $this->getClientScalingThumbnailImage( $image, $scalerParams ); + } + + if ( $scaler == 'client' ) { + # Client-side image scaling, use the source URL + # Using the destination URL in a TRANSFORM_LATER request would be incorrect + return $this->getClientScalingThumbnailImage( $image, $scalerParams ); + } + + if ( $flags & self::TRANSFORM_LATER ) { + wfDebug( __METHOD__ . ": Transforming later per flags.\n" ); + $newParams = array( + 'width' => $scalerParams['clientWidth'], + 'height' => $scalerParams['clientHeight'] + ); + if ( isset( $params['quality'] ) ) { + $newParams['quality'] = $params['quality']; + } + if ( isset( $params['page'] ) && $params['page'] ) { + $newParams['page'] = $params['page']; + } + return new ThumbnailImage( $image, $dstUrl, false, $newParams ); + } + + # Try to make a target path for the thumbnail + if ( !wfMkdirParents( dirname( $dstPath ), null, __METHOD__ ) ) { + wfDebug( __METHOD__ . ": Unable to create thumbnail destination " . + "directory, falling back to client scaling\n" ); + + return $this->getClientScalingThumbnailImage( $image, $scalerParams ); + } + + # Transform functions and binaries need a FS source file + $thumbnailSource = $this->getThumbnailSource( $image, $params ); + + $scalerParams['srcPath'] = $thumbnailSource['path']; + $scalerParams['srcWidth'] = $thumbnailSource['width']; + $scalerParams['srcHeight'] = $thumbnailSource['height']; + + if ( $scalerParams['srcPath'] === false ) { // Failed to get local copy + wfDebugLog( 'thumbnail', + sprintf( 'Thumbnail failed on %s: could not get local copy of "%s"', + wfHostname(), $image->getName() ) ); + + return new MediaTransformError( 'thumbnail_error', + $scalerParams['clientWidth'], $scalerParams['clientHeight'], + wfMessage( 'filemissing' )->text() + ); + } + + # Try a hook. Called "Bitmap" for historical reasons. + /** @var $mto MediaTransformOutput */ + $mto = null; + wfRunHooks( 'BitmapHandlerTransform', array( $this, $image, &$scalerParams, &$mto ) ); + if ( !is_null( $mto ) ) { + wfDebug( __METHOD__ . ": Hook to BitmapHandlerTransform created an mto\n" ); + $scaler = 'hookaborted'; + } + + // $scaler will return a MediaTransformError on failure, or false on success. + // If the scaler is succesful, it will have created a thumbnail at the destination + // path. + if ( is_array( $scaler ) && is_callable( $scaler ) ) { + // Allow subclasses to specify their own rendering methods. + $err = call_user_func( $scaler, $image, $scalerParams ); + } else { + switch ( $scaler ) { + case 'hookaborted': + # Handled by the hook above + $err = $mto->isError() ? $mto : false; + break; + case 'im': + $err = $this->transformImageMagick( $image, $scalerParams ); + break; + case 'custom': + $err = $this->transformCustom( $image, $scalerParams ); + break; + case 'imext': + $err = $this->transformImageMagickExt( $image, $scalerParams ); + break; + case 'gd': + default: + $err = $this->transformGd( $image, $scalerParams ); + break; + } + } + + # Remove the file if a zero-byte thumbnail was created, or if there was an error + $removed = $this->removeBadFile( $dstPath, (bool)$err ); + if ( $err ) { + # transform returned MediaTransforError + return $err; + } elseif ( $removed ) { + # Thumbnail was zero-byte and had to be removed + return new MediaTransformError( 'thumbnail_error', + $scalerParams['clientWidth'], $scalerParams['clientHeight'], + wfMessage( 'unknown-error' )->text() + ); + } elseif ( $mto ) { + return $mto; + } else { + $newParams = array( + 'width' => $scalerParams['clientWidth'], + 'height' => $scalerParams['clientHeight'] + ); + if ( isset( $params['quality'] ) ) { + $newParams['quality'] = $params['quality']; + } + if ( isset( $params['page'] ) && $params['page'] ) { + $newParams['page'] = $params['page']; + } + return new ThumbnailImage( $image, $dstUrl, $dstPath, $newParams ); + } + } + + /** + * Get the source file for the transform + * + * @param $file File + * @param $params Array + * @return Array Array with keys width, height and path. + */ + protected function getThumbnailSource( $file, $params ) { + return $file->getThumbnailSource( $params ); + } + + /** + * Returns what sort of scaler type should be used. + * + * Values can be one of client, im, custom, gd, imext, or an array + * of object, method-name to call that specific method. + * + * If specifying a custom scaler command with array( Obj, method ), + * the method in question should take 2 parameters, a File object, + * and a $scalerParams array with various options (See doTransform + * for what is in $scalerParams). On error it should return a + * MediaTransformError object. On success it should return false, + * and simply make sure the thumbnail file is located at + * $scalerParams['dstPath']. + * + * If there is a problem with the output path, it returns "client" + * to do client side scaling. + * + * @param string $dstPath + * @param bool $checkDstPath Check that $dstPath is valid + * @return string|Callable One of client, im, custom, gd, imext, or a Callable array. + */ + abstract protected function getScalerType( $dstPath, $checkDstPath = true ); + + /** + * Get a ThumbnailImage that respresents an image that will be scaled + * client side + * + * @param File $image File associated with this thumbnail + * @param array $scalerParams Array with scaler params + * @return ThumbnailImage + * + * @todo FIXME: No rotation support + */ + protected function getClientScalingThumbnailImage( $image, $scalerParams ) { + $params = array( + 'width' => $scalerParams['clientWidth'], + 'height' => $scalerParams['clientHeight'] + ); + + return new ThumbnailImage( $image, $image->getURL(), null, $params ); + } + + /** + * Transform an image using ImageMagick + * + * This is a stub method. The real method is in BitmapHander. + * + * @param File $image File associated with this thumbnail + * @param array $params Array with scaler params + * + * @return MediaTransformError Error object if error occurred, false (=no error) otherwise + */ + protected function transformImageMagick( $image, $params ) { + return $this->getMediaTransformError( $params, "Unimplemented" ); + } + + /** + * Transform an image using the Imagick PHP extension + * + * This is a stub method. The real method is in BitmapHander. + * + * @param File $image File associated with this thumbnail + * @param array $params Array with scaler params + * + * @return MediaTransformError Error object if error occurred, false (=no error) otherwise + */ + protected function transformImageMagickExt( $image, $params ) { + return $this->getMediaTransformError( $params, "Unimplemented" ); + } + + /** + * Transform an image using a custom command + * + * This is a stub method. The real method is in BitmapHander. + * + * @param File $image File associated with this thumbnail + * @param array $params Array with scaler params + * + * @return MediaTransformError Error object if error occurred, false (=no error) otherwise + */ + protected function transformCustom( $image, $params ) { + return $this->getMediaTransformError( $params, "Unimplemented" ); + } + + /** + * Get a MediaTransformError with error 'thumbnail_error' + * + * @param array $params Parameter array as passed to the transform* functions + * @param string $errMsg Error message + * @return MediaTransformError + */ + public function getMediaTransformError( $params, $errMsg ) { + return new MediaTransformError( 'thumbnail_error', $params['clientWidth'], + $params['clientHeight'], $errMsg ); + } + + /** + * Transform an image using the built in GD library + * + * This is a stub method. The real method is in BitmapHander. + * + * @param File $image File associated with this thumbnail + * @param array $params Array with scaler params + * + * @return MediaTransformError Error object if error occurred, false (=no error) otherwise + */ + protected function transformGd( $image, $params ) { + return $this->getMediaTransformError( $params, "Unimplemented" ); + } + + /** + * Escape a string for ImageMagick's property input (e.g. -set -comment) + * See InterpretImageProperties() in magick/property.c + * @param string $s + * @return string + */ + function escapeMagickProperty( $s ) { + // Double the backslashes + $s = str_replace( '\\', '\\\\', $s ); + // Double the percents + $s = str_replace( '%', '%%', $s ); + // Escape initial - or @ + if ( strlen( $s ) > 0 && ( $s[0] === '-' || $s[0] === '@' ) ) { + $s = '\\' . $s; + } + + return $s; + } + + /** + * Escape a string for ImageMagick's input filenames. See ExpandFilenames() + * and GetPathComponent() in magick/utility.c. + * + * This won't work with an initial ~ or @, so input files should be prefixed + * with the directory name. + * + * Glob character unescaping is broken in ImageMagick before 6.6.1-5, but + * it's broken in a way that doesn't involve trying to convert every file + * in a directory, so we're better off escaping and waiting for the bugfix + * to filter down to users. + * + * @param string $path The file path + * @param bool|string $scene The scene specification, or false if there is none + * @throws MWException + * @return string + */ + function escapeMagickInput( $path, $scene = false ) { + # Die on initial metacharacters (caller should prepend path) + $firstChar = substr( $path, 0, 1 ); + if ( $firstChar === '~' || $firstChar === '@' ) { + throw new MWException( __METHOD__ . ': cannot escape this path name' ); + } + + # Escape glob chars + $path = preg_replace( '/[*?\[\]{}]/', '\\\\\0', $path ); + + return $this->escapeMagickPath( $path, $scene ); + } + + /** + * Escape a string for ImageMagick's output filename. See + * InterpretImageFilename() in magick/image.c. + * @param string $path The file path + * @param bool|string $scene The scene specification, or false if there is none + * @return string + */ + function escapeMagickOutput( $path, $scene = false ) { + $path = str_replace( '%', '%%', $path ); + + return $this->escapeMagickPath( $path, $scene ); + } + + /** + * Armour a string against ImageMagick's GetPathComponent(). This is a + * helper function for escapeMagickInput() and escapeMagickOutput(). + * + * @param string $path The file path + * @param bool|string $scene The scene specification, or false if there is none + * @throws MWException + * @return string + */ + protected function escapeMagickPath( $path, $scene = false ) { + # Die on format specifiers (other than drive letters). The regex is + # meant to match all the formats you get from "convert -list format" + if ( preg_match( '/^([a-zA-Z0-9-]+):/', $path, $m ) ) { + if ( wfIsWindows() && is_dir( $m[0] ) ) { + // OK, it's a drive letter + // ImageMagick has a similar exception, see IsMagickConflict() + } else { + throw new MWException( __METHOD__ . ': unexpected colon character in path name' ); + } + } + + # If there are square brackets, add a do-nothing scene specification + # to force a literal interpretation + if ( $scene === false ) { + if ( strpos( $path, '[' ) !== false ) { + $path .= '[0--1]'; + } + } else { + $path .= "[$scene]"; + } + + return $path; + } + + /** + * Retrieve the version of the installed ImageMagick + * You can use PHPs version_compare() to use this value + * Value is cached for one hour. + * @return string Representing the IM version. + */ + protected function getMagickVersion() { + global $wgMemc; + + $cache = $wgMemc->get( "imagemagick-version" ); + if ( !$cache ) { + global $wgImageMagickConvertCommand; + $cmd = wfEscapeShellArg( $wgImageMagickConvertCommand ) . ' -version'; + wfDebug( __METHOD__ . ": Running convert -version\n" ); + $retval = ''; + $return = wfShellExec( $cmd, $retval ); + $x = preg_match( '/Version: ImageMagick ([0-9]*\.[0-9]*\.[0-9]*)/', $return, $matches ); + if ( $x != 1 ) { + wfDebug( __METHOD__ . ": ImageMagick version check failed\n" ); + + return null; + } + $wgMemc->set( "imagemagick-version", $matches[1], 3600 ); + + return $matches[1]; + } + + return $cache; + } + + /** + * Returns whether the current scaler supports rotation. + * + * @since 1.24 No longer static + * @return bool + */ + public function canRotate() { + return false; + } + + /** + * Should we automatically rotate an image based on exif + * + * @since 1.24 No longer static + * @see $wgEnableAutoRotation + * @return bool Whether auto rotation is enabled + */ + public function autoRotateEnabled() { + return false; + } + + /** + * Rotate a thumbnail. + * + * This is a stub. See BitmapHandler::rotate. + * + * @param File $file + * @param array $params Rotate parameters. + * 'rotation' clockwise rotation in degrees, allowed are multiples of 90 + * @since 1.24 Is non-static. From 1.21 it was static + * @return bool + */ + public function rotate( $file, $params ) { + return new MediaTransformError( 'thumbnail_error', 0, 0, + "$scaler rotation not implemented" ); + } + + /** + * Returns whether the file needs to be rendered. Returns true if the + * file requires rotation and we are able to rotate it. + * + * @param File $file + * @return bool + */ + public function mustRender( $file ) { + return $this->canRotate() && $this->getRotation( $file ) != 0; + } +} diff --git a/includes/media/XCF.php b/includes/media/XCF.php index aa77fa819f..48b7a47c40 100644 --- a/includes/media/XCF.php +++ b/includes/media/XCF.php @@ -209,7 +209,7 @@ class XCFHandler extends BitmapHandler { * @param bool $checkDstPath * @return string */ - protected static function getScalerType( $dstPath, $checkDstPath = true ) { + protected function getScalerType( $dstPath, $checkDstPath = true ) { return "im"; } diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php index e455ef11ef..78fe8e0186 100644 --- a/includes/resourceloader/ResourceLoaderStartUpModule.php +++ b/includes/resourceloader/ResourceLoaderStartUpModule.php @@ -94,7 +94,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { 'wgDBname' => $conf->get( 'DBname' ), // This sucks, it is only needed on Special:Upload, but I could // not find a way to add vars only for a certain module - 'wgFileCanRotate' => BitmapHandler::canRotate(), + 'wgFileCanRotate' => SpecialUpload::rotationEnabled(), 'wgAvailableSkins' => Skin::getSkinNames(), 'wgExtensionAssetsPath' => $conf->get( 'ExtensionAssetsPath' ), // MediaWiki sets cookies to have this prefix by default diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php index 13a00ed35b..55d09dd6d0 100644 --- a/includes/specials/SpecialUpload.php +++ b/includes/specials/SpecialUpload.php @@ -738,6 +738,18 @@ class SpecialUpload extends SpecialPage { protected function getGroupName() { return 'media'; } + + /** + * Should we rotate images in the preview on Special:Upload. + * + * This controls js: mw.config.get( 'wgFileCanRotate' ) + * + * @todo What about non-BitmapHandler handled files? + */ + static public function rotationEnabled() { + $bitmapHandler = new BitmapHandler(); + return $bitmapHandler->autoRotateEnabled(); + } } /** diff --git a/tests/phpunit/includes/media/ExifRotationTest.php b/tests/phpunit/includes/media/ExifRotationTest.php index 247e352c65..f0bd42a078 100644 --- a/tests/phpunit/includes/media/ExifRotationTest.php +++ b/tests/phpunit/includes/media/ExifRotationTest.php @@ -32,7 +32,7 @@ class ExifRotationTest extends MediaWikiMediaTestCase { * @dataProvider provideFiles */ public function testMetadata( $name, $type, $info ) { - if ( !BitmapHandler::canRotate() ) { + if ( !$this->handler->canRotate() ) { $this->markTestSkipped( "This test needs a rasterizer that can auto-rotate." ); } $file = $this->dataFile( $name, $type ); @@ -40,12 +40,29 @@ class ExifRotationTest extends MediaWikiMediaTestCase { $this->assertEquals( $info['height'], $file->getHeight(), "$name: height check" ); } + /** + * Same as before, but with auto-rotation set to auto. + * + * This sets scaler to image magick, which we should detect as + * supporting rotation. + * @dataProvider provideFiles + */ + public function testMetadataAutoRotate( $name, $type, $info ) { + $this->setMwGlobals( 'wgEnableAutoRotation', null ); + $this->setMwGlobals( 'wgUseImageMagick', true ); + $this->setMwGlobals( 'wgUseImageResize', true ); + + $file = $this->dataFile( $name, $type ); + $this->assertEquals( $info['width'], $file->getWidth(), "$name: width check" ); + $this->assertEquals( $info['height'], $file->getHeight(), "$name: height check" ); + } + /** * * @dataProvider provideFiles */ public function testRotationRendering( $name, $type, $info, $thumbs ) { - if ( !BitmapHandler::canRotate() ) { + if ( !$this->handler->canRotate() ) { $this->markTestSkipped( "This test needs a rasterizer that can auto-rotate." ); } foreach ( $thumbs as $size => $out ) { @@ -133,6 +150,19 @@ class ExifRotationTest extends MediaWikiMediaTestCase { $this->assertEquals( $info['height'], $file->getHeight(), "$name: height check" ); } + /** + * Same as before, but with auto-rotation set to auto and an image scaler that doesn't support it. + * @dataProvider provideFilesNoAutoRotate + */ + public function testMetadataAutoRotateUnsupported( $name, $type, $info ) { + $this->setMwGlobals( 'wgEnableAutoRotation', null ); + $this->setMwGlobals( 'wgUseImageResize', false ); + + $file = $this->dataFile( $name, $type ); + $this->assertEquals( $info['width'], $file->getWidth(), "$name: width check" ); + $this->assertEquals( $info['height'], $file->getHeight(), "$name: height check" ); + } + /** * * @dataProvider provideFilesNoAutoRotate