From 7189de5dbab81964e5ad23f438d2abf71d4de7c6 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 14 Oct 2014 23:21:30 +0100 Subject: [PATCH] Issue #2063373 by mondrake, claudiu.cristea, larowlan, arunvs: Fixed Cannot save image created from scratch. --- core/lib/Drupal/Core/Image/Image.php | 7 + core/lib/Drupal/Core/Image/ImageInterface.php | 23 ++++ .../ImageToolkitOperationBase.php | 8 +- .../src/Plugin/ImageToolkit/GDToolkit.php | 93 ++++++------- .../ImageToolkit/Operation/gd/Convert.php | 34 +++-- .../ImageToolkit/Operation/gd/CreateNew.php | 126 ++++++++++++++++++ .../Plugin/ImageToolkit/Operation/gd/Crop.php | 26 ++-- .../ImageToolkit/Operation/gd/Resize.php | 26 ++-- .../system/src/Tests/Image/ToolkitGdTest.php | 45 ++++++- .../src/Tests/Image/ToolkitTestBase.php | 2 + 10 files changed, 297 insertions(+), 93 deletions(-) create mode 100644 core/modules/system/src/Plugin/ImageToolkit/Operation/gd/CreateNew.php diff --git a/core/lib/Drupal/Core/Image/Image.php b/core/lib/Drupal/Core/Image/Image.php index aff5d814caae..757c2c4731b8 100644 --- a/core/lib/Drupal/Core/Image/Image.php +++ b/core/lib/Drupal/Core/Image/Image.php @@ -148,6 +148,13 @@ public function apply($operation, array $arguments = array()) { return $this->getToolkit()->apply($operation, $arguments); } + /** + * {@inheritdoc} + */ + public function createNew($width, $height, $extension = 'png', $transparent_color = '#ffffff') { + return $this->apply('create_new', array('width' => $width, 'height' => $height, 'extension' => $extension, 'transparent_color' => $transparent_color)); + } + /** * {@inheritdoc} */ diff --git a/core/lib/Drupal/Core/Image/ImageInterface.php b/core/lib/Drupal/Core/Image/ImageInterface.php index 3094c9ce679d..da8496b2004e 100644 --- a/core/lib/Drupal/Core/Image/ImageInterface.php +++ b/core/lib/Drupal/Core/Image/ImageInterface.php @@ -109,6 +109,29 @@ public function apply($operation, array $arguments = array()); */ public function save($destination = NULL); + /** + * Prepares a new image, without loading it from a file. + * + * For a working example, see + * \Drupal\system\Plugin\ImageToolkit\Operation\gd\CreateNew. + * + * @param int $width + * The width of the new image, in pixels. + * @param int $height + * The height of the new image, in pixels. + * @param string $extension + * (Optional) The extension of the image file (e.g. 'png', 'gif', etc.). + * Allowed values depend on the implementation of the image toolkit. + * Defaults to 'png'. + * @param string $transparent_color + * (Optional) The hexadecimal string representing the color to be used + * for transparency, needed for GIF images. Defaults to '#ffffff' (white). + * + * @return bool + * TRUE on success, FALSE on failure. + */ + public function createNew($width, $height, $extension = 'png', $transparent_color = '#ffffff'); + /** * Scales an image while maintaining aspect ratio. * diff --git a/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php index 9bff2a4077c8..e4e9b9f6f4f5 100644 --- a/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php +++ b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php @@ -59,10 +59,10 @@ public function __construct(array $configuration, $plugin_id, array $plugin_defi /** * Returns the image toolkit instance for this operation. * - * Image toolkit implementers should provide a trait that overrides this - * method to correctly document the return type of this getter. This provides - * better DX (code checking and code completion) for image toolkit operation - * developers. + * Image toolkit implementers should provide a toolkit operation base class + * that overrides this method to correctly document the return type of this + * getter. This provides better DX (code checking and code completion) for + * image toolkit operation developers. * * @return \Drupal\Core\ImageToolkit\ImageToolkitInterface */ diff --git a/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php index 7d0a28c593ea..7aa56d03eafe 100644 --- a/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php +++ b/core/modules/system/src/Plugin/ImageToolkit/GDToolkit.php @@ -7,6 +7,7 @@ namespace Drupal\system\Plugin\ImageToolkit; +use Drupal\Component\Utility\Color; use Drupal\Component\Utility\Unicode; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\ImageToolkit\ImageToolkitBase; @@ -137,16 +138,21 @@ protected function load() { return TRUE; } else { - // Convert indexed images to true color, so that filters work - // correctly and don't result in unnecessary dither. - $new_image = $this->createTmp($this->getType(), imagesx($resource), imagesy($resource)); - if ($ret = (bool) $new_image) { - imagecopy($new_image, $resource, 0, 0, 0, 0, imagesx($resource), imagesy($resource)); + // Convert indexed images to truecolor, copying the image to a new + // truecolor resource, so that filters work correctly and don't result + // in unnecessary dither. + $data = array( + 'width' => imagesx($resource), + 'height' => imagesy($resource), + 'extension' => image_type_to_extension($this->getType(), FALSE), + 'transparent_color' => $this->getTransparentColor(), + ); + if ($this->apply('create_new', $data)) { + imagecopy($this->getResource(), $resource, 0, 0, 0, 0, imagesx($resource), imagesy($resource)); imagedestroy($resource); - $this->setResource($new_image); } - return $ret; } + return (bool) $this->getResource(); } return FALSE; } @@ -211,58 +217,35 @@ public function parseFile() { } /** - * Creates a truecolor image preserving transparency from a provided image. + * Gets the color set for transparency in GIF images. * - * @param int $type - * An image type represented by a PHP IMAGETYPE_* constant (e.g. - * IMAGETYPE_JPEG, IMAGETYPE_PNG, etc.). - * @param int $width - * The new width of the new image, in pixels. - * @param int $height - * The new height of the new image, in pixels. - * - * @return resource - * A GD image handle. + * @return string|null + * A color string like '#rrggbb', or NULL if not set or not relevant. */ - public function createTmp($type, $width, $height) { - $res = imagecreatetruecolor($width, $height); - - if ($type == IMAGETYPE_GIF) { - // Find out if a transparent color is set, will return -1 if no - // transparent color has been defined in the image. - $transparent = imagecolortransparent($this->getResource()); - if ($transparent >= 0) { - // Find out the number of colors in the image palette. It will be 0 for - // truecolor images. - $palette_size = imagecolorstotal($this->getResource()); - if ($palette_size == 0 || $transparent < $palette_size) { - // Set the transparent color in the new resource, either if it is a - // truecolor image or if the transparent color is part of the palette. - // Since the index of the transparency color is a property of the - // image rather than of the palette, it is possible that an image - // could be created with this index set outside the palette size (see - // http://stackoverflow.com/a/3898007). - $transparent_color = imagecolorsforindex($this->getResource(), $transparent); - $transparent = imagecolorallocate($res, $transparent_color['red'], $transparent_color['green'], $transparent_color['blue']); - - // Flood with our new transparent color. - imagefill($res, 0, 0, $transparent); - imagecolortransparent($res, $transparent); - } - } - } - elseif ($type == IMAGETYPE_PNG) { - imagealphablending($res, FALSE); - $transparency = imagecolorallocatealpha($res, 0, 0, 0, 127); - imagefill($res, 0, 0, $transparency); - imagealphablending($res, TRUE); - imagesavealpha($res, TRUE); + public function getTransparentColor() { + if (!$this->getResource() || $this->getType() != IMAGETYPE_GIF) { + return NULL; } - else { - imagefill($res, 0, 0, imagecolorallocate($res, 255, 255, 255)); + // Find out if a transparent color is set, will return -1 if no + // transparent color has been defined in the image. + $transparent = imagecolortransparent($this->getResource()); + if ($transparent >= 0) { + // Find out the number of colors in the image palette. It will be 0 for + // truecolor images. + $palette_size = imagecolorstotal($this->getResource()); + if ($palette_size == 0 || $transparent < $palette_size) { + // Return the transparent color, either if it is a truecolor image + // or if the transparent color is part of the palette. + // Since the index of the transparent color is a property of the + // image rather than of the palette, it is possible that an image + // could be created with this index set outside the palette size. + // (see http://stackoverflow.com/a/3898007). + $rgb = imagecolorsforindex($this->getResource(), $transparent); + unset($rgb['alpha']); + return Color::rgbToHex($rgb); + } } - - return $res; + return NULL; } /** diff --git a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Convert.php b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Convert.php index 49f7fbe87ed3..51d79aa97316 100644 --- a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Convert.php +++ b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Convert.php @@ -47,19 +47,29 @@ protected function validateArguments(array $arguments) { * {@inheritdoc} */ protected function execute(array $arguments) { - $type = $this->getToolkit()->extensionToImageType($arguments['extension']); - - $res = $this->getToolkit()->createTmp($type, $this->getToolkit()->getWidth(), $this->getToolkit()->getHeight()); - if (!imagecopyresampled($res, $this->getToolkit()->getResource(), 0, 0, 0, 0, $this->getToolkit()->getWidth(), $this->getToolkit()->getHeight(), $this->getToolkit()->getWidth(), $this->getToolkit()->getHeight())) { - return FALSE; + // Create a new resource of the required dimensions and format, and copy + // the original resource on it with resampling. Destroy the original + // resource upon success. + $width = $this->getToolkit()->getWidth(); + $height = $this->getToolkit()->getHeight(); + $original_resource = $this->getToolkit()->getResource(); + $original_type = $this->getToolkit()->getType(); + $data = array( + 'width' => $width, + 'height' => $height, + 'extension' => $arguments['extension'], + 'transparent_color' => $this->getToolkit()->getTransparentColor() + ); + if ($this->getToolkit()->apply('create_new', $data)) { + if (imagecopyresampled($this->getToolkit()->getResource(), $original_resource, 0, 0, 0, 0, $width, $height, $width, $height)) { + imagedestroy($original_resource); + return TRUE; + } + // In case of error, reset resource and type to as it was. + $this->getToolkit()->setResource($original_resource); + $this->getToolkit()->setType($original_type); } - imagedestroy($this->getToolkit()->getResource()); - - // Update the image object. - $this->getToolkit()->setType($type); - $this->getToolkit()->setResource($res); - - return TRUE; + return FALSE; } } diff --git a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/CreateNew.php b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/CreateNew.php new file mode 100644 index 000000000000..126f53cca20a --- /dev/null +++ b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/CreateNew.php @@ -0,0 +1,126 @@ +<?php + +/** + * @file + * Contains \Drupal\system\Plugin\ImageToolkit\Operation\gd\CreateNew. + */ + +namespace Drupal\system\Plugin\ImageToolkit\Operation\gd; + +use Drupal\Component\Utility\Color; +use Drupal\Component\Utility\String; + +/** + * Defines GD2 create_new image operation. + * + * @ImageToolkitOperation( + * id = "gd_create_new", + * toolkit = "gd", + * operation = "create_new", + * label = @Translation("Set a new image"), + * description = @Translation("Creates a new transparent resource and sets it for the image.") + * ) + */ +class CreateNew extends GDImageToolkitOperationBase { + + /** + * {@inheritdoc} + */ + protected function arguments() { + return array( + 'width' => array( + 'description' => 'The width of the image, in pixels', + ), + 'height' => array( + 'description' => 'The height of the image, in pixels', + ), + 'extension' => array( + 'description' => 'The extension of the image file (e.g. png, gif, etc.)', + 'required' => FALSE, + 'default' => 'png', + ), + 'transparent_color' => array( + 'description' => 'The RGB hex color for GIF transparency', + 'required' => FALSE, + 'default' => '#ffffff', + ), + ); + } + + /** + * {@inheritdoc} + */ + protected function validateArguments(array $arguments) { + // Assure extension is supported. + if (!in_array($arguments['extension'], $this->getToolkit()->getSupportedExtensions())) { + throw new \InvalidArgumentException(String::format("Invalid extension (@value) specified for the image 'convert' operation", array('@value' => $arguments['extension']))); + } + + // Assure integers for width and height. + $arguments['width'] = (int) round($arguments['width']); + $arguments['height'] = (int) round($arguments['height']); + + // Fail when width or height are 0 or negative. + if ($arguments['width'] <= 0) { + throw new \InvalidArgumentException(String::format("Invalid width (@value) specified for the image 'create_new' operation", array('@value' => $arguments['width']))); + } + if ($arguments['height'] <= 0) { + throw new \InvalidArgumentException(String::format("Invalid height (@value) specified for the image 'create_new' operation", array('@value' => $arguments['height']))); + } + + // Assure transparent color is a valid hex string. + if ($arguments['transparent_color'] && !Color::validateHex($arguments['transparent_color'])) { + throw new \InvalidArgumentException(String::format("Invalid transparent color (@value) specified for the image 'create_new' operation", array('@value' => $arguments['transparent_color']))); + } + + return $arguments; + } + + /** + * {@inheritdoc} + */ + protected function execute(array $arguments) { + // Get the image type. + $type = $this->getToolkit()->extensionToImageType($arguments['extension']); + + // Create the resource. + if (!$res = imagecreatetruecolor($arguments['width'], $arguments['height'])) { + return FALSE; + } + + // Fill the resource with transparency as possible. + switch ($type) { + case IMAGETYPE_PNG: + imagealphablending($res, FALSE); + $transparency = imagecolorallocatealpha($res, 0, 0, 0, 127); + imagefill($res, 0, 0, $transparency); + imagealphablending($res, TRUE); + imagesavealpha($res, TRUE); + break; + + case IMAGETYPE_GIF: + if (empty($arguments['transparent_color'])) { + // No transparency color specified, fill white. + $fill_color = imagecolorallocate($res, 255, 255, 255); + } + else { + $fill_rgb = Color::hexToRgb($arguments['transparent_color']); + $fill_color = imagecolorallocate($res, $fill_rgb['red'], $fill_rgb['green'], $fill_rgb['blue']); + imagecolortransparent($res, $fill_color); + } + imagefill($res, 0, 0, $fill_color); + break; + + case IMAGETYPE_JPEG: + imagefill($res, 0, 0, imagecolorallocate($res, 255, 255, 255)); + break; + + } + + // Update the toolkit properties. + $this->getToolkit()->setType($type); + $this->getToolkit()->setResource($res); + return TRUE; + } + +} diff --git a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Crop.php b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Crop.php index 95d8fd7c84d2..fbda1ec576f4 100644 --- a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Crop.php +++ b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Crop.php @@ -80,17 +80,23 @@ protected function validateArguments(array $arguments) { * {@inheritdoc} */ protected function execute(array $arguments) { - $res = $this->getToolkit()->createTmp($this->getToolkit()->getType(), $arguments['width'], $arguments['height']); - - if (!imagecopyresampled($res, $this->getToolkit()->getResource(), 0, 0, $arguments['x'], $arguments['y'], $arguments['width'], $arguments['height'], $arguments['width'], $arguments['height'])) { - return FALSE; + // Create a new resource of the required dimensions, and copy and resize + // the original resource on it with resampling. Destroy the original + // resource upon success. + $original_resource = $this->getToolkit()->getResource(); + $data = array( + 'width' => $arguments['width'], + 'height' => $arguments['height'], + 'extension' => image_type_to_extension($this->getToolkit()->getType(), FALSE), + 'transparent_color' => $this->getToolkit()->getTransparentColor() + ); + if ($this->getToolkit()->apply('create_new', $data)) { + if (imagecopyresampled($this->getToolkit()->getResource(), $original_resource, 0, 0, $arguments['x'], $arguments['y'], $arguments['width'], $arguments['height'], $arguments['width'], $arguments['height'])) { + imagedestroy($original_resource); + return TRUE; + } } - - // Destroy the original image and return the modified image. - imagedestroy($this->getToolkit()->getResource()); - $this->getToolkit()->setResource($res); - - return TRUE; + return FALSE; } } diff --git a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Resize.php b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Resize.php index bc192c756045..a5f63622b349 100644 --- a/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Resize.php +++ b/core/modules/system/src/Plugin/ImageToolkit/Operation/gd/Resize.php @@ -59,17 +59,23 @@ protected function validateArguments(array $arguments) { * {@inheritdoc} */ protected function execute(array $arguments = array()) { - $res = $this->getToolkit()->createTmp($this->getToolkit()->getType(), $arguments['width'], $arguments['height']); - - if (!imagecopyresampled($res, $this->getToolkit()->getResource(), 0, 0, 0, 0, $arguments['width'], $arguments['height'], $this->getToolkit()->getWidth(), $this->getToolkit()->getHeight())) { - return FALSE; + // Create a new resource of the required dimensions, and copy and resize + // the original resource on it with resampling. Destroy the original + // resource upon success. + $original_resource = $this->getToolkit()->getResource(); + $data = array( + 'width' => $arguments['width'], + 'height' => $arguments['height'], + 'extension' => image_type_to_extension($this->getToolkit()->getType(), FALSE), + 'transparent_color' => $this->getToolkit()->getTransparentColor() + ); + if ($this->getToolkit()->apply('create_new', $data)) { + if (imagecopyresampled($this->getToolkit()->getResource(), $original_resource, 0, 0, 0, 0, $arguments['width'], $arguments['height'], imagesx($original_resource), imagesy($original_resource))) { + imagedestroy($original_resource); + return TRUE; + } } - - imagedestroy($this->getToolkit()->getResource()); - // Update image object. - $this->getToolkit()->setResource($res); - - return TRUE; + return FALSE; } } diff --git a/core/modules/system/src/Tests/Image/ToolkitGdTest.php b/core/modules/system/src/Tests/Image/ToolkitGdTest.php index 4639e4210847..a65e683dd4c0 100644 --- a/core/modules/system/src/Tests/Image/ToolkitGdTest.php +++ b/core/modules/system/src/Tests/Image/ToolkitGdTest.php @@ -252,6 +252,10 @@ function testManipulations() { ); } + // Prepare a directory for test file results. + $directory = $this->public_files_directory .'/imagetest'; + file_prepare_directory($directory, FILE_CREATE_DIRECTORY); + foreach ($files as $file) { foreach ($operations as $op => $values) { // Load up a fresh image. @@ -305,8 +309,6 @@ function testManipulations() { $correct_dimensions_object = FALSE; } - $directory = $this->public_files_directory .'/imagetest'; - file_prepare_directory($directory, FILE_CREATE_DIRECTORY); $file_path = $directory . '/' . $op . image_type_to_extension($image->getToolkit()->getType()); $image->save($file_path); @@ -358,6 +360,45 @@ function testManipulations() { $resource = $image_reloaded->getToolkit()->getResource(); } } + + // Test creation of image from scratch, and saving to storage. + foreach (array(IMAGETYPE_PNG, IMAGETYPE_GIF, IMAGETYPE_JPEG) as $type) { + $image = $this->imageFactory->get(); + $image->createNew(50, 20, image_type_to_extension($type, FALSE), '#ffff00'); + $file = 'from_null' . image_type_to_extension($type); + $file_path = $directory . '/' . $file ; + $this->assertEqual(50, $image->getWidth(), String::format('Image file %file has the correct width.', array('%file' => $file))); + $this->assertEqual(20, $image->getHeight(), String::format('Image file %file has the correct height.', array('%file' => $file))); + $this->assertEqual(image_type_to_mime_type($type), $image->getMimeType(), String::format('Image file %file has the correct MIME type.', array('%file' => $file))); + $this->assertTrue($image->save($file_path), String::format('Image %file created anew from a null image was saved.', array('%file' => $file))); + + // Reload saved image. + $image_reloaded = $this->imageFactory->get($file_path); + if (!$image_reloaded->isValid()) { + $this->fail(String::format('Could not load image %file.', array('%file' => $file))); + continue; + } + $this->assertEqual(50, $image_reloaded->getWidth(), String::format('Image file %file has the correct width.', array('%file' => $file))); + $this->assertEqual(20, $image_reloaded->getHeight(), String::format('Image file %file has the correct height.', array('%file' => $file))); + $this->assertEqual(image_type_to_mime_type($type), $image_reloaded->getMimeType(), String::format('Image file %file has the correct MIME type.', array('%file' => $file))); + if ($image_reloaded->getToolkit()->getType() == IMAGETYPE_GIF) { + $this->assertEqual('#ffff00', $image_reloaded->getToolkit()->getTransparentColor(), String::format('Image file %file has the correct transparent color channel set.', array('%file' => $file))); + } + else { + $this->assertEqual(NULL, $image_reloaded->getToolkit()->getTransparentColor(), String::format('Image file %file has no color channel set.', array('%file' => $file))); + } + } + + // Test failures of CreateNew. + $image = $this->imageFactory->get(); + $image->createNew(-50, 20); + $this->assertFalse($image->isValid(), 'CreateNew with negative width fails.'); + $image->createNew(50, 20, 'foo'); + $this->assertFalse($image->isValid(), 'CreateNew with invalid extension fails.'); + $image->createNew(50, 20, 'gif', '#foo'); + $this->assertFalse($image->isValid(), 'CreateNew with invalid color hex string fails.'); + $image->createNew(50, 20, 'gif', '#ff0000'); + $this->assertTrue($image->isValid(), 'CreateNew with valid arguments validates the Image.'); } /** diff --git a/core/modules/system/src/Tests/Image/ToolkitTestBase.php b/core/modules/system/src/Tests/Image/ToolkitTestBase.php index 9b295f7a3304..44b8437376d9 100644 --- a/core/modules/system/src/Tests/Image/ToolkitTestBase.php +++ b/core/modules/system/src/Tests/Image/ToolkitTestBase.php @@ -88,6 +88,7 @@ function assertToolkitOperationsCalled(array $expected) { 'rotate', 'crop', 'desaturate', + 'create_new', 'scale', 'scale_and_crop', 'my_operation', @@ -135,6 +136,7 @@ function imageTestReset() { 'rotate' => array(), 'crop' => array(), 'desaturate' => array(), + 'create_new' => array(), 'scale' => array(), 'scale_and_crop' => array(), 'convert' => array(), -- GitLab