From 52420b8715625fdda606e4cf47eaad94cfa0ae03 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 30 Jun 2022 15:22:08 +0200 Subject: [PATCH 01/13] Add Image::getInlineImage method --- models/Image.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/models/Image.php b/models/Image.php index c8205f83..a43df139 100644 --- a/models/Image.php +++ b/models/Image.php @@ -67,14 +67,18 @@ class Image extends Asset return EXIF::fromFile($this->getPath()); } - public function getPath() + public function getInlineImage($width = null, $height = null, $className = '') { - return ASSETSDIR . '/' . $this->subdir . '/' . $this->filename; - } + if (isset($width) && isset($height)) + $image_url = $this->getThumbnailUrl($width, $height, false); + elseif (isset($width)) + $image_url = $this->getThumbnailUrl($width, null, false); + elseif (isset($height)) + $image_url = $this->getThumbnailUrl(null, $height, false); + else + $image_url = $this->getUrl(); - public function getUrl() - { - return ASSETSURL . '/' . $this->subdir . '/' . $this->filename; + return ''; } /** From 1c02cbea931c60fd9bf584b22c1da6c84ff18ecc Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Tue, 5 Jul 2022 11:41:40 +0200 Subject: [PATCH 02/13] Rewrite Image::getInlineImage to support double density displays --- models/Image.php | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/models/Image.php b/models/Image.php index a43df139..1f4f061f 100644 --- a/models/Image.php +++ b/models/Image.php @@ -67,18 +67,20 @@ class Image extends Asset return EXIF::fromFile($this->getPath()); } - public function getInlineImage($width = null, $height = null, $className = '') + public function getInlineImage($width = null, $height = null, $className = 'inline-image') { - if (isset($width) && isset($height)) - $image_url = $this->getThumbnailUrl($width, $height, false); - elseif (isset($width)) - $image_url = $this->getThumbnailUrl($width, null, false); - elseif (isset($height)) - $image_url = $this->getThumbnailUrl(null, $height, false); + $image_urls = []; + if (isset($width) || isset($height)) + { + $thumbnail = new Thumbnail($this); + $image_urls[1] = $this->getThumbnailUrl($width, $height, false); + $image_urls[2] = $this->getThumbnailUrl($width * 2, $height * 2, false); + } else - $image_url = $this->getUrl(); + $image_urls[1] = $this->getUrl(); - return ''; + return ''; } /** From fc59708914871ebb87d4fc70322f69f72c827163 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Tue, 5 Jul 2022 12:01:02 +0200 Subject: [PATCH 03/13] Split Image::getImageUrls from Image::getInlineImage --- models/Image.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/models/Image.php b/models/Image.php index 1f4f061f..80a5cb46 100644 --- a/models/Image.php +++ b/models/Image.php @@ -67,7 +67,7 @@ class Image extends Asset return EXIF::fromFile($this->getPath()); } - public function getInlineImage($width = null, $height = null, $className = 'inline-image') + public function getImageUrls($width = null, $height = null) { $image_urls = []; if (isset($width) || isset($height)) @@ -79,6 +79,13 @@ class Image extends Asset else $image_urls[1] = $this->getUrl(); + return $image_urls; + } + + public function getInlineImage($width = null, $height = null, $className = 'inline-image') + { + $image_urls = $this->getImageUrls($width, $height); + return ''; } From 56b60b74bc3ae5039f7c9f4aa72ad062339d4622 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 7 Jul 2022 13:33:40 +0200 Subject: [PATCH 04/13] Thumbnail class: refactor getUrl method --- models/Thumbnail.php | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/models/Thumbnail.php b/models/Thumbnail.php index b6508548..c7c45ba5 100644 --- a/models/Thumbnail.php +++ b/models/Thumbnail.php @@ -9,6 +9,7 @@ class Thumbnail { private $image; + private $image_meta; private $thumbnails; private $properly_initialised; @@ -23,7 +24,7 @@ class Thumbnail const CROP_MODE_SLICE_CENTRE = 4; const CROP_MODE_SLICE_BOTTOM = 5; - public function __construct($image) + public function __construct(Image $image) { $this->image = $image; $this->image_meta = $image->getMeta(); @@ -45,51 +46,45 @@ class Thumbnail $thumb_selector = $this->width . 'x' . $this->height . $this->filename_suffix; if (!empty($this->thumbnails[$thumb_selector])) { - $thumb_path = '/' . $this->image->getSubdir() . '/' . $this->thumbnails[$thumb_selector]; - if (file_exists(THUMBSDIR . $thumb_path)) - return THUMBSURL . $thumb_path; + $thumb_filename = $this->image->getSubdir() . '/' . $this->thumbnails[$thumb_selector]; + if (file_exists(THUMBSDIR . '/' . $thumb_filename)) + return THUMBSURL . '/' . $thumb_filename; } // Do we have a custom thumbnail on file? $custom_selector = 'custom_' . $this->width . 'x' . $this->height; if (isset($this->image_meta[$custom_selector])) { - $custom_thumb_path = '/' . $this->image->getSubdir() . '/' . $this->image_meta[$custom_selector]; - if (file_exists(ASSETSDIR . $custom_thumb_path)) + $custom_filename = $this->image->getSubdir() . '/' . $this->image_meta[$custom_selector]; + if (file_exists(ASSETSDIR . '/' . $custom_filename)) { - // Ensure destination thumbnail directory exists. - if (!file_exists($this->image->getSubdir())) - @mkdir(THUMBSDIR . '/' . $this->image->getSubdir(), 0755, true); - // Copy the custom thumbail to the general thumbnail directory. - copy(ASSETSDIR . $custom_thumb_path, THUMBSDIR . $custom_thumb_path); + copy(ASSETSDIR . '/' . $custom_filename, THUMBSDIR . '/' . $custom_filename); // Let's remember this for future reference. $this->markAsGenerated($this->image_meta[$custom_selector]); - return THUMBSURL . $custom_thumb_path; + return THUMBSURL . '/' . $custom_filename; } else throw new UnexpectedValueException('Custom thumbnail expected, but missing in file system!'); } // Is this the right moment to generate a thumbnail, then? - if ($generate && array_key_exists($thumb_selector, $this->thumbnails)) + if ($generate) { - return $this->generate(); + if (array_key_exists($thumb_selector, $this->thumbnails)) + return $this->generate(); + else + throw new Exception("Trying to generate a thumbnail not previously queued by the system\n" . + print_r(func_get_args(), true)); } // If not, queue it for generation at another time, and return a URL to generate it with. - elseif (!$generate) - { - $this->markAsQueued(); - return BASEURL . '/thumbnail/' . $this->image->getId() . '/' . $this->width . 'x' . $this->height . $this->filename_suffix . '/'; - } - - // Still here..? What are you up to? ..Sneaking? else { - throw new Exception("Trying to generate a thumbnail for selector " . $thumb_selector . ", which does not appear to have been requested by the system.\n" . print_r(func_get_args(), true)); + $this->markAsQueued(); + return BASEURL . '/thumbnail/' . $this->image->getId() . '/' . $thumb_selector . '/'; } } @@ -338,6 +333,8 @@ class Thumbnail $thumb_selector = $this->width . 'x' . $this->height . $this->filename_suffix; $this->thumbnails[$thumb_selector] = $filename !== 'NULL' ? $filename : ''; } + else + throw new UnexpectedValueException('Thumbnail queuing query failed'); return $success; } From 086102d0078a9cdf06de797a36b8d1e1457a69dd Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 7 Jul 2022 13:51:03 +0200 Subject: [PATCH 05/13] Thumbnail class: minor refactor of generate method --- models/Thumbnail.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/models/Thumbnail.php b/models/Thumbnail.php index c7c45ba5..5eb562c4 100644 --- a/models/Thumbnail.php +++ b/models/Thumbnail.php @@ -255,14 +255,18 @@ class Thumbnail '_' . $this->width . 'x' . $this->height . $this->filename_suffix . '.' . $ext; // Ensure the thumbnail subdirectory exists. - if (!is_dir(THUMBSDIR . '/' . $this->image->getSubdir())) - mkdir(THUMBSDIR . '/' . $this->image->getSubdir(), 0755, true); + $target_dir = THUMBSDIR . '/' . $this->image->getSubdir(); + if (!is_dir($target_dir)) + mkdir($target_dir, 0755, true); + + if (!is_writable($target_dir)) + throw new Exception('Thumbnail directory is not writable!'); // No need to preserve every detail. $thumb->setImageCompressionQuality(80); // Save it in a public spot. - $thumb->writeImage(THUMBSDIR . '/' . $this->image->getSubdir() . '/' . $thumb_filename); + $thumb->writeImage($target_dir . '/' . $thumb_filename); // Let's remember this for future reference... $this->markAsGenerated($thumb_filename); @@ -273,7 +277,6 @@ class Thumbnail // Finally, return the URL for the generated thumbnail image. return THUMBSURL . '/' . $this->image->getSubdir() . '/' . $thumb_filename; } - // Blast! Curse your sudden but inevitable betrayal! catch (ImagickException $e) { throw new Exception('ImageMagick error occurred while generating thumbnail. Output: ' . $e->getMessage()); @@ -331,12 +334,11 @@ class Thumbnail if ($success) { $thumb_selector = $this->width . 'x' . $this->height . $this->filename_suffix; - $this->thumbnails[$thumb_selector] = $filename !== 'NULL' ? $filename : ''; + $this->thumbnails[$thumb_selector] = $filename !== 'NULL' ? $filename : null; + return $success; } else throw new UnexpectedValueException('Thumbnail queuing query failed'); - - return $success; } private function markAsQueued() From 54fb7ab41095d9a765ded926997ef5eec99f8784 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 7 Jul 2022 13:55:55 +0200 Subject: [PATCH 06/13] Write new thumbnail filenames to parent Image object as well --- models/Thumbnail.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/models/Thumbnail.php b/models/Thumbnail.php index 5eb562c4..d35c2603 100644 --- a/models/Thumbnail.php +++ b/models/Thumbnail.php @@ -335,6 +335,11 @@ class Thumbnail { $thumb_selector = $this->width . 'x' . $this->height . $this->filename_suffix; $this->thumbnails[$thumb_selector] = $filename !== 'NULL' ? $filename : null; + + // For consistency, write new thumbnail filename to parent Image object. + // TODO: there could still be an inconsistency if multiple objects exists for the same image asset. + $this->image->getThumbnails()[$thumb_selector] = $this->thumbnails[$thumb_selector]; + return $success; } else From b5edf09a696a43ec556a67fcf200cbb54a464457 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 7 Jul 2022 14:05:33 +0200 Subject: [PATCH 07/13] Don't try to generate double-density thumbs for small images --- models/Image.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/models/Image.php b/models/Image.php index 80a5cb46..99a9fbe6 100644 --- a/models/Image.php +++ b/models/Image.php @@ -74,7 +74,11 @@ class Image extends Asset { $thumbnail = new Thumbnail($this); $image_urls[1] = $this->getThumbnailUrl($width, $height, false); - $image_urls[2] = $this->getThumbnailUrl($width * 2, $height * 2, false); + + // Can we afford to generate double-density thumbnails as well? + if ((!isset($width) || $this->image_width >= $width * 2) && + (!isset($height) || $this->image_height >= $height * 2)) + $image_urls[2] = $this->getThumbnailUrl($width * 2, $height * 2, false); } else $image_urls[1] = $this->getUrl(); From c0d69f7205dd620c9902f8e1919c298cd3e3d47f Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 7 Jul 2022 14:22:22 +0200 Subject: [PATCH 08/13] Do not delete thumbnail queue when replacing an asset Thumbnails are normally created on demand, e.g. when processing the format codes in a post's body text. Normally, the temporary URL is only used once to generate thumbnails ad-hoc. However, when cache is enabled, a reference to the asset may be used in a cached version of a formatted body text, skipping the normal thumbnail generation routine. When an asset is replaced, currently, all thumbnails are removed and references to them are removed from the database. In case the asset is still referenced in a cached formatted body text, this could lead to an error when requesting the thumbnail, as the thumbnail request is no longer present in the system. As we do not know what posts use particular assets at this point in the code, it is best to work around this issue by unsetting the thumbnail filenames rather than deleting the entries outright. This effectively generates them again on the next request. In the future, we should aim to keep track of what posts make use of assets, so cache may be invalidated in a more targeted way. --- models/Image.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/Image.php b/models/Image.php index 99a9fbe6..212c1dc0 100644 --- a/models/Image.php +++ b/models/Image.php @@ -158,7 +158,8 @@ class Image extends Asset } return Registry::get('db')->query(' - DELETE FROM assets_thumbs + UPDATE assets_thumbs + SET filename = NULL WHERE id_asset = {int:id_asset}', ['id_asset' => $this->id_asset]); } From d8858c78bb9c37b789a14a558e04c952b9c48831 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 7 Jul 2022 14:54:00 +0200 Subject: [PATCH 09/13] Thumbnails: crop from original size if 2x is unavailable --- models/Image.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/Image.php b/models/Image.php index 212c1dc0..73fa459c 100644 --- a/models/Image.php +++ b/models/Image.php @@ -79,6 +79,8 @@ class Image extends Asset if ((!isset($width) || $this->image_width >= $width * 2) && (!isset($height) || $this->image_height >= $height * 2)) $image_urls[2] = $this->getThumbnailUrl($width * 2, $height * 2, false); + else + $image_urls[2] = $this->getThumbnailUrl($this->image_width, $this->image_height, true); } else $image_urls[1] = $this->getUrl(); From b3808144caf282190240f84027bcbab766cced23 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Fri, 8 Jul 2022 23:52:03 +0200 Subject: [PATCH 10/13] Address deprecation notices for certain function signatures --- models/Database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/Database.php b/models/Database.php index aaa0dfc2..8507fa71 100644 --- a/models/Database.php +++ b/models/Database.php @@ -488,7 +488,7 @@ class Database /** * This function can be used to insert data into the database in a secure way. */ - public function insert($method = 'replace', $table, $columns, $data) + public function insert($method, $table, $columns, $data) { // With nothing to insert, simply return. if (empty($data)) From 6369187eb7c3a3e6b91a2ca6ad03a6f419d00018 Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Fri, 8 Jul 2022 23:53:28 +0200 Subject: [PATCH 11/13] Add double-density thumbnails to albums and photo pages --- templates/PhotoPage.php | 6 ++---- templates/PhotosIndex.php | 10 ++++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/templates/PhotoPage.php b/templates/PhotoPage.php index 2fdfbafd..800c5218 100644 --- a/templates/PhotoPage.php +++ b/templates/PhotoPage.php @@ -65,11 +65,9 @@ class PhotoPage extends SubTemplate '; if ($this->photo->isPortrait()) - echo ' - '; + echo $this->photo->getInlineImage(null, 960); else - echo ' - '; + echo $this->photo->getInlineImage(1280, null); echo ' diff --git a/templates/PhotosIndex.php b/templates/PhotosIndex.php index 2d911294..1e8c7933 100644 --- a/templates/PhotosIndex.php +++ b/templates/PhotosIndex.php @@ -90,8 +90,14 @@ class PhotosIndex extends SubTemplate Edit'; echo ' - - '; + + width() >= $width * 2 && $image->height() >= $height * 2) + echo ' srcset="', $image->getThumbnailUrl($width * 2, $height * 2, $crop, $fit), ' 2x"'; + + echo ' alt="" title="', $image->getTitle(), '">'; if ($this->show_labels) echo ' From c763967463d70d8de5cc7b51e110f2683780ca7d Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 14 Jul 2022 16:45:17 +0200 Subject: [PATCH 12/13] Prevent current page from being 0 if no items are present --- models/GenericTable.php | 2 +- models/PageIndex.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/models/GenericTable.php b/models/GenericTable.php index df54bda2..0da224c9 100644 --- a/models/GenericTable.php +++ b/models/GenericTable.php @@ -43,7 +43,7 @@ class GenericTable $this->start = empty($options['start']) || !is_numeric($options['start']) || $options['start'] < 0 || $options['start'] > $this->recordCount ? 0 : $options['start']; // Figure out where we are on the whole, too. - $numPages = ceil($this->recordCount / $this->items_per_page); + $numPages = max(1, ceil($this->recordCount / $this->items_per_page)); $this->currentPage = min(ceil($this->start / $this->items_per_page) + 1, $numPages); // Let's bear a few things in mind... diff --git a/models/PageIndex.php b/models/PageIndex.php index 6033c9a5..0c988ccf 100644 --- a/models/PageIndex.php +++ b/models/PageIndex.php @@ -63,9 +63,9 @@ class PageIndex lower current/cont. pgs. center upper */ - $this->num_pages = ceil($this->recordCount / $this->items_per_page); + $this->num_pages = max(1, ceil($this->recordCount / $this->items_per_page)); $this->current_page = min(ceil($this->start / $this->items_per_page) + 1, $this->num_pages); - if ($this->num_pages == 0) + if ($this->num_pages <= 1) { $this->needsPageIndex = false; return; From 3de87809bb26809d8e460f1843bcb8d78926252a Mon Sep 17 00:00:00 2001 From: Aaron van Geffen Date: Thu, 14 Jul 2022 16:45:32 +0200 Subject: [PATCH 13/13] GenericTable: prevent passing NULL to strtotime --- models/GenericTable.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/GenericTable.php b/models/GenericTable.php index 0da224c9..0ead3689 100644 --- a/models/GenericTable.php +++ b/models/GenericTable.php @@ -234,7 +234,9 @@ class GenericTable else $pattern = $options['data']['pattern']; - if (!is_numeric($rowData[$options['data']['timestamp']])) + if (!isset($rowData[$options['data']['timestamp']])) + $timestamp = 0; + elseif (!is_numeric($rowData[$options['data']['timestamp']])) $timestamp = strtotime($rowData[$options['data']['timestamp']]); else $timestamp = (int) $rowData[$options['data']['timestamp']];