feat(theming): Only convert a background image if it benefits from it

* WebP images are generally quite small, converting to pngs would increase
  the filesize a lot.
* Small JPEG and PNG images do not benefit from any conversion, so skip it.
* JPEG images will get quite bigger when converted to PNG so instead convert to progressive JPEG

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>

diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php
index f7b0c12844a72e16a1eabff52d06c5c650276227..fe9c3332f7edd9abe382975e8a22e77d0a92aa34 100644 (file)
--- a/apps/theming/lib/ImageManager.php
+++ b/apps/theming/lib/ImageManager.php
@@ -243,7 +243,7 @@ class ImageManager {
                        throw new \Exception('Unsupported image type');
                }
 
-               if ($key === 'background' && strpos($detectedMimeType, 'image/svg') === false && strpos($detectedMimeType, 'image/gif') === false) {
+               if ($key === 'background' && $this->shouldOptimizeBackgroundImage($detectedMimeType, filesize($tmpFile))) {
                        // Optimize the image since some people may upload images that will be
                        // either to big or are not progressive rendering.
                        $newImage = @imagecreatefromstring(file_get_contents($tmpFile));
@@ -258,7 +258,11 @@ class ImageManager {
                        $outputImage = imagescale($newImage, $newWidth, $newHeight);
 
                        imageinterlace($outputImage, 1);
-                       imagepng($outputImage, $tmpFile, 8);
+                       if (strpos($detectedMimeType, 'image/jpeg') !== false) {
+                               imagejpeg($outputImage, $tmpFile, 90);
+                       } else {
+                               imagepng($outputImage, $tmpFile, 8);
+                       }
                        imagedestroy($outputImage);
 
                        $target->putContent(file_get_contents($tmpFile));
@@ -269,6 +273,31 @@ class ImageManager {
                return $detectedMimeType;
        }
 
+       /**
+        * Decide whether an image benefits from shrinking and reconverting
+        *
+        * @param string $mimeType the mime type of the image
+        * @param int $contentSize size of the image file
+        * @return bool
+        */
+       private function shouldOptimizeBackgroundImage(string $mimeType, int $contentSize): bool {
+               // Do not touch SVGs
+               if (strpos($mimeType, 'image/svg') !== false) {
+                       return false;
+               }
+               // GIF does not benefit from converting
+               if (strpos($mimeType, 'image/gif') !== false) {
+                       return false;
+               }
+               // WebP also does not benefit from converting
+               // We could possibly try to convert to progressive image, but normally webP images are quite small
+               if (strpos($mimeType, 'image/webp') !== false) {
+                       return false;
+               }
+               // As a rule of thumb background images should be max. 150-300 KiB, small images do not benefit from converting
+               return $contentSize > 150000;
+       }
+
        /**
         * Returns a list of supported mime types for image uploads.
         * "favicon" images are only allowed to be SVG when imagemagick with SVG support is available.

diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php
index ffb023c970fc24a50221df458eaffcb77e2afec8..22432a00103f3bb28ad7a09d8252a3fb755962dc 100644 (file)
--- a/apps/theming/tests/ImageManagerTest.php
+++ b/apps/theming/tests/ImageManagerTest.php
@@ -337,9 +337,12 @@ class ImageManagerTest extends TestCase {
 
        public function dataUpdateImage() {
                return [
-                       ['background', __DIR__ . '/../../../tests/data/testimage.png', true, true],
-                       ['background', __DIR__ . '/../../../tests/data/testimage.png', false, true],
-                       ['background', __DIR__ . '/../../../tests/data/testimage.jpg', true, true],
+                       ['background', __DIR__ . '/../../../tests/data/testimage.png', true, false],
+                       ['background', __DIR__ . '/../../../tests/data/testimage.png', false, false],
+                       ['background', __DIR__ . '/../../../tests/data/testimage.jpg', true, false],
+                       ['background', __DIR__ . '/../../../tests/data/testimage.webp', true, false],
+                       ['background', __DIR__ . '/../../../tests/data/testimage-large.jpg', true, true],
+                       ['background', __DIR__ . '/../../../tests/data/testimage-wide.png', true, true],
                        ['logo', __DIR__ . '/../../../tests/data/testimagelarge.svg', true, false],
                ];
        }