Merge pull request #4410 from jonom/force-resampling

API Force resampling by default

model/Image.php

@@ -70,7 +70,7 @@ class Image extends File implements Flushable {
 	 * @config
 	 * @var bool Force all images to resample in all cases
 	 */
-	private static $force_resample = false;
+	private static $force_resample = true;
 
 	/**
 	 * @config
@@ -163,7 +163,34 @@ class Image extends File implements Flushable {
 	public function forTemplate() {
 		return $this->getTag();
 	}
+	
+	/**
+	 * Gets the source image URL for this resource
+	 * 
+	 * @return string
+	 */
+	public function getSourceURL() {
+		return parent::getURL();
+	}
 
+	/**
+	 * Gets the relative URL accessible through the web. If forced resampling is enabled 
+	 * the URL will point to an optimised file, if it is smaller than the original
+	 *
+	 * @uses Director::baseURL()
+	 * @return string
+	 */
+	public function getURL() {
+		if ($this->config()->force_resample) {
+			//return $resampled->getURL();
+			$resampled = $this->getFormattedImage('Resampled');
+			if ($resampled->getAbsoluteSize() < $this->getAbsoluteSize()) {
+				return $resampled->getURL();
+			}
+		}
+		return $this->getSourceURL();
+	}
+	
 	/**
 	 * File names are filtered through {@link FileNameFilter}, see class documentation
 	 * on how to influence this behaviour.
@@ -232,10 +259,10 @@ class Image extends File implements Flushable {
 		
 		if( $widthRatio < $heightRatio ) {
 			// Target is higher aspect ratio than image, so check width
-			if($this->isWidth($width) && !Config::inst()->get('Image', 'force_resample')) return $this;
+			if($this->isWidth($width)) return $this;
 		} else {
 			// Target is wider or same aspect ratio as image, so check height
-			if($this->isHeight($height) && !Config::inst()->get('Image', 'force_resample')) return $this;
+			if($this->isHeight($height)) return $this;
 		}
 
 		// Item must be regenerated
@@ -281,7 +308,7 @@ class Image extends File implements Flushable {
 	 * @return Image
 	 */
 	public function Fill($width, $height) {
-		return $this->isSize($width, $height) && !Config::inst()->get('Image', 'force_resample')
+		return $this->isSize($width, $height)
 			? $this
 			: $this->getFormattedImage('Fill', $width, $height);
 	}
@@ -338,7 +365,7 @@ class Image extends File implements Flushable {
 	 * @return Image
 	 */
 	public function Pad($width, $height, $backgroundColor='FFFFFF') {
-		return $this->isSize($width, $height) && !Config::inst()->get('Image', 'force_resample')
+		return $this->isSize($width, $height)
 			? $this
 			: $this->getFormattedImage('Pad', $width, $height, $backgroundColor);
 	}
@@ -362,7 +389,7 @@ class Image extends File implements Flushable {
 	 * @return Image
 	 */
 	public function ScaleWidth($width) {
-		return $this->isWidth($width) && !Config::inst()->get('Image', 'force_resample')
+		return $this->isWidth($width)
 			? $this
 			: $this->getFormattedImage('ScaleWidth', $width);
 	}
@@ -402,7 +429,7 @@ class Image extends File implements Flushable {
 	 * @return Image
 	 */
 	public function ScaleHeight($height) {
-		return $this->isHeight($height) && !Config::inst()->get('Image', 'force_resample')
+		return $this->isHeight($height)
 			? $this
 			: $this->getFormattedImage('ScaleHeight', $height);
 	}
@@ -468,6 +495,26 @@ class Image extends File implements Flushable {
 			? $this->Fill($this->getWidth(), $height)
 			: $this;
 	}
+	
+	/**
+	 * Resample this Image to ensure quality preference is applied.
+	 * Warning: it's possible this will produce a larger file of lower quality
+	 *
+	 * @return Image
+	 */
+	public function Resampled() {
+		return $this->getFormattedImage('Resampled');
+	}
+
+	/**
+	 * Resample this Image to apply quality preference
+	 *
+	 * @param Image_Backend $backend
+	 * @return Image_Backend
+	 */
+	public function generateResampled(Image_Backend $backend){
+		return $backend;
+	}
 
 	/**
 	 * Resize the image by preserving aspect ratio, keeping the image inside the
@@ -768,7 +815,7 @@ class Image extends File implements Flushable {
 	 * @return Image
 	 */
 	public function ResizedImage($width, $height) {
-		return $this->isSize($width, $height) && !Config::inst()->get('Image', 'force_resample')
+		return $this->isSize($width, $height)
 			? $this
 			: $this->getFormattedImage('ResizedImage', $width, $height);
 	}
@@ -1028,7 +1075,11 @@ class Image_Cached extends Image {
 		$this->ID = -1;
 		$this->Filename = $filename;
 	}
-
+	
+	public function getURL() {
+		return $this->getSourceURL();
+	}
+	
 	public function getRelativePath() {
 		return $this->getField('Filename');
 	}

tests/model/ImageTest.php

@@ -69,6 +69,8 @@ class ImageTest extends SapphireTest {
 	}
 
 	public function testGetTagWithTitle() {
+		Config::inst()->update('Image', 'force_resample', false);
+		
 		$image = $this->objFromFixture('Image', 'imageWithTitle');
 		$expected = '<img src="' . Director::baseUrl()
 			. 'assets/ImageTest/test_image.png" alt="This is a image Title" />';
@@ -78,6 +80,8 @@ class ImageTest extends SapphireTest {
 	}
 
 	public function testGetTagWithoutTitle() {
+		Config::inst()->update('Image', 'force_resample', false);
+		
 		$image = $this->objFromFixture('Image', 'imageWithoutTitle');
 		$expected = '<img src="' . Director::baseUrl() . 'assets/ImageTest/test_image.png" alt="test_image" />';
 		$actual = $image->getTag();
@@ -86,6 +90,8 @@ class ImageTest extends SapphireTest {
 	}
 
 	public function testGetTagWithoutTitleContainingDots() {
+		Config::inst()->update('Image', 'force_resample', false);
+		
 		$image = $this->objFromFixture('Image', 'imageWithoutTitleContainingDots');
 		$expected = '<img src="' . Director::baseUrl()
 			. 'assets/ImageTest/test.image.with.dots.png" alt="test.image.with.dots" />';
@@ -166,58 +172,27 @@ class ImageTest extends SapphireTest {
 	}
 
 	/**
-	 * Tests that image manipulations that do not affect the resulting dimensions
+	 * Tests that a URL to a resampled image is provided when force_resample is 
-	 * of the output image resample the file when force_resample is set to true.
+	 * set to true, if the resampled file is smaller than the original.
 	 */
 	public function testForceResample() {
-
+		$imageHQ = $this->objFromFixture('Image', 'highQualityJPEG');
-		$image = $this->objFromFixture('Image', 'imageWithoutTitle');
+		$imageHQR = $imageHQ->Resampled();
-		$this->assertTrue($image->isSize(300, 300));
+		$imageLQ = $this->objFromFixture('Image', 'lowQualityJPEG');
-
+		$imageLQR = $imageLQ->Resampled();
-		$origForceResample = Config::inst()->get('Image', 'force_resample');
+		
+		// Test resampled file is served when force_resample = true
 		Config::inst()->update('Image', 'force_resample', true);
-
+		$this->assertLessThan($imageHQ->getAbsoluteSize(), $imageHQR->getAbsoluteSize(), 'Resampled image is smaller than original');
-		// Set width to 300 pixels
+		$this->assertEquals($imageHQ->getURL(), $imageHQR->getSourceURL(), 'Path to a resampled image was returned by getURL()');
-		$imageScaleWidth = $image->ScaleWidth(300);
+		
-		$this->assertEquals($imageScaleWidth->getWidth(), 300);
+		// Test original file is served when force_resample = true but original file is low quality
-		$this->assertNotEquals($image->Filename, $imageScaleWidth->Filename);
+		$this->assertGreaterThanOrEqual($imageLQ->getAbsoluteSize(), $imageLQR->getAbsoluteSize(), 'Resampled image is larger or same size as original');
-
+		$this->assertNotEquals($imageLQ->getURL(), $imageLQR->getSourceURL(), 'Path to the original image file was returned by getURL()');
-		// Set height to 300 pixels
+		
-		$imageScaleHeight = $image->ScaleHeight(300);
+		// Test original file is served when force_resample = false
-		$this->assertEquals($imageScaleHeight->getHeight(), 300);
+		Config::inst()->update('Image', 'force_resample', false);
-		$this->assertNotEquals($image->Filename, $imageScaleHeight->Filename);
+		$this->assertNotEquals($imageHQ->getURL(), $imageHQR->getSourceURL(), 'Path to the original image file was returned by getURL()');
-
-		// Crop image to 300 x 300
-		$imageCropped = $image->Fill(300, 300);
-		$this->assertTrue($imageCropped->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imageCropped->Filename);
-
-		// Resize (padded) to 300 x 300
-		$imageSized = $image->Pad(300, 300);
-		$this->assertTrue($imageSized->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imageSized->Filename);
-
-		// Padded image 300 x 300 (same as above)
-		$imagePadded = $image->Pad(300, 300);
-		$this->assertTrue($imagePadded->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imagePadded->Filename);
-
-		// Resized (stretched) to 300 x 300
-		$imageStretched = $image->ResizedImage(300, 300);
-		$this->assertTrue($imageStretched->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imageStretched->Filename);
-
-		// Fit (various options)
-		$imageFit = $image->Fit(300, 600);
-		$this->assertTrue($imageFit->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imageFit->Filename);
-		$imageFit = $image->Fit(600, 300);
-		$this->assertTrue($imageFit->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imageFit->Filename);
-		$imageFit = $image->Fit(300, 300);
-		$this->assertTrue($imageFit->isSize(300, 300));
-		$this->assertNotEquals($image->Filename, $imageFit->Filename);
-		Config::inst()->update('Image', 'force_resample', $origForceResample);
 	}
 
 	public function testImageResize() {