BUG GD::greyscale did not correctly preserve alpha component of image…

…s Added test cases to test greyscale operation across various image formats Replaced various magic numbers with IMAGETYPE_XXX definitions

filesystem/GD.php

@@ -403,6 +403,10 @@ public function greyscale($rv=38, $gv=36, $bv=26) {
 		$height = $this->height;
 		$newGD = imagecreatetruecolor($this->width, $this->height);
 
+		// Preserves transparency between images
+		imagealphablending($newGD, false);
+		imagesavealpha($newGD, true);
+
 		$rt = $rv + $bv + $gv;
 		$rr = ($rv == 0) ? 0 : 1/($rt/$rv);
 		$br = ($bv == 0) ? 0 : 1/($rt/$bv);
@@ -412,7 +416,7 @@ public function greyscale($rv=38, $gv=36, $bv=26) {
 				$pxrgb = imagecolorat($this->gd, $dx, $dy);
 				$heightgb = ImageColorsforIndex($this->gd, $pxrgb);
 				$newcol = ($rr*$heightgb['red']) + ($br*$heightgb['blue']) + ($gr*$heightgb['green']);
-				$setcol = ImageColorAllocate($newGD, $newcol, $newcol, $newcol);
+				$setcol = ImageColorAllocateAlpha($newGD, $newcol, $newcol, $newcol, $heightgb['alpha']);
 				imagesetpixel($newGD, $dx, $dy, $setcol);
 			}
 		}
@@ -437,16 +441,16 @@ public function writeTo($filename) {
 
 			$ext = strtolower(substr($filename, strrpos($filename,'.')+1));
 			if(!isset($type)) switch($ext) {
-				case "gif": $type = 1; break;
-				case "jpeg": case "jpg": case "jpe": $type = 2; break;
-				default: $type = 3; break;
+				case "gif": $type = IMAGETYPE_GIF; break;
+				case "jpeg": case "jpg": case "jpe": $type = IMAGETYPE_JPEG; break;
+				default: $type = IMAGETYPE_PNG; break;
 			}
 
 			// if the extension does not exist, the file will not be created!
 
 			switch($type) {
-				case 1: imagegif($this->gd, $filename); break;
-				case 2: imagejpeg($this->gd, $filename, $this->quality); break;
+				case IMAGETYPE_GIF: imagegif($this->gd, $filename); break;
+				case IMAGETYPE_JPEG: imagejpeg($this->gd, $filename, $this->quality); break;
 
 				// case 3, and everything else
 				default: 

tests/filesystem/GDTest.php

@@ -0,0 +1,138 @@
+<?php
+
+/**
+ * Tests for the {@link GD} class.
+ *
+ * @package framework
+ * @subpackage tests
+ */
+class GDTest extends SapphireTest {
+
+	public static $filenames = array(
+		'gif' => 'test_gif.gif',
+		'jpg' => 'test_jpg.jpg',
+		'png8' => 'test_png8.png',
+		'png32' => 'test_png32.png'
+	);
+
+	/**
+	 * Loads all images into an associative array of GD objects.
+	 * Optionally applies an operation to each GD
+	 * @param callable $callback Action to perform on each GD
+	 * @return array List of GD
+	 */
+	protected function applyToEachImage($callback = null) {
+		$gds = array();
+		foreach(self::$filenames as $type => $file) {
+			$fullPath = realpath(dirname(__FILE__) . '/gdtest/' . $file);
+			$gd = new GD($fullPath);
+			if($callback) {
+				$gd = $callback($gd);
+			}
+			$gds[$type] = $gd;
+		}
+		return $gds;
+	}
+
+	/**
+	 * Takes samples from the given GD at 5 pixel increments
+	 * @param GD $gd The source image
+	 * @param integer $horizontal Number of samples to take horizontally
+	 * @param integer $vertical Number of samples to take vertically
+	 * @return array List of colours for each sample, each given as an associative
+	 * array with red, blue, green, and alpha components
+	 */
+	protected function sampleAreas(GD $gd, $horizontal = 4, $vertical = 4) {
+		$samples = array();
+		for($y = 0; $y < $vertical; $y++) {
+			for($x = 0; $x < $horizontal; $x++) {
+				$colour = imagecolorat($gd->getGD(), $x * 5, $y * 5);
+				$samples[] = ImageColorsforIndex($gd->getGD(), $colour);
+			}
+		}
+		return $samples;
+	}
+
+	/**
+	 * Asserts that two colour channels are equivalent within a given tolerance range
+	 * @param integer $expected
+	 * @param integer $actual
+	 * @param integer $tolerance
+	 */
+	protected function assertColourEquals($expected, $actual, $tolerance = 0) {
+		$match =
+			($expected + $tolerance >= $actual) && 
+			($expected - $tolerance <= $actual);	
+		$this->assertTrue($match);
+	}
+
+	/**
+	 * Asserts that all samples given correctly correspond to a greyscale version
+	 * of the test image pattern
+	 * @param array $samples List of 16 colour samples representing each of the
+	 * 8 x 8 squares on the image pattern
+	 * @param int $alphaBits Depth of alpha channel in bits
+	 * @param int $tolerance Reasonable tolerance level for colour comparison
+	 */
+	protected function assertGreyscale($samples, $alphaBits = 0, $tolerance = 0) {
+
+		// Check that all colour samples match
+		foreach($samples as $sample) {
+			$matches =
+				($sample['red'] === $sample['green']) && 
+				($sample['blue'] === $sample['green']);
+			$this->assertTrue($matches, 'Assert colour is greyscale');
+			if(!$matches) return;
+		}
+
+		// check various sample points
+		$this->assertColourEquals(96, $samples[0]['red'], $tolerance);
+		$this->assertColourEquals(91, $samples[2]['red'], $tolerance);
+		$this->assertColourEquals(0, $samples[8]['red'], $tolerance);
+		$this->assertColourEquals(127, $samples[9]['red'], $tolerance);
+
+		// check alpha of various points
+		switch($alphaBits) {
+			case 0:
+				$this->assertColourEquals(0, $samples[2]['alpha'], $tolerance);
+				$this->assertColourEquals(0, $samples[12]['alpha'], $tolerance);
+				break;
+			case 1:
+				$this->assertColourEquals(0, $samples[2]['alpha'], $tolerance);
+				$this->assertColourEquals(127, $samples[12]['alpha'], $tolerance);
+				break;
+			default:
+				$this->assertColourEquals(63, $samples[2]['alpha'], $tolerance);
+				$this->assertColourEquals(127, $samples[12]['alpha'], $tolerance);
+				break;
+		}
+
+	}
+
+	/**
+	 * Tests that images are correctly transformed to greyscale
+	 */
+	function testGreyscale() {
+
+		// Apply greyscaling to each image
+		$images = $this->applyToEachImage(function(GD $gd) {
+			return $gd->greyscale();
+		});
+
+		// Test GIF (256 colour, transparency)
+		$samplesGIF = $this->sampleAreas($images['gif']);
+		$this->assertGreyscale($samplesGIF, 1);
+
+		// Test JPG
+		$samplesJPG = $this->sampleAreas($images['jpg']);
+		$this->assertGreyscale($samplesJPG, 0, 4);
+
+		// Test PNG 8 (indexed with alpha transparency)
+		$samplesPNG8 = $this->sampleAreas($images['png8']);
+		$this->assertGreyscale($samplesPNG8, 8, 4);
+
+		// Test PNG 32 (full alpha transparency)
+		$samplesPNG32 = $this->sampleAreas($images['png32']);
+		$this->assertGreyscale($samplesPNG32, 8);
+	}
+}