From 692b8549f5eaeee119a857cdf4673d484446af8c Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Thu, 16 Apr 2026 20:06:04 -0500 Subject: [PATCH 1/2] Make thumbnails faster --- faststack/app.py | 4 +- faststack/imaging/editor.py | 7 +- faststack/repro.py | 13 +-- faststack/tests/test_editor_reopening.py | 5 +- faststack/tests/test_mask.py | 7 +- .../tests/thumbnail_view/test_folder_stats.py | 6 +- .../tests/thumbnail_view/test_prefetcher.py | 26 ++++- .../thumbnail_view/test_provider_logic.py | 18 ++++ faststack/thumbnail_view/__init__.py | 7 +- faststack/thumbnail_view/prefetcher.py | 101 ++++++++++++------ faststack/thumbnail_view/provider.py | 22 +++- 11 files changed, 149 insertions(+), 67 deletions(-) diff --git a/faststack/app.py b/faststack/app.py index fda5ad9..127d7a3 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -78,6 +78,7 @@ from faststack.imaging.mask_engine import inverse_transform from faststack.imaging.metadata import get_exif_data from faststack.thumbnail_view import ( + DEFAULT_THUMBNAIL_CACHE_BYTES, ThumbnailModel, ThumbnailPrefetcher, ThumbnailCache, @@ -351,7 +352,7 @@ def __init__( self._grid_model_dirty = True # Start dirty to ensure initial load self._folder_loaded = False # Track whether the current folder scan is complete self._thumbnail_cache = ThumbnailCache( - max_bytes=256 * 1024 * 1024, # 256 MB + max_bytes=DEFAULT_THUMBNAIL_CACHE_BYTES, # cache-ready thumbnail QImages max_items=5000, ) self._path_resolver = PathResolver() @@ -8030,7 +8031,6 @@ def quick_auto_white_balance(self): t_start = time.perf_counter() - image_file = self.image_files[self.current_index] if self.view_override_path: active_path = Path(self.view_override_path) else: diff --git a/faststack/imaging/editor.py b/faststack/imaging/editor.py index 2092eb4..e4d6f60 100644 --- a/faststack/imaging/editor.py +++ b/faststack/imaging/editor.py @@ -14,6 +14,9 @@ import numpy as np from PIL import ExifTags, Image, ImageFilter, ImageOps +# Mask subsystem (lazy imports avoided — lightweight dataclasses) +from faststack.imaging.mask import MaskData +from faststack.imaging.mask_engine import MaskRasterCache from faststack.imaging.math_utils import ( _analyze_highlight_state, _apply_headroom_shoulder, @@ -27,10 +30,6 @@ from faststack.imaging.orientation import apply_orientation_to_np, get_exif_orientation from faststack.models import DecodedImage -# Mask subsystem (lazy imports avoided — lightweight dataclasses) -from faststack.imaging.mask import DarkenSettings, MaskData -from faststack.imaging.mask_engine import MaskRasterCache - try: from PySide6.QtGui import QImage except ImportError: diff --git a/faststack/repro.py b/faststack/repro.py index 940296a..b25ad56 100644 --- a/faststack/repro.py +++ b/faststack/repro.py @@ -1,17 +1,18 @@ import sys +import traceback from pathlib import Path from unittest.mock import MagicMock, patch -# Repo root +# Repo root must be on sys.path before importing faststack when this script is run directly. sys.path.append(str(Path(".").resolve().parent)) -print(f"DEBUG: sys.path[-1] is {sys.path[-1]}") +from faststack.app import AppController # noqa: E402 +from faststack.models import ImageFile # noqa: E402 -from faststack.app import AppController +print(f"DEBUG: sys.path[-1] is {sys.path[-1]}") # Mock dependencies mock_engine = MagicMock() -mock_config = MagicMock() with ( patch("config.config"), @@ -25,8 +26,6 @@ # Setup state # Use real list to avoid mock issues -from faststack.models import ImageFile - mock_image = ImageFile(Path("test.jpg")) controller.image_files = [mock_image] controller.current_index = 0 @@ -46,6 +45,4 @@ print("Assertion passed!") except Exception as e: print(f"\nAssertion FAILED: {type(e).__name__}: {e}") - import traceback - traceback.print_exc() diff --git a/faststack/tests/test_editor_reopening.py b/faststack/tests/test_editor_reopening.py index 1c209e3..fc5193e 100644 --- a/faststack/tests/test_editor_reopening.py +++ b/faststack/tests/test_editor_reopening.py @@ -1,5 +1,5 @@ -import unittest import sys +import unittest from pathlib import Path from unittest.mock import MagicMock, patch @@ -130,7 +130,6 @@ def test_reuse_returns_REUSED_not_True(self): def test_prepare_darken_skips_reset_on_reuse(self): """_prepare_darken_image_state must NOT call _reset_darken_on_navigation when load_image_for_editing returns _REUSED.""" - target = Path("test.jpg") self.controller.image_editor.current_filepath = None # Force a load self.controller.image_editor.float_image = None # Force needs_load=True self.controller.image_editor.current_edits = {} @@ -184,7 +183,7 @@ def test_save_closes_ui_immediately_but_keeps_memory(self): # Mock snapshot self.controller.image_editor.snapshot_for_export.return_value = MagicMock() - with patch.object(self.controller, "_save_executor") as mock_executor: + with patch.object(self.controller, "_save_executor"): # 2. CALL SAVE self.controller.save_edited_image() diff --git a/faststack/tests/test_mask.py b/faststack/tests/test_mask.py index dad1134..bb2af24 100644 --- a/faststack/tests/test_mask.py +++ b/faststack/tests/test_mask.py @@ -1,6 +1,5 @@ """Tests for the reusable mask subsystem and background darkening tool.""" -import math import unittest import numpy as np @@ -427,7 +426,6 @@ def test_apply_edits_with_darken(self): # Create a small test image img = PILImage.new("RGB", (50, 50), color=(128, 128, 128)) import tempfile - from pathlib import Path with tempfile.NamedTemporaryFile(suffix=".jpg", delete=False) as f: img.save(f.name) @@ -712,10 +710,11 @@ def test_mask_overlay_returns_transparent_when_no_overlay(self): """Verify that requesting mask_overlay with no image returns a transparent QImage, not an opaque placeholder.""" try: + from unittest.mock import Mock + from PySide6.QtGui import QImage - from PySide6.QtCore import Qt + from faststack.ui.provider import ImageProvider - from unittest.mock import Mock except ImportError: self.skipTest("PySide6 not available") diff --git a/faststack/tests/thumbnail_view/test_folder_stats.py b/faststack/tests/thumbnail_view/test_folder_stats.py index 2220377..97e5776 100644 --- a/faststack/tests/thumbnail_view/test_folder_stats.py +++ b/faststack/tests/thumbnail_view/test_folder_stats.py @@ -272,7 +272,7 @@ def test_single_file_uploaded(self): buckets = _compute_coverage_buckets(jpg_files, entries, num_buckets=1) assert len(buckets) == 1 - assert buckets[0] == (1.0, 0.0, 0.0) # uploaded, not stacked, not todo + assert buckets[0] == (1.0, 0.0, 0.0, 0.0) def test_single_file_stacked(self): """Test with single stacked file.""" @@ -282,7 +282,7 @@ def test_single_file_stacked(self): buckets = _compute_coverage_buckets(jpg_files, entries, num_buckets=1) assert len(buckets) == 1 - assert buckets[0] == (0.0, 1.0, 0.0) # not uploaded, stacked, not todo + assert buckets[0] == (0.0, 0.0, 1.0, 0.0) def test_even_distribution(self): """Test even distribution across buckets.""" @@ -356,7 +356,7 @@ def test_coverage_buckets_support_path_keys(self, temp_folder): stats = read_folder_stats(temp_folder) assert stats is not None - assert stats.coverage_buckets == [(1.0, 0.0, 0.0)] + assert stats.coverage_buckets == [(1.0, 0.0, 0.0, 0.0)] class TestCountImagesInFolder: diff --git a/faststack/tests/thumbnail_view/test_prefetcher.py b/faststack/tests/thumbnail_view/test_prefetcher.py index fb28144..4ee30bc 100644 --- a/faststack/tests/thumbnail_view/test_prefetcher.py +++ b/faststack/tests/thumbnail_view/test_prefetcher.py @@ -5,6 +5,7 @@ import pytest from PIL import Image +from PySide6.QtGui import QImage from faststack.io.utils import compute_path_hash from faststack.thumbnail_view.prefetcher import ThumbnailCache, ThumbnailPrefetcher @@ -185,6 +186,16 @@ def test_size_and_bytes_used(self, cache): assert cache.size == 2 assert cache.bytes_used == 10 + def test_qimage_size_accounting(self): + """QImage entries should count their in-memory pixel footprint.""" + cache = ThumbnailCache(max_bytes=1024 * 1024, max_items=10) + image = QImage(10, 10, QImage.Format.Format_RGB888) + + cache.put("img", image) + + assert cache.get("img") is image + assert cache.bytes_used == image.bytesPerLine() * image.height() + def test_update_existing_key(self, cache): """Test updating an existing key.""" cache.put("key1", b"old") @@ -216,7 +227,9 @@ def test_submit_schedules_job(self, prefetcher, test_image, cache, qt_app): lambda: cache.get(cache_key) is not None, timeout_s=2.0, qt_app=qt_app ) - assert cache.get(cache_key) is not None + cached_image = cache.get(cache_key) + assert isinstance(cached_image, QImage) + assert not cached_image.isNull() def test_submit_skips_if_cached(self, prefetcher, test_image, cache): """Test that submit skips if already cached.""" @@ -328,9 +341,10 @@ def test_decode_applies_exif_orientation(self, cache, temp_folder, qt_app): lambda: cache.get(cache_key) is not None, timeout_s=2.0, qt_app=qt_app ) - cached_bytes = cache.get(cache_key) - assert cached_bytes is not None - assert len(cached_bytes) > 0 + cached_image = cache.get(cache_key) + assert isinstance(cached_image, QImage) + assert not cached_image.isNull() + assert cached_image.height() >= cached_image.width() finally: prefetcher.shutdown() @@ -357,7 +371,9 @@ def test_decode_handles_png(self, cache, temp_folder, qt_app): assert _wait_until( lambda: cache.get(cache_key) is not None, timeout_s=2.0, qt_app=qt_app ) - assert cache.get(cache_key) is not None + cached_image = cache.get(cache_key) + assert isinstance(cached_image, QImage) + assert not cached_image.isNull() finally: prefetcher.shutdown() diff --git a/faststack/tests/thumbnail_view/test_provider_logic.py b/faststack/tests/thumbnail_view/test_provider_logic.py index 1f16584..d1e76b7 100644 --- a/faststack/tests/thumbnail_view/test_provider_logic.py +++ b/faststack/tests/thumbnail_view/test_provider_logic.py @@ -5,6 +5,7 @@ import pytest from PySide6.QtCore import QSize +from PySide6.QtGui import QColor, QImage from faststack.thumbnail_view.prefetcher import ThumbnailCache from faststack.thumbnail_view.provider import ( @@ -155,3 +156,20 @@ def test_bad_cached_bytes_evicts_and_submits(self, wired_provider): assert args[1] == 999 # mtime_ns assert args[2] == 200 # thumb_size assert kwargs["priority"] == prefetcher.PRIO_HIGH + + def test_qimage_cache_hit_returns_cached_image_without_submit(self, wired_provider): + provider, cache, prefetcher = wired_provider + + cache_key = "200/abc123/999" + cached = QImage(20, 12, QImage.Format.Format_RGB888) + cached.fill(QColor(12, 34, 56)) + cache.put(cache_key, cached) + + out_size = QSize() + result = provider.requestImage(f"{cache_key}?r=1", out_size, QSize()) + + assert result is cached + pixel = result.pixelColor(0, 0) + assert (pixel.red(), pixel.green(), pixel.blue()) == (12, 34, 56) + assert (out_size.width(), out_size.height()) == (20, 12) + prefetcher.submit.assert_not_called() diff --git a/faststack/thumbnail_view/__init__.py b/faststack/thumbnail_view/__init__.py index 6cd99ff..f7bb8d7 100644 --- a/faststack/thumbnail_view/__init__.py +++ b/faststack/thumbnail_view/__init__.py @@ -2,11 +2,16 @@ from .folder_stats import FolderStats, read_folder_stats from .model import ThumbnailEntry, ThumbnailModel -from .prefetcher import ThumbnailCache, ThumbnailPrefetcher +from .prefetcher import ( + DEFAULT_THUMBNAIL_CACHE_BYTES, + ThumbnailCache, + ThumbnailPrefetcher, +) from .provider import PathResolver, ThumbnailProvider __all__ = [ "FolderStats", + "DEFAULT_THUMBNAIL_CACHE_BYTES", "PathResolver", "read_folder_stats", "ThumbnailCache", diff --git a/faststack/thumbnail_view/prefetcher.py b/faststack/thumbnail_view/prefetcher.py index bd69377..f106caa 100644 --- a/faststack/thumbnail_view/prefetcher.py +++ b/faststack/thumbnail_view/prefetcher.py @@ -6,15 +6,10 @@ import time from collections import OrderedDict from concurrent.futures import Future +from contextlib import nullcontext from pathlib import Path from threading import Lock -from typing import TYPE_CHECKING, Callable, Dict, Optional, Tuple - -if TYPE_CHECKING: - from faststack.imaging.cache import ByteLRUCache - -import io -from contextlib import nullcontext +from typing import Callable, Dict, Optional, Tuple import numpy as np from PIL import Image @@ -27,9 +22,12 @@ log = logging.getLogger(__name__) +DEFAULT_THUMBNAIL_CACHE_BYTES = 768 * 1024 * 1024 + # Optional Qt dispatch so callbacks always run on Qt thread when available try: from PySide6.QtCore import QCoreApplication, QObject, Qt, Signal + from PySide6.QtGui import QImage class _ReadyEmitter(QObject): ready = Signal(str) @@ -39,6 +37,7 @@ class _ReadyEmitter(QObject): _ReadyEmitter = None _HAS_QT = False QCoreApplication = None + QImage = None # Try to initialize turbojpeg with shared discovery logic. _tj, HAS_TURBOJPEG = create_turbojpeg() @@ -46,6 +45,37 @@ class _ReadyEmitter(QObject): log.debug("TurboJPEG unavailable, using PIL for thumbnail decoding") +def _thumbnail_cache_item_size(value: object) -> int: + """Return the memory footprint we want to charge against the cache.""" + if QImage is not None and isinstance(value, QImage): + return value.bytesPerLine() * value.height() + return len(value) + + +def _rgb_to_qimage(rgb_array: np.ndarray) -> "QImage": + """Materialize a Qt-owned QImage from an RGB numpy array.""" + if QImage is None: + raise RuntimeError("QImage is unavailable for thumbnail caching") + + if not rgb_array.flags.c_contiguous: + rgb_array = np.ascontiguousarray(rgb_array) + + height, width = rgb_array.shape[:2] + # NOTE: image_view borrows rgb_array's buffer until .copy() returns. + # Keep rgb_array alive as a local and do not inline or reorder this sequence. + image_view = QImage( + rgb_array.data, + width, + height, + rgb_array.strides[0], + QImage.Format.Format_RGB888, + ) + image = image_view.copy() + if image.isNull(): + raise RuntimeError("Failed to materialize thumbnail QImage") + return image + + class ThumbnailPrefetcher: """Background thumbnail decoder with ThreadPoolExecutor. @@ -62,7 +92,7 @@ class ThumbnailPrefetcher: def __init__( self, - cache: "ByteLRUCache", + cache: "ThumbnailCache", on_ready_callback: Optional[Callable[[str], None]] = None, max_workers: int = None, target_size: int = 200, @@ -259,10 +289,10 @@ def _decode_worker( mtime_ns: int, size: int, timer: Optional["thumb_debug.ThumbTimer"] = None, - ) -> Optional[bytes]: + ) -> Optional["QImage"]: """Worker function to decode a thumbnail. - Returns JPEG bytes or None on error. + Returns a cache-ready QImage or None on error. """ if timer: timer.t_worker_start = time.perf_counter() @@ -302,12 +332,9 @@ def _decode_worker( ) return None - # Encode to JPEG bytes for storage - with timer.stage("encode") if timer else nullcontext(): - pil_image = Image.fromarray(rgb_array, mode="RGB") - buf = io.BytesIO() - pil_image.save(buf, format="JPEG", quality=85) - result = buf.getvalue() + # Materialize a Qt-owned image once so cache hits can return immediately. + with timer.stage("qimage") if timer else nullcontext(): + result = _rgb_to_qimage(rgb_array) if timer and timer.cancelled: thumb_debug.log_trace("cancel_too_late", rid=timer.rid) @@ -388,7 +415,7 @@ def _decode_image( pil_img.thumbnail( (target_size, target_size), Image.Resampling.LANCZOS ) - return np.array(pil_img.copy()) + return np.array(pil_img) except Exception as e: log.debug("PIL decode failed for %s: %s", path, e) @@ -432,10 +459,10 @@ def _on_decode_done( thumb_debug.log_trace(event, rid=timer.rid) return - jpeg_bytes = future.result() - if jpeg_bytes: + cached_image = future.result() + if cached_image is not None: # Store in cache - self._cache.put(cache_key, jpeg_bytes) + self._cache.put(cache_key, cached_image) if timer: thumb_debug.inc("decode_done_ok") @@ -486,40 +513,48 @@ def shutdown(self): class ThumbnailCache: - """Simple byte-based LRU cache for thumbnails with dual capacity limit. + """Simple size-aware LRU cache for thumbnails with dual capacity limit. Limits: - max_bytes: Maximum total bytes - max_items: Maximum number of items """ - def __init__(self, max_bytes: int = 256 * 1024 * 1024, max_items: int = 5000): + def __init__( + self, + max_bytes: int = DEFAULT_THUMBNAIL_CACHE_BYTES, + max_items: int = 5000, + size_of: Callable[[object], int] = _thumbnail_cache_item_size, + ): self._max_bytes = max_bytes self._max_items = max_items - self._cache: Dict[str, bytes] = OrderedDict() + self._size_of = size_of + self._cache: Dict[str, Tuple[object, int]] = OrderedDict() self._current_bytes = 0 self._lock = Lock() - def get(self, key: str) -> Optional[bytes]: + def get(self, key: str) -> Optional[object]: """Get item from cache, returns None if not found.""" with self._lock: if key not in self._cache: return None # Move to end (most recently used) self._cache.move_to_end(key, last=True) - return self._cache[key] + return self._cache[key][0] - def put(self, key: str, value: bytes): + def put(self, key: str, value: object): """Put item in cache, evicting if necessary.""" with self._lock: + value_size = self._size_of(value) # If already present, remove old entry logic by just updating (OrderedDict handles key existence) # But we must update _current_bytes first if it exists if key in self._cache: - self._current_bytes -= len(self._cache[key]) + _, old_size = self._cache[key] + self._current_bytes -= old_size self._cache.move_to_end(key, last=True) - self._cache[key] = value - self._current_bytes += len(value) + self._cache[key] = (value, value_size) + self._current_bytes += value_size # Evict if over limits while ( @@ -527,8 +562,8 @@ def put(self, key: str, value: bytes): or len(self._cache) > self._max_items ): # Pop oldest (first item) - _, oldest_val = self._cache.popitem(last=False) - self._current_bytes -= len(oldest_val) + _, (_, oldest_size) = self._cache.popitem(last=False) + self._current_bytes -= oldest_size def discard(self, key: str) -> bool: """Remove a single entry if present. No-op if missing. @@ -537,10 +572,10 @@ def discard(self, key: str) -> bool: """ with self._lock: try: - val = self._cache.pop(key) + _, size = self._cache.pop(key) except KeyError: return False - self._current_bytes -= len(val) + self._current_bytes -= size return True def clear(self): diff --git a/faststack/thumbnail_view/provider.py b/faststack/thumbnail_view/provider.py index d684955..55130de 100644 --- a/faststack/thumbnail_view/provider.py +++ b/faststack/thumbnail_view/provider.py @@ -236,19 +236,20 @@ def requestImage(self, id_str: str, size: QSize, _requestedSize: QSize) -> QImag # Check cache (O(1) lookup) t_cache_get_start = time.perf_counter() if timer else 0 - cached_bytes = self._cache.get(cache_key) + cached_value = self._cache.get(cache_key) dt_cache_get = (time.perf_counter() - t_cache_get_start) * 1000 if timer else 0 - if cached_bytes: + if cached_value is not None: if timer: thumb_debug.inc("req_cache_hit") thumb_debug.log_trace( "cache_hit", rid=timer.rid, ms=f"{dt_cache_get:.3f}" ) - # Decode JPEG bytes to QImage + # Most cache hits are already materialized QImages. Keep a bytes fallback + # for tests and defensive handling of manually injected bad entries. t_decode_start = time.perf_counter() if timer else 0 - image = self._bytes_to_image(cached_bytes) + image = self._cached_to_image(cached_value) dt_decode = (time.perf_counter() - t_decode_start) * 1000 if timer else 0 if image is not None and not image.isNull(): @@ -288,6 +289,19 @@ def requestImage(self, id_str: str, size: QSize, _requestedSize: QSize) -> QImag size.setHeight(self._placeholder.height()) return self._placeholder + def _cached_to_image(self, cached_value: object) -> Optional[QImage]: + """Convert a cached thumbnail payload into a QImage.""" + if isinstance(cached_value, QImage): + return cached_value + if isinstance(cached_value, (bytes, bytearray, memoryview)): + return self._bytes_to_image(bytes(cached_value)) + + log.warning( + "Unsupported cached thumbnail type: %s", + type(cached_value).__name__, + ) + return None + def _bytes_to_image(self, jpeg_bytes: bytes) -> Optional[QImage]: """Convert JPEG bytes to QImage. From 52e9f994d727c0750ee8812fc873f48bb4388f13 Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Thu, 16 Apr 2026 20:32:41 -0500 Subject: [PATCH 2/2] clean up unused script --- faststack/repro.py | 48 ---------------------------------------------- 1 file changed, 48 deletions(-) delete mode 100644 faststack/repro.py diff --git a/faststack/repro.py b/faststack/repro.py deleted file mode 100644 index b25ad56..0000000 --- a/faststack/repro.py +++ /dev/null @@ -1,48 +0,0 @@ -import sys -import traceback -from pathlib import Path -from unittest.mock import MagicMock, patch - -# Repo root must be on sys.path before importing faststack when this script is run directly. -sys.path.append(str(Path(".").resolve().parent)) - -from faststack.app import AppController # noqa: E402 -from faststack.models import ImageFile # noqa: E402 - -print(f"DEBUG: sys.path[-1] is {sys.path[-1]}") - -# Mock dependencies -mock_engine = MagicMock() - -with ( - patch("config.config"), - patch("faststack.io.watcher.Watcher"), - patch("faststack.io.sidecar.SidecarManager"), - patch("faststack.imaging.prefetch.Prefetcher"), - patch("faststack.imaging.cache.ByteLRUCache"), - patch("faststack.thumbnail_view.ThumbnailProvider"), -): - controller = AppController(Path("."), mock_engine) - -# Setup state -# Use real list to avoid mock issues -mock_image = ImageFile(Path("test.jpg")) -controller.image_files = [mock_image] -controller.current_index = 0 -controller.auto_level_threshold = 0.001 - -# Mock image_editor -controller.image_editor = MagicMock() -controller.image_editor.auto_levels.return_value = (10, 240, 10, 240) # Not full range -controller.image_editor.current_filepath = Path("test.jpg") -controller.image_editor.load_image.return_value = True - -print("Calling controller.auto_levels()...") -try: - result = controller.auto_levels() - print(f"Result: {result}") - controller.image_editor.auto_levels.assert_called_once() - print("Assertion passed!") -except Exception as e: - print(f"\nAssertion FAILED: {type(e).__name__}: {e}") - traceback.print_exc()