Revert image crop for now:
authorBrion Vibber <brion@users.mediawiki.org>
Wed, 19 Aug 2009 02:07:00 +0000 (02:07 +0000)
committerBrion Vibber <brion@users.mediawiki.org>
Wed, 19 Aug 2009 02:07:00 +0000 (02:07 +0000)
r54746 "Update formatting for r54745"
r54745 "Added crop support for inline images."
r54748 "Making demon happy (adding public/protected to function definitions) and add some comments along the way."

Several issues:
* Implementation is ImageMagick-specific, no support for GD backend
* Specification syntax is pretty ugly and non-obvious imo... [[File:foo.jpg|<width>px|<left>x<top>x<width>x<height]]
* Crop syntax seems to be tied to pixels... I _presume_ source pixels? This would break if a file is re-uploaded with a new size.
* In general I'm very leery of tacking on more infinite-options-in-infinte-combinations for image rendering; our treatment of resizing already has implications for CPU and disk usage and this just adds another level of arbitrary-rendered horror. ;)

I'd much rather we move towards limiting the number of rendered variants we have, not increase them... IMO cropping would be better served with an interface allowing for explicit, visible cropping which creates either a new saved version, or a 'cloned' virtual file which can be referred to by name... and more importantly, uses of it would be trackable so that if crops needs to be updated they can be cleanly.

RELEASE-NOTES
includes/media/Bitmap.php
languages/messages/MessagesEn.php

index d777907..a37a9d2 100644 (file)
@@ -195,7 +195,6 @@ this. Was used when mwEmbed was going to be an extension.
 ** Unnecessary type="" attribute removed for CSS and JS.
 ** If $wgWellFormedXml is set to false, some bytes will be shaved off of HTML
    output by omitting some things like quotation marks where HTML 5 allows.
-* Added crop for inline images.
 * The description message in $wgExtensionCredits can be an array with parameters
 * New hook SpecialRandomGetRandomTitle allows extensions to modify the selection
   criteria used by Special:Random and subclasses, or substitute a custom result,
index e573e23..d4a7c35 100644 (file)
@@ -8,75 +8,8 @@
  * @ingroup Media
  */
 class BitmapHandler extends ImageHandler {
-       /**
-        * Override parent method to add crop to param map.
-        */
-       public function getParamMap() {
-               return array(
-                       'img_width'     => 'width',
-                       'img_crop' => 'crop',
-               );
-       }
-       
-       /**
-        * Override param map to validate the crop param.
-        */
-       public function validateParam( $name, $value ) {
-               if ( $name == 'crop' ) {
-                       return $this->splitCropParam( $value ) !== false;
-               } else {
-                       return parent::validateParam( $name, $value );
-               }               
-       }
-       /**
-        * Split the crop param into up to 4 parts and convert them to integers.
-        * Returns false in case of malformed param.
-        */
-       protected function splitCropParam( $value ) {
-               $parts = explode( 'x', $value );
-               if ( count( $parts ) > 4 )
-                       return false;
-               foreach ( $parts as &$part ) {
-                       $intVal = intval( $part );
-                       if ( $intVal === 0 && !( $part === '0' || $part === '' ) )
-                               return false;
-                       if ( $intVal < 0 )
-                               return false;
-                       
-                       $part = $intVal;
-               }
-               
-               return $parts;
-       }
-       
-       /**
-        * Override parent method to check for optional crop parameter in param 
-        * string.
-        */
-       public function parseParamString( $str ) {
-               $res = parent::parseParamString( $str );
-               if ( $res === false ) {
-                       $m = false;
-                       if ( preg_match( '/^(\d+)px-([x0-9])crop$/', $str, $m ) ) {
-                               return array( 'width' => $m[1], 'crop' => $m[2] );
-                       } else {
-                               return false;
-                       }
-               }
-       }
-       /**
-        * Add the crop parameter the string generated by the parent.
-        */
-       public function makeParamString( $params ) { 
-               $res = parent::makeParamString( $params );
-               if ( !empty( $params['crop'] ) )
-                       $res .= '-'.implode( 'x', $params['crop'] ).'crop';
-               return $res;
-       }
-       
-       public function normaliseParams( $image, &$params ) {
+       function normaliseParams( $image, &$params ) {
                global $wgMaxImageArea;
-               # Parent fills in width, height and page and normalizes them.
                if ( !parent::normaliseParams( $image, $params ) ) {
                        return false;
                }
@@ -103,58 +36,18 @@ class BitmapHandler extends ImageHandler {
                        $params['physicalHeight'] = $srcHeight;
                        return true;
                }
-               
-               # Validate crop params
-               if ( isset( $params['crop'] ) ) {
-                       # $cropParams = array( x, y, width, height );
-                       $cropParams = $this->splitCropParam( $params['crop'] );
-                       # Fill up params
-                       switch ( count( $cropParams ) ) {
-                               # All fall through
-                               case 1:
-                                       $cropParams[1] = 0;
-                               case 2:
-                                       $cropParams[2] = $srcWidth - $cropParams[0];
-                               case 3:
-                                       $cropParams[3] = $srcHeight - $cropParams[1];
-                       }
-                       $cx = $cropParams[0] + $cropParams[2];
-                       $cy = $cropParams[1] + $cropParams[3];
-                       $targetWidth = $cropParams[2];
-                       $targetHeight = $cropParams[3];
-                       # Can't go outside image
-                       if ( $cx > $srcWidth || $cy > $srcHeight ) {
-                               # TODO: Maybe should fail gracefully?
-                               return false; 
-                       }
-                       if ( $targetWidth == $srcWidth && $targetHeight == $srcHeight )
-                       {
-                               # No need to crop
-                               $params['crop'] = false;
-                       }
-                       else
-                       {
-                               $params['crop'] = $cropParams;
-                               $params['height'] = $params['physicalHeight'] = File::scaleHeight( 
-                                               $targetWidth, $targetHeight, $params['width'] );
-                       }
-               } else {
-                       $params['crop'] = false;
-               }
 
                return true;
        }
        
        
-       /**
-        * Function that returns the number of pixels to be thumbnailed.
-        * Intended for animated GIFs to multiply by the number of frames.
-        */
-       protected function getImageArea( $image, $width, $height ) {
+       // Function that returns the number of pixels to be thumbnailed.
+       // Intended for animated GIFs to multiply by the number of frames.
+       function getImageArea( $image, $width, $height ) {
                return $width * $height;
        }
 
-       public function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) {
+       function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) {
                global $wgUseImageMagick, $wgImageMagickConvertCommand, $wgImageMagickTempDir;
                global $wgCustomConvertCommand, $wgUseImageResize;
                global $wgSharpenParameter, $wgSharpenReductionThreshold;
@@ -243,13 +136,6 @@ class BitmapHandler extends ImageHandler {
                        } else {
                                $tempEnv = '';
                        }
-                       
-                       if ( $params['crop'] ) {
-                               $crop = $params['crop'];
-                               $cropCmd = "-crop {$crop[2]}x{$crop[3]}+{$crop[0]}+{$crop[1]}";
-                       }
-                       else
-                               $cropCmd = '';
 
                        # Specify white background color, will be used for transparent images
                        # in Internet Explorer/Windows instead of default black.
@@ -261,7 +147,7 @@ class BitmapHandler extends ImageHandler {
                        $cmd  = 
                                $tempEnv .
                                wfEscapeShellArg($wgImageMagickConvertCommand) .
-                               " {$cropCmd} {$quality} -background white -size {$physicalWidth} ".
+                               " {$quality} -background white -size {$physicalWidth} ".
                                wfEscapeShellArg($srcPath . $frame) .
                                $animation .
                                // For the -resize option a "!" is needed to force exact size,
@@ -361,13 +247,13 @@ class BitmapHandler extends ImageHandler {
                }
        }
 
-       public static function imageJpegWrapper( $dst_image, $thumbPath ) {
+       static function imageJpegWrapper( $dst_image, $thumbPath ) {
                imageinterlace( $dst_image );
                imagejpeg( $dst_image, $thumbPath, 95 );
        }
 
 
-       public function getMetadata( $image, $filename ) {
+       function getMetadata( $image, $filename ) {
                global $wgShowEXIF;
                if( $wgShowEXIF && file_exists( $filename ) ) {
                        $exif = new Exif( $filename );
@@ -383,11 +269,11 @@ class BitmapHandler extends ImageHandler {
                }
        }
 
-       public function getMetadataType( $image ) {
+       function getMetadataType( $image ) {
                return 'exif';
        }
 
-       public function isMetadataValid( $image, $metadata ) {
+       function isMetadataValid( $image, $metadata ) {
                global $wgShowEXIF;
                if ( !$wgShowEXIF ) {
                        # Metadata disabled and so an empty field is expected
@@ -415,7 +301,7 @@ class BitmapHandler extends ImageHandler {
         * @return array of strings
         * @access private
         */
-       public function visibleMetadataFields() {
+       function visibleMetadataFields() {
                $fields = array();
                $lines = explode( "\n", wfMsgForContent( 'metadata-fields' ) );
                foreach( $lines as $line ) {
@@ -428,7 +314,7 @@ class BitmapHandler extends ImageHandler {
                return $fields;
        }
 
-       public function formatMetadata( $image ) {
+       function formatMetadata( $image ) {
                $result = array(
                        'visible' => array(),
                        'collapsed' => array()
index 89e7960..9c6521d 100644 (file)
@@ -288,7 +288,6 @@ $magicWords = array(
        'img_text_bottom'        => array( 1,    'text-bottom'            ),
        'img_link'               => array( 1,    'link=$1'                ),
        'img_alt'                => array( 1,    'alt=$1'                 ),
-       'img_crop'               => array( 1,    'crop=$1'                ),
        'int'                    => array( 0,    'INT:'                   ),
        'sitename'               => array( 1,    'SITENAME'               ),
        'ns'                     => array( 0,    'NS:'                    ),