From f2da422c803e0407686cc35d90f43271ff0426d6 Mon Sep 17 00:00:00 2001 From: buzzingwires Date: Wed, 28 Jun 2023 01:23:49 -0400 Subject: [PATCH 1/5] Add thumbnail.save configuration option. If this option is set to False, then do not save newly-generated thumbnails to the disk. Previously saved thumbnail images will be used normally and will never be deleted. This saves space and reduces drive writes at the cost of extra computational overhead by making the original image be reopened each time the thumbnail is loaded. --- docs/changelog.rst | 1 + tests/integration/test_read_settings.py | 5 ++-- tests/unit/utils/test_thumbnail_manager.py | 23 +++++++++++++++++ vimiv/api/settings.py | 1 + vimiv/utils/thumbnail_manager.py | 29 ++++++++++++++++------ 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8c6717a97..ff401f729 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -91,6 +91,7 @@ Added: the original image. Thanks `@Yutsuten`_ for reviving this! * Add the ``none`` sorting type for the ``sort.image_order`` and ``sort.directory_order`` options, implemented by `@buzzingwires`_ +* Add the ``thumbnail.save`` option, implemented by `@buzzingwires`_ Changed: ^^^^^^^^ diff --git a/tests/integration/test_read_settings.py b/tests/integration/test_read_settings.py index f52cb66bd..6a863d5a9 100644 --- a/tests/integration/test_read_settings.py +++ b/tests/integration/test_read_settings.py @@ -19,7 +19,7 @@ UPDATED_CONFIG = { "SORT": {"shuffle": "True"}, "IMAGE": {"overzoom": "4.2"}, - "THUMBNAIL": {"size": "64"}, + "THUMBNAIL": {"size": "64", "save": "False"}, } @@ -46,7 +46,7 @@ UPDATED_CONFIG_INVALID = { "SORT": {"shuffle": "not a bool"}, "IMAGE": {"overzoom": "not a float"}, - "THUMBNAIL": {"size": "not an int"}, + "THUMBNAIL": {"size": "not an int", "save": "not a bool"}, } @@ -86,6 +86,7 @@ def test_read_config(configpath): assert api.settings.sort.shuffle.value is True assert api.settings.image.overzoom.value == 4.2 assert api.settings.thumbnail.size.value == 64 + assert api.settings.thumbnail.save.value is False @pytest.mark.parametrize( diff --git a/tests/unit/utils/test_thumbnail_manager.py b/tests/unit/utils/test_thumbnail_manager.py index f7a155035..34a6510d1 100644 --- a/tests/unit/utils/test_thumbnail_manager.py +++ b/tests/unit/utils/test_thumbnail_manager.py @@ -11,6 +11,7 @@ import pytest +from vimiv.api import settings from vimiv.utils import thumbnail_manager @@ -24,6 +25,28 @@ def manager(qtbot, tmp_path, mocker): yield thumbnail_manager.ThumbnailManager(None) +def test_thumbnail_save_disabled(monkeypatch, qtbot, tmp_path, manager): + monkeypatch.setattr(settings.thumbnail.save, "value", False) + no_thumbnail_path = str(tmp_path / "no_thumbnail.jpg") + QPixmap(300, 300).save(no_thumbnail_path, "jpg") + manager.create_thumbnails_async([no_thumbnail_path]) + check_thumbails_created(qtbot, manager, 0) + + +def test_thumbnail_save_disabled_no_delete_old(monkeypatch, qtbot, tmp_path, manager): + monkeypatch.setattr(settings.thumbnail.save, "value", True) + has_thumbnail_path = str(tmp_path / "has_thumbnail.jpg") + QPixmap(300, 300).save(has_thumbnail_path, "jpg") + manager.create_thumbnails_async([has_thumbnail_path]) + check_thumbails_created(qtbot, manager, 1) + + monkeypatch.setattr(settings.thumbnail.save, "value", False) + no_thumbnail_path = str(tmp_path / "no_thumbnail.jpg") + QPixmap(300, 300).save(no_thumbnail_path, "jpg") + manager.create_thumbnails_async([has_thumbnail_path, no_thumbnail_path]) + check_thumbails_created(qtbot, manager, 1) + + @pytest.mark.parametrize("n_paths", (1, 5)) def test_create_n_thumbnails(qtbot, tmp_path, manager, n_paths): # Create images to create thumbnails of diff --git a/vimiv/api/settings.py b/vimiv/api/settings.py index 4d1d9c4c1..3f14e6692 100644 --- a/vimiv/api/settings.py +++ b/vimiv/api/settings.py @@ -470,6 +470,7 @@ class thumbnail: # pylint: disable=invalid-name """Namespace for thumbnail related settings.""" size = ThumbnailSizeSetting("thumbnail.size", 128, desc="Size of thumbnails") + save = BoolSetting("thumbnail.save", True, desc="Save new thumbnails to disk for later use") class slideshow: # pylint: disable=invalid-name diff --git a/vimiv/utils/thumbnail_manager.py b/vimiv/utils/thumbnail_manager.py index a547f0f93..aed51f666 100644 --- a/vimiv/utils/thumbnail_manager.py +++ b/vimiv/utils/thumbnail_manager.py @@ -22,6 +22,7 @@ from PyQt5.QtGui import QIcon, QPixmap, QImage import vimiv +from vimiv import api from vimiv.utils import xdg, imagereader, Pool @@ -131,6 +132,24 @@ def _get_thumbnail_filename(self, path: str) -> str: def _get_source_mtime(path: str) -> int: return int(os.path.getmtime(path)) + def _save_thumbnail(self, image: QImage, thumbnail_path: str) -> None: + """Save the thumbnail file to the disk. + + Args: + image: The QImage representing the thumbnail. + thumbnail_path: Path to which the thumbnail is stored. + Returns: + None. + """ + # First create temporary file and then move it. This avoids + # problems with concurrent access of the thumbnail cache, since + # "move" is an atomic operation + handle, tmp_filename = tempfile.mkstemp(dir=self._manager.directory) + os.close(handle) + os.chmod(tmp_filename, 0o600) + image.save(tmp_filename, format="png") + os.replace(tmp_filename, thumbnail_path) + def _create_thumbnail(self, path: str, thumbnail_path: str) -> QPixmap: """Create thumbnail for an image. @@ -153,14 +172,8 @@ def _create_thumbnail(self, path: str, thumbnail_path: str) -> QPixmap: return self._manager.fail_pixmap for key, value in attributes.items(): image.setText(key, value) - # First create temporary file and then move it. This avoids - # problems with concurrent access of the thumbnail cache, since - # "move" is an atomic operation - handle, tmp_filename = tempfile.mkstemp(dir=self._manager.directory) - os.close(handle) - os.chmod(tmp_filename, 0o600) - image.save(tmp_filename, format="png") - os.replace(tmp_filename, thumbnail_path) + if api.settings.thumbnail.save: + self._save_thumbnail(image, thumbnail_path) return QPixmap(image) def _get_thumbnail_attributes(self, path: str, image: QImage) -> Dict[str, str]: From 0669eaff39ae7bc16c52ea3593e992ac5edf9419 Mon Sep 17 00:00:00 2001 From: buzzingwires Date: Tue, 11 Jul 2023 01:46:49 -0400 Subject: [PATCH 2/5] Fix careless lint error. --- vimiv/api/settings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vimiv/api/settings.py b/vimiv/api/settings.py index 3f14e6692..71afa1848 100644 --- a/vimiv/api/settings.py +++ b/vimiv/api/settings.py @@ -470,7 +470,9 @@ class thumbnail: # pylint: disable=invalid-name """Namespace for thumbnail related settings.""" size = ThumbnailSizeSetting("thumbnail.size", 128, desc="Size of thumbnails") - save = BoolSetting("thumbnail.save", True, desc="Save new thumbnails to disk for later use") + save = BoolSetting( + "thumbnail.save", True, desc="Save new thumbnails to disk for later use" + ) class slideshow: # pylint: disable=invalid-name From f27e246290171e4390d2262782e7a7985575b428 Mon Sep 17 00:00:00 2001 From: buzzingwires Date: Tue, 11 Jul 2023 13:54:12 -0400 Subject: [PATCH 3/5] Remove redundant configuration file reading tests. --- tests/integration/test_read_settings.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_read_settings.py b/tests/integration/test_read_settings.py index 6a863d5a9..f52cb66bd 100644 --- a/tests/integration/test_read_settings.py +++ b/tests/integration/test_read_settings.py @@ -19,7 +19,7 @@ UPDATED_CONFIG = { "SORT": {"shuffle": "True"}, "IMAGE": {"overzoom": "4.2"}, - "THUMBNAIL": {"size": "64", "save": "False"}, + "THUMBNAIL": {"size": "64"}, } @@ -46,7 +46,7 @@ UPDATED_CONFIG_INVALID = { "SORT": {"shuffle": "not a bool"}, "IMAGE": {"overzoom": "not a float"}, - "THUMBNAIL": {"size": "not an int", "save": "not a bool"}, + "THUMBNAIL": {"size": "not an int"}, } @@ -86,7 +86,6 @@ def test_read_config(configpath): assert api.settings.sort.shuffle.value is True assert api.settings.image.overzoom.value == 4.2 assert api.settings.thumbnail.size.value == 64 - assert api.settings.thumbnail.save.value is False @pytest.mark.parametrize( From fcf6cc8bac26be57750dfe22ba9f61d03e44e9a2 Mon Sep 17 00:00:00 2001 From: buzzingwires Date: Tue, 11 Jul 2023 13:54:46 -0400 Subject: [PATCH 4/5] Specify shared icon cache as location for thumbnail.save --- vimiv/api/settings.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vimiv/api/settings.py b/vimiv/api/settings.py index 71afa1848..a45cd22af 100644 --- a/vimiv/api/settings.py +++ b/vimiv/api/settings.py @@ -471,7 +471,9 @@ class thumbnail: # pylint: disable=invalid-name size = ThumbnailSizeSetting("thumbnail.size", 128, desc="Size of thumbnails") save = BoolSetting( - "thumbnail.save", True, desc="Save new thumbnails to disk for later use" + "thumbnail.save", + True, + desc="Save new thumbnails to the disk in the shared icon cache for later use" ) From 3f287e8d15cf75592f16bb318993feeb7925760b Mon Sep 17 00:00:00 2001 From: buzzingwires Date: Tue, 11 Jul 2023 14:02:00 -0400 Subject: [PATCH 5/5] Again, fix careless linting error. --- vimiv/api/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vimiv/api/settings.py b/vimiv/api/settings.py index a45cd22af..af1e3ac3c 100644 --- a/vimiv/api/settings.py +++ b/vimiv/api/settings.py @@ -472,8 +472,8 @@ class thumbnail: # pylint: disable=invalid-name size = ThumbnailSizeSetting("thumbnail.size", 128, desc="Size of thumbnails") save = BoolSetting( "thumbnail.save", - True, - desc="Save new thumbnails to the disk in the shared icon cache for later use" + True, + desc="Save new thumbnails to the disk in the shared icon cache for later use" )