Commit 23e7a3cf authored by catch's avatar catch
Browse files

Issue #1338428 by jbrown, BTMash, Tor Arne Thune: Fixed...

Issue #1338428 by jbrown, BTMash, Tor Arne Thune: Fixed image_dimensions_scale() and image_scale_effect() are ungrokable and buggy.
parent e6b94024
......@@ -186,14 +186,14 @@ function image_scale_and_crop(stdClass $image, $width, $height) {
* Dimensions to be modified - an array with components width and height, in
* pixels.
* @param $width
* The target width, in pixels. This value is omitted then the scaling will
* The target width, in pixels. If this value is NULL then the scaling will be
* based only on the height value.
* @param $height
* The target height, in pixels. This value is omitted then the scaling will
* based only on the width value.
* The target height, in pixels. If this value is NULL then the scaling will
* be based only on the width value.
* @param $upscale
* Boolean indicating that files smaller than the dimensions will be scaled
* up. This generally results in a low quality image.
* Boolean indicating that images smaller than the target dimensions will be
* scaled up. This generally results in a low quality image.
*
* @return
* TRUE if $dimensions was modified, FALSE otherwise.
......@@ -203,31 +203,25 @@ function image_scale_and_crop(stdClass $image, $width, $height) {
function image_dimensions_scale(array &$dimensions, $width = NULL, $height = NULL, $upscale = FALSE) {
$aspect = $dimensions['height'] / $dimensions['width'];
if ($upscale) {
// Set width/height according to aspect ratio if either is empty.
$width = !empty($width) ? $width : $height / $aspect;
$height = !empty($height) ? $height : $width / $aspect;
// Calculate one of the dimensions from the other target dimension,
// ensuring the same aspect ratio as the source dimensions. If one of the
// target dimensions is missing, that is the one that is calculated. If both
// are specified then the dimension calculated is the one that would not be
// calculated to be bigger than its target.
if (($width && !$height) || ($width && $height && $aspect < $height / $width)) {
$height = (int) round($width * $aspect);
}
else {
// Set impossibly large values if the width and height aren't set.
$width = !empty($width) ? $width : 9999999;
$height = !empty($height) ? $height : 9999999;
// Don't scale up.
if (round($width) >= $dimensions['width'] && round($height) >= $dimensions['height']) {
return FALSE;
}
$width = (int) round($height / $aspect);
}
if ($aspect < $height / $width) {
$dimensions['width'] = $width;
$dimensions['height'] = (int) round($width * $aspect);
}
else {
$dimensions['width'] = (int) round($height / $aspect);
$dimensions['height'] = $height;
// Don't upscale if the option isn't enabled.
if (!$upscale && ($width >= $dimensions['width'] || $height >= $dimensions['height'])) {
return FALSE;
}
$dimensions['width'] = $width;
$dimensions['height'] = $height;
return TRUE;
}
......
......@@ -124,13 +124,11 @@ function image_resize_dimensions(array &$dimensions, array $data) {
function image_scale_effect(&$image, $data) {
// Set sane default values.
$data += array(
'width' => NULL,
'height' => NULL,
'upscale' => FALSE,
);
// Set impossibly large values if the width and height aren't set.
$data['width'] = empty($data['width']) ? PHP_INT_MAX : $data['width'];
$data['height'] = empty($data['height']) ? PHP_INT_MAX : $data['height'];
if (!image_scale($image, $data['width'], $data['height'], $data['upscale'])) {
watchdog('image', 'Image scale failed using the %toolkit toolkit on %path (%mimetype, %dimensions)', array('%toolkit' => $image->toolkit, '%path' => $image->source, '%mimetype' => $image->info['mime_type'], '%dimensions' => $image->info['width'] . 'x' . $image->info['height']), WATCHDOG_ERROR);
return FALSE;
......
......@@ -1129,3 +1129,144 @@ class ImageDimensionsUnitTest extends DrupalWebTestCase {
$this->assertEqual($img_tag, '<img typeof="foaf:Image" src="' . $url . '" alt="" />', t('Expected img tag was found.'));
}
}
/**
* Tests image_dimensions_scale().
*/
class ImageDimensionsScaleTestCase extends DrupalUnitTestCase {
public static function getInfo() {
return array(
'name' => 'image_dimensions_scale()',
'description' => 'Tests all control flow branches in image_dimensions_scale().',
'group' => 'Image',
);
}
/**
* Tests all control flow branches in image_dimensions_scale().
*/
function testImageDimensionsScale() {
// Define input / output datasets to test different branch conditions.
$test = array();
// Test branch conditions:
// - No height.
// - Upscale, don't need to upscale.
$tests[] = array(
'input' => array(
'dimensions' => array(
'width' => 1000,
'height' => 2000,
),
'width' => 200,
'height' => NULL,
'upscale' => TRUE,
),
'output' => array(
'dimensions' => array(
'width' => 200,
'height' => 400,
),
'return_value' => TRUE,
),
);
// Test branch conditions:
// - No width.
// - Don't upscale, don't need to upscale.
$tests[] = array(
'input' => array(
'dimensions' => array(
'width' => 1000,
'height' => 800,
),
'width' => NULL,
'height' => 140,
'upscale' => FALSE,
),
'output' => array(
'dimensions' => array(
'width' => 175,
'height' => 140,
),
'return_value' => TRUE,
),
);
// Test branch conditions:
// - Source aspect ratio greater than target.
// - Upscale, need to upscale.
$tests[] = array(
'input' => array(
'dimensions' => array(
'width' => 8,
'height' => 20,
),
'width' => 200,
'height' => 140,
'upscale' => TRUE,
),
'output' => array(
'dimensions' => array(
'width' => 56,
'height' => 140,
),
'return_value' => TRUE,
),
);
// Test branch condition: target aspect ratio greater than source.
$tests[] = array(
'input' => array(
'dimensions' => array(
'width' => 2000,
'height' => 800,
),
'width' => 200,
'height' => 140,
'upscale' => FALSE,
),
'output' => array(
'dimensions' => array(
'width' => 200,
'height' => 80,
),
'return_value' => TRUE,
),
);
// Test branch condition: don't upscale, need to upscale.
$tests[] = array(
'input' => array(
'dimensions' => array(
'width' => 100,
'height' => 50,
),
'width' => 200,
'height' => 140,
'upscale' => FALSE,
),
'output' => array(
'dimensions' => array(
'width' => 100,
'height' => 50,
),
'return_value' => FALSE,
),
);
foreach ($tests as $test) {
// Process the test dataset.
$return_value = image_dimensions_scale($test['input']['dimensions'], $test['input']['width'], $test['input']['height'], $test['input']['upscale']);
// Check the width.
$this->assertEqual($test['output']['dimensions']['width'], $test['input']['dimensions']['width'], t('Computed width (@computed_width) equals expected width (@expected_width)', array('@computed_width' => $test['output']['dimensions']['width'], '@expected_width' => $test['input']['dimensions']['width'])));
// Check the height.
$this->assertEqual($test['output']['dimensions']['height'], $test['input']['dimensions']['height'], t('Computed height (@computed_height) equals expected height (@expected_height)', array('@computed_height' => $test['output']['dimensions']['height'], '@expected_height' => $test['input']['dimensions']['height'])));
// Check the return value.
$this->assertEqual($test['output']['return_value'], $return_value, t('Correct return value.'));
}
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment