Commit 42da7fa4 authored by alexpott's avatar alexpott

Issue #2531678 by mondrake, MattA: The 'create_new' and 'rotate' image...

Issue #2531678 by mondrake, MattA: The 'create_new' and 'rotate' image operations do not release memory
parent 2e198fda
......@@ -196,6 +196,7 @@ protected function load() {
'height' => imagesy($resource),
'extension' => image_type_to_extension($this->getType(), FALSE),
'transparent_color' => $this->getTransparentColor(),
'is_temp' => TRUE,
);
if ($this->apply('create_new', $data)) {
imagecopy($this->getResource(), $resource, 0, 0, 0, 0, imagesx($resource), imagesy($resource));
......
......@@ -56,7 +56,8 @@ protected function execute(array $arguments) {
'width' => $width,
'height' => $height,
'extension' => $arguments['extension'],
'transparent_color' => $this->getToolkit()->getTransparentColor()
'transparent_color' => $this->getToolkit()->getTransparentColor(),
'is_temp' => TRUE,
);
if ($this->getToolkit()->apply('create_new', $data)) {
if (imagecopyresampled($this->getToolkit()->getResource(), $original_resource, 0, 0, 0, 0, $width, $height, $width, $height)) {
......
......@@ -43,6 +43,11 @@ protected function arguments() {
'required' => FALSE,
'default' => '#ffffff',
),
'is_temp' => array(
'description' => 'If TRUE, this operation is being used to create a temporary image by another GD operation. After performing its function, the caller is responsible for destroying the original GD resource.',
'required' => FALSE,
'default' => FALSE,
),
);
}
......@@ -82,6 +87,9 @@ protected function execute(array $arguments) {
// Get the image type.
$type = $this->getToolkit()->extensionToImageType($arguments['extension']);
// Store the original GD resource.
$original_res = $this->getToolkit()->getResource();
// Create the resource.
if (!$res = imagecreatetruecolor($arguments['width'], $arguments['height'])) {
return FALSE;
......@@ -119,6 +127,12 @@ protected function execute(array $arguments) {
// Update the toolkit properties.
$this->getToolkit()->setType($type);
$this->getToolkit()->setResource($res);
// Destroy the original resource if it is not needed by other operations.
if (!$arguments['is_temp'] && is_resource($original_res)) {
imagedestroy($original_res);
}
return TRUE;
}
......
......@@ -86,13 +86,20 @@ protected function execute(array $arguments) {
'width' => $arguments['width'],
'height' => $arguments['height'],
'extension' => image_type_to_extension($this->getToolkit()->getType(), FALSE),
'transparent_color' => $this->getToolkit()->getTransparentColor()
'transparent_color' => $this->getToolkit()->getTransparentColor(),
'is_temp' => TRUE,
);
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;
}
else {
// In case of failure, destroy the temporary resource and restore
// the original one.
imagedestroy($this->getToolkit()->getResource());
$this->getToolkit()->setResource($original_resource);
}
}
return FALSE;
}
......
......@@ -65,13 +65,20 @@ protected function execute(array $arguments = array()) {
'width' => $arguments['width'],
'height' => $arguments['height'],
'extension' => image_type_to_extension($this->getToolkit()->getType(), FALSE),
'transparent_color' => $this->getToolkit()->getTransparentColor()
'transparent_color' => $this->getToolkit()->getTransparentColor(),
'is_temp' => TRUE,
);
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;
}
else {
// In case of failure, destroy the temporary resource and restore
// the original one.
imagedestroy($this->getToolkit()->getResource());
$this->getToolkit()->setResource($original_resource);
}
}
return FALSE;
}
......
......@@ -98,17 +98,25 @@ protected function execute(array $arguments) {
return FALSE;
}
$this->getToolkit()->setResource(imagerotate($this->getToolkit()->getResource(), 360 - $arguments['degrees'], $arguments['background_idx']));
// GIFs need to reassign the transparent color after performing the rotate,
// but only do so, if the image already had transparency of its own, or the
// rotate added a transparent background.
if (!empty($arguments['gif_transparent_color'])) {
$transparent_idx = imagecolorexactalpha($this->getToolkit()->getResource(), $arguments['gif_transparent_color']['red'], $arguments['gif_transparent_color']['green'], $arguments['gif_transparent_color']['blue'], $arguments['gif_transparent_color']['alpha']);
imagecolortransparent($this->getToolkit()->getResource(), $transparent_idx);
// Stores the original GD resource.
$original_res = $this->getToolkit()->getResource();
if ($new_res = imagerotate($this->getToolkit()->getResource(), 360 - $arguments['degrees'], $arguments['background_idx'])) {
$this->getToolkit()->setResource($new_res);
imagedestroy($original_res);
// GIFs need to reassign the transparent color after performing the
// rotate, but only do so, if the image already had transparency of its
// own, or the rotate added a transparent background.
if (!empty($arguments['gif_transparent_color'])) {
$transparent_idx = imagecolorexactalpha($this->getToolkit()->getResource(), $arguments['gif_transparent_color']['red'], $arguments['gif_transparent_color']['green'], $arguments['gif_transparent_color']['blue'], $arguments['gif_transparent_color']['alpha']);
imagecolortransparent($this->getToolkit()->getResource(), $transparent_idx);
}
return TRUE;
}
return TRUE;
return FALSE;
}
}
......@@ -278,9 +278,19 @@ function testManipulations() {
}
}
// Store the original GD resource.
$old_res = $toolkit->getResource();
// Perform our operation.
$image->apply($values['function'], $values['arguments']);
// If the operation replaced the resource, check that the old one has
// been destroyed.
$new_res = $toolkit->getResource();
if ($new_res !== $old_res) {
$this->assertFalse(is_resource($old_res), SafeMarkup::format("'%operation' destroyed the original resource.", ['%operation' => $values['function']]));
}
// To keep from flooding the test with assert values, make a general
// value for whether each group of values fail.
$correct_dimensions_real = TRUE;
......@@ -389,7 +399,7 @@ function testManipulations() {
}
}
// Test failures of CreateNew.
// Test failures of the 'create_new' operation.
$image = $this->imageFactory->get();
$image->createNew(-50, 20);
$this->assertFalse($image->isValid(), 'CreateNew with negative width fails.');
......@@ -405,12 +415,24 @@ function testManipulations() {
* Tests that GD resources are freed from memory.
*/
public function testResourceDestruction() {
// Test that an Image object going out of scope releases its GD resource.
$image = $this->imageFactory->get(drupal_get_path('module', 'simpletest') . '/files/image-test.png');
$res = $image->getToolkit()->getResource();
$this->assertTrue(is_resource($res), 'Successfully loaded image resource.');
// Force the toolkit to go out of scope.
$image = NULL;
$this->assertFalse(is_resource($res), 'Image resource was destroyed after losing scope.');
// Test that 'create_new' operation does not leave orphaned GD resources.
$image = $this->imageFactory->get(drupal_get_path('module', 'simpletest') . '/files/image-test.png');
$old_res = $image->getToolkit()->getResource();
// Check if resource has been created successfully.
$this->assertTrue(is_resource($old_res));
$image->createNew(20, 20);
$new_res = $image->getToolkit()->getResource();
// Check if the original resource has been destroyed.
$this->assertFalse(is_resource($old_res));
// Check if a new resource has been created successfully.
$this->assertTrue(is_resource($new_res));
}
/**
......
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