diff --git a/src/applications/files/transform/PhabricatorFileImageTransform.php b/src/applications/files/transform/PhabricatorFileImageTransform.php --- a/src/applications/files/transform/PhabricatorFileImageTransform.php +++ b/src/applications/files/transform/PhabricatorFileImageTransform.php @@ -31,13 +31,14 @@ protected function applyCropAndScale( $dst_w, $dst_h, $src_x, $src_y, - $src_w, $src_h) { + $src_w, $src_h, + $use_w, $use_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); - $cpy_h = min($dst_h, $src_h); + $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; diff --git a/src/applications/files/transform/PhabricatorFileThumbnailTransform.php b/src/applications/files/transform/PhabricatorFileThumbnailTransform.php --- a/src/applications/files/transform/PhabricatorFileThumbnailTransform.php +++ b/src/applications/files/transform/PhabricatorFileThumbnailTransform.php @@ -59,32 +59,60 @@ } public function applyTransform(PhabricatorFile $file) { - $xformer = new PhabricatorImageTransformer(); - if ($this->dstY === null) { - return $xformer->executePreviewTransform($file, $this->dstX); - } - $this->willTransformFile($file); list($src_x, $src_y) = $this->getImageDimensions(); $dst_x = $this->dstX; $dst_y = $this->dstY; - // 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_y = min(($dst_y / $src_y), 1); - - if ($scale_x > $scale_y) { - // This image is relatively tall and narrow. We're going to crop off the - // top and bottom. + if ($dst_y === null) { + // If we only have one dimension, it represents a maximum dimension. + // The other dimension of the transform is scaled appropriately, except + // that we never generate images with crazily extreme aspect ratios. + if ($src_x < $src_y) { + // This is a tall, narrow image. Use the maximum dimension for the + // height and scale the width. + $use_y = $dst_x; + $dst_y = $dst_x; + + $use_x = $dst_y * ($src_x / $src_y); + $dst_x = max($dst_y / 4, $use_x); + } else { + // This is a short, wide image. Use the maximum dimension for the width + // and scale the height. + $use_x = $dst_x; + + $use_y = $dst_x * ($src_y / $src_x); + $dst_y = max($dst_x / 4, $use_y); + } + + // In this mode, we always copy the entire source image. We may generate + // margins in the output. $copy_x = $src_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); $copy_y = $src_y; + } else { + // 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_y = min(($dst_y / $src_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; + $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); + $copy_y = $src_y; + } + + // In this mode, we always use the entire destination image. We may + // crop the source input. + $use_x = $dst_x; + $use_y = $dst_y; } return $this->applyCropAndScale( @@ -93,7 +121,9 @@ ($src_x - $copy_x) / 2, ($src_y - $copy_y) / 2, $copy_x, - $copy_y); + $copy_y, + $use_x, + $use_y); } public function getDefaultTransform(PhabricatorFile $file) {