Workaround image magick issue with greyscale xcf files

IM doesn't seem to properly interpret greyscale xcf files
as being greyscale. Tell it to just take the red channel
in such a case.

Bug: 66323
Change-Id: I46302d43e1029d815be99f481f3942481becd74f

diff --git a/includes/media/Bitmap.php b/includes/media/Bitmap.php
index 44be178..76dab03 100644 (file)
--- a/includes/media/Bitmap.php
+++ b/includes/media/Bitmap.php
@@ -355,6 +355,19 @@ class BitmapHandler extends ImageHandler {
                        }
                } elseif ( $params['mimeType'] == 'image/x-xcf' ) {
                        $animation_post = array( '-layers', 'merge' );
+                       wfSuppressWarnings();
+                       $xcfMeta = unserialize( $image->getMetadata() );
+                       wfRestoreWarnings();
+                       if ( $xcfMeta
+                               && isset( $xcfMeta['colorType'] )
+                               && $xcfMeta['colorType'] === 'greyscale-alpha'
+                               && version_compare( $this->getMagickVersion(), "6.8.9-3" ) < 0
+                       ) {
+                               // bug 66323 - Greyscale images not rendered properly.
+                               // So only take the "red" channel.
+                               $channelOnly = array( '-channel', 'R', '-separate' );
+                               $animation_post = array_merge( $animation_post, $channelOnly );
+                       }
                }
 
                // Use one thread only, to avoid deadlock bugs on OOM

diff --git a/includes/media/XCF.php b/includes/media/XCF.php
index 41e6f03..658112f 100644 (file)
--- a/includes/media/XCF.php
+++ b/includes/media/XCF.php
@@ -61,7 +61,28 @@ class XCFHandler extends BitmapHandler {
         * @return array
         */
        function getImageSize( $image, $filename ) {
-               return self::getXCFMetaData( $filename );
+               $header = self::getXCFMetaData( $filename );
+               if ( !$header ) {
+                       return false;
+               }
+
+               # Forge a return array containing metadata information just like getimagesize()
+               # See PHP documentation at: http://www.php.net/getimagesize
+               $metadata = array();
+               $metadata[0] = $header['width'];
+               $metadata[1] = $header['height'];
+               $metadata[2] = null; # IMAGETYPE constant, none exist for XCF.
+               $metadata[3] = sprintf(
+                       'height="%s" width="%s"', $header['height'], $header['width']
+               );
+               $metadata['mime'] = 'image/x-xcf';
+               $metadata['channels'] = null;
+               $metadata['bits'] = 8; # Always 8-bits per color
+
+               assert( '7 == count($metadata); ' .
+                       '# return array must contains 7 elements just like getimagesize() return' );
+
+               return $metadata;
        }
 
        /**
@@ -124,23 +145,61 @@ class XCFHandler extends BitmapHandler {
                wfDebug( __METHOD__ .
                        ": canvas size of '$filename' is {$header['width']} x {$header['height']} px\n" );
 
-               # Forge a return array containing metadata information just like getimagesize()
-               # See PHP documentation at: http://www.php.net/getimagesize
+               return $header;
+       }
+
+       /**
+        * Store the channel type
+        *
+        * Greyscale files need different command line options.
+        *
+        * @param File $file The image object, or false if there isn't one.
+        *   Warning, FSFile::getPropsFromPath might pass an (object)array() instead (!)
+        * @param string $path The filename
+        * @return string
+        */
+       public function getMetadata( $file, $filename ) {
+               $header = self::getXCFMetadata( $filename );
                $metadata = array();
-               $metadata[0] = $header['width'];
-               $metadata[1] = $header['height'];
-               $metadata[2] = null; # IMAGETYPE constant, none exist for XCF.
-               $metadata[3] = sprintf(
-                       'height="%s" width="%s"', $header['height'], $header['width']
-               );
-               $metadata['mime'] = 'image/x-xcf';
-               $metadata['channels'] = null;
-               $metadata['bits'] = 8; # Always 8-bits per color
+               if ( $header ) {
+                       // Try to be consistent with the names used by PNG files.
+                       // Unclear from base media type if it has an alpha layer,
+                       // so just assume that it does since it "potentially" could.
+                       switch( $header['base_type'] ) {
+                       case 0:
+                               $metadata['colorType'] = 'truecolour-alpha';
+                               break;
+                       case 1:
+                               $metadata['colorType'] = 'greyscale-alpha';
+                               break;
+                       case 2:
+                               $metadata['colorType'] = 'index-coloured';
+                               break;
+                       default:
+                               $metadata['colorType'] = 'unknown';
 
-               assert( '7 == count($metadata); ' .
-                       '# return array must contains 7 elements just like getimagesize() return' );
+                       }
+               } else {
+                       // Marker to prevent repeated attempted extraction
+                       $metadata['error'] = true;
+               }
+               return serialize( $metadata );
+       }
 
-               return $metadata;
+       /**
+        * Should we refresh the metadata
+        *
+        * @param File $file The file object for the file in question
+        * @param string $metadata Serialized metadata
+        * @return bool One of the self::METADATA_(BAD|GOOD|COMPATIBLE) constants
+        */
+       public function isMetadataValid( $file, $metadata ) {
+               if ( !$metadata ) {
+                       // Old metadata when we just put an empty string in there
+                       return self::METADATA_BAD;
+               } else {
+                       return self::METADATA_GOOD;
+               }
        }
 
        /**

diff --git a/tests/phpunit/includes/media/XCFTest.php b/tests/phpunit/includes/media/XCFTest.php
new file mode 100644 (file)
index 0000000..ae4fa8b
--- /dev/null
+++ b/tests/phpunit/includes/media/XCFTest.php
@@ -0,0 +1,74 @@
+<?php
+class XCFHandlerTest extends MediaWikiMediaTestCase {
+
+       /** @var XCFHandler */
+       protected $handler;
+
+       protected function setUp() {
+               parent::setUp();
+               $this->handler = new XCFHandler();
+       }
+
+
+       /**
+        * @param string $filename
+        * @param int $expectedWidth width
+        * @param int $expectedHeigh height
+        * @dataProvider provideGetImageSize
+        * @covers XCFHandler::getImageSize
+        */
+       public function testGetImageSize( $filename, $expectedWidth, $expectedHeight ) {
+               $file = $this->dataFile( $filename, 'image/x-xcf' );
+               $actual = $this->handler->getImageSize( $file, $file->getLocalRefPath() );
+               $this->assertEquals( $expectedWidth, $actual[0] );
+               $this->assertEquals( $expectedHeight, $actual[1] );
+       }
+
+       public static function provideGetImageSize() {
+               return array(
+                       array( '80x60-2layers.xcf', 80, 60 ),
+                       array( '80x60-RGB.xcf', 80, 60 ),
+                       array( '80x60-Greyscale.xcf', 80, 60 ),
+               );
+       }
+
+       /**
+        * @param string $metadata Serialized metadata
+        * @param int $expected One of the class constants of XCFHandler
+        * @dataProvider provideIsMetadataValid
+        * @covers XCFHandler::isMetadataValid
+        */
+       public function testIsMetadataValid( $metadata, $expected ) {
+               $actual = $this->handler->isMetadataValid( null, $metadata );
+               $this->assertEquals( $expected, $actual );
+       }
+
+       public static function provideIsMetadataValid() {
+               return array(
+                       array( '', XCFHandler::METADATA_BAD ),
+                       array( serialize( array( 'error' => true ) ), XCFHandler::METADATA_GOOD ),
+                       array( false, XCFHandler::METADATA_BAD ),
+                       array( serialize( array( 'colorType' => 'greyscale-alpha' ) ), XCFHandler::METADATA_GOOD ),
+               );
+       }
+
+       /**
+        * @param string $filename
+        * @param string $expected Serialized array
+        * @dataProvider provideGetMetadata
+        * @covers XCFHandler::getMetadata
+        */
+       public function testGetMetadata( $filename, $expected ) {
+               $file = $this->dataFile( $filename, 'image/png' );
+               $actual = $this->handler->getMetadata( $file, "$this->filePath/$filename" );
+               $this->assertEquals( $expected, $actual );
+       }
+
+       public static function provideGetMetadata() {
+               return array(
+                       array( '80x60-2layers.xcf', 'a:1:{s:9:"colorType";s:16:"truecolour-alpha";}' ),
+                       array( '80x60-RGB.xcf', 'a:1:{s:9:"colorType";s:16:"truecolour-alpha";}' ),
+                       array( '80x60-Greyscale.xcf', 'a:1:{s:9:"colorType";s:15:"greyscale-alpha";}' ),
+               );
+       }
+}