From d1fb290f4e13496f8fb10bf3f538fdfd1d740b42 Mon Sep 17 00:00:00 2001 From: "Michael J. Rubinsky" Date: Sat, 20 Feb 2010 14:59:56 -0500 Subject: [PATCH] Remove the $id parameter from loadString and loadFile It's not necessary, client code should know that it has loaded data, it was causing all kinds of problems with Image effects, and takes up resources. --- ansel/lib/Faces/Base.php | 2 +- ansel/lib/Image.php | 25 +++++++-------- framework/Image/lib/Horde/Image/Base.php | 36 ++++++---------------- .../lib/Horde/Image/Effect/Imagick/PhotoStack.php | 2 +- framework/Image/lib/Horde/Image/Gd.php | 7 ++--- framework/Image/lib/Horde/Image/Im.php | 2 +- framework/Image/lib/Horde/Image/Imagick.php | 6 ++-- horde/services/images/view.php | 2 +- imp/lib/Mime/Viewer/Images.php | 2 +- imp/lib/Mime/Viewer/Pdf.php | 2 +- 10 files changed, 33 insertions(+), 53 deletions(-) diff --git a/ansel/lib/Faces/Base.php b/ansel/lib/Faces/Base.php index 16d2083c4..648b0a219 100644 --- a/ansel/lib/Faces/Base.php +++ b/ansel/lib/Faces/Base.php @@ -380,7 +380,7 @@ class Ansel_Faces_Base if ($data instanceof PEAR_Error) { throw new Horde_Exception_Prior($data); } - $img->loadString($face_id, $data); + $img->loadString($data); return $img; } diff --git a/ansel/lib/Image.php b/ansel/lib/Image.php index 6944c8044..696de4a9a 100644 --- a/ansel/lib/Image.php +++ b/ansel/lib/Image.php @@ -284,7 +284,7 @@ class Ansel_Image Implements Iterator // already have the full data loaded. If we auto-rotate the image // then there is no need to save it just to load it again. if ($view == 'full' && !empty($this->_data['full'])) { - $this->_image->loadString('original', $this->_data['full']); + $this->_image->loadString($this->_data['full']); $this->_loaded['full'] = true; return true; } @@ -302,9 +302,6 @@ class Ansel_Image Implements Iterator return; } - /* We've definitely successfully loaded the image now. */ - $this->_loaded[$viewHash] = true; - /* Get the VFS info. */ $vfspath = $this->getVFSPath($view, $style); @@ -315,8 +312,11 @@ class Ansel_Image Implements Iterator throw new Ansel_Exception($data); } + /* We've definitely successfully loaded the image now. */ + $this->_loaded[$viewHash] = true; $this->_data[$viewHash] = $data; - $this->_image->loadString($vfspath . '/' . $this->id, $data); + $this->_image->loadString($data); + return true; } @@ -392,7 +392,9 @@ class Ansel_Image Implements Iterator Horde::logMessage($data, __FILE__, __LINE__, PEAR_LOG_ERR); throw new Ansel_Exception($data); } - $this->_image->loadString($this->getVFSPath('full') . '/' . $this->id, $data); + + $vHash = $this->getViewHash($view, $style); + $this->_image->loadString($data); $styleDef = Ansel::getStyleDefinition($style); if ($view == 'prettythumb') { $viewType = $styleDef['thumbstyle']; @@ -419,17 +421,16 @@ class Ansel_Image Implements Iterator $iview->create(); /* Cache the data from the new imageview */ - $view = $this->getViewHash($view, $style); try { - $this->_data[$view] = $this->_image->raw(); + $this->_data[$vHash] = $this->_image->raw(); } catch (Horde_Image_Exception $e) { throw new Ansel_Exception($e); } /* ...and put it in Horde_Image obejct, then save */ - $this->_image->loadString($vfspath . '/' . $this->id, $this->_data[$view]); - $this->_loaded[$view] = true; - $GLOBALS['ansel_vfs']->writeData($vfspath, $this->getVFSName($view), $this->_data[$view], true); + $this->_image->loadString($this->_data[$vHash]); + $this->_loaded[$vHash] = true; + $GLOBALS['ansel_vfs']->writeData($vfspath, $this->getVFSName($vHash), $this->_data[$vHash], true); /* Autowatermark the screen view */ if ($view == 'screen' && @@ -1089,7 +1090,7 @@ class Ansel_Image Implements Iterator try { $this->_image->addEffect($type, $params); } catch (Horde_Image_Exception $e) { - Horde::logMessage($e->getMessage(), __FILE__, __LINE__, PEAR_LOG_DEBUG); + Horde::logMessage($e->getMessage(), __FILE__, __LINE__, PEAR_LOG_ERR); throw new Ansel_Exception($e); } } diff --git a/framework/Image/lib/Horde/Image/Base.php b/framework/Image/lib/Horde/Image/Base.php index 0e97750a8..0f9b74874 100644 --- a/framework/Image/lib/Horde/Image/Base.php +++ b/framework/Image/lib/Horde/Image/Base.php @@ -41,16 +41,6 @@ abstract class Horde_Image_Base Implements Iterator protected $_data = ''; /** - * The current image id. - * - * @TODO: Do we *really* need an image id...and if so, we can make the - * parameter optional in the methods that take one? - * - * @var string - */ - protected $_id = ''; - - /** * Logger */ protected $_logger; @@ -224,7 +214,6 @@ abstract class Horde_Image_Base Implements Iterator public function reset() { $this->_data = ''; - $this->_id = ''; $this->_width = null; $this->_height = null; $this->_background = 'white'; @@ -256,13 +245,10 @@ abstract class Horde_Image_Base Implements Iterator * @param string $id An arbitrary id for the image. * @param string $image_data The data to use for the image. */ - public function loadString($id, $image_data) + public function loadString($image_data) { - if ($id != $this->_id) { - $this->reset(); - $this->_data = $image_data; - $this->_id = $id; - } + $this->reset(); + $this->_data = $image_data; } /** @@ -278,16 +264,12 @@ abstract class Horde_Image_Base Implements Iterator */ public function loadFile($filename) { - if ($filename != $this->_id) { - $this->reset(); - if (!file_exists($filename)) { - throw new Horde_Image_Exception(sprintf("The image file, %s, does not exist.", $filename)); - } - if ($this->_data = file_get_contents($filename)) { - $this->_id = $filename; - } else { - throw new Horde_Image_Exception(sprintf("Could not load the image file %s", $filename)); - } + $this->reset(); + if (!file_exists($filename)) { + throw new Horde_Image_Exception(sprintf("The image file, %s, does not exist.", $filename)); + } + if (!$this->_data = file_get_contents($filename)) { + throw new Horde_Image_Exception(sprintf("Could not load the image file %s", $filename)); } return true; diff --git a/framework/Image/lib/Horde/Image/Effect/Imagick/PhotoStack.php b/framework/Image/lib/Horde/Image/Effect/Imagick/PhotoStack.php index 3f67e5863..60f594dab 100644 --- a/framework/Image/lib/Horde/Image/Effect/Imagick/PhotoStack.php +++ b/framework/Image/lib/Horde/Image/Effect/Imagick/PhotoStack.php @@ -201,7 +201,7 @@ class Horde_Image_Effect_Imagick_PhotoStack extends Horde_Image_Effect $context = array('tmpdir' => $this->_image->getTmpDir()); $size = $image->getImageGeometry(); $new = Horde_Image::factory('Imagick', array('context' => $context)); - $new->loadString('somestring', $image->getImageBlob()); + $new->loadString($image->getImageBlob()); $image->destroy(); $new->addEffect('RoundCorners', array('border' => 2, 'bordercolor' => '#111')); $new->applyEffects(); diff --git a/framework/Image/lib/Horde/Image/Gd.php b/framework/Image/lib/Horde/Image/Gd.php index 7e6bd801f..a5cee5bfc 100644 --- a/framework/Image/lib/Horde/Image/Gd.php +++ b/framework/Image/lib/Horde/Image/Gd.php @@ -175,12 +175,9 @@ class Horde_Image_Gd extends Horde_Image_Base * @param string $id An arbitrary id for the image. * @param string $image_data The data to use for the image. */ - public function loadString($id, $image_data) + public function loadString($image_data) { - if ($id != $this->_id) { - $this->_im = $this->_call('imageCreateFromString', array($image_data)); - $this->_id = $id; - } + $this->_im = $this->_call('imageCreateFromString', array($image_data)); } /** diff --git a/framework/Image/lib/Horde/Image/Im.php b/framework/Image/lib/Horde/Image/Im.php index 2ac9435f1..439af8dda 100644 --- a/framework/Image/lib/Horde/Image/Im.php +++ b/framework/Image/lib/Horde/Image/Im.php @@ -101,7 +101,7 @@ class Horde_Image_Im extends Horde_Image_Base if (!empty($params['filename'])) { $this->loadFile($params['filename']); } elseif (!empty($params['data'])) { - $this->loadString(md5($params['data']), $params['data']); + $this->loadString($params['data']); } else { $cmd = "-size {$this->_width}x{$this->_height} xc:{$this->_background} +profile \"*\" {$this->_type}:__FILEOUT__"; $this->executeConvertCmd($cmd); diff --git a/framework/Image/lib/Horde/Image/Imagick.php b/framework/Image/lib/Horde/Image/Imagick.php index 8db9ea3b0..0e61c1b66 100644 --- a/framework/Image/lib/Horde/Image/Imagick.php +++ b/framework/Image/lib/Horde/Image/Imagick.php @@ -60,7 +60,7 @@ class Horde_Image_Imagick extends Horde_Image_Base if (!empty($params['filename'])) { $this->loadFile($params['filename']); } elseif(!empty($params['data'])) { - $this->loadString(md5($params['data']), $params['data']); + $this->loadString($params['data']); } else { $this->_width = max(array($this->_width, 1)); $this->_height = max(array($this->_height, 1)); @@ -82,9 +82,9 @@ class Horde_Image_Imagick extends Horde_Image_Base * * @return void */ - public function loadString($id, $image_data) + public function loadString($image_data) { - parent::loadString($id, $image_data); + parent::loadString($image_data); $this->_imagick->clear(); try { $this->_imagick->readImageBlob($image_data); diff --git a/horde/services/images/view.php b/horde/services/images/view.php index add8ba2af..011f0612a 100644 --- a/horde/services/images/view.php +++ b/horde/services/images/view.php @@ -74,7 +74,7 @@ if (!empty($conf['image']['convert'])) { $context['identify'] = $conf['image']['identify']; } $image = Horde_Image::factory($driver, array('context' => $context)); -$image->loadString($file, $file_data); +$image->loadString($file_data); /* Check if no editing action required and send the image to browser. */ if (empty($action)) { diff --git a/imp/lib/Mime/Viewer/Images.php b/imp/lib/Mime/Viewer/Images.php index d12763bc4..9d46bc160 100644 --- a/imp/lib/Mime/Viewer/Images.php +++ b/imp/lib/Mime/Viewer/Images.php @@ -272,7 +272,7 @@ EOD; if ($load) { try { - $ret = $img->loadString(1, $this->_mimepart->getContents()); + $ret = $img->loadString($this->_mimepart->getContents()); } catch (Horde_Image_Exception $e) { return false; } diff --git a/imp/lib/Mime/Viewer/Pdf.php b/imp/lib/Mime/Viewer/Pdf.php index 39c63697b..db1c1d06c 100644 --- a/imp/lib/Mime/Viewer/Pdf.php +++ b/imp/lib/Mime/Viewer/Pdf.php @@ -126,7 +126,7 @@ class IMP_Horde_Mime_Viewer_Pdf extends Horde_Mime_Viewer_Pdf if ($load) { try { - $ret = $img->loadString(1, $this->_mimepart->getContents()); + $ret = $img->loadString($this->_mimepart->getContents()); } catch (Horde_Image_Exception $e) { return false; } -- 2.11.0