From 4869208a445fb2d1740508b66412581c59cafd18 Mon Sep 17 00:00:00 2001 From: Samuel CHEMLA Date: Thu, 5 Sep 2019 22:42:43 +0200 Subject: [PATCH 1/2] Remove unnecessary max-size preview file generation Signed-off-by: Samuel CHEMLA --- lib/private/Preview/Generator.php | 98 ++++--------------------------- 1 file changed, 12 insertions(+), 86 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 79c512c84aac2..29c0b068b1821 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -25,7 +25,6 @@ namespace OC\Preview; -use OC\Preview\GeneratorHelper; use OCP\Files\File; use OCP\Files\IAppData; use OCP\Files\NotFoundException; @@ -33,11 +32,9 @@ use OCP\Files\SimpleFS\ISimpleFile; use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IConfig; -use OCP\IImage; +use OCP\Image as OCPImage; use OCP\IPreview; -use OCP\Preview\IProvider; use OCP\Preview\IVersionedPreviewFile; -use OCP\Preview\IProviderV2; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\GenericEvent; @@ -49,8 +46,6 @@ class Generator { private $config; /** @var IAppData */ private $appData; - /** @var GeneratorHelper */ - private $helper; /** @var EventDispatcherInterface */ private $eventDispatcher; @@ -71,8 +66,8 @@ public function __construct( $this->config = $config; $this->previewManager = $previewManager; $this->appData = $appData; - $this->helper = $helper; $this->eventDispatcher = $eventDispatcher; + //TODO remove GeneratorHelper $helper } /** @@ -122,14 +117,8 @@ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $previewVersion = $file->getPreviewVersion() . '-'; } - // Get the max preview and infer the max preview sizes from that - $maxPreview = $this->getMaxPreview($previewFolder, $file, $mimeType, $previewVersion); - if ($maxPreview->getSize() === 0) { - $maxPreview->delete(); - throw new NotFoundException('Max preview size 0, invalid!'); - } - - list($maxWidth, $maxHeight) = $this->getPreviewSize($maxPreview, $previewVersion); + $maxWidth = (int)$this->config->getSystemValue('preview_max_x', 4096); + $maxHeight = (int)$this->config->getSystemValue('preview_max_y', 4096); // If both width and heigth are -1 we just want the max preview if ($width === -1 && $height === -1) { @@ -141,16 +130,18 @@ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, list($width, $height) = $this->calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHeight); // No need to generate a preview that is just the max preview + // TODO think about this use case... + /* if ($width === $maxWidth && $height === $maxHeight) { return $maxPreview; - } + }*/ // Try to get a cached preview. Else generate (and store) one try { try { - $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion); + $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $mimeType, $previewVersion); } catch (NotFoundException $e) { - $preview = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + $preview = $this->generatePreview($previewFolder, $file, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); } } catch (\InvalidArgumentException $e) { throw new NotFoundException(); @@ -164,72 +155,6 @@ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, return $preview; } - /** - * @param ISimpleFolder $previewFolder - * @param File $file - * @param string $mimeType - * @param string $prefix - * @return ISimpleFile - * @throws NotFoundException - */ - private function getMaxPreview(ISimpleFolder $previewFolder, File $file, $mimeType, $prefix) { - $nodes = $previewFolder->getDirectoryListing(); - - foreach ($nodes as $node) { - $name = $node->getName(); - if (($prefix === '' || strpos($name, $prefix) === 0) && strpos($name, 'max')) { - return $node; - } - } - - $previewProviders = $this->previewManager->getProviders(); - foreach ($previewProviders as $supportedMimeType => $providers) { - if (!preg_match($supportedMimeType, $mimeType)) { - continue; - } - - foreach ($providers as $providerClosure) { - $provider = $this->helper->getProvider($providerClosure); - if (!($provider instanceof IProviderV2)) { - continue; - } - - if (!$provider->isAvailable($file)) { - continue; - } - - $maxWidth = (int)$this->config->getSystemValue('preview_max_x', 4096); - $maxHeight = (int)$this->config->getSystemValue('preview_max_y', 4096); - - $preview = $this->helper->getThumbnail($provider, $file, $maxWidth, $maxHeight); - - if (!($preview instanceof IImage)) { - continue; - } - - // Try to get the extention. - try { - $ext = $this->getExtention($preview->dataMimeType()); - } catch (\InvalidArgumentException $e) { - // Just continue to the next iteration if this preview doesn't have a valid mimetype - continue; - } - - $path = $prefix . (string)$preview->width() . '-' . (string)$preview->height() . '-max.' . $ext; - try { - $file = $previewFolder->newFile($path); - $file->putContent($preview->data()); - } catch (NotPermittedException $e) { - throw new NotFoundException(); - } - - return $file; - } - } - - throw new NotFoundException(); - } - /** * @param ISimpleFile $file * @param string $prefix @@ -361,8 +286,9 @@ private function calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHei * @throws NotFoundException * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid) */ - private function generatePreview(ISimpleFolder $previewFolder, ISimpleFile $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $prefix) { - $preview = $this->helper->getImage($maxPreview); + private function generatePreview(ISimpleFolder $previewFolder, File $file, $width, $height, $crop, $maxWidth, $maxHeight, $prefix) { + $preview = new OCPImage(); + $preview->loadFromData($file->getContent()); if (!$preview->valid()) { throw new \InvalidArgumentException('Failed to generate preview, failed to load image'); From 51041f979fa091e8bf8a20776230dd378eeb1f4b Mon Sep 17 00:00:00 2001 From: Samuel CHEMLA Date: Fri, 6 Sep 2019 00:14:30 +0200 Subject: [PATCH 2/2] Fix image ration not preserved when cropping Signed-off-by: Samuel CHEMLA --- lib/private/Preview/Generator.php | 33 ++++++++++++++++++------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 29c0b068b1821..75e734e7eb21b 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -117,8 +117,18 @@ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $previewVersion = $file->getPreviewVersion() . '-'; } - $maxWidth = (int)$this->config->getSystemValue('preview_max_x', 4096); - $maxHeight = (int)$this->config->getSystemValue('preview_max_y', 4096); + // Compute max width and height preserving aspect ratio + // TODO test this with images smaller than preview_max_x and/or preview_max_y + // TODO Does this code really belong to here... see lib/private/legacy/image.php::resize() for example + $image = new OCPImage(); + $image->loadFromData($file->getContent()); + $heightOrig = $image->height(); + $widthOrig = $image->width(); + $ratio = $widthOrig / $heightOrig; + $maxWidthConfig = (int)$this->config->getSystemValue('preview_max_x', 4096); + $maxHeightConfig = (int)$this->config->getSystemValue('preview_max_y', 4096); + $maxWidth = min($maxWidthConfig, $ratio * $maxHeightConfig); + $maxHeight = min($maxHeightConfig, $maxWidthConfig / $ratio); // If both width and heigth are -1 we just want the max preview if ($width === -1 && $height === -1) { @@ -136,12 +146,13 @@ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, return $maxPreview; }*/ + // Try to get a cached preview. Else generate (and store) one try { try { $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $mimeType, $previewVersion); } catch (NotFoundException $e) { - $preview = $this->generatePreview($previewFolder, $file, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + $preview = $this->generatePreview($previewFolder, $image, $width, $height, $crop, $previewVersion); } } catch (\InvalidArgumentException $e) { throw new NotFoundException(); @@ -279,17 +290,12 @@ private function calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHei * @param int $width * @param int $height * @param bool $crop - * @param int $maxWidth - * @param int $maxHeight * @param string $prefix * @return ISimpleFile * @throws NotFoundException * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid) */ - private function generatePreview(ISimpleFolder $previewFolder, File $file, $width, $height, $crop, $maxWidth, $maxHeight, $prefix) { - $preview = new OCPImage(); - $preview->loadFromData($file->getContent()); - + private function generatePreview(ISimpleFolder $previewFolder, OCPImage $preview, $width, $height, $crop, $prefix) { if (!$preview->valid()) { throw new \InvalidArgumentException('Failed to generate preview, failed to load image'); } @@ -297,14 +303,13 @@ private function generatePreview(ISimpleFolder $previewFolder, File $file, $widt if ($crop) { if ($height !== $preview->height() && $width !== $preview->width()) { //Resize - $widthR = $preview->width() / $width; - $heightR = $preview->height() / $height; + $ratioOrig = $preview->width() / $preview->height(); - if ($widthR > $heightR) { + if ($ratioOrig > 1) { $scaleH = $height; - $scaleW = $maxWidth / $heightR; + $scaleW = $height * $ratioOrig; } else { - $scaleH = $maxHeight / $widthR; + $scaleH = $width / $ratioOrig; $scaleW = $width; } $preview->preciseResize((int)round($scaleW), (int)round($scaleH));