From 6a55f7fc324285604aac1f6afd4ef6bd45025318 Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Mon, 12 Sep 2016 00:00:57 +0200 Subject: [PATCH 1/9] ImageMedium->derivatives now works with image filters Previously, using ImageMedium->derivatives would not work well in combination with image filters or the other method of generating srcset variants of images (by appending eg. "@2x" to their filenames). This commit hopefully fixes that. --- .../Grav/Common/Page/Medium/ImageMedium.php | 81 +++++++++---------- system/src/Grav/Common/Page/Medium/Medium.php | 2 +- .../Grav/Common/Page/Medium/MediumFactory.php | 4 +- 3 files changed, 41 insertions(+), 46 deletions(-) diff --git a/system/src/Grav/Common/Page/Medium/ImageMedium.php b/system/src/Grav/Common/Page/Medium/ImageMedium.php index 521c41c0a8..1f68027c98 100644 --- a/system/src/Grav/Common/Page/Medium/ImageMedium.php +++ b/system/src/Grav/Common/Page/Medium/ImageMedium.php @@ -65,11 +65,6 @@ class ImageMedium extends Medium 'zoomCrop' => [ 0, 1 ] ]; - /** - * @var array - */ - protected $derivatives = []; - /** * @var string */ @@ -197,28 +192,19 @@ public function cache() */ public function srcset($reset = true) { - if (empty($this->alternatives) && empty($this->derivatives)) { + if (empty($this->alternatives)) { if ($reset) { $this->reset(); } + return ''; } - if (!empty($this->derivatives)) { - asort($this->derivatives); - - foreach ($this->derivatives as $url => $width) { - $srcset[] = $url . ' ' . $width . 'w'; - } - - $srcset[] = $this->url($reset) . ' ' . $this->get('width') . 'w'; - } - else { - $srcset = [ $this->url($reset) . ' ' . $this->get('width') . 'w' ]; - foreach ($this->alternatives as $ratio => $medium) { - $srcset[] = $medium->url($reset) . ' ' . $medium->get('width') . 'w'; - } + $srcset = []; + foreach ($this->alternatives as $ratio => $medium) { + $srcset[] = $medium->url($reset) . ' ' . $medium->get('width') . 'w'; } + $srcset[] = $this->url($reset) . ' ' . $this->get('width') . 'w'; return implode(', ', $srcset); } @@ -232,31 +218,36 @@ public function srcset($reset = true) * @return $this */ public function derivatives($min_width, $max_width, $step = 200) { - $width = $min_width; - - // Do not upscale images. - if ($max_width > $this->get('width')) { - $max_width = $this->get('width'); - } - - while ($width <= $max_width) { - $ratio = $width / $this->get('width'); - $derivative = MediumFactory::scaledFromMedium($this, 1, $ratio); - if (is_array($derivative)) { - $this->addDerivative($derivative['file']); + $width = $min_width; + + // Do not upscale images. + if ($max_width > $this->get('width')) { + $max_width = $this->get('width'); } - $width += $step; - } - return $this; - } - /** - * Add a derivative - * - * @param ImageMedium $image - */ - public function addDerivative(ImageMedium $image) { - $this->derivatives[$image->url()] = $image->get('width'); + // Clear any alternatives that have already been implied by the image's + // filename (eg. ones that had "@2x" or similar appended to their + // filenames) + $this->clearAlternatives(); + + while ($width <= $max_width) { + $ratio = $width / $this->get('width'); + $width += $step; + + if ($ratio == 1) { + continue; + } + + $derivative = MediumFactory::fromFile($this->get('filepath')); + $derivative_width = $derivative->get('width') * $ratio; + $derivative_height = $derivative->get('height') * $ratio; + $derivative->resize($derivative_width, $derivative_height); + $derivative->set('width', $derivative_width); + $derivative->set('height', $derivative_height); + $this->addAlternative($ratio, $derivative); + } + + return $this; } /** @@ -464,6 +455,10 @@ public function __call($method, $args) call_user_func_array([$this->image, $method], $args); foreach ($this->alternatives as $ratio => $medium) { + if (!$medium->image) { + $medium->image(); + } + $args_copy = $args; // regular image: resize 400x400 -> 200x200 diff --git a/system/src/Grav/Common/Page/Medium/Medium.php b/system/src/Grav/Common/Page/Medium/Medium.php index b13306f525..f5d9fd1ab7 100644 --- a/system/src/Grav/Common/Page/Medium/Medium.php +++ b/system/src/Grav/Common/Page/Medium/Medium.php @@ -100,7 +100,7 @@ public function addAlternative($ratio, Medium $alternative) } $alternative->set('ratio', $ratio); - $this->alternatives[(float) $ratio] = $alternative; + $this->alternatives[(string) $ratio] = $alternative; } /** diff --git a/system/src/Grav/Common/Page/Medium/MediumFactory.php b/system/src/Grav/Common/Page/Medium/MediumFactory.php index c94a549e14..b55194d988 100644 --- a/system/src/Grav/Common/Page/Medium/MediumFactory.php +++ b/system/src/Grav/Common/Page/Medium/MediumFactory.php @@ -119,8 +119,8 @@ public static function scaledFromMedium($medium, $from, $to) } $ratio = $to / $from; - $width = (int) ($medium->get('width') * $ratio); - $height = (int) ($medium->get('height') * $ratio); + $width = $medium->get('width') * $ratio; + $height = $medium->get('height') * $ratio; $basename = $medium->get('basename'); $basename = str_replace('@'.$from.'x', '@'.$to.'x', $basename); From c2e6292f13bff82d80d2c474d0e23fcba02bb78f Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Mon, 12 Sep 2016 15:24:08 +0200 Subject: [PATCH 2/9] Modified initialization of image alternatives The biggest alternative will now become the base medium, and alternatives will be filled out as necessary in a descending manner. --- system/src/Grav/Common/Page/Media.php | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/system/src/Grav/Common/Page/Media.php b/system/src/Grav/Common/Page/Media.php index c8385ea155..cf4974f510 100644 --- a/system/src/Grav/Common/Page/Media.php +++ b/system/src/Grav/Common/Page/Media.php @@ -74,16 +74,12 @@ public function __construct($path) } // Create the base medium - if (!empty($types['base'])) { + if (empty($types['base'])) { + $max = max(array_keys($types['alternative'])); + $medium = $types['alternative'][$max]['file']; + } else { $medium = MediumFactory::fromFile($types['base']['file']); $medium && $medium->set('size', $types['base']['size']); - } else if (!empty($types['alternative'])) { - $altMedium = reset($types['alternative']); - $ratio = key($types['alternative']); - - $altMedium = $altMedium['file']; - - $medium = MediumFactory::scaledFromMedium($altMedium, $ratio, 1)['file']; } if (empty($medium)) { @@ -103,10 +99,9 @@ public function __construct($path) // Build missing alternatives if (!empty($types['alternative'])) { $alternatives = $types['alternative']; - $max = max(array_keys($alternatives)); - for ($i=2; $i < $max; $i++) { + for ($i=$max; $i > 0; $i--) { if (isset($alternatives[$i])) { continue; } @@ -115,7 +110,9 @@ public function __construct($path) } foreach ($types['alternative'] as $ratio => $altMedium) { - $medium->addAlternative($ratio, $altMedium['file']); + if ($altMedium['file'] != $medium) { + $medium->addAlternative($ratio, $altMedium['file']); + } } } From 4d1962041b24f5cf87ed0c640f9e55b4aac0a91b Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Mon, 12 Sep 2016 15:46:08 +0200 Subject: [PATCH 3/9] Fully reset image when derivatives method is called Otherwise we get some funky results, with the possibility of two different images being rendered between the full-width srcset version and the original src version. --- system/src/Grav/Common/Page/Medium/ImageMedium.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/src/Grav/Common/Page/Medium/ImageMedium.php b/system/src/Grav/Common/Page/Medium/ImageMedium.php index 1f68027c98..caaa7d226c 100644 --- a/system/src/Grav/Common/Page/Medium/ImageMedium.php +++ b/system/src/Grav/Common/Page/Medium/ImageMedium.php @@ -228,7 +228,7 @@ public function derivatives($min_width, $max_width, $step = 200) { // Clear any alternatives that have already been implied by the image's // filename (eg. ones that had "@2x" or similar appended to their // filenames) - $this->clearAlternatives(); + $this->reset(); while ($width <= $max_width) { $ratio = $width / $this->get('width'); From 7efd55dac7bd2a3e82b8c02783297c141c4a2508 Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Sat, 1 Oct 2016 11:44:45 +0200 Subject: [PATCH 4/9] Account for risk of original file not existing when generating image derivatives --- .../Grav/Common/Page/Medium/ImageMedium.php | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/system/src/Grav/Common/Page/Medium/ImageMedium.php b/system/src/Grav/Common/Page/Medium/ImageMedium.php index caaa7d226c..37163feb68 100644 --- a/system/src/Grav/Common/Page/Medium/ImageMedium.php +++ b/system/src/Grav/Common/Page/Medium/ImageMedium.php @@ -239,12 +239,20 @@ public function derivatives($min_width, $max_width, $step = 200) { } $derivative = MediumFactory::fromFile($this->get('filepath')); - $derivative_width = $derivative->get('width') * $ratio; - $derivative_height = $derivative->get('height') * $ratio; - $derivative->resize($derivative_width, $derivative_height); - $derivative->set('width', $derivative_width); - $derivative->set('height', $derivative_height); - $this->addAlternative($ratio, $derivative); + + // It's possible that MediumFactory::fromFile returns null if the + // original image file no longer exists and this class instance was + // retrieved from the page cache + if (isset($derivative)) { + $derivative_width = $derivative->get('width') * $ratio; + $derivative_height = $derivative->get('height') * $ratio; + + $derivative->resize($derivative_width, $derivative_height); + $derivative->set('width', $derivative_width); + $derivative->set('height', $derivative_height); + + $this->addAlternative($ratio, $derivative); + } } return $this; From 07c9591369a997edaac9ce538e1dbe1481f38757 Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Wed, 5 Oct 2016 14:38:59 +0200 Subject: [PATCH 5/9] Fixed an issue where too many alternatives would be generated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using naming conventions to generate image alternatives, this patch would previously generate a “@1x” alternative if one didn’t exist. That’s no longer the case --- system/src/Grav/Common/Page/Media.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/src/Grav/Common/Page/Media.php b/system/src/Grav/Common/Page/Media.php index cf4974f510..805aacb085 100644 --- a/system/src/Grav/Common/Page/Media.php +++ b/system/src/Grav/Common/Page/Media.php @@ -101,7 +101,7 @@ public function __construct($path) $alternatives = $types['alternative']; $max = max(array_keys($alternatives)); - for ($i=$max; $i > 0; $i--) { + for ($i=$max; $i > 1; $i--) { if (isset($alternatives[$i])) { continue; } From 1d2cc2cce198c4d4d335ef7438e162c9aab2a9cf Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Sat, 15 Oct 2016 11:36:04 +0200 Subject: [PATCH 6/9] Add an "@1x" alternative when an image lacks a base medium When an image only has an alternative medium - ie. the only version of the image ends in eg. "@3x", then we construct the missing alternatives automatically. Previously, we would only do this down till "@2x", meaning that the image that would have been the base medium, had the image been manually resized, wasn't created. This has now been fixed. --- system/src/Grav/Common/Page/Media.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/system/src/Grav/Common/Page/Media.php b/system/src/Grav/Common/Page/Media.php index 805aacb085..7e0df52fb4 100644 --- a/system/src/Grav/Common/Page/Media.php +++ b/system/src/Grav/Common/Page/Media.php @@ -77,6 +77,11 @@ public function __construct($path) if (empty($types['base'])) { $max = max(array_keys($types['alternative'])); $medium = $types['alternative'][$max]['file']; + + // We do fill in missing alternatives later, but when the base + // medium is actually the biggest alternative, we need to also + // construct what would have been the "@1x", ie. the base medium + $types['alternative'][1] = MediumFactory::scaledFromMedium($medium, $max, 1); } else { $medium = MediumFactory::fromFile($types['base']['file']); $medium && $medium->set('size', $types['base']['size']); From 778cc1bf7dbc23d101b30aa1a950d0ad6fe86463 Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Sun, 16 Oct 2016 15:40:55 +0200 Subject: [PATCH 7/9] Always make smallest image alternative the base medium When an image lacks a base medium on disk (eg. the only existing image is an @2x version), then we make a scaled down version the base medium, which ensures that the smaller version is served up in the src attribute in the HTML. Also, don't reset the image alternatives when calling ImageMedium#derivatives, instead only generate the image alternatives that are missing. --- system/src/Grav/Common/Page/Media.php | 6 +-- .../Grav/Common/Page/Medium/ImageMedium.php | 39 +++++++++---------- system/src/Grav/Common/Page/Medium/Medium.php | 4 +- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/system/src/Grav/Common/Page/Media.php b/system/src/Grav/Common/Page/Media.php index 7e0df52fb4..c4451e08ce 100644 --- a/system/src/Grav/Common/Page/Media.php +++ b/system/src/Grav/Common/Page/Media.php @@ -77,11 +77,7 @@ public function __construct($path) if (empty($types['base'])) { $max = max(array_keys($types['alternative'])); $medium = $types['alternative'][$max]['file']; - - // We do fill in missing alternatives later, but when the base - // medium is actually the biggest alternative, we need to also - // construct what would have been the "@1x", ie. the base medium - $types['alternative'][1] = MediumFactory::scaledFromMedium($medium, $max, 1); + $medium = MediumFactory::scaledFromMedium($medium, $max, 1)['file']; } else { $medium = MediumFactory::fromFile($types['base']['file']); $medium && $medium->set('size', $types['base']['size']); diff --git a/system/src/Grav/Common/Page/Medium/ImageMedium.php b/system/src/Grav/Common/Page/Medium/ImageMedium.php index 7deec0efbc..82e8e4c58e 100644 --- a/system/src/Grav/Common/Page/Medium/ImageMedium.php +++ b/system/src/Grav/Common/Page/Medium/ImageMedium.php @@ -244,38 +244,37 @@ public function getImagePrettyName() * @return $this */ public function derivatives($min_width, $max_width, $step = 200) { - $width = $min_width; - - // Do not upscale images. - if ($max_width > $this->get('width')) { - $max_width = $this->get('width'); + if (!empty($this->alternatives)) { + $max = max(array_keys($this->alternatives)); + $base = $this->alternatives[$max]; + } else { + $base = $this; } - // Clear any alternatives that have already been implied by the image's - // filename (eg. ones that had "@2x" or similar appended to their - // filenames) - $this->reset(); - - while ($width <= $max_width) { - $ratio = $width / $this->get('width'); - $width += $step; + // Do not upscale images. + $max_width = min($max_width, $base->get('width')); - if ($ratio == 1) { + for ($width = $min_width; $width < $max_width; $width = $width + $step) { + // Only generate image alternatives that don't already exist + if (array_key_exists((int) $width, $this->alternatives)) { continue; } - $derivative = MediumFactory::fromFile($this->get('filepath')); + $derivative = MediumFactory::fromFile($base->get('filepath')); // It's possible that MediumFactory::fromFile returns null if the // original image file no longer exists and this class instance was // retrieved from the page cache if (isset($derivative)) { - $derivative_width = $derivative->get('width') * $ratio; - $derivative_height = $derivative->get('height') * $ratio; + $ratio = $base->get('width') / $width; + $height = $derivative->get('height') / $ratio; + + $basename = preg_replace('/(@\d+x)*$/', "@{$ratio}x", $base->get('basename'), 1); + $derivative->setImagePrettyName($basename); - $derivative->resize($derivative_width, $derivative_height); - $derivative->set('width', $derivative_width); - $derivative->set('height', $derivative_height); + $derivative->resize($width, $height); + $derivative->set('width', $width); + $derivative->set('height', $height); $this->addAlternative($ratio, $derivative); } diff --git a/system/src/Grav/Common/Page/Medium/Medium.php b/system/src/Grav/Common/Page/Medium/Medium.php index f5d9fd1ab7..b9e338854d 100644 --- a/system/src/Grav/Common/Page/Medium/Medium.php +++ b/system/src/Grav/Common/Page/Medium/Medium.php @@ -100,7 +100,9 @@ public function addAlternative($ratio, Medium $alternative) } $alternative->set('ratio', $ratio); - $this->alternatives[(string) $ratio] = $alternative; + $width = $alternative->get('width'); + + $this->alternatives[$width] = $alternative; } /** From 91cea78a84b6dd50d9dba2dda315de4b638ab58e Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Sun, 16 Oct 2016 21:11:07 +0200 Subject: [PATCH 8/9] Set better prettynames for image derivatives --- .../src/Grav/Common/Page/Medium/ImageMedium.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/system/src/Grav/Common/Page/Medium/ImageMedium.php b/system/src/Grav/Common/Page/Medium/ImageMedium.php index 82e8e4c58e..084313a58a 100644 --- a/system/src/Grav/Common/Page/Medium/ImageMedium.php +++ b/system/src/Grav/Common/Page/Medium/ImageMedium.php @@ -266,12 +266,22 @@ public function derivatives($min_width, $max_width, $step = 200) { // original image file no longer exists and this class instance was // retrieved from the page cache if (isset($derivative)) { - $ratio = $base->get('width') / $width; - $height = $derivative->get('height') / $ratio; + $index = 2; + $widths = array_keys($this->alternatives); + sort($widths); + + foreach ($widths as $i => $key) { + if ($width > $key) { + $index += max($i, 1); + } + } - $basename = preg_replace('/(@\d+x)*$/', "@{$ratio}x", $base->get('basename'), 1); + $basename = preg_replace('/(@\d+x)*$/', "@{$index}x", $base->get('basename'), 1); $derivative->setImagePrettyName($basename); + $ratio = $base->get('width') / $width; + $height = $derivative->get('height') / $ratio; + $derivative->resize($width, $height); $derivative->set('width', $width); $derivative->set('height', $height); From baa1d8732e780bcb5ceb311d6c9921a37ebbd3f1 Mon Sep 17 00:00:00 2001 From: Fredrik Ekelund Date: Fri, 21 Oct 2016 16:27:18 +0200 Subject: [PATCH 9/9] Changed image derivatives prettynames to be width based Instead of example2x.jpeg, we now have eg. example1280w.jpeg --- system/src/Grav/Common/Page/Medium/ImageMedium.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/src/Grav/Common/Page/Medium/ImageMedium.php b/system/src/Grav/Common/Page/Medium/ImageMedium.php index 084313a58a..a2bb7b169a 100644 --- a/system/src/Grav/Common/Page/Medium/ImageMedium.php +++ b/system/src/Grav/Common/Page/Medium/ImageMedium.php @@ -276,7 +276,7 @@ public function derivatives($min_width, $max_width, $step = 200) { } } - $basename = preg_replace('/(@\d+x)*$/', "@{$index}x", $base->get('basename'), 1); + $basename = preg_replace('/(@\d+x){0,1}$/', "@{$width}w", $base->get('basename'), 1); $derivative->setImagePrettyName($basename); $ratio = $base->get('width') / $width;