Improve image thumbnailing and increase the size of Macro thumbnails

Summary:
Alternate proposal for D3635.

  - Works better with small images.
  - Produces a predictable thumbnail size.
  - Somewhat reasonable output on 3000x10 images.
  - Increase the size of Macro thumbnails to 240px.

Test Plan: {F20497}

Reviewers: vrana, chad

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D3638

src/applications/files/PhabricatorImageTransformer.php

@@ -47,6 +47,20 @@ final class PhabricatorImageTransformer {
       ));
   }
 
+  public function executePreviewTransform(
+    PhabricatorFile $file,
+    $size) {
+
+    $image = $this->generatePreview($file, $size);
+
+    return PhabricatorFile::newFromFileData(
+      $image,
+      array(
+        'name' => 'preview-'.$file->getName(),
+      ));
+  }
+
+
   private function crudelyCropTo(PhabricatorFile $file, $x, $min_y, $max_y) {
     $data = $file->loadFileData();
     $img = imagecreatefromstring($data);
@@ -86,27 +100,56 @@ final class PhabricatorImageTransformer {
     $x = imagesx($src);
     $y = imagesy($src);
 
-    $scale = min($x / $dx, $y / $dy);
+    $scale = min(($dx / $x), ($dy / $y), 1);
+
     $dst = imagecreatetruecolor($dx, $dy);
     imagesavealpha($dst, true);
-    imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 0, 0, 0, 127));
+    imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127));
 
-    // If we need to chop off some pixels, chop them off from the sides instead
+    $sdx = $scale * $x;
-    // of scaling in on <0, 0>.
+    $sdy = $scale * $y;
-    $sdx = $scale * $dx;
-    $sdy = $scale * $dy;
 
     imagecopyresampled(
       $dst,
       $src,
+      ($dx - $sdx) / 2,  ($dy - $sdy) / 2,
       0, 0,
-      ($x - $sdx) / 2,  ($y - $sdy) / 2,
+      $sdx, $sdy,
-      $dx, $dy,
+      $x, $y);
-      $sdx, $sdy);
 
     return $dst;
   }
 
+  private function generatePreview(PhabricatorFile $file, $size) {
+    $data = $file->loadFileData();
+    $src = imagecreatefromstring($data);
+
+    $x = imagesx($src);
+    $y = imagesy($src);
+
+    $scale = min($size / $x, $size / $y, 1);
+
+    $dx = max($size / 4, $scale * $x);
+    $dy = max($size / 4, $scale * $y);
+
+    $dst = imagecreatetruecolor($dx, $dy);
+    imagesavealpha($dst, true);
+    imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127));
+
+    $sdx = $scale * $x;
+    $sdy = $scale * $y;
+
+    imagecopyresampled(
+      $dst,
+      $src,
+      ($dx - $sdx) / 2, ($dy - $sdy) / 2,
+      0, 0,
+      $sdx, $sdy,
+      $x, $y);
+
+    return $this->saveImageDataInAnyFormat($dst, $file->getMimeType());
+  }
+
   private function saveImageDataInAnyFormat($data, $preferred_mime = '') {
     switch ($preferred_mime) {
       case 'image/gif': // GIF doesn't support true color.

src/applications/files/controller/PhabricatorFileTransformController.php

@@ -55,6 +55,12 @@ final class PhabricatorFileTransformController
     $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
 
     switch ($this->transform) {
+      case 'thumb-220x165':
+        $xformed_file = $this->executeThumbTransform($file, 220, 165);
+        break;
+      case 'preview-220':
+        $xformed_file = $this->executePreviewTransform($file, 220);
+        break;
       case 'thumb-160x120':
         $xformed_file = $this->executeThumbTransform($file, 160, 120);
         break;
@@ -133,6 +139,11 @@ final class PhabricatorFileTransformController
     return id(new AphrontRedirectResponse())->setURI($uri);
   }
 
+  private function executePreviewTransform(PhabricatorFile $file, $size) {
+    $xformer = new PhabricatorImageTransformer();
+    return $xformer->executePreviewTransform($file, $size);
+  }
+
   private function executeThumbTransform(PhabricatorFile $file, $x, $y) {
     $xformer = new PhabricatorImageTransformer();
     return $xformer->executeThumbTransform($file, $x, $y);

src/applications/files/storage/PhabricatorFile.php

@@ -315,6 +315,13 @@ final class PhabricatorFile extends PhabricatorFileDAO {
     return '/file/xform/thumb-160x120/'.$this->getPHID().'/';
   }
 
+  public function getPreview220URI() {
+    return '/file/xform/preview-220/'.$this->getPHID().'/';
+  }
+
+  public function getThumb220x165URI() {
+    return '/file/xform/thumb-220x165/'.$this->getPHID().'/';
+  }
 
   public function isViewableInBrowser() {
     return ($this->getViewableMimeType() !== null);

src/applications/macro/controller/PhabricatorMacroListController.php

@@ -99,8 +99,8 @@ final class PhabricatorMacroListController
 
         $item = new PhabricatorPinboardItemView();
         if ($file) {
-          $item->setImageURI($file->getThumb160x120URI());
+          $item->setImageURI($file->getThumb220x165URI());
-          $item->setImageSize(160, 120);
+          $item->setImageSize(220, 165);
           if ($file->getAuthorPHID()) {
             $author_handle = $this->getHandle($file->getAuthorPHID());
             $item->appendChild(
@@ -112,7 +112,6 @@ final class PhabricatorMacroListController
               'div',
               array(),
               'Created on '.$datetime));
-
         }
         $item->setURI($this->getApplicationURI('/edit/'.$macro->getID().'/'));
         $item->setHeader($macro->getName());

src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php

@@ -68,9 +68,7 @@ final class PhabricatorRemarkupRuleEmbedFile
       return $this->getEngine()->storeText($link);
     }
 
-    $attrs = array(
+    $attrs = array();
-      'class' => 'phabricator-remarkup-embed-image',
-    );
 
     switch ($options['size']) {
       case 'full':
@@ -79,9 +77,7 @@ final class PhabricatorRemarkupRuleEmbedFile
         break;
       case 'thumb':
       default:
-        $attrs['src'] = $file->getThumb160x120URI();
+        $attrs['src'] = $file->getPreview220URI();
-        $attrs['width'] = 160;
-        $attrs['height'] = 120;
         $link = $file->getBestURI();
         break;
     }
@@ -93,6 +89,7 @@ final class PhabricatorRemarkupRuleEmbedFile
         'a',
         array(
           'href'    => $link,
+          'class'   => 'phabricator-remarkup-embed-image',
           'target'  => '_blank',
         ),
         $embed);

webroot/rsrc/css/core/remarkup.css

@@ -219,14 +219,10 @@
   margin: .5em 1em 0;
 }
 
-img.phabricator-remarkup-embed-image {
+.phabricator-remarkup-embed-image {
-  display: inline;
+  display: inline-block;
-  border: 2px solid white;
+  border: 3px solid white;
-  background: white;
+  box-shadow: 1px 1px 2px rgba(0, 0, 0, 0.20);
-
-  box-shadow: 1px 1px 6px rgba(0,0,0,.5);
-  -moz-box-shadow: 1px 1px 6px rgba(0,0,0,.5);
-  -webkit-box-shadow: 1px 1px 6px rgba(0,0,0,.5);
 }
 
 .phabricator-remarkup table.remarkup-table {