From 5cd12cd7c39c2ea156ab35266c9359cdd0f4c070 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Sun, 16 Feb 2020 01:45:47 +0100 Subject: [PATCH 1/3] allow generating multiple preview sizes for a single file at once this saves having to do some of the overhead multiple times Signed-off-by: Robin Appelman --- lib/private/Preview/Generator.php | 94 ++++++++++++++++++++----------- lib/private/PreviewManager.php | 44 ++++++++++----- lib/public/IPreview.php | 13 +++++ 3 files changed, 104 insertions(+), 47 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index bb597460f9d..073001d6788 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -91,22 +91,37 @@ class Generator { * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid) */ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) { + $specification = [ + 'width' => $width, + 'height' => $height, + 'crop' => $crop, + 'mode' => $mode, + ]; + $this->eventDispatcher->dispatch( + IPreview::EVENT, + new GenericEvent($file, $specification) + ); + + // since we only ask for one preview, and the generate method return the last one it created, it returns the one we want + return $this->generatePreviews($file, [$specification], $mimeType); + } + + /** + * Generates previews of a file + * + * @param File $file + * @param array $specifications + * @param string $mimeType + * @return ISimpleFile the last preview that was generated + * @throws NotFoundException + * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid) + */ + public function generatePreviews(File $file, array $specifications, $mimeType = null) { //Make sure that we can read the file if (!$file->isReadable()) { throw new NotFoundException('Cannot read file'); } - - $this->eventDispatcher->dispatch( - IPreview::EVENT, - new GenericEvent($file, [ - 'width' => $width, - 'height' => $height, - 'crop' => $crop, - 'mode' => $mode - ]) - ); - if ($mimeType === null) { $mimeType = $file->getMimeType(); } @@ -128,36 +143,47 @@ class Generator { throw new NotFoundException('Max preview size 0, invalid!'); } - list($maxWidth, $maxHeight) = $this->getPreviewSize($maxPreview, $previewVersion); + [$maxWidth, $maxHeight] = $this->getPreviewSize($maxPreview, $previewVersion); - // If both width and heigth are -1 we just want the max preview - if ($width === -1 && $height === -1) { - $width = $maxWidth; - $height = $maxHeight; - } + $preview = null; - // Calculate the preview size - list($width, $height) = $this->calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHeight); + foreach ($specifications as $specification) { + $width = $specification['width'] ?? -1; + $height = $specification['height'] ?? -1; + $crop = $specification['crop'] ?? false; + $mode = $specification['mode'] ?? IPreview::MODE_FILL; - // No need to generate a preview that is just the max preview - if ($width === $maxWidth && $height === $maxHeight) { - return $maxPreview; - } + // If both width and heigth are -1 we just want the max preview + if ($width === -1 && $height === -1) { + $width = $maxWidth; + $height = $maxHeight; + } - // Try to get a cached preview. Else generate (and store) one - try { + // Calculate the preview size + [$width, $height] = $this->calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHeight); + + // No need to generate a preview that is just the max preview + if ($width === $maxWidth && $height === $maxHeight) { + // ensure correct return value if this was the last one + $preview = $maxPreview; + continue; + } + + // Try to get a cached preview. Else generate (and store) one try { - $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion); - } catch (NotFoundException $e) { - $preview = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + try { + $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion); + } catch (NotFoundException $e) { + $preview = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + } + } catch (\InvalidArgumentException $e) { + throw new NotFoundException(); } - } catch (\InvalidArgumentException $e) { - throw new NotFoundException(); - } - if ($preview->getSize() === 0) { - $preview->delete(); - throw new NotFoundException('Cached preview size 0, invalid!'); + if ($preview->getSize() === 0) { + $preview->delete(); + throw new NotFoundException('Cached preview size 0, invalid!'); + } } return $preview; diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php index 652a442deec..e50b43a51d2 100644 --- a/lib/private/PreviewManager.php +++ b/lib/private/PreviewManager.php @@ -151,6 +151,22 @@ class PreviewManager implements IPreview { return !empty($this->providers); } + private function getGenerator(): Generator { + if ($this->generator === null) { + $this->generator = new Generator( + $this->config, + $this, + $this->appData, + new GeneratorHelper( + $this->rootFolder, + $this->config + ), + $this->eventDispatcher + ); + } + return $this->generator; + } + /** * Returns a preview of a file * @@ -169,20 +185,22 @@ class PreviewManager implements IPreview { * @since 11.0.0 - \InvalidArgumentException was added in 12.0.0 */ public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) { - if ($this->generator === null) { - $this->generator = new Generator( - $this->config, - $this, - $this->appData, - new GeneratorHelper( - $this->rootFolder, - $this->config - ), - $this->eventDispatcher - ); - } + return $this->getGenerator()->getPreview($file, $width, $height, $crop, $mode, $mimeType); + } - return $this->generator->getPreview($file, $width, $height, $crop, $mode, $mimeType); + /** + * Generates previews of a file + * + * @param File $file + * @param array $specifications + * @param string $mimeType + * @return ISimpleFile the last preview that was generated + * @throws NotFoundException + * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid) + * @since 19.0.0 + */ + public function generatePreviews(File $file, array $specifications, $mimeType = null) { + return $this->getGenerator()->generatePreviews($file, $specifications, $mimeType); } /** diff --git a/lib/public/IPreview.php b/lib/public/IPreview.php index b377d285acb..164cbbac2bf 100644 --- a/lib/public/IPreview.php +++ b/lib/public/IPreview.php @@ -115,4 +115,17 @@ interface IPreview { * @since 8.0.0 */ public function isAvailable(\OCP\Files\FileInfo $file); + + /** + * Generates previews of a file + * + * @param File $file + * @param array $specifications + * @param string $mimeType + * @return ISimpleFile the last preview that was generated + * @throws NotFoundException + * @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid) + * @since 19.0.0 + */ + public function generatePreviews(File $file, array $specifications, $mimeType = null); } From 7d386872e5f4c5530a936465f71b4e1ea85a565e Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Sun, 16 Feb 2020 02:34:09 +0100 Subject: [PATCH 2/3] optimize batch generation of previews by allowing the generation of multiple previews at once we save on having to find, open and decode the max-preview for every preview of the same file the main use case for this is the preview generator app (pr for that comming next) in my local testing this saves about 25% of time when using the preview generator app Signed-off-by: Robin Appelman --- lib/private/Preview/Generator.php | 18 ++++-- lib/private/legacy/OC_Image.php | 102 +++++++++++++++++++++++++++--- lib/public/IImage.php | 39 ++++++++++++ 3 files changed, 144 insertions(+), 15 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 073001d6788..eedf80d8522 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -138,6 +138,7 @@ class Generator { // Get the max preview and infer the max preview sizes from that $maxPreview = $this->getMaxPreview($previewFolder, $file, $mimeType, $previewVersion); + $maxPreviewImage = null; // only load the image when we need it if ($maxPreview->getSize() === 0) { $maxPreview->delete(); throw new NotFoundException('Max preview size 0, invalid!'); @@ -174,7 +175,11 @@ class Generator { try { $preview = $this->getCachedPreview($previewFolder, $width, $height, $crop, $maxPreview->getMimeType(), $previewVersion); } catch (NotFoundException $e) { - $preview = $this->generatePreview($previewFolder, $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); + if ($maxPreviewImage === null) { + $maxPreviewImage = $this->helper->getImage($maxPreview); + } + + $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); } } catch (\InvalidArgumentException $e) { throw new NotFoundException(); @@ -386,9 +391,8 @@ class Generator { * @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, IImage $maxPreview, $width, $height, $crop, $maxWidth, $maxHeight, $prefix) { + $preview = $maxPreview; if (!$preview->valid()) { throw new \InvalidArgumentException('Failed to generate preview, failed to load image'); } @@ -406,13 +410,13 @@ class Generator { $scaleH = $maxHeight / $widthR; $scaleW = $width; } - $preview->preciseResize((int)round($scaleW), (int)round($scaleH)); + $preview = $preview->preciseResizeCopy((int)round($scaleW), (int)round($scaleH)); } $cropX = (int)floor(abs($width - $preview->width()) * 0.5); $cropY = (int)floor(abs($height - $preview->height()) * 0.5); - $preview->crop($cropX, $cropY, $width, $height); + $preview = $preview->cropCopy($cropX, $cropY, $width, $height); } else { - $preview->resize(max($width, $height)); + $preview = $maxPreview->resizeCopy(max($width, $height)); } diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index 7d23ece7ffa..73d380eb99a 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -39,6 +39,8 @@ * */ +use OCP\IImage; + /** * Class for basic image manipulation */ @@ -845,6 +847,17 @@ class OC_Image implements \OCP\IImage { * @return bool */ public function resize($maxSize) { + $result = $this->resizeNew($maxSize); + imagedestroy($this->resource); + $this->resource = $result; + return is_resource($result); + } + + /** + * @param $maxSize + * @return resource | bool + */ + private function resizeNew($maxSize) { if (!$this->valid()) { $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); return false; @@ -861,8 +874,7 @@ class OC_Image implements \OCP\IImage { $newHeight = $maxSize; } - $this->preciseResize((int)round($newWidth), (int)round($newHeight)); - return true; + return $this->preciseResizeNew((int)round($newWidth), (int)round($newHeight)); } /** @@ -871,6 +883,19 @@ class OC_Image implements \OCP\IImage { * @return bool */ public function preciseResize(int $width, int $height): bool { + $result = $this->preciseResizeNew($width, $height); + imagedestroy($this->resource); + $this->resource = $result; + return is_resource($result); + } + + + /** + * @param int $width + * @param int $height + * @return resource | bool + */ + public function preciseResizeNew(int $width, int $height) { if (!$this->valid()) { $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); return false; @@ -896,9 +921,7 @@ class OC_Image implements \OCP\IImage { imagedestroy($process); return false; } - imagedestroy($this->resource); - $this->resource = $process; - return true; + return $process; } /** @@ -969,6 +992,22 @@ class OC_Image implements \OCP\IImage { * @return bool for success or failure */ public function crop(int $x, int $y, int $w, int $h): bool { + $result = $this->cropNew($x, $y, $w, $h); + imagedestroy($this->resource); + $this->resource = $result; + return is_resource($result); + } + + /** + * Crops the image from point $x$y with dimension $wx$h. + * + * @param int $x Horizontal position + * @param int $y Vertical position + * @param int $w Width + * @param int $h Height + * @return resource | bool + */ + public function cropNew(int $x, int $y, int $w, int $h) { if (!$this->valid()) { $this->logger->error(__METHOD__ . '(): No image loaded', ['app' => 'core']); return false; @@ -993,9 +1032,7 @@ class OC_Image implements \OCP\IImage { imagedestroy($process); return false; } - imagedestroy($this->resource); - $this->resource = $process; - return true; + return $process; } /** @@ -1045,6 +1082,55 @@ class OC_Image implements \OCP\IImage { return false; } + public function copy(): IImage { + $image = new OC_Image(null, $this->logger, $this->config); + $image->resource = imagecreatetruecolor($this->width(), $this->height()); + imagecopy( + $image->resource(), + $this->resource(), + 0, + 0, + 0, + 0, + $this->width(), + $this->height() + ); + + return $image; + } + + public function cropCopy(int $x, int $y, int $w, int $h): IImage { + $image = new OC_Image(null, $this->logger, $this->config); + $image->resource = $this->cropNew($x, $y, $w, $h); + + return $image; + } + + public function preciseResizeCopy(int $width, int $height): IImage { + $image = new OC_Image(null, $this->logger, $this->config); + $image->resource = $this->preciseResizeNew($width, $height); + + return $image; + } + + public function resizeCopy(int $maxSize): IImage { + $image = new OC_Image(null, $this->logger, $this->config); + $image->resource = $this->resizeNew($maxSize); + + return $image; + } + + + /** + * Resizes the image preserving ratio, returning a new copy + * + * @param integer $maxSize The maximum size of either the width or height. + * @return bool + */ + public function copyResize($maxSize): IImage { + + } + /** * Destroys the current image and resets the object */ diff --git a/lib/public/IImage.php b/lib/public/IImage.php index 67db6b097ef..6e6c28609d8 100644 --- a/lib/public/IImage.php +++ b/lib/public/IImage.php @@ -190,4 +190,43 @@ interface IImage { * @since 8.1.0 */ public function scaleDownToFit($maxWidth, $maxHeight); + + /** + * create a copy of this image + * + * @return IImage + * @since 19.0.0 + */ + public function copy(): IImage; + + /** + * create a new cropped copy of this image + * + * @param int $x Horizontal position + * @param int $y Vertical position + * @param int $w Width + * @param int $h Height + * @return IImage + * @since 19.0.0 + */ + public function cropCopy(int $x, int $y, int $w, int $h): IImage; + + /** + * create a new resized copy of this image + * + * @param int $width + * @param int $height + * @return IImage + * @since 19.0.0 + */ + public function preciseResizeCopy(int $width, int $height): IImage; + + /** + * create a new resized copy of this image + * + * @param integer $maxSize The maximum size of either the width or height. + * @return IImage + * @since 19.0.0 + */ + public function resizeCopy(int $maxSize): IImage; } From 8f9bac26f874e105c017de2b1791b23c2a135b28 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 9 Apr 2020 15:59:18 +0200 Subject: [PATCH 3/3] fix preview generation tests Signed-off-by: Robin Appelman --- lib/private/Preview/Generator.php | 4 ++-- tests/lib/Preview/GeneratorTest.php | 37 ++++++++++++++++++----------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index eedf80d8522..cf57f9474f1 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -182,7 +182,7 @@ class Generator { $preview = $this->generatePreview($previewFolder, $maxPreviewImage, $width, $height, $crop, $maxWidth, $maxHeight, $previewVersion); } } catch (\InvalidArgumentException $e) { - throw new NotFoundException(); + throw new NotFoundException("", 0, $e); } if ($preview->getSize() === 0) { @@ -478,7 +478,7 @@ class Generator { case 'image/gif': return 'gif'; default: - throw new \InvalidArgumentException('Not a valid mimetype'); + throw new \InvalidArgumentException('Not a valid mimetype: "' . $mimeType . '"'); } } } diff --git a/tests/lib/Preview/GeneratorTest.php b/tests/lib/Preview/GeneratorTest.php index ae84d8eb4b3..cc14e78087c 100644 --- a/tests/lib/Preview/GeneratorTest.php +++ b/tests/lib/Preview/GeneratorTest.php @@ -227,19 +227,11 @@ class GeneratorTest extends \Test\TestCase { ->with($this->equalTo('256-256.png')) ->willThrowException(new NotFoundException()); - $image = $this->createMock(IImage::class); + $image = $this->getMockImage(2048, 2048, 'my resized data'); $this->helper->method('getImage') ->with($this->equalTo($maxPreview)) ->willReturn($image); - $image->expects($this->once()) - ->method('resize') - ->with(256); - $image->method('data') - ->willReturn('my resized data'); - $image->method('valid')->willReturn(true); - $image->method('dataMimeType')->willReturn('image/png'); - $previewFile->expects($this->once()) ->method('putContent') ->with('my resized data'); @@ -325,6 +317,27 @@ class GeneratorTest extends \Test\TestCase { $this->generator->getPreview($file, 100, 100); } + private function getMockImage($width, $height, $data = null) { + $image = $this->createMock(IImage::class); + $image->method('height')->willReturn($width); + $image->method('width')->willReturn($height); + $image->method('valid')->willReturn(true); + $image->method('dataMimeType')->willReturn('image/png'); + $image->method('data')->willReturn($data); + + $image->method('resizeCopy')->willReturnCallback(function($size) use ($data) { + return $this->getMockImage($size, $size, $data); + }); + $image->method('preciseResizeCopy')->willReturnCallback(function($width, $height) use ($data) { + return $this->getMockImage($width, $height, $data); + }); + $image->method('cropCopy')->willReturnCallback(function($x, $y, $width, $height) use ($data) { + return $this->getMockImage($width, $height, $data); + }); + + return $image; + } + public function dataSize() { return [ [1024, 2048, 512, 512, false, IPreview::MODE_FILL, 256, 512], @@ -409,14 +422,10 @@ class GeneratorTest extends \Test\TestCase { ->with($this->equalTo($filename)) ->willThrowException(new NotFoundException()); - $image = $this->createMock(IImage::class); + $image = $this->getMockImage($maxX, $maxY); $this->helper->method('getImage') ->with($this->equalTo($maxPreview)) ->willReturn($image); - $image->method('height')->willReturn($maxY); - $image->method('width')->willReturn($maxX); - $image->method('valid')->willReturn(true); - $image->method('dataMimeType')->willReturn('image/png'); $preview = $this->createMock(ISimpleFile::class); $previewFolder->method('newFile')