Skip to content

Commit

Permalink
Make cropThumbnailImage behave correctly.
Browse files Browse the repository at this point in the history
  • Loading branch information
@Danack
Danack committed Dec 10, 2015
1 parent dab0396 commit df7d59c
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 45 deletions.
12 changes: 12 additions & 0 deletions imagick.c
View file
Expand Up @@ -563,6 +563,7 @@ PHP_IMAGICK_API zend_class_entry *php_imagickpixel_get_class_entry()
ZEND_BEGIN_ARG_INFO_EX(imagick_cropthumbnailimage_args, 0, 0, 2)
ZEND_ARG_INFO(0, width)
ZEND_ARG_INFO(0, height)
ZEND_ARG_INFO(0, legacy)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(imagick_setimagefilename_args, 0, 0, 1)
Expand Down Expand Up @@ -913,6 +914,14 @@ PHP_IMAGICK_API zend_class_entry *php_imagickpixel_get_class_entry()
ZEND_ARG_OBJ_INFO(0, ImagickDraw, ImagickDraw, 0)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(imagick_calculatecrop_args, 0, 0, 4)
ZEND_ARG_INFO(0, orig_width)
ZEND_ARG_INFO(0, orig_height)
ZEND_ARG_INFO(0, desired_width)
ZEND_ARG_INFO(0, desired_height)
ZEND_ARG_INFO(0, legacy)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(imagick_borderimage_args, 0, 0, 3)
ZEND_ARG_INFO(0, color)
ZEND_ARG_INFO(0, width)
Expand Down Expand Up @@ -944,6 +953,7 @@ PHP_IMAGICK_API zend_class_entry *php_imagickpixel_get_class_entry()
ZEND_BEGIN_ARG_INFO_EX(imagick_colorizeimage_args, 0, 0, 2)
ZEND_ARG_INFO(0, colorize_color)
ZEND_ARG_INFO(0, opacity)
ZEND_ARG_INFO(0, legacy)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(imagick_compareimagechannels_args, 0, 0, 3)
Expand Down Expand Up @@ -1252,6 +1262,7 @@ PHP_IMAGICK_API zend_class_entry *php_imagickpixel_get_class_entry()
ZEND_BEGIN_ARG_INFO_EX(imagick_tintimage_args, 0, 0, 2)
ZEND_ARG_INFO(0, tint_color)
ZEND_ARG_INFO(0, opacity)
ZEND_ARG_INFO(0, legacy)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_INFO_EX(imagick_unsharpmaskimage_args, 0, 0, 4)
Expand Down Expand Up @@ -2423,6 +2434,7 @@ static zend_function_entry php_imagick_class_methods[] =
#endif
#endif
PHP_ME(imagick, borderimage, imagick_borderimage_args, ZEND_ACC_PUBLIC)
PHP_ME(imagick, calculatecrop, imagick_calculatecrop_args, ZEND_ACC_STATIC|ZEND_ACC_PUBLIC)
PHP_ME(imagick, chopimage, imagick_chopimage_args, ZEND_ACC_PUBLIC)
PHP_ME(imagick, clipimage, imagick_zero_args, ZEND_ACC_PUBLIC)
PHP_ME(imagick, clippathimage, imagick_clippathimage_args, ZEND_ACC_PUBLIC)
Expand Down
141 changes: 113 additions & 28 deletions imagick_class.c
View file
Expand Up @@ -7560,12 +7560,107 @@ PHP_METHOD(imagick, thumbnailimage)
}
/* }}} */



static inline double im_round_helper(double value) {
double tmp_value;

if (value >= 0.0) {
return floor(value + 0.5);

} else {
return ceil(value - 0.5);
}
}


static
zend_bool s_crop_thumbnail_image(MagickWand *magick_wand, long desired_width, long desired_height TSRMLS_DC)
{
void s_calculate_crop(
long orig_width, long orig_height,
long desired_width, long desired_height,
long *new_width, long *new_height,
long *offset_x, long *offset_y,
zend_bool legacy
) {
double ratio_x, ratio_y;
long crop_x = 0, crop_y = 0, new_width, new_height;
long temp_new_width, temp_new_height;

ratio_x = ((double) desired_width / (double) orig_width);
ratio_y = ((double) desired_height / (double) orig_height);

if (ratio_x > ratio_y) {
temp_new_width = desired_width;

if (legacy) {
temp_new_height = (long)(ratio_x * (double)orig_height);
}
else {
temp_new_height = im_round_helper(ratio_x * (double)orig_height);
}
} else {
temp_new_height = desired_height;
if (legacy) {
temp_new_width = (long)(ratio_y * (double)orig_width);
}
else {
temp_new_width = im_round_helper(ratio_y * (double)orig_width);
}
}

*new_width = temp_new_width;
*new_height = temp_new_height;

*offset_x = (long) ((temp_new_width - desired_width) / 2);
*offset_y = (long) ((temp_new_height - desired_height) / 2);
}


/* {{{ proto array Imagick::calculateCrop(long orig_width, long orig_height, long desired_width, long desired_height[, bool legacy = false])
Calculates the cropping values that will be used by a crop operation.
*/
PHP_METHOD(imagick, calculatecrop)
{
long orig_width, orig_height;
long desired_width, desired_height;
long new_width, new_height;
long offset_x, offset_y;
zend_bool legacy = 0;

/* Parse parameters given to function */
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "llll|b",
&orig_width, &orig_height, &desired_width, &desired_height, &legacy) == FAILURE) {
return;
}

if (orig_width <= 0 || orig_height <= 0 ||
desired_width <= 0 || desired_height <= 0) {
php_imagick_throw_exception(IMAGICK_CLASS, "All values must be above zero." TSRMLS_CC);
}

s_calculate_crop(
orig_width, orig_height,
desired_width, desired_height,
&new_width, &new_height,
&offset_x, &offset_y,
legacy
);

array_init(return_value);
add_assoc_long(return_value, "width", new_width);
add_assoc_long(return_value, "height", new_height);

add_assoc_long(return_value, "offset_x", offset_x);
add_assoc_long(return_value, "offset_y", offset_y);

return;
}
/* }}} */


static
zend_bool s_crop_thumbnail_image(MagickWand *magick_wand, long desired_width, long desired_height, zend_bool legacy TSRMLS_DC)
{
long offset_x = 0, offset_y = 0, new_width, new_height;
long orig_width = MagickGetImageWidth(magick_wand);
long orig_height = MagickGetImageHeight(magick_wand);

Expand All @@ -7577,21 +7672,13 @@ zend_bool s_crop_thumbnail_image(MagickWand *magick_wand, long desired_width, lo
return 1;
}

ratio_x = ((double) desired_width / (double) orig_width);
ratio_y = ((double) desired_height / (double) orig_height);

if (desired_width == desired_height) {
new_width = desired_width;
new_height = desired_height;
} else if (ratio_x > ratio_y) {
new_width = desired_width;
//TODO - this should be round() when we target C99
new_height = ratio_x * (double)orig_height;
} else {
new_height = desired_height;
//TODO - this should be round() when we target C99
new_width = ratio_y * (double)orig_width;
}
s_calculate_crop(
orig_width, orig_height,
desired_width, desired_height,
&new_width, &new_height,
&offset_x, &offset_y,
legacy
);

if (MagickThumbnailImage(magick_wand, new_width, new_height) == MagickFalse) {
return 0;
Expand All @@ -7602,10 +7689,7 @@ zend_bool s_crop_thumbnail_image(MagickWand *magick_wand, long desired_width, lo
return 1;
}

crop_x = (long) ((new_width - desired_width) / 2);
crop_y = (long) ((new_height - desired_height) / 2);

if (MagickCropImage(magick_wand, desired_width, desired_height, crop_x, crop_y) == MagickFalse) {
if (MagickCropImage(magick_wand, desired_width, desired_height, offset_x, offset_y) == MagickFalse) {
return 0;
}

Expand All @@ -7614,16 +7698,19 @@ zend_bool s_crop_thumbnail_image(MagickWand *magick_wand, long desired_width, lo
}

//
/* {{{ proto bool Imagick::cropthumbnailImage(int columns, int rows)
Creates a crop thumbnail
/* {{{ proto bool Imagick::cropthumbnailImage(int columns, int rows[, bool legacy = false] )
Creates a cropped thumbnail at the requested size. If legacy is true, uses the
incorrect behaviour that was present until Imagick 3.4.0. If false it uses the correct
behaviour.
*/
PHP_METHOD(imagick, cropthumbnailimage)
{
long crop_width, crop_height;
zend_bool legacy = 0;
php_imagick_object *intern;

/* Parse parameters given to function */
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ll", &crop_width, &crop_height) == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ll|b", &crop_width, &crop_height, &legacy) == FAILURE) {
return;
}

Expand All @@ -7632,7 +7719,7 @@ PHP_METHOD(imagick, cropthumbnailimage)
return;

/* The world collapses.. */
if (!s_crop_thumbnail_image(intern->magick_wand, crop_width, crop_height TSRMLS_CC)) {
if (!s_crop_thumbnail_image(intern->magick_wand, crop_width, crop_height, legacy TSRMLS_CC)) {
php_imagick_convert_imagick_exception(intern->magick_wand, "Unable to crop-thumbnail image" TSRMLS_CC);
return;
}
Expand Down Expand Up @@ -8200,8 +8287,6 @@ void s_add_named_strings (zval *array, const char *haystack TSRMLS_DC)
#endif
}
efree (buffer);


}

/* {{{ proto array Imagick::identifyImage([bool appendRawOutput] )
Expand Down
1 change: 1 addition & 0 deletions php_imagick_defs.h
View file
Expand Up @@ -656,6 +656,7 @@ PHP_METHOD(imagick, averageimages);
#endif
#endif
PHP_METHOD(imagick, borderimage);
PHP_METHOD(imagick, calculatecrop);
PHP_METHOD(imagick, chopimage);
PHP_METHOD(imagick, clipimage);
PHP_METHOD(imagick, clippathimage);
Expand Down
64 changes: 47 additions & 17 deletions tests/064_cropThumbNailImage.phpt
View file
Expand Up @@ -8,35 +8,65 @@ if (getenv('SKIP_SLOW_TESTS')) die('skip slow tests excluded by request');
--FILE--
<?php

//Test the the calculated values are actually correct.
$desired_height = 250;
$imageWidth = 1128;

for ($desired_width=245; $desired_width<255 ; $desired_width++) {
for ($imageHeight=1125 ; $imageHeight<1135 ; $imageHeight++) {
//Test the the calculated values are actually correct.
$desired_height = 250;
$imageWidth = 1128;
$imageHeight = 1128;

$legacySettings = [0, 1];

foreach($legacySettings as $legacy) {
for ($desired_width = 245; $desired_width < 255; $desired_width++) {
$imagick = new Imagick();
$imagick->newPseudoImage($imageWidth, $imageHeight, 'xc:white');

$imagick->newImage($imageWidth, $imageHeight, '#dddddd', 'MIFF' );
$imagick->cropThumbnailImage($desired_width, $desired_height);
$imagick->cropThumbnailImage(
$desired_width, $desired_height,
$legacy
);
$error = false;

if ($imagick->getImageWidth() != $desired_width) {
printf(
"Wrong width %d for test case: ".PHP_EOL,
$imagick->getImageWidth()
);
var_dump($testCase);
$thumbnailImageWidth = $imagick->getImageWidth();
$thumbnailImageHeight = $imagick->getImageHeight();

if ($thumbnailImageHeight != $desired_height) {
echo "Incorrect height for desired_width $desired_width imageHeight $imageHeight".PHP_EOL;
$error = true;
}

$expectedWidth = $desired_width;
$expectedHeight = $desired_height;

if ($legacy == true &&
$desired_width == 250 &&
$desired_height == 250) {
// Thumbnail size of 249 x 250 does not matched desired size 250 x 250 for source image of 1128 x 1128
$expectedWidth = 249;
}

if ($thumbnailImageWidth != $expectedWidth) {
echo "Incorrect width for desired_width $desired_width imageHeight $imageHeight".PHP_EOL;
$error = true;
}

if ($imagick->getImageHeight() != $desired_height) {
if ($thumbnailImageHeight != $expectedHeight) {
echo "Incorrect width for desired_width $desired_width imageHeight $imageHeight".PHP_EOL;
$error = true;
}

if ($error) {
printf(
"Wrong height %d for test case:".PHP_EOL,
$imagick->getImageHeight()
"Thumbnail size of %d x %d does not matched expected size %d x %d for source image of %d x %d. Legacy is %d\n",
$thumbnailImageWidth, $thumbnailImageHeight,
$desired_width, $desired_height,
$imageWidth, $imageHeight,
$legacy
);
var_dump($testCase);
}

$imagick->destroy();
$imagick = null;
}
}

Expand Down

0 comments on commit df7d59c

Please sign in to comment.