r90508 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r90507‎ | r90508 | r90509 >
Date:03:40, 21 June 2011
Author:neilk
Status:ok (Comments)
Tags:brion 
Comment:
create images with orientation (had to use exiv2 hack, imagemagick not helpful)
Modified paths:
  • /trunk/phase3/tests/phpunit/includes/api/RandomImageGenerator.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/api/generateRandomImages.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/api/RandomImageGenerator.php
@@ -6,7 +6,7 @@
77 *
88 * Because MediaWiki tests the uniqueness of media upload content, and filenames, it is sometimes useful to generate
99 * files that are guaranteed (or at least very likely) to be unique in both those ways.
10 - * This generates a number of filenames with random names and random content (colored circles)
 10+ * This generates a number of filenames with random names and random content (colored triangles)
1111 *
1212 * It is also useful to have fresh content because our tests currently run in a "destructive" mode, and don't create a fresh new wiki for each
1313 * test run.
@@ -31,11 +31,45 @@
3232 private $maxWidth = 800;
3333 private $minHeight = 400;
3434 private $maxHeight = 800;
35 - private $circlesToDraw = 5;
 35+ private $shapesToDraw = 5;
3636 private $imageWriteMethod;
3737
 38+ /**
 39+ * Orientations: 0th row, 0th column, EXIF orientation code, rotation 2x2 matrix that is opposite of orientation
 40+ * n.b. we do not handle the 'flipped' orientations, which is why there is no entry for 2, 4, 5, or 7. Those
 41+ * seem to be rare in real images anyway
 42+ * (we also would need a non-symmetric shape for the images to test those, like a letter F)
 43+ */
 44+ private static $orientations = array(
 45+ array(
 46+ '0thRow' => 'top',
 47+ '0thCol' => 'left',
 48+ 'exifCode' => 1,
 49+ 'counterRotation' => array( array( 1, 0 ), array( 0, 1 ) )
 50+ ),
 51+ array(
 52+ '0thRow' => 'bottom',
 53+ '0thCol' => 'right',
 54+ 'exifCode' => 3,
 55+ 'counterRotation' => array( array( -1, 0 ), array( 0, -1 ) )
 56+ ),
 57+ array(
 58+ '0thRow' => 'right',
 59+ '0thCol' => 'top',
 60+ 'exifCode' => 6,
 61+ 'counterRotation' => array( array( 0, 1 ), array( 1, 0 ) )
 62+ ),
 63+ array(
 64+ '0thRow' => 'left',
 65+ '0thCol' => 'bottom',
 66+ 'exifCode' => 8,
 67+ 'counterRotation' => array( array( 0, -1 ), array( -1, 0 ) )
 68+ )
 69+ );
 70+
 71+
3872 public function __construct( $options = array() ) {
39 - foreach ( array( 'dictionaryFile', 'minWidth', 'minHeight', 'maxHeight', 'circlesToDraw' ) as $property ) {
 73+ foreach ( array( 'dictionaryFile', 'minWidth', 'minHeight', 'maxHeight', 'shapesToDraw' ) as $property ) {
4074 if ( isset( $options[$property] ) ) {
4175 $this->$property = $options[$property];
4276 }
@@ -136,8 +170,11 @@
137171 $diagonalLength = sqrt( pow( $spec['width'], 2 ) + pow( $spec['height'], 2 ) );
138172
139173 $draws = array();
140 - for ( $i = 0; $i <= $this->circlesToDraw; $i++ ) {
 174+ for ( $i = 0; $i <= $this->shapesToDraw; $i++ ) {
141175 $radius = mt_rand( 0, $diagonalLength / 4 );
 176+ if ( $radius == 0 ) {
 177+ continue;
 178+ }
142179 $originX = mt_rand( -1 * $radius, $spec['width'] + $radius );
143180 $originY = mt_rand( -1 * $radius, $spec['height'] + $radius );
144181 $angle = mt_rand( 0, ( 3.141592/2 ) * $radius ) / $radius;
@@ -179,7 +216,7 @@
180217 /**
181218 * Based on image specification, write a very simple SVG file to disk.
182219 * Ignores the background spec because transparency is cool. :)
183 - * @param $spec: spec describing background and circles to draw
 220+ * @param $spec: spec describing background and shapes to draw
184221 * @param $format: file format to write (which is obviously always svg here)
185222 * @param $filename: filename to write to
186223 */
@@ -211,7 +248,20 @@
212249 * @param $filename: filename to write to
213250 */
214251 public function writeImageWithApi( $spec, $format, $filename ) {
 252+ // this is a hack because I can't get setImageOrientation() to work. See below.
 253+ global $wgExiv2Command;
 254+
215255 $image = new Imagick();
 256+ /**
 257+ * If the format is 'jpg', will also add a random orientation -- the image will be drawn rotated with triangle points
 258+ * facing in some direction (0, 90, 180 or 270 degrees) and a countering rotation should turn the triangle points upward again
 259+ */
 260+ $orientation = self::$orientations[0]; // default is normal orientation
 261+ if ( $format == 'jpg' ) {
 262+ $orientation = self::$orientations[ array_rand( self::$orientations ) ];
 263+ $spec = self::rotateImageSpec( $spec, $orientation['counterRotation'] );
 264+ }
 265+
216266 $image->newImage( $spec['width'], $spec['height'], new ImagickPixel( $spec['fill'] ) );
217267
218268 foreach ( $spec['draws'] as $drawSpec ) {
@@ -222,11 +272,85 @@
223273 }
224274
225275 $image->setImageFormat( $format );
 276+
 277+ // this doesn't work, even though it's documented to do so...
 278+ // $image->setImageOrientation( $orientation['exifCode'] );
 279+
226280 $image->writeImage( $filename );
 281+
 282+ // because the above setImageOrientation call doesn't work... nor can I get an external imagemagick binary to do this either...
 283+ // hacking this for now (only works if you have exiv2 installed, a program to read and manipulate exif)
 284+ if ( $wgExiv2Command ) {
 285+ $cmd = wfEscapeShellArg( $wgExiv2Command )
 286+ . " -M "
 287+ . wfEscapeShellArg( "set Exif.Image.Orientation " . $orientation['exifCode'] )
 288+ . " "
 289+ . wfEscapeShellArg( $filename );
 290+
 291+ $retval = 0;
 292+ $err = wfShellExec( $cmd, $retval );
 293+ if ( $retval !== 0 ) {
 294+ print "Error with $cmd: $retval, $err\n";
 295+ }
 296+ }
 297+
 298+
227299 }
228300
 301+ /**
 302+ * Given an image specification, produce rotated version
 303+ * This is used when simulating a rotated image capture with EXIF orientation
 304+ * @param $spec Object returned by getImageSpec
 305+ * @param $matrix 2x2 transformation matrix
 306+ * @return transformed Spec
 307+ */
 308+ private static function rotateImageSpec( &$spec, $matrix ) {
 309+ $tSpec = array();
 310+ $dims = self::matrixMultiply2x2( $matrix, $spec['width'], $spec['height'] );
 311+ $correctionX = 0;
 312+ $correctionY = 0;
 313+ if ( $dims['x'] < 0 ) {
 314+ $correctionX = abs( $dims['x'] );
 315+ }
 316+ if ( $dims['y'] < 0 ) {
 317+ $correctionY = abs( $dims['y'] );
 318+ }
 319+ $tSpec['width'] = abs( $dims['x'] );
 320+ $tSpec['height'] = abs( $dims['y'] );
 321+ $tSpec['fill'] = $spec['fill'];
 322+ $tSpec['draws'] = array();
 323+ foreach( $spec['draws'] as $draw ) {
 324+ $tDraw = array(
 325+ 'fill' => $draw['fill'],
 326+ 'shape' => array()
 327+ );
 328+ foreach( $draw['shape'] as $point ) {
 329+ $tPoint = self::matrixMultiply2x2( $matrix, $point['x'], $point['y'] );
 330+ $tPoint['x'] += $correctionX;
 331+ $tPoint['y'] += $correctionY;
 332+ $tDraw['shape'][] = $tPoint;
 333+ }
 334+ $tSpec['draws'][] = $tDraw;
 335+ }
 336+ return $tSpec;
 337+ }
229338
230339 /**
 340+ * Given a matrix and a pair of images, return new position
 341+ * @param $matrix: 2x2 rotation matrix
 342+ * @param $x: x-coordinate number
 343+ * @param $y: y-coordinate number
 344+ * @return Array transformed with properties x, y
 345+ */
 346+ private static function matrixMultiply2x2( $matrix, $x, $y ) {
 347+ return array(
 348+ 'x' => $x * $matrix[0][0] + $y * $matrix[0][1],
 349+ 'y' => $x * $matrix[1][0] + $y * $matrix[1][1]
 350+ );
 351+ }
 352+
 353+
 354+ /**
231355 * Based on an image specification, write such an image to disk, using the command line ImageMagick program ('convert').
232356 *
233357 * Sample command line:
@@ -235,7 +359,7 @@
236360 * -draw 'fill rgb(99,123,231) circle 59,39 56,57' \
237361 * -draw 'fill rgb(240,12,32) circle 50,21 50,3' filename.png
238362 *
239 - * @param $spec: spec describing background and circles to draw
 363+ * @param $spec: spec describing background and shapes to draw
240364 * @param $format: file format to write (unused by this method but kept so it has the same signature as writeImageWithApi)
241365 * @param $filename: filename to write to
242366 */
Index: trunk/phase3/tests/phpunit/includes/api/generateRandomImages.php
@@ -1,26 +1,47 @@
22 <?php
 3+/**
 4+ * Bootstrapping for test image file generation
 5+ *
 6+ * @file
 7+ */
38
 9+// Evaluate the include path relative to this file
 10+$IP = dirname( dirname( dirname( dirname( dirname( __FILE__ ) ) ) ) );
 11+
 12+// Start up MediaWiki in command-line mode
 13+require_once( "$IP/maintenance/Maintenance.php" );
414 require("RandomImageGenerator.php");
515
6 -$getOptSpec = array(
7 - 'dictionaryFile::',
8 - 'minWidth::',
9 - 'maxWidth::',
10 - 'minHeight::',
11 - 'maxHeight::',
12 - 'shapesToDraw::',
13 - 'shape::',
 16+class GenerateRandomImages extends Maintenance {
1417
15 - 'number::',
16 - 'format::'
17 -);
18 -$options = getopt( null, $getOptSpec );
 18+ public function execute() {
1919
20 -$format = isset( $options['format'] ) ? $options['format'] : 'jpg';
21 -unset( $options['format'] );
 20+ $getOptSpec = array(
 21+ 'dictionaryFile::',
 22+ 'minWidth::',
 23+ 'maxWidth::',
 24+ 'minHeight::',
 25+ 'maxHeight::',
 26+ 'shapesToDraw::',
 27+ 'shape::',
2228
23 -$number = isset( $options['number'] ) ? intval( $options['number'] ) : 10;
24 -unset( $options['number'] );
 29+ 'number::',
 30+ 'format::'
 31+ );
 32+ $options = getopt( null, $getOptSpec );
2533
26 -$randomImageGenerator = new RandomImageGenerator( $options );
27 -$randomImageGenerator->writeImages( $number, $format );
 34+ $format = isset( $options['format'] ) ? $options['format'] : 'jpg';
 35+ unset( $options['format'] );
 36+
 37+ $number = isset( $options['number'] ) ? intval( $options['number'] ) : 10;
 38+ unset( $options['number'] );
 39+
 40+ $randomImageGenerator = new RandomImageGenerator( $options );
 41+ $randomImageGenerator->writeImages( $number, $format );
 42+ }
 43+}
 44+
 45+$maintClass = 'GenerateRandomImages';
 46+require( RUN_MAINTENANCE_IF_MAIN );
 47+
 48+

Comments

#Comment by Platonides (talk | contribs)   21:31, 22 June 2011

// because the above setImageOrientation call doesn't work... nor can I get an external imagemagick binary to do this either... // hacking this for now (only works if you have exiv2 installed, a program to read and manipulate exif) if ( $wgExiv2Command ) { $wgExiv2Command is not defined anywhere.

What did you want setImageOrientation() to do? To change just the EXIF metadata?

#Comment by NeilK (talk | contribs)   21:48, 23 June 2011

to add or change EXIF metadata, yes.

There is some dispute on mailing lists about whether it will add EXIF metadata, and there's little documentation. Gave up for now.

#Comment by NeilK (talk | contribs)   20:12, 27 June 2011

You added a fixme to this, could you describe what you want done in order for it to be fixed? This was already approved as "ok".

#Comment by Platonides (talk | contribs)   20:57, 27 June 2011

You are using an undefined variable. They are usually defined in DefaultSettings.php, although being only used in a test, I'd be comfortable with having it hardcoded in the test or in a class constant, too.

Note that even guessing by its name that it should be set in LocalSettings to some arcane invocation of a program called exiv2, there's no information on which is the proper spell.

#Comment by NeilK (talk | contribs)   23:09, 27 June 2011

Fixed in r90919

Status & tagging log