Convert "profile" image transforms to the new pathway

Summary:
Ref T7707. This ends up being sort of complicated: to support 100x100 images in T4406, we need to scale small images //up// so they look OK when we scale them back down with `background-size` in CSS.

The rest of it is mostly straightforward.

Test Plan:
  - Did an OAuth handshake and saw a scaled-up, scaled-down profile picture that looked correct.
  - Used Pholio, edited pholio, embedded pholio.
  - Uploaded a bunch of small/weird/big images and regenerated all their transforms.
  - Uploaded some text files into Pholio.
  - Grepped for removed methods, etc.

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7707

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

src/applications/auth/view/PhabricatorAuthAccountView.php

@@ -89,13 +89,17 @@ final class PhabricatorAuthAccountView extends AphrontView {
         $account_uri);
     }
 
-    $image_uri = $account->getProfileImageFile()->getProfileThumbURI();
+    $image_file = $account->getProfileImageFile();
+    $xform = PhabricatorFileTransform::getTransformByKey(
+      PhabricatorFileThumbnailTransform::TRANSFORM_PROFILE);
+    $image_uri = $image_file->getURIForTransform($xform);
+    list($x, $y) = $xform->getTransformedDimensions($image_file);
 
     return phutil_tag(
       'div',
       array(
         'class' => 'auth-account-view',
-        'style' => 'background-image: url('.$image_uri.')',
+        'style' => 'background-image: url('.$image_uri.');',
       ),
       $content);
   }

src/applications/files/PhabricatorImageTransformer.php

@@ -20,21 +20,6 @@ final class PhabricatorImageTransformer {
       ));
   }
 
-  public function executeThumbTransform(
-    PhabricatorFile $file,
-    $x,
-    $y) {
-
-    $image = $this->crudelyScaleTo($file, $x, $y);
-
-    return PhabricatorFile::newFromFileData(
-      $image,
-      array(
-        'name' => 'thumb-'.$file->getName(),
-        'canCDN' => true,
-      ));
-  }
-
   public function executeProfileTransform(
     PhabricatorFile $file,
     $x,
@@ -123,21 +108,6 @@ final class PhabricatorImageTransformer {
     return self::saveImageDataInAnyFormat($dst, $file->getMimeType());
   }
 
-
-  /**
-   * Very crudely scale an image up or down to an exact size.
-   */
-  private function crudelyScaleTo(PhabricatorFile $file, $dx, $dy) {
-    $scaled = $this->applyScaleWithImagemagick($file, $dx, $dy);
-
-    if ($scaled != null) {
-      return $scaled;
-    }
-
-    $dst = $this->applyScaleTo($file, $dx, $dy);
-    return self::saveImageDataInAnyFormat($dst, $file->getMimeType());
-  }
-
   private function getBlankDestinationFile($dx, $dy) {
     $dst = imagecreatetruecolor($dx, $dy);
     imagesavealpha($dst, true);

src/applications/files/controller/PhabricatorFileTransformController.php

@@ -44,50 +44,33 @@ final class PhabricatorFileTransformController
       }
     }
 
-    $type = $file->getMimeType();
+    $xforms = PhabricatorFileTransform::getAllTransforms();
-
+    if (!isset($xforms[$transform])) {
-    if (!$file->isViewableInBrowser() || !$file->isTransformableImage()) {
+      return new Aphront404Response();
-      return $this->buildDefaultTransformation($file, $transform);
     }
 
+    $xform = $xforms[$transform];
+
     // We're essentially just building a cache here and don't need CSRF
     // protection.
     $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
 
     $xformed_file = null;
-
+    if ($xform->canApplyTransform($file)) {
-    $xforms = PhabricatorFileTransform::getAllTransforms();
+      try {
-    if (isset($xforms[$transform])) {
+        $xformed_file = $xforms[$transform]->applyTransform($file);
-      $xform = $xforms[$transform];
+      } catch (Exception $ex) {
-      if ($xform->canApplyTransform($file)) {
+        // In normal transform mode, we ignore failures and generate a
-        try {
+        // default transform below. If we're explicitly regenerating the
-          $xformed_file = $xforms[$transform]->applyTransform($file);
+        // thumbnail, rethrow the exception.
-        } catch (Exception $ex) {
+        if ($is_regenerate) {
-          // In normal transform mode, we ignore failures and generate a
+          throw $ex;
-          // default transform below. If we're explicitly regenerating the
-          // thumbnail, rethrow the exception.
-          if ($is_regenerate) {
-            throw $ex;
-          }
         }
       }
-
-      if (!$xformed_file) {
-        $xformed_file = $xform->getDefaultTransform($file);
-      }
     }
 
     if (!$xformed_file) {
-      switch ($transform) {
+      $xformed_file = $xform->getDefaultTransform($file);
-        case 'thumb-profile':
-          $xformed_file = $this->executeThumbTransform($file, 50, 50);
-          break;
-        case 'thumb-280x210':
-          $xformed_file = $this->executeThumbTransform($file, 280, 210);
-          break;
-        default:
-          return new Aphront400Response();
-      }
     }
 
     if (!$xformed_file) {
@@ -103,40 +86,6 @@ final class PhabricatorFileTransformController
     return $this->buildTransformedFileResponse($xform);
   }
 
-  private function buildDefaultTransformation(
-    PhabricatorFile $file,
-    $transform) {
-    static $regexps = array(
-      '@application/zip@'     => 'zip',
-      '@image/@'              => 'image',
-      '@application/pdf@'     => 'pdf',
-      '@.*@'                  => 'default',
-    );
-
-    $type = $file->getMimeType();
-    $prefix = 'default';
-    foreach ($regexps as $regexp => $implied_prefix) {
-      if (preg_match($regexp, $type)) {
-        $prefix = $implied_prefix;
-        break;
-      }
-    }
-
-    switch ($transform) {
-      case 'thumb-280x210':
-        $suffix = '280x210';
-        break;
-      default:
-        throw new Exception('Unsupported transformation type!');
-    }
-
-    $path = celerity_get_resource_uri(
-      "rsrc/image/icon/fatcow/thumbnails/{$prefix}{$suffix}.png");
-
-    return id(new AphrontRedirectResponse())
-      ->setURI($path);
-  }
-
   private function buildTransformedFileResponse(
     PhabricatorTransformedFile $xform) {
 
@@ -154,11 +103,6 @@ final class PhabricatorFileTransformController
     return $file->getRedirectResponse();
   }
 
-  private function executeThumbTransform(PhabricatorFile $file, $x, $y) {
-    $xformer = new PhabricatorImageTransformer();
-    return $xformer->executeThumbTransform($file, $x, $y);
-  }
-
   private function destroyTransform(PhabricatorTransformedFile $xform) {
     $file = id(new PhabricatorFileQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())

src/applications/files/storage/PhabricatorFile.php

@@ -784,14 +784,6 @@ final class PhabricatorFile extends PhabricatorFileDAO
     return PhabricatorEnv::getCDNURI($path);
   }
 
-  public function getProfileThumbURI() {
-    return $this->getTransformedURI('thumb-profile');
-  }
-
-  public function getThumb280x210URI() {
-    return $this->getTransformedURI('thumb-280x210');
-  }
-
   public function isViewableInBrowser() {
     return ($this->getViewableMimeType() !== null);
   }

src/applications/files/transform/PhabricatorFileImageTransform.php

@@ -42,13 +42,20 @@ abstract class PhabricatorFileImageTransform extends PhabricatorFileTransform {
     $dst_w, $dst_h,
     $src_x, $src_y,
     $src_w, $src_h,
-    $use_w, $use_h) {
+    $use_w, $use_h,
+    $scale_up) {
+
+    // Figure out the effective destination width, height, and offsets.
+    $cpy_w = min($dst_w, $use_w);
+    $cpy_h = min($dst_h, $use_h);
+
+    // If we aren't scaling up, and are copying a very small source image,
+    // we're just going to center it in the destination image.
+    if (!$scale_up) {
+      $cpy_w = min($cpy_w, $src_w);
+      $cpy_h = min($cpy_h, $src_h);
+    }
 
-    // Figure out the effective destination width, height, and offsets. We
-    // never want to scale images up, so if we're copying a very small source
-    // image we're just going to center it in the destination image.
-    $cpy_w = min($dst_w, $src_w, $use_w);
-    $cpy_h = min($dst_h, $src_h, $use_h);
     $off_x = ($dst_w - $cpy_w) / 2;
     $off_y = ($dst_h - $cpy_h) / 2;
 
@@ -58,11 +65,18 @@ abstract class PhabricatorFileImageTransform extends PhabricatorFileTransform {
       $argv[] = '-shave';
       $argv[] = $src_x.'x'.$src_y;
       $argv[] = '-resize';
-      $argv[] = $dst_w.'x'.$dst_h.'>';
+
+      if ($scale_up) {
+        $argv[] = $dst_w.'x'.$dst_h;
+      } else {
+        $argv[] = $dst_w.'x'.$dst_h.'>';
+      }
+
       $argv[] = '-bordercolor';
       $argv[] = 'rgba(255, 255, 255, 0)';
       $argv[] = '-border';
       $argv[] = $off_x.'x'.$off_y;
+
       return $this->applyImagemagick($argv);
     }
 
@@ -117,7 +131,13 @@ abstract class PhabricatorFileImageTransform extends PhabricatorFileTransform {
    * @param string Raw file data.
    */
   protected function newFileFromData($data) {
-    $name = $this->getTransformKey().'-'.$this->file->getName();
+    if ($this->file) {
+      $name = $this->file->getName();
+    } else {
+      $name = 'default.png';
+    }
+
+    $name = $this->getTransformKey().'-'.$name;
 
     return PhabricatorFile::newFromFileData(
       $data,

src/applications/files/transform/PhabricatorFileThumbnailTransform.php

@@ -12,6 +12,7 @@ final class PhabricatorFileThumbnailTransform
   private $key;
   private $dstX;
   private $dstY;
+  private $scaleUp;
 
   public function setName($name) {
     $this->name = $name;
@@ -29,6 +30,11 @@ final class PhabricatorFileThumbnailTransform
     return $this;
   }
 
+  public function setScaleUp($scale) {
+    $this->scaleUp = $scale;
+    return $this;
+  }
+
   public function getTransformName() {
     return $this->name;
   }
@@ -42,7 +48,8 @@ final class PhabricatorFileThumbnailTransform
       id(new PhabricatorFileThumbnailTransform())
         ->setName(pht("Profile (100px \xC3\x97 100px)"))
         ->setKey(self::TRANSFORM_PROFILE)
-        ->setDimensions(100, 100),
+        ->setDimensions(100, 100)
+        ->setScaleUp(true),
       id(new PhabricatorFileThumbnailTransform())
         ->setName(pht("Pinboard (280px \xC3\x97 210px)"))
         ->setKey(self::TRANSFORM_PINBOARD)
@@ -86,7 +93,8 @@ final class PhabricatorFileThumbnailTransform
       $copy_x,
       $copy_y,
       $use_x,
-      $use_y);
+      $use_y,
+      $this->scaleUp);
   }
 
 
@@ -144,22 +152,35 @@ final class PhabricatorFileThumbnailTransform
       $copy_x = $src_x;
       $copy_y = $src_y;
     } else {
+      $scale_up = $this->scaleUp;
+
       // Otherwise, both dimensions are fixed. Figure out how much we'd have to
       // scale the image down along each dimension to get the entire thing to
       // fit.
-      $scale_x = min(($dst_x / $src_x), 1);
+      $scale_x = ($dst_x / $src_x);
-      $scale_y = min(($dst_y / $src_y), 1);
+      $scale_y = ($dst_y / $src_y);
+
+      if (!$scale_up) {
+        $scale_x = min($scale_x, 1);
+        $scale_y = min($scale_y, 1);
+      }
 
       if ($scale_x > $scale_y) {
         // This image is relatively tall and narrow. We're going to crop off the
         // top and bottom.
-        $copy_x = $src_x;
+        $scale = $scale_x;
-        $copy_y = min($src_y, $dst_y / $scale_x);
       } else {
         // This image is relatively short and wide. We're going to crop off the
         // left and right.
-        $copy_x = min($src_x, $dst_x / $scale_y);
+        $scale = $scale_y;
-        $copy_y = $src_y;
+      }
+
+      $copy_x = $dst_x / $scale_x;
+      $copy_y = $dst_y / $scale_x;
+
+      if (!$scale_up) {
+        $copy_x = min($src_x, $copy_x);
+        $copy_y = min($src_y, $copy_y);
       }
 
       // In this mode, we always use the entire destination image. We may

src/applications/macro/query/PhabricatorMacroSearchEngine.php

@@ -179,14 +179,18 @@ final class PhabricatorMacroSearchEngine
     assert_instances_of($macros, 'PhabricatorFileImageMacro');
     $viewer = $this->requireViewer();
 
+    $xform = PhabricatorFileTransform::getTransformByKey(
+      PhabricatorFileThumbnailTransform::TRANSFORM_PINBOARD);
+
     $pinboard = new PHUIPinboardView();
     foreach ($macros as $macro) {
       $file = $macro->getFile();
 
       $item = new PHUIPinboardItemView();
       if ($file) {
-        $item->setImageURI($file->getThumb280x210URI());
+        $item->setImageURI($file->getURIForTransform($xform));
-        $item->setImageSize(280, 210);
+        list($x, $y) = $xform->getTransformedDimensions($file);
+        $item->setImageSize($x, $y);
       }
 
       if ($macro->getDateCreated()) {

src/applications/pholio/query/PholioMockSearchEngine.php

@@ -141,15 +141,22 @@ final class PholioMockSearchEngine extends PhabricatorApplicationSearchEngine {
 
     $viewer = $this->requireViewer();
 
+    $xform = PhabricatorFileTransform::getTransformByKey(
+      PhabricatorFileThumbnailTransform::TRANSFORM_PINBOARD);
+
     $board = new PHUIPinboardView();
     foreach ($mocks as $mock) {
 
+      $image = $mock->getCoverFile();
+      $image_uri = $image->getURIForTransform($xform);
+      list($x, $y) = $xform->getTransformedDimensions($image);
+
       $header = 'M'.$mock->getID().' '.$mock->getName();
       $item = id(new PHUIPinboardItemView())
         ->setHeader($header)
         ->setURI('/M'.$mock->getID())
-        ->setImageURI($mock->getCoverFile()->getThumb280x210URI())
+        ->setImageURI($image_uri)
-        ->setImageSize(280, 210)
+        ->setImageSize($x, $y)
         ->setDisabled($mock->isClosed())
         ->addIconCount('fa-picture-o', count($mock->getImages()))
         ->addIconCount('fa-trophy', $mock->getTokenCount());

src/applications/pholio/view/PholioMockEmbedView.php

@@ -28,25 +28,29 @@ final class PholioMockEmbedView extends AphrontView {
         $this->mock->getImages(), array_flip($this->images));
     }
 
+    $xform = PhabricatorFileTransform::getTransformByKey(
+      PhabricatorFileThumbnailTransform::TRANSFORM_PINBOARD);
+
     if ($images_to_show) {
-      foreach ($images_to_show as $image) {
+      $image = head($images_to_show);
-        $thumbfile = $image->getFile();
+      $thumbfile = $image->getFile();
-        $thumbnail = $thumbfile->getThumb280x210URI();
-      }
       $header = 'M'.$mock->getID().' '.$mock->getName().
         ' (#'.$image->getID().')';
       $uri = '/M'.$this->mock->getID().'/'.$image->getID().'/';
     } else {
-      $thumbnail = $mock->getCoverFile()->getThumb280x210URI();
+      $thumbfile = $mock->getCoverFile();
       $header = 'M'.$mock->getID().' '.$mock->getName();
       $uri = '/M'.$this->mock->getID();
     }
 
+    $thumbnail = $thumbfile->getURIForTransform($xform);
+    list($x, $y) = $xform->getTransformedDimensions($thumbfile);
+
     $item = id(new PHUIPinboardItemView())
       ->setHeader($header)
       ->setURI($uri)
       ->setImageURI($thumbnail)
-      ->setImageSize(280, 210)
+      ->setImageSize($x, $y)
       ->setDisabled($mock->isClosed())
       ->addIconCount('fa-picture-o', count($mock->getImages()))
       ->addIconCount('fa-trophy', $mock->getTokenCount());

src/applications/pholio/view/PholioMockImagesView.php

@@ -68,8 +68,11 @@ final class PholioMockImagesView extends AphrontView {
 
     // TODO: We could maybe do a better job with tailoring this, which is the
     // image shown on the review stage.
-    $nonimage_uri = celerity_get_resource_uri(
+    $default_name = 'image-100x100.png';
-      'rsrc/image/icon/fatcow/thumbnails/default.p100.png');
+    $builtins = PhabricatorFile::loadBuiltins(
+      $this->getUser(),
+      array($default_name));
+    $default = $builtins[$default_name];
 
     $engine = id(new PhabricatorMarkupEngine())
       ->setViewer($this->getUser());
@@ -97,7 +100,7 @@ final class PholioMockImagesView extends AphrontView {
         'fullURI' => $file->getBestURI(),
         'stageURI' => ($file->isViewableImage()
           ? $file->getBestURI()
-          : $nonimage_uri),
+          : $default->getBestURI()),
         'pageURI' => $this->getImagePageURI($image, $mock),
         'downloadURI' => $file->getDownloadURI(),
         'historyURI' => $history_uri,

src/applications/pholio/view/PholioUploadedImageView.php

@@ -38,11 +38,15 @@ final class PholioUploadedImageView extends AphrontView {
       ->setSigil('image-description')
       ->setLabel(pht('Description'));
 
+    $xform = PhabricatorFileTransform::getTransformByKey(
+      PhabricatorFileThumbnailTransform::TRANSFORM_PINBOARD);
+    $thumbnail_uri = $file->getURIForTransform($xform);
+
     $thumb_frame = phutil_tag(
       'div',
       array(
         'class' => 'pholio-thumb-frame',
-        'style' => 'background-image: url('.$file->getThumb280x210URI().');',
+        'style' => 'background-image: url('.$thumbnail_uri.');',
       ));
 
     $handle = javelin_tag(

webroot/rsrc/css/application/auth/auth.css

@@ -28,6 +28,7 @@
   border: 1px solid {$lightblueborder};
   background-repeat: no-repeat;
   background-position: 4px 4px;
+  background-size: 50px 50px;
   padding: 4px 4px 4px 62px;
   min-height: 50px;
   border-radius: 2px;