Merge branch 'master' into talmdal

6 files changed, 54 insertions, 35 deletions

diff --git a/modules/gallery/helpers/gallery_task.php b/modules/gallery/helpers/gallery_task.php
index c9557324..1b56ab97 100644
--- a/modules/gallery/helpers/gallery_task.php
+++ b/modules/gallery/helpers/gallery_task.php
@@ -60,14 +60,15 @@ class gallery_task_Core {
 
         $item = ORM::factory("item", $row->id);
         if ($item->loaded) {
-          $success = graphics::generate($item);
-          if (!$success) {
+          try {
+            graphics::generate($item);
             $ignored[$item->id] = 1;
-            $errors[] = t("Unable to rebuild images for '%title'",
-                          array("title" => html::purify($item->title)));
-          } else {
             $errors[] = t("Successfully rebuilt images for '%title'",
                           array("title" => html::purify($item->title)));
+          } catch (Exception $e) {
+            $errors[] = t("Unable to rebuild images for '%title'",
+                          array("title" => html::purify($item->title)));
+            $errors[] = $e->__toString();
           }
         }
 
diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php
index 521dc5a4..78812794 100644
--- a/modules/gallery/helpers/graphics.php
+++ b/modules/gallery/helpers/graphics.php
@@ -102,12 +102,12 @@ class graphics_Core {
   /**
    * Rebuild the thumb and resize for the given item.
    * @param Item_Model $item
-   * @return true on successful generation
    */
   static function generate($item) {
     if ($item->is_album()) {
       if (!$cover = $item->album_cover()) {
-        return false;
+        // This album has no cover; there's nothing to generate.
+        return;
       }
       $input_file = $cover->file_path();
       $input_item = $cover;
@@ -127,7 +127,7 @@ class graphics_Core {
       $item->thumb_dirty = 0;
       $item->resize_dirty = 0;
       $item->save();
-      return true;
+      return;
     }
 
     try {
@@ -176,10 +176,8 @@ class graphics_Core {
       // @todo we should handle this better.
       Kohana::log("error", "Caught exception rebuilding image: {$item->title}\n" .
                   $e->getMessage() . "\n" . $e->getTraceAsString());
-      return false;
+      throw $e;
     }
-
-    return true;
   }
 
   /**
@@ -206,11 +204,13 @@ class graphics_Core {
       // Image would get upscaled; do nothing
       copy($input_file, $output_file);
     } else {
-      Image::factory($input_file)
+      $image = Image::factory($input_file)
         ->resize($options["width"], $options["height"], $options["master"])
-        ->quality(module::get_var("gallery", "image_quality"))
-        ->sharpen(module::get_var("gallery", "image_sharpen"))
-        ->save($output_file);
+        ->quality(module::get_var("gallery", "image_quality"));
+      if (graphics::can("sharpen")) {
+        $image->sharpen(module::get_var("gallery", "image_sharpen"));
+      }
+      $image->save($output_file);
     }
 
     module::event("graphics_resize_completed", $input_file, $output_file, $options);
@@ -352,13 +352,22 @@ class graphics_Core {
       $toolkits->gd->installed = true;
       $toolkits->gd->version = $gd["GD Version"];
       $toolkits->gd->rotate = function_exists("imagerotate");
+      $toolkits->gd->sharpen = function_exists("imageconvolution");
       $toolkits->gd->binary = "";
       $toolkits->gd->dir = "";
 
-      if (!$toolkits->gd->rotate) {
+      if (!$toolkits->gd->rotate && !$toolkits->gd->sharpen) {
+        $toolkits->gd->error =
+          t("You have GD version %version, but it lacks image rotation and sharpening.",
+            array("version" => $gd["GD Version"]));
+      } else if (!$toolkits->gd->rotate) {
         $toolkits->gd->error =
           t("You have GD version %version, but it lacks image rotation.",
             array("version" => $gd["GD Version"]));
+      } else if (!$toolkits->gd->sharpen) {
+        $toolkits->gd->error =
+          t("You have GD version %version, but it lacks image sharpening.",
+            array("version" => $gd["GD Version"]));
       }
     }
 
@@ -387,6 +396,7 @@ class graphics_Core {
           $toolkits->imagemagick->binary = $path;
           $toolkits->imagemagick->dir = dirname($path);
           $toolkits->imagemagick->rotate = true;
+          $toolkits->imagemagick->sharpen = true;
         } else {
           $toolkits->imagemagick->installed = false;
           $toolkits->imagemagick->error =
@@ -411,6 +421,7 @@ class graphics_Core {
           $toolkits->graphicsmagick->binary = $path;
           $toolkits->graphicsmagick->dir = dirname($path);
           $toolkits->graphicsmagick->rotate = true;
+          $toolkits->graphicsmagick->sharpen = true;
         } else {
           $toolkits->graphicsmagick->installed = false;
           $toolkits->graphicsmagick->error =
@@ -475,14 +486,18 @@ class graphics_Core {
 
   /**
    * Verify that a specific graphics function is available with the active toolkit.
-   * @param  string  $func (eg rotate, resize)
+   * @param  string  $func (eg rotate, sharpen)
    * @return boolean
    */
   static function can($func) {
-    if (module::get_var("gallery", "graphics_toolkit") == "gd" &&
-        $func == "rotate" &&
-        !function_exists("imagerotate")) {
-      return false;
+    if (module::get_var("gallery", "graphics_toolkit") == "gd") {
+      switch ($func) {
+      case "rotate":
+        return function_exists("imagerotate");
+
+      case "sharpen":
+        return function_exists("imageconvolution");
+      }
     }
 
     return true;
diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php
index 96a66d29..40b645a2 100644
--- a/modules/gallery/helpers/photo.php
+++ b/modules/gallery/helpers/photo.php
@@ -111,9 +111,11 @@ class photo_Core {
 
     // Build our thumbnail/resizes.  If we fail to build thumbnail/resize we assume that the image
     // is bad in some way and discard it.
-    if (!graphics::generate($photo)) {
+    try {
+      graphics::generate($photo);
+    } catch (Exception $e) {
       $photo->delete();
-      throw new Exception("@todo BAD_IMAGE_FILE");
+      throw $e;
     }
 
     // If the parent has no cover item, make this it.
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 68e89db6..8905a627 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -188,9 +188,8 @@ class Item_Model extends ORM_MPTT {
    * photo: http://example.com/gallery3/var/albums/album1/photo.jpg
    */
   public function file_url($full_uri=false) {
-    return $full_uri ?
-      url::abs_file("var/albums/" . $this->relative_path()) :
-      url::file("var/albums/" . $this->relative_path());
+    $relative_path = "var/albums/" . $this->relative_path();
+    return $full_uri ? url::abs_file($relative_path) : url::file($relative_path);
   }
 
   /**
@@ -222,9 +221,8 @@ class Item_Model extends ORM_MPTT {
    */
   public function thumb_url($full_uri=false) {
     $cache_buster = "?m=" . $this->updated;
-    $base = ($full_uri ?
-             url::abs_file("var/thumbs/" . $this->relative_path()) :
-             url::file("var/thumbs/" . $this->relative_path()));
+    $relative_path = "var/thumbs/" . $this->relative_path();
+    $base = ($full_uri ? url::abs_file($relative_path) : url::file($relative_path));
     if ($this->is_photo()) {
       return $base . $cache_buster;
     } else if ($this->is_album()) {
@@ -250,9 +248,8 @@ class Item_Model extends ORM_MPTT {
    * photo: http://example.com/gallery3/var/albums/album1/photo.resize.jpg
    */
   public function resize_url($full_uri=false) {
-    return ($full_uri ?
-            url::abs_file("var/resizes/" . $this->relative_path()) :
-            url::file("var/resizes/" . $this->relative_path())) .
+    $relative_path = "var/resizes/" . $this->relative_path();
+    return ($full_uri ? url::abs_file($relative_path) : url::file($relative_path)) .
       ($this->is_album() ? "/.album.jpg" : "");
   }
 
diff --git a/modules/gallery/tests/Photos_Controller_Test.php b/modules/gallery/tests/Photos_Controller_Test.php
index f7d3f72f..5c4802c2 100644
--- a/modules/gallery/tests/Photos_Controller_Test.php
+++ b/modules/gallery/tests/Photos_Controller_Test.php
@@ -33,8 +33,8 @@ class Photos_Controller_Test extends Unit_Test_Case {
   public function change_photo_test() {
     $controller = new Photos_Controller();
     $root = ORM::factory("item", 1);
-    $this->_photo = photo::create($root, MODPATH . "gallery/tests/test.jpg", "test.jpeg", "test",
-                                  "test");
+    $this->_photo = photo::create(
+      $root, MODPATH . "gallery/tests/test.jpg", "test.jpeg", "test", "test");
     $orig_name = $this->_photo->name;
 
     $_POST["filename"] = "test.jpeg";
@@ -63,7 +63,8 @@ class Photos_Controller_Test extends Unit_Test_Case {
   public function change_photo_no_csrf_fails_test() {
     $controller = new Photos_Controller();
     $root = ORM::factory("item", 1);
-    $this->_photo = photo::create($root, MODPATH . "gallery/tests/test.jpg", "test", "test", "test");
+    $this->_photo = photo::create(
+      $root, MODPATH . "gallery/tests/test.jpg", "test.jpg", "test", "test");
     $_POST["name"] = "new name";
     $_POST["title"] = "new title";
     $_POST["description"] = "new description";
diff --git a/modules/gallery_unit_test/controllers/gallery_unit_test.php b/modules/gallery_unit_test/controllers/gallery_unit_test.php
index 8f3353dc..a5dbcc1f 100644
--- a/modules/gallery_unit_test/controllers/gallery_unit_test.php
+++ b/modules/gallery_unit_test/controllers/gallery_unit_test.php
@@ -122,6 +122,9 @@ class Gallery_Unit_Test_Controller extends Controller {
         module::activate($module_name);
       }
 
+      // Trigger late-binding install actions (defined in gallery_event::user_login)
+      graphics::choose_default_toolkit();
+
       $filter = count($_SERVER["argv"]) > 2 ? $_SERVER["argv"][2] : null;
       print new Unit_Test($modules, $filter);
     } catch (Exception $e) {