r97675 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97674‎ | r97675 | r97676 >
Date:22:40, 20 September 2011
Author:brion
Status:ok
Tags:
Comment:
MFT r97651, r97656, r97659 updated test cases for bug 6672, 31024
MFT r97671 fix for bug 31024
MFT r97672 fix for bug 31048
these fixes resolve remaining issues on bug 6672.
Modified paths:
  • /branches/REL1_18/phase3/includes/media/Bitmap.php (modified) (history)
  • /branches/REL1_18/phase3/includes/media/ExifBitmap.php (modified) (history)
  • /branches/REL1_18/phase3/resources/mediawiki.special/mediawiki.special.upload.js (modified) (history)
  • /branches/REL1_18/phase3/tests/phpunit/includes/media/ExifRotationTest.php (modified) (history)

Diff [purge]

Index: branches/REL1_18/phase3/tests/phpunit/includes/media/ExifRotationTest.php
@@ -9,41 +9,74 @@
1010 parent::setUp();
1111 $this->filePath = dirname( __FILE__ ) . '/../../data/media/';
1212 $this->handler = new BitmapHandler();
 13+ $this->repo = new FSRepo(array(
 14+ 'name' => 'temp',
 15+ 'directory' => wfTempDir() . '/exif-test-' . time(),
 16+ 'url' => 'http://localhost/thumbtest'
 17+ ));
 18+ if ( !wfDl( 'exif' ) ) {
 19+ $this->markTestSkipped( "This test needs the exif extension." );
 20+ }
 21+ global $wgShowEXIF;
 22+ $this->show = $wgShowEXIF;
 23+ $wgShowEXIF = true;
1324 }
 25+ public function tearDown() {
 26+ global $wgShowEXIF;
 27+ $wgShowEXIF = $this->show;
 28+ }
1429
1530 /**
1631 *
1732 * @dataProvider providerFiles
1833 */
1934 function testMetadata( $name, $type, $info ) {
20 - # Force client side resizing
21 - $params = array( 'width' => 10000, 'height' => 10000 );
2235 $file = UnregisteredLocalFile::newFromPath( $this->filePath . $name, $type );
23 -
24 - # Normalize parameters
25 - $this->assertTrue( $this->handler->normaliseParams( $file, $params ) );
26 - $rotation = $this->handler->getRotation( $file );
27 -
28 - # Check if pre-rotation dimensions are still good
29 - list( $width, $height ) = $this->handler->extractPreRotationDimensions( $params, $rotation );
30 - $this->assertEquals( $file->getWidth(), $width,
31 - "$name: pre-rotation width check, $rotation:$width" );
32 - $this->assertEquals( $file->getHeight(), $height,
33 - "$name: pre-rotation height check, $rotation" );
34 -
35 - # Any further test require a scaler that can rotate
36 - if ( !BitmapHandler::canRotate() ) {
37 - $this->markTestIncomplete( 'Scaler does not support rotation' );
38 - return;
 36+ $this->assertEquals( $info['width'], $file->getWidth(), "$name: width check" );
 37+ $this->assertEquals( $info['height'], $file->getHeight(), "$name: height check" );
 38+ }
 39+
 40+ /**
 41+ *
 42+ * @dataProvider providerFiles
 43+ */
 44+ function testRotationRendering( $name, $type, $info, $thumbs ) {
 45+ foreach( $thumbs as $size => $out ) {
 46+ if( preg_match('/^(\d+)px$/', $size, $matches ) ) {
 47+ $params = array(
 48+ 'width' => $matches[1],
 49+ );
 50+ } elseif ( preg_match( '/^(\d+)x(\d+)px$/', $size, $matches ) ) {
 51+ $params = array(
 52+ 'width' => $matches[1],
 53+ 'height' => $matches[2]
 54+ );
 55+ } else {
 56+ throw new MWException('bogus test data format ' . $size);
 57+ }
 58+
 59+ $file = $this->localFile( $name, $type );
 60+ $thumb = $file->transform( $params, File::RENDER_NOW );
 61+
 62+ $this->assertEquals( $out[0], $thumb->getWidth(), "$name: thumb reported width check for $size" );
 63+ $this->assertEquals( $out[1], $thumb->getHeight(), "$name: thumb reported height check for $size" );
 64+
 65+ $gis = getimagesize( $thumb->getPath() );
 66+ if ($out[0] > $info['width']) {
 67+ // Physical image won't be scaled bigger than the original.
 68+ $this->assertEquals( $info['width'], $gis[0], "$name: thumb actual width check for $size");
 69+ $this->assertEquals( $info['height'], $gis[1], "$name: thumb actual height check for $size");
 70+ } else {
 71+ $this->assertEquals( $out[0], $gis[0], "$name: thumb actual width check for $size");
 72+ $this->assertEquals( $out[1], $gis[1], "$name: thumb actual height check for $size");
 73+ }
3974 }
40 -
41 - # Check post-rotation width
42 - $this->assertEquals( $params['physicalWidth'], $info['width'],
43 - "$name: post-rotation width check" );
44 - $this->assertEquals( $params['physicalHeight'], $info['height'],
45 - "$name: post-rotation height check" );
4675 }
4776
 77+ private function localFile( $name, $type ) {
 78+ return new UnregisteredLocalFile( false, $this->repo, $this->filePath . $name, $type );
 79+ }
 80+
4881 function providerFiles() {
4982 return array(
5083 array(
@@ -52,6 +85,12 @@
5386 array(
5487 'width' => 1024,
5588 'height' => 768,
 89+ ),
 90+ array(
 91+ '800x600px' => array( 800, 600 ),
 92+ '9999x800px' => array( 1067, 800 ),
 93+ '800px' => array( 800, 600 ),
 94+ '600px' => array( 600, 450 ),
5695 )
5796 ),
5897 array(
@@ -60,6 +99,12 @@
61100 array(
62101 'width' => 768, // as rotated
63102 'height' => 1024, // as rotated
 103+ ),
 104+ array(
 105+ '800x600px' => array( 450, 600 ),
 106+ '9999x800px' => array( 600, 800 ),
 107+ '800px' => array( 800, 1067 ),
 108+ '600px' => array( 600, 800 ),
64109 )
65110 )
66111 );
@@ -100,22 +145,5 @@
101146 ),
102147 );
103148 }
104 -
105 - function testWidthFlipping() {
106 - $file = UnregisteredLocalFile::newFromPath( $this->filePath . 'portrait-rotated.jpg', 'image/jpeg' );
107 - $params = array( 'width' => '50' );
108 - $this->assertTrue( $this->handler->normaliseParams( $file, $params ) );
109 -
110 - $this->assertEquals( 50, $params['height'] );
111 - $this->assertEquals( round( (768/1024)*50 ), $params['width'], '', 0.1 );
112 - }
113 - function testWidthNotFlipping() {
114 - $file = UnregisteredLocalFile::newFromPath( $this->filePath . 'landscape-plain.jpg', 'image/jpeg' );
115 - $params = array( 'width' => '50' );
116 - $this->assertTrue( $this->handler->normaliseParams( $file, $params ) );
117 -
118 - $this->assertEquals( 50, $params['width'] );
119 - $this->assertEquals( round( (768/1024)*50 ), $params['height'], '', 0.1 );
120 - }
121149 }
122150
Index: branches/REL1_18/phase3/includes/media/Bitmap.php
@@ -42,20 +42,6 @@
4343 return true;
4444 }
4545 }
46 -
47 -
48 - if ( self::canRotate() ) {
49 - $rotation = $this->getRotation( $image );
50 - if ( $rotation == 90 || $rotation == 270 ) {
51 - wfDebug( __METHOD__ . ": Swapping width and height because the file will be rotated $rotation degrees\n" );
52 -
53 - list( $params['width'], $params['height'] ) =
54 - array( $params['height'], $params['width'] );
55 - list( $params['physicalWidth'], $params['physicalHeight'] ) =
56 - array( $params['physicalHeight'], $params['physicalWidth'] );
57 - }
58 - }
59 -
6046
6147 # Don't thumbnail an image so big that it will fill hard drives and send servers into swap
6248 # JPEG has the handy property of allowing thumbnailing without full decompression, so we make
@@ -73,6 +59,11 @@
7460 /**
7561 * Extracts the width/height if the image will be scaled before rotating
7662 *
 63+ * This will match the physical size/aspect ratio of the original image
 64+ * prior to application of the rotation -- so for a portrait image that's
 65+ * stored as raw landscape with 90-degress rotation, the resulting size
 66+ * will be wider than it is tall.
 67+ *
7768 * @param $params array Parameters as returned by normaliseParams
7869 * @param $rotation int The rotation angle that will be applied
7970 * @return array ($width, $height) array
@@ -249,6 +240,8 @@
250241 * @param $image File File associated with this thumbnail
251242 * @param $params array Array with scaler params
252243 * @return ThumbnailImage
 244+ *
 245+ * @fixme no rotation support
253246 */
254247 protected function getClientScalingThumbnailImage( $image, $params ) {
255248 return new ThumbnailImage( $image, $image->getURL(),
@@ -685,33 +678,21 @@
686679 }
687680
688681 /**
689 - * Try to read out the orientation of the file and return the angle that
690 - * the file needs to be rotated to be viewed
 682+ * On supporting image formats, try to read out the low-level orientation
 683+ * of the file and return the angle that the file needs to be rotated to
 684+ * be viewed.
691685 *
 686+ * This information is only useful when manipulating the original file;
 687+ * the width and height we normally work with is logical, and will match
 688+ * any produced output views.
 689+ *
 690+ * The base BitmapHandler doesn't understand any metadata formats, so this
 691+ * is left up to child classes to implement.
 692+ *
692693 * @param $file File
693694 * @return int 0, 90, 180 or 270
694695 */
695696 public function getRotation( $file ) {
696 - $data = $file->getMetadata();
697 - if ( !$data ) {
698 - return 0;
699 - }
700 - wfSuppressWarnings();
701 - $data = unserialize( $data );
702 - wfRestoreWarnings();
703 - if ( isset( $data['Orientation'] ) ) {
704 - # See http://sylvana.net/jpegcrop/exif_orientation.html
705 - switch ( $data['Orientation'] ) {
706 - case 8:
707 - return 90;
708 - case 3:
709 - return 180;
710 - case 6:
711 - return 270;
712 - default:
713 - return 0;
714 - }
715 - }
716697 return 0;
717698 }
718699
Index: branches/REL1_18/phase3/includes/media/ExifBitmap.php
@@ -124,5 +124,77 @@
125125 function getMetadataType( $image ) {
126126 return 'exif';
127127 }
 128+
 129+ /**
 130+ * Wrapper for base classes ImageHandler::getImageSize() that checks for
 131+ * rotation reported from metadata and swaps the sizes to match.
 132+ *
 133+ * @param File $image
 134+ * @param string $path
 135+ * @return array
 136+ */
 137+ function getImageSize( $image, $path ) {
 138+ $gis = parent::getImageSize( $image, $path );
 139+
 140+ // Don't just call $image->getMetadata(); File::getPropsFromPath() calls us with a bogus object.
 141+ // This may mean we read EXIF data twice on initial upload.
 142+ $meta = $this->getMetadata( $image, $path );
 143+ $rotation = $this->getRotationForExif( $meta );
 144+
 145+ if ($rotation == 90 || $rotation == 270) {
 146+ $width = $gis[0];
 147+ $gis[0] = $gis[1];
 148+ $gis[1] = $width;
 149+ }
 150+ return $gis;
 151+ }
 152+
 153+ /**
 154+ * On supporting image formats, try to read out the low-level orientation
 155+ * of the file and return the angle that the file needs to be rotated to
 156+ * be viewed.
 157+ *
 158+ * This information is only useful when manipulating the original file;
 159+ * the width and height we normally work with is logical, and will match
 160+ * any produced output views.
 161+ *
 162+ * @param $file File
 163+ * @return int 0, 90, 180 or 270
 164+ */
 165+ public function getRotation( $file ) {
 166+ $data = $file->getMetadata();
 167+ return $this->getRotationForExif( $data );
 168+ }
 169+
 170+ /**
 171+ * Given a chunk of serialized Exif metadata, return the orientation as
 172+ * degrees of rotation.
 173+ *
 174+ * @param string $data
 175+ * @return int 0, 90, 180 or 270
 176+ * @fixme orientation can include flipping as well; see if this is an issue!
 177+ */
 178+ protected function getRotationForExif( $data ) {
 179+ if ( !$data ) {
 180+ return 0;
 181+ }
 182+ wfSuppressWarnings();
 183+ $data = unserialize( $data );
 184+ wfRestoreWarnings();
 185+ if ( isset( $data['Orientation'] ) ) {
 186+ # See http://sylvana.net/jpegcrop/exif_orientation.html
 187+ switch ( $data['Orientation'] ) {
 188+ case 8:
 189+ return 90;
 190+ case 3:
 191+ return 180;
 192+ case 6:
 193+ return 270;
 194+ default:
 195+ return 0;
 196+ }
 197+ }
 198+ return 0;
 199+ }
128200 }
129201
Index: branches/REL1_18/phase3/resources/mediawiki.special/mediawiki.special.upload.js
@@ -83,7 +83,7 @@
8484 }
8585
8686 img.onload = function() {
87 - var width, height, x, y, dx, dy;
 87+ var width, height, x, y, dx, dy, logicalWidth, logicalHeight;
8888 // Fit the image within the previewSizexpreviewSize box
8989 if ( img.width > img.height ) {
9090 width = previewSize;
@@ -103,19 +103,27 @@
104104 case 0:
105105 x = dx;
106106 y = dy;
 107+ logicalWidth = img.width;
 108+ logicalHeight = img.height;
107109 break;
108110 case 90:
109111
110112 x = dx;
111113 y = dy - previewSize;
 114+ logicalWidth = img.height;
 115+ logicalHeight = img.width;
112116 break;
113117 case 180:
114118 x = dx - previewSize;
115119 y = dy - previewSize;
 120+ logicalWidth = img.width;
 121+ logicalHeight = img.height;
116122 break;
117123 case 270:
118124 x = dx - previewSize;
119125 y = dy;
 126+ logicalWidth = img.height;
 127+ logicalHeight = img.width;
120128 break;
121129 }
122130
@@ -124,7 +132,7 @@
125133 ctx.drawImage( img, x, y, width, height );
126134
127135 // Image size
128 - var info = mw.msg( 'widthheight', img.width, img.height ) +
 136+ var info = mw.msg( 'widthheight', logicalWidth, logicalHeight ) +
129137 ', ' + prettySize( file.size );
130138 $( '#mw-upload-thumbnail .fileinfo' ).text( info );
131139 };

Follow-up revisions

RevisionCommit summaryAuthorDate
r97676Merge from REL1_18 r97675 -- exif rotation fixesbrion22:41, 20 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r79859Follow-up r79845: add function documentation; only call wfMakeDirsParent if w...btongminh11:49, 8 January 2011
r90016Start on test cases for bug 6672 (Exif orientation support), follow up to r79......brion22:13, 13 June 2011
r90123add canvas scaling and rotation. mostly fixes bug 29383 (a few issues remaini...neilk17:35, 15 June 2011
r97651Partial revert of broken test changes from r92246 -- for some reason it was t...brion19:13, 20 September 2011
r97656Further tweaks to r96687, r90016, r97398 etc tests: actually produce a thumbn...brion19:39, 20 September 2011
r97659Further tweaks to r96687, r90016, r97398, r97656 etc tests: try several thumb...brion20:04, 20 September 2011
r97671* (bug 6672, 31024) Fixes for handling of images with an EXIF orientation...brion22:13, 20 September 2011
r97672* (bug 31048) Fix for width/height reported on Special:Upload thumbnail for E...brion22:25, 20 September 2011

Status & tagging log