Merge pull request #70 from manuelmeister/bugfix/#33

FEATURE: Only output existing images and allow upscaling
sitegeist · Sep 27, 2024 · 1aa673e · 1aa673e
2 parents 0146228 + 742fab2
commit 1aa673e
Show file tree

Hide file tree

Showing 7 changed files with 199 additions and 61 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -16,8 +16,6 @@ jobs:
                 php-versions: ['8.0', '8.1']
                 neos-versions: ['8.3']
                 include:
-                  - php-versions: '7.3'
-                    neos-versions: '7.3'
                   - php-versions: '7.4'
                     neos-versions: '7.3'
                   - php-versions: '8.0'

diff --git a/Classes/Domain/AbstractImageSource.php b/Classes/Domain/AbstractImageSource.php
@@ -7,7 +7,6 @@
 use Neos\Eel\ProtectedContextAwareInterface;
 use Neos\Flow\Annotations as Flow;
 use Neos\Flow\Log\Utility\LogEnvironment;
-use Neos\Utility\Arrays;
 use Psr\Log\LoggerInterface;
 use Sitegeist\Kaleidoscope\EelHelpers\ImageSourceHelperInterface;
 
@@ -218,39 +217,14 @@ public function withVariantPreset(string $presetIdentifier, string $presetVarian
     }
 
     /**
-     * Render sourceset Attribute for various media descriptors.
+     * Render sourceset Attribute non-scalable media.
      *
      * @param mixed $mediaDescriptors
      *
      * @return string
      */
     public function srcset($mediaDescriptors): string
     {
-        if ($this instanceof ScalableImageSourceInterface) {
-            $srcsetArray = [];
-
-            if (is_array($mediaDescriptors) || $mediaDescriptors instanceof \Traversable) {
-                $descriptors = $mediaDescriptors;
-            } else {
-                $descriptors = Arrays::trimExplode(',', (string) $mediaDescriptors);
-            }
-
-            foreach ($descriptors as $descriptor) {
-                if (preg_match('/^(?<width>[0-9]+)w$/u', $descriptor, $matches)) {
-                    $width = (int) $matches['width'];
-                    $scaleFactor = $width / $this->width();
-                    $scaled = $this->scale($scaleFactor);
-                    $srcsetArray[] = $scaled->src() . ' ' . $width . 'w';
-                } elseif (preg_match('/^(?<factor>[0-9\\.]+)x$/u', $descriptor, $matches)) {
-                    $factor = (float) $matches['factor'];
-                    $scaled = $this->scale($factor);
-                    $srcsetArray[] = $scaled->src() . ' ' . $factor . 'x';
-                }
-            }
-
-            return implode(', ', $srcsetArray);
-        }
-
         return $this->src();
     }
 

diff --git a/Classes/Domain/AbstractScalableImageSource.php b/Classes/Domain/AbstractScalableImageSource.php
@@ -7,12 +7,14 @@
 use Imagine\Image\Box;
 use Imagine\Image\ImagineInterface;
 use Neos\Flow\Annotations as Flow;
+use Neos\Flow\Log\Utility\LogEnvironment;
 use Neos\Media\Domain\Model\Adjustment\CropImageAdjustment;
 use Neos\Media\Domain\Model\Adjustment\ImageAdjustmentInterface;
 use Neos\Media\Domain\Model\Adjustment\ResizeImageAdjustment;
 use Neos\Media\Domain\ValueObject\Configuration\Adjustment;
 use Neos\Media\Domain\ValueObject\Configuration\VariantPreset;
 use Neos\Utility\ObjectAccess;
+use Neos\Utility\Arrays;
 use Sitegeist\Kaleidoscope\EelHelpers\ScalableImageSourceHelperInterface;
 
 abstract class AbstractScalableImageSource extends AbstractImageSource implements ScalableImageSourceInterface, ScalableImageSourceHelperInterface
@@ -38,9 +40,9 @@ abstract class AbstractScalableImageSource extends AbstractImageSource implement
      * @param int|null $targetWidth
      * @param bool     $preserveAspect
      *
-     * @return ImageSourceInterface
+     * @return ScalableImageSourceInterface
      */
-    public function withWidth(int $targetWidth = null, bool $preserveAspect = false): ImageSourceInterface
+    public function withWidth(int $targetWidth = null, bool $preserveAspect = false): ScalableImageSourceInterface
     {
         $newSource = clone $this;
         $newSource->targetWidth = $targetWidth;
@@ -60,9 +62,9 @@ public function withWidth(int $targetWidth = null, bool $preserveAspect = false)
      * @param int|null $targetHeight
      * @param bool     $preserveAspect
      *
-     * @return ImageSourceInterface
+     * @return ScalableImageSourceInterface
      */
-    public function withHeight(int $targetHeight = null, bool $preserveAspect = false): ImageSourceInterface
+    public function withHeight(int $targetHeight = null, bool $preserveAspect = false): ScalableImageSourceInterface
     {
         $newSource = clone $this;
         $newSource->targetHeight = $targetHeight;
@@ -78,12 +80,28 @@ public function withHeight(int $targetHeight = null, bool $preserveAspect = fals
         return $newSource;
     }
 
+    /**
+     * @param int $targetWidth
+     * @param int $targetHeight
+     *
+     * @return ScalableImageSourceInterface
+     */
+    public function withDimensions(int $targetWidth, int $targetHeight): ScalableImageSourceInterface
+    {
+        $newSource = clone $this;
+        $newSource->targetWidth = $targetWidth;
+        $newSource->targetHeight = $targetHeight;
+
+        return $newSource;
+    }
+
+
     /**
      * @param float $factor
      *
-     * @return ImageSourceInterface
+     * @return ScalableImageSourceInterface
      */
-    public function scale(float $factor): ImageSourceInterface
+    public function scale(float $factor): ScalableImageSourceInterface
     {
         $scaledHelper = clone $this;
 
@@ -213,4 +231,71 @@ protected function createAdjustment(Adjustment $adjustmentConfiguration): ImageA
 
         return $adjustment;
     }
+
+    /**
+     * Render srcset Attribute for various media descriptors.
+     *
+     * If upscaling is not allowed and the width is greater than the base width,
+     * use the base width.
+     *
+     * @param $mediaDescriptors
+     *
+     * @return string
+     */
+    public function srcset($mediaDescriptors): string
+    {
+        $srcsetArray = [];
+
+        if (is_array($mediaDescriptors) || $mediaDescriptors instanceof \Traversable) {
+            $descriptors = $mediaDescriptors;
+        } else {
+            $descriptors = Arrays::trimExplode(',', (string)$mediaDescriptors);
+        }
+
+        $srcsetType = null;
+        $maxScaleFactor = min($this->baseWidth / $this->width(), $this->baseHeight / $this->height());
+
+        foreach ($descriptors as $descriptor) {
+            $hasDescriptor = preg_match('/^(?<width>[0-9]+)w$|^(?<factor>[0-9\\.]+)x$/u', $descriptor, $matches, PREG_UNMATCHED_AS_NULL);
+
+            if (!$hasDescriptor) {
+                $this->logger->warning(sprintf('Invalid media descriptor "%s". Missing type "x" or "w"', $descriptor), LogEnvironment::fromMethodName(__METHOD__));
+                continue;
+            }
+
+            if (!$srcsetType) {
+                $srcsetType = isset($matches['width']) ? 'width' : 'factor';
+            } elseif (($srcsetType === 'width' && isset($matches['factor'])) || ($srcsetType === 'factor' && isset($matches['width']))) {
+                $this->logger->warning(sprintf('Mixed media descriptors are not valid: [%s]', implode(', ', is_array($descriptors) ? $descriptors : iterator_to_array($descriptors))), LogEnvironment::fromMethodName(__METHOD__));
+                continue;
+            }
+
+            if ($srcsetType === 'width') {
+                $width = (int)$matches['width'];
+                $scaleFactor = $width / $this->width();
+                if (!$this->supportsUpscaling() && ($width / $this->baseWidth > 1)) {
+                    $srcsetArray[] = $this->src() . ' ' . $this->baseWidth . 'w';
+                } else {
+                    $scaled = $this->scale($scaleFactor);
+                    $srcsetArray[] = $scaled->src() . ' ' . $width . 'w';
+                }
+            } elseif ($srcsetType === 'factor') {
+                $factor = (float)$matches['factor'];
+                if (
+                    !$this->supportsUpscaling() && (
+                        ($this->targetHeight && ($maxScaleFactor < $factor)) ||
+                        ($this->targetWidth && ($maxScaleFactor < $factor))
+                    )
+                ) {
+                    $scaled = $this->scale($maxScaleFactor);
+                    $srcsetArray[] = $scaled->src() . ' ' . $maxScaleFactor . 'x';
+                } else {
+                    $scaled = $this->scale($factor);
+                    $srcsetArray[] = $scaled->src() . ' ' . $factor . 'x';
+                }
+            }
+        }
+
+        return implode(', ', array_unique($srcsetArray));
+    }
 }
diff --git a/Classes/Domain/AssetImageSource.php b/Classes/Domain/AssetImageSource.php
@@ -69,6 +69,11 @@ public function __construct(ImageInterface $asset, ?string $title = null, ?strin
         $this->baseHeight = $this->asset->getHeight();
     }
 
+    public function supportsUpscaling(): bool
+    {
+        return false;
+    }
+
     /**
      * Use the variant generated from the given variant preset in this image source.
      *
@@ -128,7 +133,7 @@ public function src(): string
 
         $async = $this->request ? $this->async : false;
         $allowCropping = true;
-        $allowUpScaling = false;
+        $allowUpScaling = $this->supportsUpscaling();
         $thumbnailConfiguration = new ThumbnailConfiguration(
             $width,
             $width,
@@ -168,7 +173,7 @@ public function dataSrc(): string
 
         $async = false;
         $allowCropping = true;
-        $allowUpScaling = false;
+        $allowUpScaling = $this->supportsUpscaling();
         $thumbnailConfiguration = new ThumbnailConfiguration(
             $width,
             $width,

diff --git a/Classes/Domain/DummyImageSource.php b/Classes/Domain/DummyImageSource.php
@@ -59,6 +59,11 @@ class DummyImageSource extends AbstractScalableImageSource
      */
     protected $baseUri;
 
+    /**
+     * @var bool
+     */
+    private $allowUpScaling;
+
     /**
      * @param string|null $baseUri
      * @param string|null $title
@@ -68,8 +73,9 @@ class DummyImageSource extends AbstractScalableImageSource
      * @param string|null $backgroundColor
      * @param string|null $foregroundColor
      * @param string|null $text
+     * @param bool        $allowUpScaling
      */
-    public function __construct(?string $baseUri, ?string $title = null, ?string $alt = null, ?int $baseWidth = null, ?int $baseHeight = null, ?string $backgroundColor = null, ?string $foregroundColor = null, ?string $text = null)
+    public function __construct(?string $baseUri, ?string $title = null, ?string $alt = null, ?int $baseWidth = null, ?int $baseHeight = null, ?string $backgroundColor = null, ?string $foregroundColor = null, ?string $text = null, $allowUpScaling = false)
     {
         parent::__construct($title, $alt);
         $this->baseUri = $baseUri;
@@ -78,6 +84,12 @@ public function __construct(?string $baseUri, ?string $title = null, ?string $al
         $this->backgroundColor = $backgroundColor ?? '999';
         $this->foregroundColor = $foregroundColor ?? 'fff';
         $this->text = $text ?? '';
+        $this->allowUpScaling = $allowUpScaling;
+    }
+
+    public function supportsUpscaling(): bool
+    {
+        return $this->allowUpScaling;
     }
 
     /**

diff --git a/Classes/Domain/ScalableImageSourceInterface.php b/Classes/Domain/ScalableImageSourceInterface.php
@@ -6,5 +6,6 @@
 
 interface ScalableImageSourceInterface extends ImageSourceInterface
 {
+    public function supportsUpscaling(): bool;
     public function scale(float $factor): ImageSourceInterface;
 }