diff --git a/.gitignore b/.gitignore index 4b7ba0d..c364b5a 100644 --- a/.gitignore +++ b/.gitignore @@ -5,8 +5,10 @@ prompt.md WARP.md ARCHITECTURE.md AGENTS.md +ANTIGRAVITY_RULES.md .claude/ .agent/ +tools/ # We don't have any good docs yet docs/ @@ -70,6 +72,7 @@ Thumbs.db .idea/ *.swp *.swo +pyrightconfig.json # ---------------------------- # Local scratch / test / debug outputs (anywhere) @@ -84,6 +87,7 @@ test_report*.log test_results.txt smoke_test_output.txt verify_result.txt +*test_output.txt **/*fail*.txt **/*final*.txt diff --git a/ChangeLog.md b/ChangeLog.md index f4b00c6..7a07064 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -12,6 +12,7 @@ Todo: Make it work on Linux / Mac. Create Windows .exe. Write better docum - Improved **thumbnail responsiveness**: visible thumbnails are now queued with higher priority than background prefetch. - Improved **prefetch stability/performance**: prefetch work runs on daemon threads and cleans up finished futures. - UI tweaks: recycle-bin details text is selectable and uses updated colors; metadata filename now shows RAW extension when present (e.g., `IMG_0001.JPG + ORF`). +- Helicon Focus: Avoid a race condition by deferring deletion of temporary file lists until app shutdown. ## 1.5.7 (2026-02-09) diff --git a/README.md b/README.md index 1336089..184fe69 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # FastStack -# Version 1.5.8 - February 12, 2026 +# Version 1.5.9 - February 16, 2026 # By Alan Rockefeller Ultra-fast, caching JPG viewer designed for culling and selecting RAW or JPG files for focus stacking and website upload. @@ -12,6 +12,7 @@ This tool is optimized for speed, using `libjpeg-turbo` for decoding, aggressive - **Crop:** Added the ability to crop and rotate images via the cr(O)p hotkey (or right mouse click). It can be a freeform crop, or constrained to several popular aspect ratios. - **Zoom & Pan:** Smooth zooming and panning. - **Stack Selection:** Group images into stacks (`[`, `]`) and select them for processing (`S`). +- **Speak Line**: In grid view, a spark line is visible on each folder, so you can see how far you have gotten in uploading photos in each directory. - **Helicon Focus Integration:** Launch Helicon Focus with your selected RAW files with a single keypress (`Enter`). - **Instant Navigation:** Sub-10ms next/previous image switching, high performance decoding via `PyTurboJPEG`. - **Image Editor:** Built-in editor with exposure, contrast, white balance, sharpness, and more (E key) @@ -26,6 +27,7 @@ This tool is optimized for speed, using `libjpeg-turbo` for decoding, aggressive - **Configurable:** Adjust cache sizes, prefetch behavior, and Helicon Focus / Photoshop paths via a settings dialog and a persistent `.ini` file. - **Accurate Colors:** Uses monitor ICC profile to display colors correctly. - **RGB Histogram:** Pressing H brings up a RGB histogram which is designed to show even a little bit of highlight clipping and updates as you zoom in. +- **Full Screen Mode:** Pressing F11 enters full screen mode - Esc/F11 exits. ## Installation @@ -70,6 +72,7 @@ faststack - `K` / `Left Arrow`: Previous Image - `G`: Jump to Image Number - `I`: Show EXIF Data +- `F11`: Toggle Fullscreen (Loupe View) - `S`: Toggle current image in/out of stack - `X`: Remove current image from batch/stack - `B`: Toggle current image in/out of batch @@ -91,7 +94,7 @@ faststack - `Ctrl+Shift+B`: Quick auto white balance (alternate) - `L`: Quick auto levels (saves automatically) - `E`: Toggle Image Editor -- `Esc`: Close active dialog, editor, or cancel crop +- `Esc`: Close active dialog, editor, cancel crop, or exit fullscreen - `H`: Toggle histogram window - `Ctrl+C`: Copy image path to clipboard - `Ctrl+0`: Reset zoom and pan to fit window diff --git a/faststack/app.py b/faststack/app.py index 21cae14..9d53e5b 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -52,7 +52,11 @@ from faststack.config import config from faststack.logging_setup import setup_logging from faststack.models import ImageFile, DecodedImage -from faststack.io.indexer import find_images, image_sort_key +from faststack.io.indexer import find_images, find_images_with_variants, image_sort_key +from faststack.io.variants import ( + VariantGroup, build_badge_list, get_group_key_for_path, + norm_path, +) from faststack.io.sidecar import SidecarManager from faststack.io.watcher import Watcher from faststack.io.helicon import launch_helicon_focus @@ -128,8 +132,10 @@ def make_hdrop(paths): log = logging.getLogger(__name__) -# Global flag for debug mode - set by main() +# Global flags for debug modes - set by main() _debug_mode = False +_debug_thumb_timing = False +_debug_thumb_trace = False # Cache Thrashing Detection Constants CACHE_THRASH_WINDOW_SECS = 2.0 @@ -171,9 +177,17 @@ class ProgressReporter(QObject): ) # Signal for async delete completion (result dict from worker) def __init__( - self, image_dir: Path, engine: QQmlApplicationEngine, debug_cache: bool = False + self, image_dir: Path, engine: QQmlApplicationEngine, + debug_cache: bool = False, + debug_thumb_timing: bool = False, + debug_thumb_trace: bool = False ): super().__init__() + self.debug_thumb_timing = debug_thumb_timing + self.debug_thumb_trace = debug_thumb_trace + + import faststack.util.thumb_debug as thumb_debug + thumb_debug.init(timing=self.debug_thumb_timing, trace=self.debug_thumb_trace) # Histogram Offloading Setup self._hist_executor = create_daemon_threadpool_executor(max_workers=1, thread_name_prefix="Histogram") self._hist_inflight = False @@ -211,6 +225,11 @@ def __init__( self._opencv_warning_shown = False # Only show OpenCV warning once per session self._last_auto_levels_msg: str = "" # Detail message from last auto_levels() call + # Variant state + self._variant_map: Dict[str, VariantGroup] = {} + self.view_override_path: Optional[str] = None # normalized absolute string + self.view_override_kind: Optional[str] = None # "main"|"developed"|"backup" + self.image_dir = image_dir self.image_files: List[ImageFile] = [] # Filtered list for display self._all_images: List[ImageFile] = [] # Cached full list from disk @@ -232,6 +251,7 @@ def __init__( # Cache Warning State self._last_cache_warning_time = 0 + self._eviction_lock = threading.Lock() self._eviction_timestamps = [] # List of eviction timestamps for rate detection self.display_ready = False # Track if display size has been reported self.pending_prefetch_index: Optional[int] = None # Deferred prefetch index @@ -247,6 +267,7 @@ def __init__( self.sidecar = SidecarManager(self.image_dir, self.watcher, debug=_debug_mode) self.image_editor = ImageEditor() # Initialize the editor self._dialog_open_count = 0 # Track nested dialogs + self._temp_files_to_clean: List[Path] = [] # Track temp files for cleanup on shutdown # -- Caching & Prefetching -- cache_size_gb = config.getfloat("core", "cache_size_gb", 1.5) @@ -278,6 +299,12 @@ def __init__( self._grid_nav_history: list[Path] = ( [] ) # Stack of previous directories for back navigation + + # -- Optimization & Instrumentation -- + self._scan_count_variant = 0 + self._grid_refreshes = 0 + self._grid_model_dirty = True # Start dirty to ensure initial load + self._folder_loaded = False # Track if initial scan is complete self._thumbnail_cache = ThumbnailCache( max_bytes=256 * 1024 * 1024, # 256 MB max_items=5000, @@ -287,6 +314,8 @@ def __init__( cache=self._thumbnail_cache, on_ready_callback=self._on_thumbnail_ready, target_size=200, + debug_timing=self.debug_thumb_timing, + debug_trace=self.debug_thumb_trace, ) self._thumbnail_model = ThumbnailModel( base_directory=self.image_dir, @@ -302,6 +331,8 @@ def __init__( prefetcher=self._thumbnail_prefetcher, path_resolver=self._path_resolver.resolve, default_size=200, + debug_timing=self.debug_thumb_timing, + debug_trace=self.debug_thumb_trace, ) # Connect thread-safe thumbnail ready signal to GUI thread handler # The callback is invoked from worker threads, so we use a signal to hop to GUI thread @@ -315,6 +346,7 @@ def __init__( self.ui_state.theme = self.get_theme() self.ui_state.debugCache = self.debug_cache self.ui_state.debugMode = _debug_mode # Set debug mode from global + self.ui_state.debugThumbTiming = self.debug_thumb_timing self.keybinder = Keybinder(self) self.ui_state.debugCache = self.debug_cache # Pass debug_cache state to UI self.ui_state.isDecoding = False # Initialize decoding indicator @@ -383,6 +415,13 @@ def __init__( self._watcher_debounce_timer.setInterval(200) # 200ms debounce self._watcher_debounce_timer.timeout.connect(self.refresh_image_list) + # Periodic summary for thumbnail debug logging + if self.debug_thumb_timing or self.debug_thumb_trace: + self._thumb_summary_timer = QTimer(self) + self._thumb_summary_timer.setInterval(1000) # Check every second + self._thumb_summary_timer.timeout.connect(thumb_debug.check_periodic_summary) + self._thumb_summary_timer.start() + # Debounce timer for metadata/highlight signals during rapid navigation # Only emits these signals once user stops navigating (16ms = 1 frame debounce) @@ -505,9 +544,20 @@ def apply_filter(self, filter_string: str, filter_flags: list): # Sync filter to grid view model; # cancel stale thumbnail jobs so the filtered view's thumbnails load quickly - self._thumbnail_prefetcher.cancel_all() - self._thumbnail_model.set_filter(filter_string) - self._thumbnail_model.set_filter_flags(flags) + if self._is_grid_view_active: + self._thumbnail_prefetcher.cancel_all() + # Silent updates - we will refresh manually via refresh_from_controller + if self._thumbnail_model: + self._thumbnail_model.set_filter(filter_string, refresh=False) + self._thumbnail_model.set_filter_flags(flags, refresh=False) + + if self._is_grid_view_active and self._thumbnail_model: + self._grid_refreshes += 1 + self._thumbnail_model.refresh_from_controller(self.image_files) + self._path_resolver.update_from_model(self._thumbnail_model) + self._grid_model_dirty = False + else: + self._grid_model_dirty = True # reset to start of filtered list self.current_index = 0 @@ -540,9 +590,20 @@ def clear_filter(self): # Sync filter to grid view model; # cancel stale thumbnail jobs so the new view's thumbnails load quickly - self._thumbnail_prefetcher.cancel_all() - self._thumbnail_model.set_filter("") - self._thumbnail_model.set_filter_flags([]) + if self._is_grid_view_active: + self._thumbnail_prefetcher.cancel_all() + # Silent updates - we will refresh manually via refresh_from_controller + if self._thumbnail_model: + self._thumbnail_model.set_filter("", refresh=False) + self._thumbnail_model.set_filter_flags([], refresh=False) + + if self._is_grid_view_active and self._thumbnail_model: + self._grid_refreshes += 1 + self._thumbnail_model.refresh_from_controller(self.image_files) + self._path_resolver.update_from_model(self._thumbnail_model) + self._grid_model_dirty = False + else: + self._grid_model_dirty = True self.current_index = min(self.current_index, max(0, len(self.image_files) - 1)) self.sync_ui_state() @@ -587,12 +648,13 @@ def _handle_resize(self): self.prefetcher.cancel_all() # Cancel stale tasks to avoid wasted work - # On first resize, execute deferred prefetch; on subsequent resizes, do normal prefetch - if is_first_resize and self.pending_prefetch_index is not None: - self.prefetcher.update_prefetch(self.pending_prefetch_index) - self.pending_prefetch_index = None - else: - self.prefetcher.update_prefetch(self.current_index) + if self._loupe_decode_allowed(): + # On first resize, execute deferred prefetch; on subsequent resizes, do normal prefetch + if is_first_resize and self.pending_prefetch_index is not None: + self.prefetcher.update_prefetch(self.pending_prefetch_index) + self.pending_prefetch_index = None + else: + self.prefetcher.update_prefetch(self.current_index) self.sync_ui_state() # To refresh the image @@ -702,7 +764,7 @@ def eventFilter(self, watched, event) -> bool: return super().eventFilter(watched, event) def _do_prefetch( - self, index: int, is_navigation: bool = False, direction: Optional[int] = None + self, index: int, is_navigation: bool = False, direction: Optional[int] = None, override_path: Optional[Path] = None ): """Helper to defer prefetch until display size is stable. @@ -710,7 +772,11 @@ def _do_prefetch( index: The index to prefetch around is_navigation: True if called from user navigation (arrow keys, etc.) direction: 1 for forward, -1 for backward, None to use last direction + override_path: Optional explicit path to decode for the current index. """ + if not self._loupe_decode_allowed(): + return + # If navigation occurs during resize debounce, cancel timer and apply resize immediately # to ensure prefetch uses correct dimensions if is_navigation and self.resize_timer.isActive(): @@ -721,16 +787,28 @@ def _do_prefetch( log.debug("Display not ready, deferring prefetch for index %d", index) self.pending_prefetch_index = index return + + # If override_path is provided, it's a priority request for the current image + if override_path is not None: + self.prefetcher.submit_task( + index, + self.prefetcher.generation, + priority=True, + override_path=override_path, + ) + return # Skip adjacent prefetch when viewing a variant override + self.prefetcher.update_prefetch( index, is_navigation=is_navigation, direction=direction ) - def load(self, skip_thumbnail_refresh: bool = False): - """Loads images, sidecar data, and starts services. + def load(self): + """Loads images, sidecar data, and starts services.""" + # Reset instrumentation for this load operation + self._scan_count_variant = 0 + self._grid_refreshes = 0 + self._grid_model_dirty = True - Args: - skip_thumbnail_refresh: If True, skip thumbnail model refresh (caller already did it). - """ self.refresh_image_list() # Initial scan from disk if not self.image_files: self.current_index = 0 @@ -746,13 +824,24 @@ def load(self, skip_thumbnail_refresh: bool = False): # Defer initial UI sync until after images are loaded self.sync_ui_state() - # Initialize grid view if starting in grid mode (unless already done) - if self._is_grid_view_active and not skip_thumbnail_refresh: - self._thumbnail_model.refresh() - self._path_resolver.update_from_model(self._thumbnail_model) - # Mark folder as loaded so QML can show "No images" message if truly empty - self._folder_loaded = True - self.ui_state.isFolderLoadedChanged.emit() + # Mark folder as loaded for UI + self._folder_loaded = True + self.ui_state.isFolderLoadedChanged.emit() + + if self._is_grid_view_active: + + # Ensure grid model is populated if starting in grid mode + if self._thumbnail_model and self._grid_model_dirty and self._thumbnail_model.rowCount() == 0: + self._grid_refreshes += 1 + self._thumbnail_model.refresh_from_controller(self.image_files) + self._path_resolver.update_from_model(self._thumbnail_model) + self._grid_model_dirty = False + + log.info( + "Load summary: scans=variant:%d grid_refreshes:%d", + self._scan_count_variant, + self._grid_refreshes, + ) def _request_watcher_refresh(self, path=None): """Thread-safe entry point for the filesystem watcher. @@ -807,12 +896,21 @@ def refresh_image_list(self): # Clear folder stats cache so subfolder counts are fresh clear_raw_count_cache() - self._all_images = find_images(self.image_dir) + self._scan_count_variant += 1 + images, variant_map = find_images_with_variants(self.image_dir) + self._all_images = images + self._variant_map = variant_map self._apply_filter_to_cached_list() - # Refresh thumbnail model if it exists (for external file changes) + # Mark model as dirty since the underlying directory was rescanned + self._grid_model_dirty = True + + # Refresh thumbnail model if it exists (for external file changes or startup) if self._thumbnail_model and self._is_grid_view_active: - self._thumbnail_model.refresh() + self._grid_refreshes += 1 + self._thumbnail_model.refresh_from_controller(self.image_files) + self._path_resolver.update_from_model(self._thumbnail_model) + self._grid_model_dirty = False def _apply_filter_to_cached_list(self): """Applies current filter to cached image list without disk I/O.""" @@ -888,6 +986,97 @@ def _reindex_after_save(self, saved_path: str) -> bool: ) return False + # --- Variant Badge Logic --- + + def get_variant_badges(self) -> list: + """Return badge list for the current image's variant group.""" + if not self.image_files or self.current_index >= len(self.image_files): + return [] + img = self.image_files[self.current_index] + key_cf = get_group_key_for_path(img.path, self._variant_map) + if key_cf is None: + return [] + group = self._variant_map[key_cf] + if len(group.all_files) <= 1: + return [] + badges = build_badge_list(group) + + # Determine which badge is active + if self.view_override_path: + active_norm = self.view_override_path + else: + active_norm = norm_path(img.path) + + # Create a new list with COPIED dicts to avoid mutating the cached result from build_badge_list + result_badges = [] + for badge in badges: + b_copy = badge.copy() + b_copy["active"] = (badge["path"] == active_norm) + result_badges.append(b_copy) + return result_badges + + def set_variant_override(self, path_str: str): + """Switch loupe view to a different variant file.""" + norm = norm_path(Path(path_str)) + + # If selecting main, clear override + if self.image_files and self.current_index < len(self.image_files): + main_norm = norm_path(self.image_files[self.current_index].path) + if norm == main_norm: + self.view_override_path = None + self.view_override_kind = None + else: + self.view_override_path = norm + # Determine kind + img = self.image_files[self.current_index] + key_cf = get_group_key_for_path(img.path, self._variant_map) + if key_cf and key_cf in self._variant_map: + group = self._variant_map[key_cf] + if group.developed_path and norm_path(group.developed_path) == norm: + self.view_override_kind = "developed" + else: + self.view_override_kind = "backup" + else: + self.view_override_kind = "backup" + + # Bump generation to bust cache, trigger re-render + self.ui_refresh_generation += 1 + + # Trigger decode for the new variant + override_p = Path(self.view_override_path) if self.view_override_path else None + self._maybe_decode_current_image(reason="variant-switch", override_path=override_p) + + if self.ui_state: + self.ui_state.variantBadgesChanged.emit() + self.ui_state.variantSaveHintChanged.emit() + self.ui_state.currentImageSourceChanged.emit() + + def _clear_variant_override(self): + """Clear variant override state (called on navigation).""" + if self.view_override_path is not None: + self.view_override_path = None + self.view_override_kind = None + if self.ui_state: + self.ui_state.variantBadgesChanged.emit() + self.ui_state.variantSaveHintChanged.emit() + + def _loupe_decode_allowed(self) -> bool: + """Predicate to check if full-resolution decoding is allowed.""" + # Only decode full-res when we are in loupe mode and the folder is loaded. + return (not self._is_grid_view_active) and self._folder_loaded + + def _maybe_decode_current_image(self, reason: str = "", override_path: Optional[Path] = None) -> None: + """Trigger decode of current image if allowed.""" + if not self._loupe_decode_allowed(): + return + + # Log reason for debugging + if _debug_mode and reason: + log.debug(f"Triggering decode: {reason}") + + # Trigger prefetch/decode for current index + self._do_prefetch(self.current_index, override_path=override_path) + def get_decoded_image(self, index: int) -> Optional[DecodedImage]: """Retrieves a decoded image, blocking until ready to ensure correct display. @@ -895,6 +1084,11 @@ def get_decoded_image(self, index: int) -> Optional[DecodedImage]: where users expect to see the correct image immediately. The prefetcher minimizes cache misses by decoding adjacent images in advance. """ + if not self._loupe_decode_allowed(): + if _debug_mode: + log.debug("get_decoded_image called but loupe decode not allowed (index=%d)", index) + return None + if self.debug_cache: _t_start = time.perf_counter() print(f"[DBGCACHE] {_t_start*1000:.3f} get_decoded_image: START index={index}") @@ -932,7 +1126,19 @@ def get_decoded_image(self, index: int) -> Optional[DecodedImage]: return self._last_rendered_preview _, _, display_gen = self.get_display_info() - image_path = self.image_files[index].path + + # Variant override: use override path for current index + current_override = None + if ( + self.view_override_path + and index == self.current_index + ): + if _debug_mode: + log.debug("DISPLAY OVERRIDE kind=%s path=%s", self.view_override_kind, self.view_override_path) + image_path = Path(self.view_override_path) + current_override = image_path + else: + image_path = self.image_files[index].path path_str = image_path.as_posix() cache_key = build_cache_key(image_path, display_gen) @@ -990,7 +1196,10 @@ def get_decoded_image(self, index: int) -> Optional[DecodedImage]: try: # Submit with priority=True to cancel pending prefetch tasks and free up workers future = self.prefetcher.submit_task( - index, self.prefetcher.generation, priority=True + index, + self.prefetcher.generation, + priority=True, + override_path=current_override, ) if not future: with self._last_image_lock: @@ -1149,6 +1358,8 @@ def _emit_debounced_metadata_signals(self): self.ui_state.highlightStateChanged.emit() self.ui_state.metadataChanged.emit() + self.ui_state.variantBadgesChanged.emit() + self.ui_state.variantSaveHintChanged.emit() if self.debug_cache: _t_end = time.perf_counter() @@ -1162,8 +1373,35 @@ def _emit_debounced_metadata_signals(self): self.ui_state.batchInfoText, ) + def get_variant_save_hint(self) -> str: + """Returns a string describing the save behavior when viewing a variant.""" + if self.view_override_path and self._get_save_target_path_for_current_view(): + return "Saving will restore to main image (backup will be created)." + return "" + # --- Image Editor Integration --- + def _get_save_target_path_for_current_view(self) -> Optional[Path]: + """Determine the target path for saving edits based on current view. + + If we are viewing a variant (backup or developed) via override, we want + to save changes "as" the main image so that a NEW backup of the main + image is created (Policy A). + """ + if not self.view_override_path: + return None + + # Policy Change: When editing a "developed" variant (e.g. from RawTherapee), + # we want to save IN-PLACE (overwrite the developed file) rather than + # overwriting the Main source file. This prevents accidental data loss/confusion. + # Editing a "backup" still targets the Main file (restore behavior). + if self.view_override_kind == "developed": + return None + + if self.current_index is not None and 0 <= self.current_index < len(self.image_files): + return self.image_files[self.current_index].path + return None + @Slot() def save_edited_image(self): """Saves the edited image in a background thread to keep UI responsive. @@ -1184,6 +1422,9 @@ def save_edited_image(self): if write_sidecar and 0 <= self.current_index < len(self.image_files): dev_path = self.image_files[self.current_index].developed_jpg_path + # Determine save_target_path for variant saves + save_target_path = self._get_save_target_path_for_current_view() + # Store save token to prevent "surprise close" if user navigates away during save self._save_initiated_path = self.image_editor.current_filepath @@ -1196,7 +1437,9 @@ def do_save(): """Worker function that runs in background thread.""" try: result = self.image_editor.save_image( - write_developed_jpg=write_sidecar, developed_path=dev_path + write_developed_jpg=write_sidecar, + developed_path=dev_path, + save_target_path=save_target_path, ) return {"success": True, "result": result} except RuntimeError as e: @@ -1258,6 +1501,10 @@ def _on_save_finished(self, save_result: dict): if editor_still_on_same_image: self.image_editor.clear() + # 2b. Clear variant override (save always targets Main) + if editor_still_on_same_image: + self._clear_variant_override() + # 3. Refresh List and Handle Selection if editor_still_on_same_image: # Full refresh to see new file or updated timestamp @@ -1283,6 +1530,9 @@ def _on_save_finished(self, save_result: dict): self.prefetcher.cancel_all() self.prefetcher.update_prefetch(self.current_index) self.sync_ui_state() + # Refresh variant badges (backup was created) + if self.ui_state: + self.ui_state.variantBadgesChanged.emit() else: # User navigated away - skip full refresh to preserve their selection # Just clear stale cache entry for the saved image @@ -1324,6 +1574,9 @@ def _set_current_index( self.current_index = index # Set index first so signals pick up correct image + # Clear variant override on navigation + self._clear_variant_override() + self._reset_crop_settings() if self.debug_cache: @@ -1476,6 +1729,18 @@ def toggle_grid_view(self): """Toggle between grid view and loupe (single image) view.""" self._set_grid_view_active(not self._is_grid_view_active) + @Slot() + def refresh_grid(self): + """Full manual refresh: Rescans the disk directory and rebuilds the grid view. + + This is a heavy operation that clears caches and rescans the filesystem. + Use this only when you need to pick up external changes that the watcher + might have missed. + """ + log.info("Manual grid refresh requested (Full Rescan)") + # Ensure all images are rescanned from disk and the grid follows + self.refresh_image_list() + def switch_to_grid_view(self): """Switch to grid view (from loupe view). Called by Esc key.""" if not self._is_grid_view_active: @@ -1489,10 +1754,25 @@ def _set_grid_view_active(self, active: bool): self._is_grid_view_active = active if active: - # Entering grid view - refresh the model - self._thumbnail_model.refresh() - # Update path resolver for the current directory - self._path_resolver.update_from_model(self._thumbnail_model) + # Entering grid view + self.pending_prefetch_index = None + # Cancel inflight full-res decode jobs to free up resources + self.prefetcher.cancel_all() + # Cancel stale thumbnail jobs (e.g. from transient top-of-list state) + if hasattr(self, "_thumbnail_prefetcher"): + self._thumbnail_prefetcher.cancel_all() + + # Refresh the model if dirty or empty + needs_refresh = self._grid_model_dirty or self._thumbnail_model.rowCount() == 0 + if needs_refresh: + self._grid_refreshes += 1 + + # Always use controller's list, even if empty. + self._thumbnail_model.refresh_from_controller(self.image_files) + + # Update path resolver for the current directory + self._path_resolver.update_from_model(self._thumbnail_model) + self._grid_model_dirty = False # Find current loupe image in grid and scroll to it if self.image_files and 0 <= self.current_index < len(self.image_files): @@ -1508,7 +1788,10 @@ def _set_grid_view_active(self, active: bool): log.info("Switched to grid view") else: + # Entering loupe view log.info("Switched to loupe view") + # Trigger exactly one decode for the current index + self._maybe_decode_current_image("enter-loupe") # Notify UI state via signal self.ui_state.isGridViewActiveChanged.emit(active) @@ -2535,8 +2818,8 @@ def _launch_helicon_with_files(self, files: List[Path]) -> bool: unique_files = sorted(list(set(files))) success, tmp_path = launch_helicon_focus(unique_files) if success and tmp_path: - # Schedule delayed deletion of the temporary file - QTimer.singleShot(5000, lambda: self._delete_temp_file(tmp_path)) + # Defer deletion until shutdown to avoid race condition with Helicon Focus + self._temp_files_to_clean.append(tmp_path) # Record stacking metadata today = date.today().isoformat() @@ -2555,14 +2838,7 @@ def _launch_helicon_with_files(self, files: List[Path]) -> bool: return success - def _delete_temp_file(self, tmp_path: Path): - """Deletes the temporary file list passed to Helicon Focus.""" - if tmp_path.exists(): - try: - os.remove(tmp_path) - log.info("Deleted temporary file: %s", tmp_path) - except OSError as e: - log.error("Error deleting temporary file %s: %s", tmp_path, e) + def clear_all_stacks(self): log.info("Clearing all defined stacks.") @@ -4419,6 +4695,17 @@ def shutdown_nonqt(self): except Exception as e: log.warning("Error saving sidecar during shutdown: %s", e) + # Clean up temporary files (e.g. Helicon Focus lists) + if self._temp_files_to_clean: + log.debug("Cleaning up %d temporary files...", len(self._temp_files_to_clean)) + for tmp_path in self._temp_files_to_clean: + try: + tmp_path.unlink(missing_ok=True) + log.debug("Deleted temporary file: %s", tmp_path) + except OSError as e: + log.warning("Error deleting temporary file %s: %s", tmp_path, e) + self._temp_files_to_clean.clear() + log.info("Background shutdown complete.") def shutdown(self): @@ -4448,34 +4735,32 @@ def empty_recycle_bin(self): clear_raw_count_cache() log.info("Emptied recycle bins and cleared delete history") - def _on_cache_evict(self): + def _on_cache_evict(self, key, value): """Callback for when the image cache evicts an item.""" now = time.time() - # 1. Record eviction timestamp - self._eviction_timestamps.append(now) - - # 2. Prune timestamps older than window - # Keep list short - cutoff = now - CACHE_THRASH_WINDOW_SECS - self._eviction_timestamps = [t for t in self._eviction_timestamps if t > cutoff] - - # 3. Check for thrashing (e.g., > threshold evictions in window) - if len(self._eviction_timestamps) > CACHE_THRASH_THRESHOLD: - # 4. Rate limit the warning - if now - self._last_cache_warning_time > CACHE_WARNING_COOLDOWN_SECS: - self._last_cache_warning_time = now - self._has_warned_cache_full = True - - # Format usage info - used_gb = self.image_cache.currsize / (1024**3) - max_gb = self.image_cache.max_bytes / (1024**3) - - msg = f"Cache thrashing! {len(self._eviction_timestamps)} evictions in {CACHE_THRASH_WINDOW_SECS}s. Usage: {used_gb:.1f}GB / {max_gb:.1f}GB." - - # Use QTimer.singleShot to ensure this runs on the main thread - QTimer.singleShot(0, lambda: self.update_status_message(msg)) - log.warning(msg) + with self._eviction_lock: + # 1. Record eviction timestamp / prune + self._eviction_timestamps.append(now) + cutoff = now - CACHE_THRASH_WINDOW_SECS + self._eviction_timestamps = [t for t in self._eviction_timestamps if t > cutoff] + + # 2. Check for thrashing (e.g., > threshold evictions in window) + if len(self._eviction_timestamps) > CACHE_THRASH_THRESHOLD: + # 3. Rate limit the warning + if now - self._last_cache_warning_time > CACHE_WARNING_COOLDOWN_SECS: + self._last_cache_warning_time = now + self._has_warned_cache_full = True + + # UI update logic + used_gb = self.image_cache.currsize / (1024**3) + max_gb = self.image_cache.max_bytes / (1024**3) + msg = f"Cache thrashing! {len(self._eviction_timestamps)} evictions in {CACHE_THRASH_WINDOW_SECS}s. Usage: {used_gb:.1f}GB / {max_gb:.1f}GB." + + # Schedule UI work safely on main thread + # QTimer.singleShot(0, ...) is thread-safe entry to main loop + QTimer.singleShot(0, lambda: self.update_status_message(msg)) + log.warning(msg) def restore_all_from_recycle_bin(self): """Restores all files from tracked recycle bins to their parent folders.""" @@ -4920,7 +5205,11 @@ def load_image_for_editing(self): This provides a centralized entry point for loading the editor correctly. """ try: - active_path = self.get_active_edit_path(self.current_index) + # Use variant override path if active + if self.view_override_path: + active_path = Path(self.view_override_path) + else: + active_path = self.get_active_edit_path(self.current_index) filepath = str(active_path) # Fetch cached preview if available for faster initial display @@ -5938,10 +6227,17 @@ def quick_auto_levels(self): return try: + # Determine save_target_path for variant saves (Policy A) + save_target_path = self._get_save_target_path_for_current_view() + # Try uint8 fast path first, fall back to regular save - save_result = self.image_editor.save_image_uint8_levels() + save_result = self.image_editor.save_image_uint8_levels( + save_target_path=save_target_path + ) if save_result is None: - save_result = self.image_editor.save_image() + save_result = self.image_editor.save_image( + save_target_path=save_target_path + ) except RuntimeError as e: log.warning(f"quick_auto_levels: Save failed: {e}") self.update_status_message(f"Failed to save image: {e}") @@ -6033,9 +6329,14 @@ def _apply_auto_levels_at_index(self, index: int) -> bool: return False try: - save_result = self.image_editor.save_image_uint8_levels() + save_target_path = self._get_save_target_path_for_current_view() + save_result = self.image_editor.save_image_uint8_levels( + save_target_path=save_target_path + ) if save_result is None: - save_result = self.image_editor.save_image() + save_result = self.image_editor.save_image( + save_target_path=save_target_path + ) except Exception as e: log.warning("batch auto levels: save failed for %s: %s", filepath, e) return False @@ -6596,10 +6897,17 @@ def cleanup_recycle_bins(self): clear_raw_count_cache() -def main(image_dir: str = "", debug: bool = False, debug_cache: bool = False): +def main( + image_dir: Optional[str] = None, + debug: bool = False, + debug_cache: bool = False, + debug_thumb_timing: bool = False, + debug_thumb_trace: bool = False, +): """FastStack Application Entry Point""" - global _debug_mode + global _debug_mode, _debug_thumb_timing _debug_mode = debug + _debug_thumb_timing = debug_thumb_timing t0 = time.perf_counter() setup_logging(debug) @@ -6666,7 +6974,13 @@ def main(image_dir: str = "", debug: bool = False, debug_cache: bool = False): os.path.join(os.path.dirname(PySide6.__file__), "qml", "Qt5Compat") ) - controller = AppController(image_dir_path, engine, debug_cache=debug_cache) + controller = AppController( + image_dir=image_dir_path, + engine=engine, + debug_cache=debug_cache, + debug_thumb_timing=debug_thumb_timing, + debug_thumb_trace=debug_thumb_trace, + ) if debug: log.info("Startup: after AppController: %.3fs", time.perf_counter() - t0) image_provider = ImageProvider(controller) @@ -6779,8 +7093,26 @@ def cli(): parser.add_argument( "--debugcache", action="store_true", help="Enable debug cache features" ) + parser.add_argument( + "--debug-thumbtiming", + action="store_true", + help="Enable thumbnail pipeline timing logs (implies --debug)", + ) + parser.add_argument( + "--debug-thumbtrace", + action="store_true", + help="Enable thumbnail pipeline trace logs (implies --debug)", + ) args = parser.parse_args() - main(image_dir=args.image_dir, debug=args.debug, debug_cache=args.debugcache) + if args.debug_thumbtiming or args.debug_thumbtrace: + args.debug = True + main( + image_dir=args.image_dir, + debug=args.debug, + debug_cache=args.debugcache, + debug_thumb_timing=args.debug_thumbtiming, + debug_thumb_trace=args.debug_thumbtrace, + ) if __name__ == "__main__": diff --git a/faststack/check_daemon.py b/faststack/check_daemon.py new file mode 100644 index 0000000..8e03bc6 --- /dev/null +++ b/faststack/check_daemon.py @@ -0,0 +1,15 @@ +import threading +from concurrent.futures import ThreadPoolExecutor + +def set_daemon(): + try: + threading.current_thread().daemon = True + print(f"Set daemon for {threading.current_thread().name}") + except Exception as e: + print(f"Failed to set daemon for {threading.current_thread().name}: {e}") + +def check_daemon(): + return threading.current_thread().daemon + +with ThreadPoolExecutor(max_workers=1, initializer=set_daemon) as executor: + print(f"Result: {executor.submit(check_daemon).result()}") diff --git a/faststack/debug_path_norm.py b/faststack/debug_path_norm.py new file mode 100644 index 0000000..40f8107 --- /dev/null +++ b/faststack/debug_path_norm.py @@ -0,0 +1,22 @@ + +import os +from pathlib import Path + +def norm_path(p: Path) -> str: + return os.path.normcase(os.path.abspath(str(p))) + +p1 = Path("C:/Test/File.JPG") +p2 = Path("C:/Test/file.jpg") + +print(f"p1: {p1}") +print(f"p2: {p2}") +print(f"p1 == p2: {p1 == p2}") + +n1 = Path(norm_path(p1)) +n2 = Path(norm_path(p2)) + +print(f"n1: {n1}") +print(f"n2: {n2}") +print(f"n1 == n2: {n1 == n2}") +print(f"n1 == p1: {n1 == p1}") + diff --git a/faststack/imaging/cache.py b/faststack/imaging/cache.py index c899468..8e9581e 100644 --- a/faststack/imaging/cache.py +++ b/faststack/imaging/cache.py @@ -3,32 +3,58 @@ import logging from pathlib import Path from typing import Any, Callable, Optional, Union - import time import threading from cachetools import LRUCache - log = logging.getLogger(__name__) +def get_decoded_image_size(item) -> int: + """Calculates the size of a DecodedImage object.""" + # In this simplified example, we only store the buffer. + # In the full app, this would also account for the QImage/QTexture. + from faststack.models import DecodedImage + + if isinstance(item, DecodedImage): + # Handle both numpy arrays and memoryview buffers + if hasattr(item.buffer, "nbytes"): + return item.buffer.nbytes + elif isinstance(item.buffer, (bytes, bytearray)): + return len(item.buffer) + else: + # Fallback: estimate from dimensions (more accurate for image buffers than sys.getsizeof) + bytes_per_pixel = getattr(item, "channels", 4) # Default to RGBA + return item.width * item.height * bytes_per_pixel + + log.warning( + f"Unexpected item type in cache: {type(item)}. Returning estimated size of 1." + ) + return 1 # Should not happen + + class ByteLRUCache(LRUCache): """An LRU Cache that respects the size of its items in bytes.""" def __init__( self, max_bytes: int, - size_of: Callable[[Any], int] = len, - on_evict: Optional[Callable[[], None]] = None, + size_of: Callable[[Any], int] = get_decoded_image_size, + on_evict: Optional[Callable[[Any, Any], None]] = None, ): super().__init__(maxsize=max_bytes, getsizeof=size_of) self.on_evict = on_evict + # RLock is required: __setitem__ holds _lock and calls super().__setitem__(), + # which may call our overridden popitem() for LRU eviction. A non-reentrant + # Lock would deadlock on that re-entry. self._lock = threading.RLock() # Tombstones to prevent race conditions where a deleted image is re-cached # by a lingering background thread. # Set of prefixes that are currently "tombstoned" (forbidden from caching). self._tombstones: set[str] = set() self._tombstone_expiry: dict[str, float] = {} + self._pending_callbacks: Optional[list[Callable[[], None]]] = None + self._pending_callbacks_owner: Optional[int] = None log.info( f"Initialized byte-aware LRU cache with {max_bytes / 1024**2:.2f} MB capacity." ) @@ -46,6 +72,7 @@ def max_bytes(self, value: int) -> None: log.debug(f"Cache max_bytes updated to {v / 1024**2:.2f} MB") def __setitem__(self, key, value): + pending_callbacks = [] with self._lock: # Check tombstones - prevent caching if key starts with a tombstoned prefix # This is critical for preventing "ghost" images after deletion @@ -65,10 +92,25 @@ def __setitem__(self, key, value): return # Before adding a new item, we might need to evict others - # This is handled by the parent class, which will call popitem if needed - super().__setitem__(key, value) + # This is handled by the parent class, which will call popitem if needed. + # We override popitem to capture callbacks if they occur during this call. + self._pending_callbacks = pending_callbacks + self._pending_callbacks_owner = threading.get_ident() + try: + super().__setitem__(key, value) + finally: + self._pending_callbacks = None + self._pending_callbacks_owner = None + log.debug(f"Cached item '{key}'. Cache size: {self.currsize / 1024**2:.2f} MB") + # Execute all captured eviction callbacks OUTSIDE the lock + for callback in pending_callbacks: + try: + callback() + except Exception: + log.exception("Error in eviction callback") + def __getitem__(self, key): """Thread-safe access (updates LRU order).""" with self._lock: @@ -85,16 +127,42 @@ def get(self, key, default=None): return super().get(key, default) def popitem(self): - """Extend popitem to log eviction.""" + """Extend popitem to log eviction. + + Lock note: acquires _lock, which is safe because _lock is an RLock. + When called from __setitem__ -> super().__setitem__() (LRU eviction), + the lock is already held by the same thread; RLock allows re-entry. + Eviction callbacks are deferred via _pending_callbacks when inside + __setitem__, and always execute OUTSIDE _lock. + """ with self._lock: key, value = super().popitem() log.debug( f"Evicted item '{key}'. Cache size after eviction: {self.currsize / 1024**2:.2f} MB" ) - callback = self.on_evict - + + # Create a bound callback for this specific item + callback = None + if self.on_evict: + # Capture key/value in closure + # We use a default arg to bind immediate values + def _callback_func(k=key, v=value): + if self.on_evict: + self.on_evict(k, v) + + # If we are inside a __setitem__ call on the SAME thread, defer the callback + if self._pending_callbacks is not None and threading.get_ident() == self._pending_callbacks_owner: + self._pending_callbacks.append(_callback_func) + callback = None # Already deferred + else: + callback = _callback_func + + # Execute callback OUTSIDE the lock to avoid deadlocks/reentrancy if callback: - callback() + try: + callback() + except Exception: + log.exception("Error in eviction callback") # In a real Qt app, `value` would be a tuple like (numpy_buffer, qtexture_id) # and we would explicitly free the GPU texture here. @@ -104,12 +172,12 @@ def clear(self): """Clear cache without triggering eviction callbacks.""" # Temporarily disable callback to prevent "thrashing" warnings during mass clear with self._lock: - callback = self.on_evict + saved_callback = self.on_evict self.on_evict = None try: super().clear() finally: - self.on_evict = callback + self.on_evict = saved_callback def pop_path(self, path: Union[Path, str]): """Targeted invalidation of all generations for a given path. @@ -193,17 +261,9 @@ def evict_paths(self, paths: list[Union[Path, str]]): # 4. Remove keys removed_bytes = 0 for k in keys_to_remove: - # We need size before removal to log correctly? - # LRUCache.pop returns value. We can ask getsizeof(value) but pop removes it anyway. - # ByteLRUCache tracks currsize. We can diff currsize. - # But simpler: just trust currsize updates. - # We want to log *how much* we removed. - # Accessing self.getsizeof(val) needs val. - # val = self.pop(k) would work. - # We want to log *how much* we removed. - if k in self: - # Pop first to avoid updating LRU order with self[k] - val = self.pop(k) + # Use super().pop to avoid re-acquiring our lock / calling our overridden pop logic. + val = super().pop(k, None) + if val is not None: try: size = get_decoded_image_size(val) except Exception: @@ -215,28 +275,6 @@ def evict_paths(self, paths: list[Union[Path, str]]): f"Evicted {len(keys_to_remove)} entries ({removed_bytes / 1024**2:.2f} MB) for {len(paths)} paths" ) -def get_decoded_image_size(item) -> int: - """Calculates the size of a decoded image tuple (buffer, qimage).""" - # In this simplified example, we only store the buffer. - # In the full app, this would also account for the QImage/QTexture. - from faststack.models import DecodedImage - - if isinstance(item, DecodedImage): - # Handle both numpy arrays and memoryview buffers - if hasattr(item.buffer, "nbytes"): - return item.buffer.nbytes - elif isinstance(item.buffer, (bytes, bytearray)): - return len(item.buffer) - else: - # Fallback: estimate from dimensions (more accurate for image buffers than sys.getsizeof) - bytes_per_pixel = getattr(item, "channels", 4) # Default to RGBA - return item.width * item.height * bytes_per_pixel - - log.warning( - f"Unexpected item type in cache: {type(item)}. Returning estimated size of 1." - ) - return 1 # Should not happen - def build_cache_key(image_path: Union[Path, str], display_generation: int) -> str: """Builds a stable cache key that survives list reordering.""" diff --git a/faststack/imaging/editor.py b/faststack/imaging/editor.py index 62460ed..164564a 100644 --- a/faststack/imaging/editor.py +++ b/faststack/imaging/editor.py @@ -1614,6 +1614,7 @@ def _apply_highlights_shadows( shadows: float, *, srgb_u8_stride: Optional[np.ndarray] = None, + srgb_u8: Optional[np.ndarray] = None, analysis_state: Optional[Dict[str, float]] = None, edits: Optional[Dict[str, Any]] = None, ) -> np.ndarray: @@ -1633,24 +1634,26 @@ def _apply_highlights_shadows( shadows: -1.0 to 1.0, positive lifts shadows, negative crushes srgb_u8_stride: Optional uint8 sRGB array (strided) for accurate JPEG clipping detection (should be the image BEFORE linearization) + srgb_u8: Keyword-only alias for srgb_u8_stride (preferred if provided). analysis_state: Optional pre-computed analysis state to avoid re-work. Returns: Adjusted float32 RGB array (linear) """ arr = linear + effective_srgb_u8 = srgb_u8 if srgb_u8 is not None else srgb_u8_stride # Analyze highlight state if needed # If caller passed analysis_state, usage that. state = analysis_state if state is None: # Re-compute locally if not provided - # We assume srgb_u8_stride is ALREADY STRIDED if passed (based on the name change) + # We assume effective_srgb_u8 is ALREADY STRIDED if passed arr_stride = arr[::4, ::4, :] - # If srgb_u8_stride was passed, use it directly (it's already small). + # If effective_srgb_u8 was passed, use it directly (it's already small). # If it wasn't passed, we can't easily recreate the source state here without the original source buffer. # But the caller (_apply_edits) usually provides it. - state = _analyze_highlight_state(arr_stride, srgb_u8=srgb_u8_stride) + state = _analyze_highlight_state(arr_stride, srgb_u8=effective_srgb_u8) # Ensure we have edits dict to check upstream hash if edits is None: @@ -1802,6 +1805,8 @@ def set_crop_box(self, crop_box: Tuple[int, int, int, int]): self.current_edits["crop_box"] = crop_box self._edits_rev += 1 + + def _write_tiff_16bit(self, path: Path, arr_float: np.ndarray): """ Writes a float32 (0-1) numpy array as an uncompressed 16-bit RGB TIFF using OpenCV. @@ -1894,15 +1899,33 @@ def _get_sanitized_exif_bytes(self) -> Optional[bytes]: def _ensure_float_image(self) -> None: """Ensure self.float_image exists. Needed when load_image(preview_only=True).""" - if self.float_image is not None: - return - if self.original_image is None: - raise RuntimeError("No image loaded") - rgb = self.original_image.convert("RGB") - self.float_image = np.array(rgb).astype(np.float32) / 255.0 + # 1. Quick check under lock + with self._lock: + if self.float_image is not None: + return + if self.original_image is None: + raise RuntimeError("No image loaded") + # Snapshot original image to convert outside lock + original_ref = self.original_image + + # 2. Expensive conversion outside lock + rgb = original_ref.convert("RGB") + float_arr = np.array(rgb).astype(np.float32) / 255.0 + + # 3. Store result under lock (checking if someone beat us to it, or if image changed) + with self._lock: + # Only assign if original_image hasn't changed + if self.original_image is original_ref: + if self.float_image is None: + self.float_image = float_arr + + def save_image( - self, write_developed_jpg: bool = False, developed_path: Optional[Path] = None + self, + write_developed_jpg: bool = False, + developed_path: Optional[Path] = None, + save_target_path: Optional[Path] = None, ) -> Optional[Tuple[Path, Path]]: """Saves the edited image, backing up the original. @@ -1911,43 +1934,71 @@ def save_image( This should be True only when editing RAW files. developed_path: Optional explicit path for the developed JPG. If not provided, it's derived from current_filepath. + save_target_path: Optional override for the output path. When saving + from a variant (backup/developed), this should be + the Main file's path. Backup is created for Main, + the variant source file is left untouched. Returns: A tuple of (saved_path, backup_path) on success, otherwise None. + + Raises: + RuntimeError: If preconditions are not met (no path, no image) or if saving fails. """ - if self.current_filepath is None or self.original_image is None: - return None + if self.current_filepath is None: + raise RuntimeError("No file path set") + if self.original_image is None: + raise RuntimeError("No image loaded") # Ensure float master exists (preview_only loads may not have it) try: self._ensure_float_image() except RuntimeError: - return None + raise _debug = log.isEnabledFor(logging.DEBUG) if _debug: t0 = time.perf_counter() # 1. Apply Edits to Full Resolution - # Skip the expensive .copy() when safe — see _edits_can_share_input(). - _safe_no_copy = self._edits_can_share_input(self.current_edits) - source_arr = self.float_image if _safe_no_copy else self.float_image.copy() - if _safe_no_copy: - log.debug("save_image: skipping float_image.copy() (safe no-copy path)") + # Snapshot state under lock to avoid races + with self._lock: + # Re-check float image existence under lock (though _ensure calls it too) + # Previously returned None, now raising to be explicit about failure + if self.float_image is None: + raise RuntimeError("save_image called with no float_image (race condition?)") + + # Determine if we can skip copy + _safe_no_copy = self._edits_can_share_input(self.current_edits) + + # Snapshot the source data + # If safe to share (read-only), we just grab the reference + # If not safe, we MUST copy it here while holding the lock + if _safe_no_copy: + source_arr = self.float_image + log.debug("save_image: skipping float_image.copy() (safe no-copy path)") + else: + source_arr = self.float_image.copy() + + # Snapshot edits + edits_snapshot = self.current_edits.copy() + + # Expensive computation runs WITHOUT the lock final_float = self._apply_edits( - source_arr, for_export=True + source_arr, edits=edits_snapshot, for_export=True ) # (H,W,3) float32 + if _debug: t_edits = time.perf_counter() - original_path = self.current_filepath + original_path = save_target_path if save_target_path else self.current_filepath try: original_stat = original_path.stat() except OSError as e: log.warning("Unable to read timestamps for %s: %s", original_path, e) original_stat = None - # 2. Backup + # 2. Backup (always backs up original_path, which is Main when save_target_path is set) backup_path = create_backup_file(original_path) if backup_path is None: return None @@ -1963,9 +2014,9 @@ def save_image( self._write_tiff_16bit(original_path, final_float) else: # Check for geometric transforms - rotation = self.current_edits.get("rotation", 0) + rotation = edits_snapshot.get("rotation", 0) straighten_angle = float( - self.current_edits.get("straighten_angle", 0.0) + edits_snapshot.get("straighten_angle", 0.0) ) transforms_applied = (rotation != 0) or (abs(straighten_angle) > 0.001) @@ -2007,9 +2058,9 @@ def save_image( developed_path = original_path.with_name(f"{stem}-developed.jpg") # Check for geometric transforms (re-check not strictly needed but for clarity) - rotation = self.current_edits.get("rotation", 0) + rotation = edits_snapshot.get("rotation", 0) straighten_angle = float( - self.current_edits.get("straighten_angle", 0.0) + edits_snapshot.get("straighten_angle", 0.0) ) transforms_applied = (rotation != 0) or (abs(straighten_angle) > 0.001) @@ -2061,13 +2112,18 @@ def save_image( log.exception("Failed to save %s: %s", self.current_filepath, e) raise RuntimeError("Save failed: %s" % str(e)) from e - def save_image_uint8_levels(self) -> Optional[Tuple[Path, Path]]: + def save_image_uint8_levels( + self, save_target_path: Optional[Path] = None, + ) -> Optional[Tuple[Path, Path]]: """Fast-path save using a uint8 LUT for levels-only edits. Instead of float_convert -> _apply_edits -> uint8, builds a 256-entry lookup table from the blacks/whites levels formula and applies it directly to the original uint8 PIL image data. + Args: + save_target_path: Optional override for the output path (variant save). + Returns: (saved_path, backup_path) on success, None if the fast path is not applicable (TIFF, missing image, non-levels edits active). @@ -2075,7 +2131,7 @@ def save_image_uint8_levels(self) -> Optional[Tuple[Path, Path]]: if self.original_image is None or self.current_filepath is None: return None - original_path = self.current_filepath + original_path = save_target_path if save_target_path else self.current_filepath # TIFF needs 16-bit pipeline if original_path.suffix.lower() in (".tif", ".tiff"): diff --git a/faststack/imaging/orientation.py b/faststack/imaging/orientation.py index ea2216d..edc7605 100644 --- a/faststack/imaging/orientation.py +++ b/faststack/imaging/orientation.py @@ -84,9 +84,21 @@ def apply_orientation_to_np(buffer: np.ndarray, orientation: int) -> np.ndarray: return result -def apply_exif_orientation( - buffer: np.ndarray, image_path: Path, exif: Optional[Image.Exif] = None -) -> np.ndarray: - """Helper that reads orientation and applies it to a numpy buffer.""" - orientation = get_exif_orientation(image_path, exif) - return apply_orientation_to_np(buffer, orientation) +def apply_exif_orientation(rgb: np.ndarray, path: Path) -> np.ndarray: + """Read EXIF orientation from path and apply it to the numpy buffer. + + Requirements: + - Reads EXIF orientation from path using PIL. + - If file missing / cannot read EXIF / no EXIF: return input unchanged (as C-contiguous). + - If orientation > 1: call apply_orientation_to_np and ensure contiguity. + - No Qt deps. + """ + orientation = get_exif_orientation(path) + if orientation <= 1: + # Return input unchanged but ensure C-contiguous + if not rgb.flags["C_CONTIGUOUS"]: + return np.ascontiguousarray(rgb) + return rgb + + # apply_orientation_to_np already ensures C-contiguity + return apply_orientation_to_np(rgb, orientation) diff --git a/faststack/imaging/prefetch.py b/faststack/imaging/prefetch.py index bf2c507..851c39f 100644 --- a/faststack/imaging/prefetch.py +++ b/faststack/imaging/prefetch.py @@ -117,9 +117,7 @@ def get_monitor_profile() -> Optional[ImageCms.ImageCmsProfile]: return _monitor_profile_cache[monitor_icc_path] -# apply_orientation_to_np imported from orientation.py - -_EXIF_ORIENTATION_TAG = 274 # Exif "Orientation" +# apply_orientation_to_np and apply_exif_orientation imported from orientation.py @@ -191,6 +189,7 @@ def __init__( ) self._futures_lock = threading.RLock() self.futures: Dict[int, Future] = {} + self.future_paths: Dict[int, Path] = {} self.generation = 0 self._scheduled: Dict[int, set] = {} # generation -> set of scheduled indices @@ -207,9 +206,10 @@ def __init__( self._direction_bias: float = 0.85 # 85% of radius in travel direction def set_image_files(self, image_files: List[ImageFile]): - if self.image_files != image_files: - self.image_files = image_files - self.cancel_all() + with self._futures_lock: + if self.image_files != image_files: + self.image_files = image_files + self._cancel_all_locked() def update_prefetch( self, @@ -268,49 +268,58 @@ def update_prefetch( ahead = max(1, int(effective_radius * (1 - self._direction_bias))) behind = effective_radius - ahead + 1 - start = max(0, current_index - behind) - end = min(len(self.image_files), current_index + ahead + 1) - - log.debug( - "Prefetch range: [%d, %d) for index %d (direction=%d, behind=%d, ahead=%d)", - start, - end, - current_index, - self._last_navigation_direction, - behind, - ahead, - ) + # Invariant: All reads/writes of self.futures, self._scheduled, self.generation, + # and self.image_files that participate in scheduling or cancellation MUST + # happen under _futures_lock. - # Cancel stale futures and remove from scheduled - tasks_submitted = 0 + # Snapshoting and range computation inside lock with self._futures_lock: + n = len(self.image_files) + if n == 0: + return + # Ensure current_index is clamped + safe_current = max(0, min(n - 1, current_index)) + + start = max(0, safe_current - behind) + end = min(n, safe_current + ahead + 1) + + log.debug( + "Prefetch range: [%d, %d) for index %d (direction=%d, behind=%d, ahead=%d)", + start, + end, + safe_current, + self._last_navigation_direction, + behind, + ahead, + ) + + # Build priority order: current first, then in direction of travel + priority_order = [safe_current] + if self._last_navigation_direction > 0: + priority_order.extend(range(safe_current + 1, end)) + priority_order.extend(range(safe_current - 1, start - 1, -1)) + else: + priority_order.extend(range(safe_current - 1, start - 1, -1)) + priority_order.extend(range(safe_current + 1, end)) + + # Cancel stale futures and remove from scheduled + tasks_submitted = 0 # Clean up old generation entries to prevent memory leak - old_generations = [g for g in self._scheduled if g < self.generation] + old_generations = [g for g in list(self._scheduled.keys()) if g < self.generation] for g in old_generations: - del self._scheduled[g] + self._scheduled.pop(g, None) - # Get scheduled set for current generation (inside lock) + # Get scheduled set for current generation scheduled = self._scheduled.setdefault(self.generation, set()) - stale_keys = [] + for index, future in list(self.futures.items()): if index < start or index >= end: if future.cancel(): - stale_keys.append(index) + self.futures.pop(index, None) scheduled.discard(index) - for key in stale_keys: - del self.futures[key] - - # Build priority order: current first, then in direction of travel - priority_order = [current_index] - if self._last_navigation_direction > 0: - priority_order.extend(range(current_index + 1, end)) - priority_order.extend(range(current_index - 1, start - 1, -1)) - else: - priority_order.extend(range(current_index - 1, start - 1, -1)) - priority_order.extend(range(current_index + 1, end)) for i in priority_order: - if i < 0 or i >= len(self.image_files): + if i < 0 or i >= n: continue if i not in scheduled and i not in self.futures: self.submit_task(i, self.generation) @@ -322,19 +331,36 @@ def update_prefetch( print(f"[DBGCACHE] {_t_end*1000:.3f} update_prefetch: DONE submitted={tasks_submitted} total={(_t_end - _t_start)*1000:.2f}ms") def submit_task( - self, index: int, generation: int, priority: bool = False + self, index: int, generation: int, priority: bool = False, override_path: Optional[Path] = None ) -> Optional[Future]: """Submits a decoding task for a given index.""" if self._stop_event.is_set(): return None + # Capture list snapshot and check bounds before accessing path + image_files = self.image_files + if index < 0 or index >= len(image_files): + return None + + requested_path = override_path if override_path is not None else image_files[index].path + if self.debug and priority: _t_start = time.perf_counter() - print(f"[DBGCACHE] {_t_start*1000:.3f} submit_task: PRIORITY index={index} gen={generation}") + print(f"[DBGCACHE] {_t_start*1000:.3f} submit_task: PRIORITY index={index} gen={generation} override={override_path}") with self._futures_lock: + # We track by index. If we already have a job for this index, + # we must cancel it if the requested path is different + # (e.g. switching between main and variants). if index in self.futures and not self.futures[index].done(): - return self.futures[index] + current_path = self.future_paths.get(index) + if current_path != requested_path: + # Force cancel the old one to switch paths + self.futures[index].cancel() + self.futures.pop(index, None) + self.future_paths.pop(index, None) + else: + return self.futures[index] if priority: cancelled_count = 0 @@ -346,7 +372,7 @@ def submit_task( if not future.done() and future.cancel(): cancelled_count += 1 - del self.futures[task_index] + self.futures.pop(task_index, None) if cancelled_count > 0: log.debug( "Cancelled %d pending prefetch tasks to prioritize index %d", @@ -365,8 +391,10 @@ def submit_task( display_width, display_height, display_generation, + override_path, ) self.futures[index] = future + self.future_paths[index] = requested_path future.add_done_callback(lambda f, idx=index: self._cleanup_future(idx, f)) return future @@ -380,16 +408,18 @@ def _decode_and_cache( display_width: int, display_height: int, display_generation: int, + override_path: Optional[Path] = None, ) -> Optional[tuple[Path, int]]: """The actual work done by the thread pool.""" if generation != self.generation or self._stop_event.is_set(): return None - exif_obj = None + # Use override path if provided, otherwise default to image_file.path + target_path = override_path if override_path is not None else image_file.path try: - if os.path.getsize(image_file.path) == 0: - log.warning("Skipping empty image file: %s", image_file.path) + if os.path.getsize(target_path) == 0: + log.warning("Skipping empty image file: %s", target_path) return None color_mode = config.get("color", "mode", fallback="none").lower() @@ -397,12 +427,12 @@ def _decode_and_cache( fast_dct = optimize_for == "speed" use_resized = optimize_for == "speed" should_resize = display_width > 0 and display_height > 0 - is_jpeg = image_file.path.suffix.lower() in {".jpg", ".jpeg", ".jpe"} + is_jpeg = target_path.suffix.lower() in {".jpg", ".jpeg", ".jpe"} buffer = None icc_bytes = None - exif_obj = None + orientation = 1 if color_mode == "icc": monitor_profile = get_monitor_profile() monitor_icc_path = config.get("color", "monitor_icc_path", fallback="").strip() @@ -410,7 +440,7 @@ def _decode_and_cache( if monitor_profile is not None: if is_jpeg: try: - with open(image_file.path, "rb") as f: + with open(target_path, "rb") as f: with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mmapped: if use_resized and should_resize: buffer = decode_jpeg_resized(mmapped, display_width, display_height, fast_dct=fast_dct) @@ -423,39 +453,40 @@ def _decode_and_cache( if buffer is not None: try: + # Seek to beginning of mmapped file for PIL mmapped.seek(0) with PILImage.open(mmapped) as pil_img: icc_bytes = pil_img.info.get("icc_profile") - if exif_obj is None: - exif_obj = pil_img.getexif() + orientation = pil_img.getexif().get(274, 1) except Exception: pass except Exception as e: - log.warning("Decode failed (ICC path) index=%d path=%s: %s", index, image_file.path, e) + log.warning("Decode failed (ICC path) index=%d path=%s: %s", index, target_path, e) buffer = None if buffer is None: try: - with PILImage.open(image_file.path) as img: + with PILImage.open(target_path) as img: + orientation = img.getexif().get(274, 1) img = img.convert("RGB") if should_resize: img.thumbnail((display_width, display_height), PILImage.Resampling.LANCZOS) buffer = np.array(img) except Exception as e: - log.warning("Decode failed (ICC fallback) index=%d path=%s: %s", index, image_file.path, e) + log.warning("Decode failed (ICC fallback) index=%d path=%s: %s", index, target_path, e) return None img = PILImage.fromarray(buffer) - if icc_bytes is None or exif_obj is None: + if icc_bytes is None: try: - with PILImage.open(image_file.path) as orig: - if icc_bytes is None: - icc_bytes = orig.info.get("icc_profile") - if exif_obj is None: - exif_obj = orig.getexif() + # Try to get ICC if we don't have it yet (but orientation should be set) + with PILImage.open(target_path) as orig: + icc_bytes = orig.info.get("icc_profile") + if orientation == 1: + orientation = orig.getexif().get(274, 1) except Exception as e: - log.warning("Failed to read metadata from %s: %s", image_file.path, e) + log.warning("Failed to read metadata from %s: %s", target_path, e) src_profile = None src_profile_key = None @@ -480,7 +511,7 @@ def _decode_and_cache( if buffer is None: if is_jpeg: try: - with open(image_file.path, "rb") as f: + with open(target_path, "rb") as f: with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mmapped: if use_resized and should_resize: buffer = decode_jpeg_resized(mmapped, display_width, display_height, fast_dct=fast_dct) @@ -491,12 +522,12 @@ def _decode_and_cache( img.thumbnail((display_width, display_height), PILImage.Resampling.LANCZOS) buffer = np.array(img) + # Capture orientation if we have a buffer if buffer is not None: try: mmapped.seek(0) with PILImage.open(mmapped) as pil_img: - if exif_obj is None: - exif_obj = pil_img.getexif() + orientation = pil_img.getexif().get(274, 1) except Exception: pass except Exception: @@ -504,37 +535,30 @@ def _decode_and_cache( if buffer is None: try: - with PILImage.open(image_file.path) as img: - # Optimization: capture EXIF while the file is open - if exif_obj is None: - exif_obj = img.getexif() - + with PILImage.open(target_path) as img: + orientation = img.getexif().get(274, 1) img = img.convert("RGB") if should_resize: img.thumbnail((display_width, display_height), PILImage.Resampling.LANCZOS) buffer = np.array(img) except Exception as e: - log.warning("Decode failed index=%d path=%s: %s", index, image_file.path, e) + log.warning("Decode failed index=%d path=%s: %s", index, target_path, e) return None if buffer is None: return None buffer = np.ascontiguousarray(buffer) - bytes_per_line = buffer.strides[0] + # Apply captured orientation try: - if exif_obj is None: - with PILImage.open(image_file.path) as orig: - exif_obj = orig.getexif() - orientation = exif_obj.get(274, 1) if exif_obj else 1 if orientation > 1: buffer = apply_orientation_to_np(buffer, orientation) - buffer = np.ascontiguousarray(buffer) - bytes_per_line = buffer.strides[0] except Exception as e: log.warning("Failed to apply EXIF orientation: %s", e) + bytes_per_line = buffer.strides[0] + if color_mode == "saturation": # Safer pattern for custom config wrappers val = config.get("color", "saturation_factor", fallback="1.0") @@ -554,9 +578,9 @@ def _decode_and_cache( if generation != self.generation or self._stop_event.is_set(): return None - cache_key = build_cache_key(image_file.path, display_generation) + cache_key = build_cache_key(target_path, display_generation) self.cache_put(cache_key, decoded) - return (image_file.path, display_generation) + return (target_path, display_generation) except Exception as e: # Downgraded from ERROR to prevent log noise on bad files @@ -569,16 +593,26 @@ def _cleanup_future(self, index: int, future: Future): # Only remove if it's the specific future we're tracking # (to avoid race if a new task for the same index was submitted) if self.futures.get(index) is future: - del self.futures[index] + self.futures.pop(index, None) + self.future_paths.pop(index, None) + + def _cancel_all_locked(self): + """Internal helper to cancel all pending prefetching tasks. + Assumes _futures_lock is already held. + """ + self.generation += 1 # Invalidate in-flight tasks + # Snapshot values before cancelling + all_futures = list(self.futures.values()) + for future in all_futures: + future.cancel() + self.futures.clear() + self.future_paths.clear() + self._scheduled.clear() def cancel_all(self): """Cancels all pending prefetching tasks.""" with self._futures_lock: - self.generation += 1 # Invalidate in-flight tasks - for index, future in list(self.futures.items()): - future.cancel() - del self.futures[index] - self._scheduled.clear() + self._cancel_all_locked() def shutdown(self): diff --git a/faststack/io/indexer.py b/faststack/io/indexer.py index dc12dc2..dc0fc31 100644 --- a/faststack/io/indexer.py +++ b/faststack/io/indexer.py @@ -8,6 +8,7 @@ from typing import List, Dict, Tuple from faststack.models import ImageFile +from faststack.io.variants import VariantGroup, build_variant_map, parse_variant_stem log = logging.getLogger(__name__) @@ -15,72 +16,70 @@ JPG_EXTENSIONS = {".jpg", ".jpeg", ".jpe"} -_DEVELOPED_SUFFIX = "-developed" - # Matches FastStack backup stems: name-backup, name-backup2, name-backup33, etc. _BACKUP_STEM_RE = re.compile(r"-backup\d*$", re.IGNORECASE) -def find_images(directory: Path) -> List[ImageFile]: - """Finds all JPGs in a directory and pairs them with RAW files.""" - t_start = time.perf_counter() - log.info("Scanning directory for images: %s", directory) +def _scan_directory( + directory: Path, +) -> Tuple[ + List[Tuple[Path, os.stat_result]], + List[Tuple[Path, os.stat_result]], + Dict[str, List[Tuple[Path, os.stat_result]]], +]: + """Single os.scandir pass, returns (visible_jpgs, all_jpgs, raws). - # Categorize files + visible_jpgs: JPGs excluding backups (legacy behavior). + all_jpgs: ALL JPGs including backups (for variant grouping). + raws: keyed by stem.casefold(). + """ + visible_jpgs: List[Tuple[Path, os.stat_result]] = [] all_jpgs: List[Tuple[Path, os.stat_result]] = [] - raws: Dict[str, List[Tuple[Path, os.stat_result]]] = {} # keyed by stem.casefold() - - try: - for entry in os.scandir(directory): - if entry.is_file(): - p = Path(entry.path) - ext = p.suffix.lower() - if ext in JPG_EXTENSIONS: - # Skip FastStack backup files (name-backup.jpg, name-backup2.jpg, etc.) - if _BACKUP_STEM_RE.search(p.stem): - continue - all_jpgs.append((p, entry.stat())) - elif ext in RAW_EXTENSIONS: - stem = p.stem.casefold() - if stem not in raws: - raws[stem] = [] - raws[stem].append((p, entry.stat())) - except OSError: - log.exception("Error scanning directory %s", directory) - return [] - - # Separate developed JPGs, build base map, and process normal JPGs - # base_map: filename.casefold() -> (mtime, name) + raws: Dict[str, List[Tuple[Path, os.stat_result]]] = {} + + for entry in os.scandir(directory): + if entry.is_file(): + p = Path(entry.path) + ext = p.suffix.lower() + if ext in JPG_EXTENSIONS: + stat = entry.stat() + all_jpgs.append((p, stat)) + if not _BACKUP_STEM_RE.search(p.stem): + visible_jpgs.append((p, stat)) + elif ext in RAW_EXTENSIONS: + stem = p.stem.casefold() + if stem not in raws: + raws[stem] = [] + raws[stem].append((p, entry.stat())) + + return visible_jpgs, all_jpgs, raws + + +def _build_image_list( + visible_jpgs: List[Tuple[Path, os.stat_result]], + raws: Dict[str, List[Tuple[Path, os.stat_result]]], +) -> List[ImageFile]: + """Build sorted image list from visible JPGs and RAWs.""" base_map: Dict[str, Tuple[float, str]] = {} - developed_candidates: List[Tuple[Path, os.stat_result, str]] = ( - [] - ) # path, stat, base_stem - + developed_candidates: List[Tuple[Path, os.stat_result, str]] = [] image_entries: List[Tuple[Tuple[float, str, int, str], ImageFile]] = [] used_raws = set() - for p, stat in all_jpgs: + for p, stat in visible_jpgs: is_dev, base_stem = _parse_developed(p) if is_dev: developed_candidates.append((p, stat, base_stem)) else: - # Register in base_map for developed images to find their parents base_map[p.name.casefold()] = (stat.st_mtime, p.name) - - # Process as normal JPG raw_pair = _find_raw_pair(p, stat, raws.get(p.stem.casefold(), [])) if raw_pair: used_raws.add(raw_pair) - img = ImageFile(path=p, raw_pair=raw_pair, timestamp=stat.st_mtime) image_entries.append((image_sort_key(img), img)) - # 2. Process Developed JPGs for p, stat, base_stem in developed_candidates: - # Try to find base image in priority order: .jpg, .jpeg, .jpe effective_ts = stat.st_mtime effective_name = p.name.casefold() - for ext in sorted(JPG_EXTENSIONS): candidate = (base_stem + ext).casefold() if candidate in base_map: @@ -88,17 +87,12 @@ def find_images(directory: Path) -> List[ImageFile]: effective_ts = base_ts effective_name = base_name.casefold() break - - # Store effective_name so image_sort_key() reproduces the exact key - # used here, even when the base file's extension differs from the - # developed file's extension (e.g. base .jpeg, developed .jpg). img = ImageFile( path=p, raw_pair=None, timestamp=effective_ts, sort_name_cf=effective_name, ) image_entries.append((image_sort_key(img), img)) - # 3. Handle orphaned RAWs for stem, raw_list in raws.items(): for raw_path, raw_stat in raw_list: if raw_path not in used_raws: @@ -107,9 +101,104 @@ def find_images(directory: Path) -> List[ImageFile]: ) image_entries.append((image_sort_key(img), img)) - # Final Sort image_entries.sort(key=lambda x: x[0]) - image_files = [x[1] for x in image_entries] + return [x[1] for x in image_entries] + + +def find_images(directory: Path) -> List[ImageFile]: + """Finds all JPGs in a directory and pairs them with RAW files. + + Backward-compatible: does NOT filter developed files or annotate variant flags. + For variant-aware loading, use find_images_with_variants() instead. + """ + t_start = time.perf_counter() + log.info("Scanning directory for images: %s", directory) + + try: + visible_jpgs, _, raws = _scan_directory(directory) + except OSError: + log.exception("Error scanning directory %s", directory) + return [] + + image_files = _build_image_list(visible_jpgs, raws) + + elapsed = time.perf_counter() - t_start + paired_count = sum( + 1 for im in image_files + if im.raw_pair and im.path.suffix.lower() in JPG_EXTENSIONS + ) + raw_only_count = sum( + 1 for im in image_files if im.path.suffix.lower() not in JPG_EXTENSIONS + ) + log.info( + "Found %d images (%d paired, %d raw-only).", + len(image_files), paired_count, raw_only_count, + ) + return image_files + + +def find_images_with_variants( + directory: Path, +) -> Tuple[List[ImageFile], Dict[str, VariantGroup]]: + """Finds images and builds variant map in a single scan. + + Returns: + (visible_image_list, variant_map) where variant_map is keyed by + group_key.casefold(). + """ + t_start = time.perf_counter() + log.info("Scanning directory for images: %s", directory) + + try: + visible_jpgs, all_jpgs, raws = _scan_directory(directory) + except OSError: + log.exception("Error scanning directory %s", directory) + return [], {} + + # Build the visible image list (legacy behavior) + image_files = _build_image_list(visible_jpgs, raws) + + # Build variant map from ALL jpgs (including backups) + all_jpg_paths = [p for p, _ in all_jpgs] + variant_map = build_variant_map(all_jpg_paths) + + # Filter visible list: keep only entries whose path equals their group's main_path. + # This removes developed files that are reachable via badges while keeping + # orphan developed files that ARE their group's main. + filtered = [] + for img in image_files: + ext = img.path.suffix.lower() + if ext not in JPG_EXTENSIONS: + # RAW-only: always keep + filtered.append(img) + continue + + group_key, _, _ = parse_variant_stem(img.path.stem) + key_cf = group_key.casefold() + group = variant_map.get(key_cf) + if group is None or len(group.all_files) <= 1: + # No variant group or singleton: keep as-is + filtered.append(img) + elif group.main_path == img.path: + # This IS the main: keep it + filtered.append(img) + else: + # This is a developed file reachable via badge: remove from visible list + log.debug("Filtering out variant %s (main=%s)", img.path.name, group.main_path.name if group.main_path else "?") + + # Annotate images with variant flags + for img in filtered: + ext = img.path.suffix.lower() + if ext not in JPG_EXTENSIONS: + continue + group_key, _, _ = parse_variant_stem(img.path.stem) + key_cf = group_key.casefold() + group = variant_map.get(key_cf) + if group: + img.has_backups = bool(group.backup_paths) + img.has_developed = group.developed_path is not None and group.developed_path != group.main_path + + image_files = filtered elapsed = time.perf_counter() - t_start paired_count = sum( @@ -136,7 +225,7 @@ def find_images(directory: Path) -> List[ImageFile]: paired_count, raw_only_count, ) - return image_files + return image_files, variant_map def _parse_developed(path: Path) -> Tuple[bool, str]: @@ -144,17 +233,11 @@ def _parse_developed(path: Path) -> Tuple[bool, str]: Detect if a file is a developed image. Returns (is_developed, base_stem). - Matches a trailing '-developed' on the filename stem, case-insensitive. - Example: 'IMG_0001-developed.jpg' -> ('IMG_0001') + Uses the robust parse_variant_stem from variants.py. """ - stem = path.stem - stem_cf = stem.casefold() - suf_cf = _DEVELOPED_SUFFIX.casefold() - - if stem_cf.endswith(suf_cf): - base_stem = stem[: -len(_DEVELOPED_SUFFIX)] - return True, base_stem - + group_key, is_developed, _ = parse_variant_stem(path.stem) + if is_developed: + return True, group_key return False, "" diff --git a/faststack/io/variants.py b/faststack/io/variants.py new file mode 100644 index 0000000..2c9f6f5 --- /dev/null +++ b/faststack/io/variants.py @@ -0,0 +1,238 @@ +"""Variant (backup + developed) parsing and grouping for image files. + +Pure-logic module with no Qt dependencies. +""" + +import logging +import os +import re +from dataclasses import dataclass, field +from pathlib import Path +from typing import Dict, List, Optional, Tuple + +log = logging.getLogger(__name__) + +# Token-boundary regex: match `-developed` as a real dash-delimited token. +# Ensures "undeveloped" or "mydeveloped" do NOT match. +_DEVELOPED_TOKEN_RE = re.compile(r"(?:^|(?<=-))[Dd][Ee][Vv][Ee][Ll][Oo][Pp][Ee][Dd](?=$|(?=-))") + +# Trailing `-backup(\d+)?` token at end of (stripped) stem. +_BACKUP_TRAILING_RE = re.compile(r"(?:^|-)([Bb][Aa][Cc][Kk][Uu][Pp])(\d+)?$") + + +@dataclass +class VariantInfo: + """Parsed information about a single image file's variant role.""" + + path: Path + group_key: str # case-preserved (NOT casefolded) + is_developed: bool + backup_number: Optional[int] # None = not a backup, 1 = -backup, N = -backupN + + +@dataclass +class VariantGroup: + """A group of related variant files sharing the same base stem.""" + + group_key: str + main_path: Optional[Path] = None + developed_path: Optional[Path] = None + backup_paths: Dict[int, Path] = field(default_factory=dict) # N -> path + all_files: List[VariantInfo] = field(default_factory=list) + + +def parse_variant_stem(stem: str) -> Tuple[str, bool, Optional[int]]: + """Parse a filename stem into (group_key, is_developed, backup_number). + + Token matching rules: + - `-developed` must be a real dash-delimited token (not a substring) + - `-backup(\\d+)?` must be a trailing token + + Returns: + (group_key, is_developed, backup_number) + - group_key: case-preserved stem with variant tokens removed + - is_developed: True if stem contains a `-developed` token + - backup_number: None if not a backup, 1 for `-backup`, N for `-backupN` + """ + # 1. Check for -developed token + is_developed = bool(_DEVELOPED_TOKEN_RE.search(stem)) + + # 2. Remove exactly one -developed token (first occurrence) -> stripped + if is_developed: + # Find the match and remove it, handling leading/trailing dashes + m = _DEVELOPED_TOKEN_RE.search(stem) + start, end = m.start(), m.end() + # Remove the token and any resulting double-dash or leading/trailing dash + before = stem[:start] + after = stem[end:] + # Clean up dashes at the join point + if before.endswith("-") and (after.startswith("-") or after == ""): + before = before[:-1] + elif before == "" and after.startswith("-"): + after = after[1:] + stripped = before + after + else: + stripped = stem + + # 3. Check stripped for trailing -backup(\d+)? token + backup_number = None + bm = _BACKUP_TRAILING_RE.search(stripped) + if bm: + num_str = bm.group(2) + backup_number = int(num_str) if num_str else 1 + group_key = stripped[: bm.start()] + else: + group_key = stripped + + return group_key, is_developed, backup_number + + +def build_variant_map( + all_jpg_paths: List[Path], +) -> Dict[str, VariantGroup]: + """Build a mapping from group_key (casefolded) to VariantGroup. + + Args: + all_jpg_paths: All JPG paths in the directory (including backups). + + Returns: + Dict keyed by group_key.casefold() -> VariantGroup with selection rules applied. + """ + # 1. Parse all files + groups: Dict[str, VariantGroup] = {} # keyed by group_key.casefold() + + for path in all_jpg_paths: + group_key, is_developed, backup_number = parse_variant_stem(path.stem) + key_cf = group_key.casefold() + + info = VariantInfo( + path=Path(norm_path(path)), + group_key=group_key, + is_developed=is_developed, + backup_number=backup_number, + ) + + if key_cf not in groups: + groups[key_cf] = VariantGroup(group_key=group_key) + groups[key_cf].all_files.append(info) + + # 2. Apply selection rules for each group + for group in groups.values(): + _select_main(group) + _select_developed(group) + _select_backups(group) + + return groups + + +def _select_main(group: VariantGroup) -> None: + """Select the main file for a variant group. + + Priority: non-backup non-developed > non-backup developed > lowest backup. + Tiebreak: str(path) lexicographic. + """ + candidates = [] + for info in group.all_files: + # Priority tier: (0=non-backup non-dev, 1=non-backup dev, 2=backup) + if info.backup_number is None and not info.is_developed: + tier = 0 + elif info.backup_number is None and info.is_developed: + tier = 1 + else: + tier = 2 + backup_n = info.backup_number if info.backup_number is not None else 0 + candidates.append((tier, backup_n, str(info.path), info)) + + candidates.sort() + if candidates: + group.main_path = candidates[0][3].path + + +def _select_developed(group: VariantGroup) -> None: + """Select the developed target for a variant group. + + Tier 1: developed + non-backup. + Tier 2: developed + backup (lowest N). + Tiebreak: str(path) lexicographic. + """ + candidates = [] + for info in group.all_files: + if not info.is_developed: + continue + if info.backup_number is None: + tier = 0 + else: + tier = 1 + backup_n = info.backup_number if info.backup_number is not None else 0 + candidates.append((tier, backup_n, str(info.path), info)) + + candidates.sort() + if candidates: + group.developed_path = candidates[0][3].path + + +def _select_backups(group: VariantGroup) -> None: + """Populate backup_paths: for each backup N, prefer non-developed.""" + by_n: Dict[int, List[VariantInfo]] = {} + for info in group.all_files: + if info.backup_number is not None: + by_n.setdefault(info.backup_number, []).append(info) + + for n, infos in by_n.items(): + # Prefer non-developed; tiebreak lexicographic + infos.sort(key=lambda i: (i.is_developed, str(i.path))) + group.backup_paths[n] = infos[0].path + + +def get_group_key_for_path( + path: Path, variant_map: Dict[str, VariantGroup], +) -> Optional[str]: + """Look up the casefolded group key for a file path.""" + group_key, _, _ = parse_variant_stem(path.stem) + key_cf = group_key.casefold() + if key_cf in variant_map: + return key_cf + return None + + +def build_badge_list(group: VariantGroup) -> List[Dict]: + """Build ordered badge list for a variant group. + + Order: Main, D (if present), B, B2, B3... by N ascending. + Each badge is a dict: {"label": str, "path": str, "kind": str}. + """ + badges = [] + norm = norm_path + + if group.main_path is not None: + badges.append({ + "label": "Main", + "path": norm(group.main_path), + "kind": "main", + }) + + if group.developed_path is not None and group.developed_path != group.main_path: + badges.append({ + "label": "D", + "path": norm(group.developed_path), + "kind": "developed", + }) + + for n in sorted(group.backup_paths.keys()): + bp = group.backup_paths[n] + # Skip if this backup is already the main or developed path + if bp == group.main_path or bp == group.developed_path: + continue + label = "Bk" if n == 1 else f"Bk{n}" + badges.append({ + "label": label, + "path": norm(bp), + "kind": "backup", + }) + + return badges + + +def norm_path(p: Path) -> str: + """Normalize a path for consistent comparison.""" + return os.path.normcase(os.path.abspath(str(p))) diff --git a/faststack/models.py b/faststack/models.py index 0820b2d..776e619 100644 --- a/faststack/models.py +++ b/faststack/models.py @@ -16,6 +16,8 @@ class ImageFile: # is the *base* image's name (so the pair sorts adjacently); for everything # else it defaults to None which means "use path.name.casefold()". sort_name_cf: Optional[str] = None + has_backups: bool = False + has_developed: bool = False @property def raw_path(self) -> Optional[Path]: diff --git a/faststack/qml/Components.qml b/faststack/qml/Components.qml index 28160ee..07c31f6 100644 --- a/faststack/qml/Components.qml +++ b/faststack/qml/Components.qml @@ -31,6 +31,12 @@ Item { } Keys.onEscapePressed: (event) => { + if (root.fullScreenLoupe) { + root.exitFullScreenLoupe() + event.accepted = true + return + } + if (uiState && uiState.isCropping) { if (mainMouseArea.isRotating) { // Revert rotation @@ -269,6 +275,7 @@ Item { Image { id: mainImage anchors.centerIn: parent + visible: uiState && !uiState.isGridViewActive // Image size is now updated atomically in updateRotatorGeometry to prevent distortion // width: sourceSize.width diff --git a/faststack/qml/Main.qml b/faststack/qml/Main.qml index 1ee847d..526aec9 100644 --- a/faststack/qml/Main.qml +++ b/faststack/qml/Main.qml @@ -16,6 +16,50 @@ ApplicationWindow { title: "FastStack - " + (uiState ? uiState.currentDirectory : "Loading...") property bool allowCloseWithRecycleBins: false + property bool fullScreenLoupe: false + property var savedWindowGeometry: ({}) + + function enterFullScreenLoupe() { + if (!uiState || uiState.isGridViewActive) return + + savedWindowGeometry = { + x: root.x, + y: root.y, + width: root.width, + height: root.height, + visibility: root.visibility + } + + fullScreenLoupe = true + root.showFullScreen() + } + + function exitFullScreenLoupe() { + if (!fullScreenLoupe) return + + fullScreenLoupe = false + + if (savedWindowGeometry.visibility === Window.Maximized) { + root.showMaximized() + } else { + root.showNormal() + if (savedWindowGeometry.visibility === Window.Windowed) { + root.x = savedWindowGeometry.x + root.y = savedWindowGeometry.y + root.width = savedWindowGeometry.width + root.height = savedWindowGeometry.height + } + } + root.requestActivate() + } + + function toggleFullScreenLoupe() { + if (fullScreenLoupe) { + exitFullScreenLoupe() + } else { + enterFullScreenLoupe() + } + } onClosing: function(close) { if (allowCloseWithRecycleBins) { @@ -67,6 +111,7 @@ ApplicationWindow { height: 40 color: "transparent" z: 100 // Ensure it's above the content + visible: !root.fullScreenLoupe // Unified "menu active" flag to avoid flashing property bool menuActive: menuBarMouseArea.containsMouse @@ -765,6 +810,21 @@ ApplicationWindow { } property int footerHeight: 60 + property int effectiveFooterHeight: fullScreenLoupe ? 0 : footerHeight + + Shortcut { + sequence: "F11" + context: Qt.ApplicationShortcut + enabled: uiState ? !uiState.isGridViewActive && !uiState.isDialogOpen : false + onActivated: root.toggleFullScreenLoupe() + } + + Shortcut { + sequence: "Escape" + context: Qt.ApplicationShortcut + enabled: root.fullScreenLoupe + onActivated: root.exitFullScreenLoupe() + } Shortcut { sequence: "E" @@ -794,6 +854,43 @@ ApplicationWindow { } } + // Handle View Switching and Prefetch Gating + Connections { + target: uiState + function onIsGridViewActiveChanged() { + if (uiState.isGridViewActive && root.fullScreenLoupe) { + root.exitFullScreenLoupe() + } + + var gridItem = gridViewLoader.item + if (!gridItem) return + + if (uiState.isGridViewActive) { + // Switching TO grid: + // 1. Immediately disable prefetch to block transient top-of-list requests + // that happen before the view layout/scroll is restored. + if (typeof gridItem.setPrefetchEnabled === "function") { + gridItem.setPrefetchEnabled(false) + } + + // 2. Re-enable on next event loop tick. + // This allows the GridView to restore its currentIndex/contentY position. + Qt.callLater(function() { + var it = gridViewLoader.item + if (uiState.isGridViewActive && it && typeof it.setPrefetchEnabled === "function") { + it.setPrefetchEnabled(true) + } + }) + } else { + // Switching AWAY from grid: + // Disable immediately to stop background work. + if (typeof gridItem.setPrefetchEnabled === "function") { + gridItem.setPrefetchEnabled(false) + } + } + } + } + // -------- MAIN VIEW -------- // StackLayout to switch between loupe and grid view StackLayout { @@ -812,10 +909,16 @@ ApplicationWindow { anchors.fill: parent source: "Components.qml" focus: !uiState || !uiState.isGridViewActive - onLoaded: item.footerHeight = Qt.binding(function() { return root.footerHeight }) + onLoaded: item.footerHeight = Qt.binding(function() { return root.effectiveFooterHeight }) // Key bindings implemented in old Main.qml Keys.onPressed: function(event) { + if (root.fullScreenLoupe && event.key === Qt.Key_Escape) { + root.exitFullScreenLoupe() + event.accepted = true + return + } + if (!uiState || !controller) { return } @@ -845,6 +948,19 @@ ApplicationWindow { visible: uiState && uiState.isGridViewActive focus: uiState && uiState.isGridViewActive + onLoaded: { + // Enable prefetch on startup if grid is active (single owner) + var loadedItem = item + if (uiState && uiState.isGridViewActive && loadedItem && typeof loadedItem.setPrefetchEnabled === "function") { + // Delay to match the toggle behavior (allow layout to settle) + Qt.callLater(function() { + if (gridViewLoader.item === loadedItem && uiState.isGridViewActive) { + loadedItem.setPrefetchEnabled(true) + } + }) + } + } + // Bind theme property to loaded item Binding { target: gridViewLoader.item @@ -863,12 +979,13 @@ ApplicationWindow { id: footerRect // Keep footer height fixed so the main image area doesn't change size when // stack/batch labels appear or disappear (prevents cache invalidations). - height: root.footerHeight - implicitHeight: root.footerHeight + height: root.effectiveFooterHeight + implicitHeight: root.effectiveFooterHeight anchors.left: parent.left anchors.right: parent.right color: Qt.rgba(root.currentBackgroundColor.r, root.currentBackgroundColor.g, root.currentBackgroundColor.b, 0.8) clip: true + visible: !root.fullScreenLoupe RowLayout { id: footerRow @@ -965,6 +1082,51 @@ ApplicationWindow { font.pixelSize: 16 } } + // Variant badges (loupe view only, when multiple variants exist) + Row { + spacing: 4 + visible: uiState && !uiState.isGridViewActive && uiState.variantBadges.length > 1 + + Repeater { + model: uiState ? uiState.variantBadges : [] + + delegate: Rectangle { + width: badgeLabel.implicitWidth + 12 + height: 22 + radius: 3 + color: modelData.active ? "white" : "#555" + border.color: modelData.active ? "#333" : "transparent" + border.width: modelData.active ? 1 : 0 + + Text { + id: badgeLabel + anchors.centerIn: parent + text: modelData.label + font.pixelSize: 11 + font.bold: true + color: modelData.active ? "black" : "white" + } + + MouseArea { + anchors.fill: parent + cursorShape: Qt.PointingHandCursor + onClicked: { + if (uiState) uiState.setVariantOverride(modelData.path) + } + } + } + } + + Label { + text: uiState ? uiState.variantSaveHint : "" + color: root.isDarkTheme ? "#aaa" : "#666" + font.pixelSize: 11 + font.italic: true + visible: text !== "" + anchors.verticalCenter: parent.verticalCenter + } + } + Rectangle { Layout.fillWidth: true color: "transparent" @@ -1174,7 +1336,8 @@ ApplicationWindow { "  G: Jump to Image Number
" + "  Alt+U: Jump to Last Uploaded
" + "  I: Show EXIF Data
" + - "  T: Toggle Thumbnail Grid / Single Image View

" + + "  T: Toggle Thumbnail Grid / Single Image View
" + + "  F11: Toggle Fullscreen (Loupe View)

" + "Thumbnail Grid View:
" + "  Arrow Keys: Navigate between images
" + "  Enter: Open current image in single view
" + @@ -1232,7 +1395,7 @@ ApplicationWindow { "  P: Edit in Photoshop
" + "  H: Toggle histogram window
" + "  Ctrl+C: Copy image path to clipboard
" + - "  Esc: Close dialog/editor, or switch to grid view" + "  Esc: Close dialog/editor, switch to grid view, or exit fullscreen" padding: 10 wrapMode: Text.WordWrap color: root.currentTextColor diff --git a/faststack/qml/ThumbnailGridView.qml b/faststack/qml/ThumbnailGridView.qml index bea1093..f896583 100644 --- a/faststack/qml/ThumbnailGridView.qml +++ b/faststack/qml/ThumbnailGridView.qml @@ -17,6 +17,11 @@ Item { // Selection count for keyboard handler (use gridSelectedCount for efficiency) property int selectedCount: uiState ? uiState.gridSelectedCount : 0 + // Wrapper to expose function to Loader + function setPrefetchEnabled(enabled) { + thumbnailGrid.setPrefetchEnabled(enabled) + } + // Grid view GridView { id: thumbnailGrid @@ -60,6 +65,8 @@ Item { tileFolderStats: folderStats || null tileIsSelected: isSelected || false tileIsParentFolder: isParentFolder || false + tileHasBackups: hasBackups || false + tileHasDeveloped: hasDeveloped || false tileHasCursor: index === thumbnailGrid.currentIndex } @@ -71,14 +78,34 @@ Item { // Visible range prefetch property int prefetchMargin: 2 // rows + property bool prefetchEnabled: false // Gate for prefetch requests (default off for startup safety) + + function setPrefetchEnabled(enabled) { + prefetchEnabled = enabled + if (enabled) { + // Restore position to ensure we don't prefetch top-of-list by accident + if (thumbnailGrid.currentIndex >= 0) { + thumbnailGrid.positionViewAtIndex(thumbnailGrid.currentIndex, GridView.Contain) + } + // Schedule a fresh prefetch with a slight delay to allow layout to settle + // This prevents "coalesced_from=prefetch" delays for visible items + Qt.callLater(function() { + if (prefetchEnabled) prefetchTimer.restart() + }) + } else { + prefetchTimer.stop() + // Cancel any queued work immediately to clear the backlog + if (uiState) uiState.cancelThumbnailPrefetch() + } + } onContentYChanged: { - prefetchTimer.restart() + if (prefetchEnabled && !prefetchTimer.running) prefetchTimer.start() // Throttle } Timer { id: prefetchTimer - interval: 100 + interval: 50 repeat: false onTriggered: { thumbnailGrid.triggerPrefetch() @@ -86,33 +113,43 @@ Item { } function triggerPrefetch() { - if (thumbnailGrid.count === 0) return + if (!prefetchEnabled) return + if (!uiState || thumbnailGrid.count === 0) return + + var cellW = thumbnailGrid.cellWidth + var cellH = thumbnailGrid.cellHeight + if (cellW <= 0 || cellH <= 0) return - // Calculate visible range - var topIndex = thumbnailGrid.indexAt(thumbnailGrid.contentX, thumbnailGrid.contentY) - var bottomIndex = thumbnailGrid.indexAt( - thumbnailGrid.contentX + thumbnailGrid.width, - thumbnailGrid.contentY + thumbnailGrid.height - ) + // Calculate columns and visible rows + var cols = Math.max(1, Math.floor(thumbnailGrid.width / cellW)) + var firstRow = Math.max(0, Math.floor(thumbnailGrid.contentY / cellH)) + var rowsVisible = Math.max(1, Math.ceil(thumbnailGrid.height / cellH)) - if (topIndex < 0) topIndex = 0 - if (bottomIndex < 0) bottomIndex = thumbnailGrid.count - 1 + // Padding rows for smoother scrolling + var padRows = thumbnailGrid.prefetchMargin || 4 + var startRow = Math.max(0, firstRow - padRows) + var endRow = firstRow + rowsVisible + padRows - // Add margin (with epsilon to handle sub-pixel rounding during resize) - var cols = Math.floor((thumbnailGrid.width + 1) / thumbnailGrid.cellWidth) - if (cols < 1) cols = 1 - var marginItems = cols * thumbnailGrid.prefetchMargin - topIndex = Math.max(0, topIndex - marginItems) - bottomIndex = Math.min(thumbnailGrid.count - 1, bottomIndex + marginItems) + // Calculate item indices + var topIndex = startRow * cols + var bottomIndex = (endRow * cols) - 1 + + // Clamp to model boundaries + topIndex = Math.max(0, Math.min(topIndex, thumbnailGrid.count - 1)) + bottomIndex = Math.max(0, Math.min(bottomIndex, thumbnailGrid.count - 1)) + + // Determine budget (intended items to prefetch) + var maxCount = (rowsVisible + 2 * padRows) * cols + maxCount = Math.max(200, Math.min(maxCount, 800)) // Log for debugging if (uiState && uiState.debugMode) { - console.log("Prefetch range:", topIndex, "-", bottomIndex) + console.log("Prefetch range:", topIndex, "-", bottomIndex, "maxCount=" + maxCount + " cols=" + cols) } // Actually trigger prefetch if (uiState) { - uiState.gridPrefetchRange(topIndex, bottomIndex) + uiState.gridPrefetchRange(topIndex, bottomIndex, maxCount) } } @@ -202,30 +239,29 @@ Item { } } - // Focus handling + // Focus and layout triggers + onWidthChanged: { if (thumbnailGrid.prefetchEnabled) prefetchTimer.restart() } + onHeightChanged: { if (thumbnailGrid.prefetchEnabled) prefetchTimer.restart() } + Component.onCompleted: { + if (uiState && uiState.debugThumbTiming) + console.log("[THUMB-TIMING] GridView Component.onCompleted t=" + Date.now() + "ms") thumbnailGrid.forceActiveFocus() - // Trigger initial prefetch after a short delay - initialPrefetchTimer.start() - } - - Timer { - id: initialPrefetchTimer - interval: 200 - repeat: false - onTriggered: { - if (thumbnailGrid.count > 0) { - thumbnailGrid.triggerPrefetch() - } + + // Sync initial cursor position from state to prevent top-of-list prefetch + if (uiState && uiState.currentIndex >= 0 && uiState.currentIndex < thumbnailGrid.count) { + thumbnailGrid.currentIndex = uiState.currentIndex + thumbnailGrid.positionViewAtIndex(thumbnailGrid.currentIndex, GridView.Center) } } + Connections { target: uiState function onIsGridViewActiveChanged() { if (uiState.isGridViewActive) { - // Trigger prefetch when grid view becomes active - thumbnailGrid.triggerPrefetch() + // Prefetch triggering is now handled by Main.qml via setPrefetchEnabled + // to avoid transient state issues. thumbnailGrid.forceActiveFocus() } } diff --git a/faststack/qml/ThumbnailTile.qml b/faststack/qml/ThumbnailTile.qml index 75de6e4..c5e2c2e 100644 --- a/faststack/qml/ThumbnailTile.qml +++ b/faststack/qml/ThumbnailTile.qml @@ -23,6 +23,8 @@ Item { property bool tileIsSelected: false property bool tileIsParentFolder: false property bool tileHasCursor: false // Keyboard cursor position + property bool tileHasBackups: false + property bool tileHasDeveloped: false // Theme property (bound by parent) property bool isDarkTheme: false @@ -248,6 +250,27 @@ Item { } } + // Variant badges row (top-right corner of thumbnail) + Row { + anchors.right: parent.right + anchors.top: parent.top + anchors.margins: 4 + spacing: 2 + visible: !tile.tileIsFolder + layoutDirection: Qt.RightToLeft + + Rectangle { + visible: tile.tileHasBackups + width: 18; height: 18; radius: 3; color: "#9C27B0" + Text { anchors.centerIn: parent; text: "Bk"; font.pixelSize: 9; font.bold: true; color: "white" } + } + Rectangle { + visible: tile.tileHasDeveloped + width: 18; height: 18; radius: 3; color: "#009688" + Text { anchors.centerIn: parent; text: "D"; font.pixelSize: 11; font.bold: true; color: "white" } + } + } + // ============================================================ // TOP STATS OVERLAY: U (left), S (center), E (right) // Colored text: U=green, S=orange, E=yellow @@ -476,6 +499,11 @@ Item { } } + Component.onCompleted: { + if (tile.tileIndex === 0 && uiState && uiState.debugThumbTiming) + console.log("[THUMB-TIMING] first delegate created (index 0) t=" + Date.now() + "ms") + } + // Mouse area for interactions MouseArea { id: tileMouseArea diff --git a/faststack/reactive_test_output.txt b/faststack/reactive_test_output.txt deleted file mode 100644 index 6bcc146..0000000 --- a/faststack/reactive_test_output.txt +++ /dev/null @@ -1,15 +0,0 @@ -============================= test session starts ============================= -platform win32 -- Python 3.12.10, pytest-9.0.2, pluggy-1.6.0 -- C:\code\faststack\faststack\verify_venv\Scripts\python.exe -rootdir: C:\code\faststack -configfile: pyproject.toml -collecting ... collected 7 items - -tests\test_reactive_delete.py::test_optimistic_ui_removal PASSED [ 14%] -tests\test_reactive_delete.py::test_undo_pending_delete_no_disk_ops PASSED [ 28%] -tests\test_reactive_delete.py::test_async_delete_completion PASSED [ 42%] -tests\test_reactive_delete.py::test_delete_rollback_on_cancel PASSED [ 57%] -tests\test_reactive_delete.py::test_debounced_refresh PASSED [ 71%] -tests\test_reactive_delete.py::test_cancel_midlight_with_real_files PASSED [ 85%] -tests\test_reactive_delete.py::test_undo_then_completion_no_bookkeeping PASSED [100%] - -============================== 7 passed in 0.56s ============================== diff --git a/faststack/repro_cache_lock.py b/faststack/repro_cache_lock.py new file mode 100644 index 0000000..a276c17 --- /dev/null +++ b/faststack/repro_cache_lock.py @@ -0,0 +1,47 @@ + +import threading +from faststack.imaging.cache import ByteLRUCache + +def repro_lock_contention(): + lock_held_during_callback = False + + def on_evict_callback(key, value): + nonlocal lock_held_during_callback + # Try to acquire the same lock. If it's held by the current thread (RLock), + # we can check if it would block others or if we can detect it's held. + # Since it's an RLock, current thread can re-acquire it. + # But we can check if the lock is "locked" by looking at internal state + # or just by the fact that we know we are in the callback. + + # A better way to check if the lock is held: + # Since it's an RLock, it doesn't expose a simple "is_locked" that works across threads easily + # but we can try to acquire it in a DIFFERENT thread. + + def check_lock(): + nonlocal lock_held_during_callback + if not cache._lock.acquire(blocking=False): + lock_held_during_callback = True + else: + cache._lock.release() + + t = threading.Thread(target=check_lock) + t.start() + t.join() + + cache = ByteLRUCache(max_bytes=100, size_of=lambda x: x, on_evict=on_evict_callback) + + print("Adding item 'a' (50 bytes)") + cache["a"] = 50 + print("Adding item 'b' (50 bytes)") + cache["b"] = 50 + + print("Adding item 'c' (50 bytes) -> should trigger eviction of 'a'") + cache["c"] = 50 + + if lock_held_during_callback: + print("FAILED: Lock was HELD during on_evict callback!") + else: + print("SUCCESS: Lock was NOT held during on_evict callback.") + +if __name__ == "__main__": + repro_lock_contention() diff --git a/faststack/test_executors.py b/faststack/test_executors.py deleted file mode 100644 index 4bb8faf..0000000 --- a/faststack/test_executors.py +++ /dev/null @@ -1,54 +0,0 @@ -import time -import threading -from faststack.util.executors import create_priority_executor as PriorityExecutor - -def test_priority_executor(): - print("Testing PriorityExecutor...") - executor = PriorityExecutor(max_workers=1, thread_name_prefix="Test") - - results = [] - def task(name, delay=0.1): - time.sleep(delay) - results.append(name) - return name - - # Fill the worker and wait a bit to ensure it STARTS - executor.submit(task, "initial", delay=0.2) - time.sleep(0.05) - - # Submit tasks with different priorities and see execution order - # Lower number = higher priority - # within same priority, higher sequence = higher priority (LIFO) - executor.submit(task, "p2_a", priority=2) - executor.submit(task, "p2_b", priority=2) - executor.submit(task, "p1", priority=1) - - print("Tasks submitted, waiting for completion...") - # Expected order: "initial" (already running), "p1", "p2_b", "p2_a" - - time.sleep(1.0) - print("Execution order:", results) - - expected = ["initial", "p1", "p2_b", "p2_a"] - if results == expected: - print("SUCCESS: Priority and LIFO ordering correct.") - else: - print(f"FAILURE: Expected {expected}, got {results}") - - print("\nTesting shutdown and cancellation...") - executor = PriorityExecutor(max_workers=1, thread_name_prefix="TestShutdown") - executor.submit(task, "long", delay=0.5) - f1 = executor.submit(task, "queued1") - f2 = executor.submit(task, "queued2") - - executor.shutdown(wait=True, cancel_futures=True) - print(f"f1 cancelled: {f1.cancelled()}") - print(f"f2 cancelled: {f2.cancelled()}") - - if f1.cancelled() and f2.cancelled(): - print("SUCCESS: Futures cancelled on shutdown.") - else: - print("FAILURE: Futures not cancelled.") - -if __name__ == "__main__": - test_priority_executor() diff --git a/faststack/tests/debug_editor_error.py b/faststack/tests/debug_editor_error.py index 085ac3e..14328b0 100644 --- a/faststack/tests/debug_editor_error.py +++ b/faststack/tests/debug_editor_error.py @@ -14,31 +14,37 @@ def test_debug_save_image(self): editor = ImageEditor() editor.float_image = np.zeros((10, 10, 3), dtype=np.float32) editor.current_filepath = Path("fake_path.jpg") + # Need original_image to pass _ensure_float_image checks if called, + # and for save logic + editor.original_image = MagicMock() # Patch create_backup_file to succeed with patch("faststack.imaging.editor.create_backup_file", return_value=Path("backup.jpg")): - # Patch Image.fromarray to return a mock that fails to save - mock_img = MagicMock() - mock_img.save.side_effect = PermissionError("Mocked save error") - - print(f"DEBUG: Real Image.fromarray before patch: {Image.fromarray}") - - with patch("PIL.Image.fromarray", return_value=mock_img) as mock_fromarray: - print(f"DEBUG: Image.fromarray is patched: {Image.fromarray}") - print(f"DEBUG: mock_fromarray: {mock_fromarray}") - - # Verify that calling Image.fromarray returns our mock - img = Image.fromarray(np.zeros((10,10,3), dtype=np.uint8)) - print(f"DEBUG: Returned img: {img}") - print(f"DEBUG: img.save side effect: {img.save.side_effect}") - - try: + # Patch Image.fromarray at the module level where it's used + with patch("faststack.imaging.editor.Image.fromarray") as mock_fromarray: + # Configure the mock object returned by fromarray + mock_img = MagicMock() + mock_img.save.side_effect = PermissionError("Mocked save error") + mock_fromarray.return_value = mock_img + + # Expect RuntimeError because save_image catches exceptions and raises RuntimeError + with self.assertRaises(RuntimeError): editor.save_image() - print("FAIL: save_image did NOT raise RuntimeError") - except RuntimeError as e: - print(f"PASS: Caught RuntimeError: {e}") - except Exception as e: - print(f"FAIL: Caught unexpected exception: {type(e)} {e}") + + def test_save_image_raises_on_missing_float(self): + editor = ImageEditor() + editor.float_image = None + editor.current_filepath = Path("fake.jpg") + editor.original_image = MagicMock() + + # Simulate race: _ensure_float_image "thinks" it succeeded (or was raced), + # but float_image is actually None when we enter the lock. + # We achieve this by silencing _ensure_float_image so it doesn't populate float_image. + editor._ensure_float_image = MagicMock() + + # Should raise RuntimeError explicitly now (instead of returning None) + with self.assertRaisesRegex(RuntimeError, "save_image called with no float_image"): + editor.save_image() if __name__ == "__main__": unittest.main() diff --git a/faststack/tests/repro_futures_cleanup.py b/faststack/tests/repro_futures_cleanup.py index 4877367..3f3137a 100644 --- a/faststack/tests/repro_futures_cleanup.py +++ b/faststack/tests/repro_futures_cleanup.py @@ -16,8 +16,6 @@ def test_newer_future_is_not_deleted_by_older_task(self): mock_cache_put = MagicMock() mock_get_display_info = MagicMock(return_value=(100, 100, 1)) image_files = [MagicMock(path=Path(f"test_{i}.jpg")) for i in range(10)] - for img in image_files: - img.path.suffix = ".jpg" prefetcher = Prefetcher( image_files=image_files, diff --git a/faststack/tests/test_exif_display_rotation.py b/faststack/tests/test_exif_display_rotation.py index 940c1fa..afdda65 100644 --- a/faststack/tests/test_exif_display_rotation.py +++ b/faststack/tests/test_exif_display_rotation.py @@ -12,7 +12,7 @@ # Adjust path to import faststack sys.path.insert(0, str(Path(__file__).parents[1])) -from faststack.imaging.prefetch import apply_exif_orientation +from faststack.imaging.orientation import apply_exif_orientation class TestExifDisplayOrientation(unittest.TestCase): diff --git a/faststack/tests/test_feedback_fixes.py b/faststack/tests/test_feedback_fixes.py new file mode 100644 index 0000000..1869b4c --- /dev/null +++ b/faststack/tests/test_feedback_fixes.py @@ -0,0 +1,120 @@ +import pytest +import numpy as np +from pathlib import Path +from faststack.imaging.cache import ByteLRUCache +from faststack.models import DecodedImage +from faststack.imaging.editor import ImageEditor +from faststack.io.variants import build_variant_map, norm_path +from faststack.io.indexer import find_images_with_variants + +def test_cache_evict_callback_payload(): + """Verify that ByteLRUCache passes (key, value) to on_evict.""" + evicted_items = [] + + def on_evict(k, v): + evicted_items.append((k, v)) + + # Small cache to trigger eviction + cache = ByteLRUCache(max_bytes=100, on_evict=on_evict) + + # Create some test images + img1 = DecodedImage(buffer=np.zeros(60, dtype=np.uint8), width=60, height=1, bytes_per_line=60, format=1) + img2 = DecodedImage(buffer=np.zeros(60, dtype=np.uint8), width=60, height=1, bytes_per_line=60, format=1) + + cache["k1"] = img1 + assert len(evicted_items) == 0 + + # Adding k2 should evict k1 + cache["k2"] = img2 + assert len(evicted_items) == 1 + assert evicted_items[0][0] == "k1" + assert evicted_items[0][1] == img1 + +def test_cache_popitem_callback_payload(): + """Verify popitem passes (key, value) to on_evict.""" + evicted_items = [] + def on_evict(k, v): + evicted_items.append((k, v)) + + cache = ByteLRUCache(max_bytes=1000, on_evict=on_evict) + img = DecodedImage(buffer=np.zeros(10, dtype=np.uint8), width=10, height=1, bytes_per_line=10, format=1) + cache["k1"] = img + + cache.popitem() + assert len(evicted_items) == 1 + assert evicted_items[0] == ("k1", img) + +def test_image_editor_save_exception(tmp_path): + """Verify ImageEditor.save_image raises RuntimeError if no float_image.""" + editor = ImageEditor() + # No image loaded + with pytest.raises(RuntimeError, match="No file path set"): + editor.save_image() + +def test_variant_normalization_consistency(): + """Verify variant mapping handles mixed-case extensions and paths consistently.""" + # Simulate mixed case extensions + paths = [ + Path("test.JPG"), + Path("test-backup.jpeg"), + Path("test-developed.jpg") + ] + + vmap = build_variant_map(paths) + key_cf = "test".casefold() + assert key_cf in vmap + + group = vmap[key_cf] + assert group.main_path is not None + assert norm_path(group.main_path) == norm_path(Path("test.JPG")) + assert group.developed_path is not None + assert group.developed_path.name.lower() == "test-developed.jpg" + +def test_orphan_developed_preservation(): + """Verify find_images_with_variants keeps developed JPGs even if no main JPG exists.""" + import tempfile + with tempfile.TemporaryDirectory() as td: + tdp = Path(td) + # Create an orphan developed file + orphan = tdp / "orphan-developed.jpg" + orphan.write_text("dummy") + + # Also create a regular pair for comparison + pair_main = tdp / "pair.jpg" + pair_dev = tdp / "pair-developed.jpg" + pair_main.write_text("dummy") + pair_dev.write_text("dummy") + + visible, vmap = find_images_with_variants(tdp) + + visible_names = [img.path.name for img in visible] + assert "orphan-developed.jpg" in visible_names + assert "pair.jpg" in visible_names + assert "pair-developed.jpg" not in visible_names + +def test_cache_evict_lock_not_held(): + """Verify that on_evict is called OUTSIDE the cache lock.""" + lock_held_during_callback = False + + def on_evict(k, v): + nonlocal lock_held_during_callback + import threading + def check_lock(): + nonlocal lock_held_during_callback + if not cache._lock.acquire(blocking=False): + lock_held_during_callback = True + else: + cache._lock.release() + + t = threading.Thread(target=check_lock) + t.start() + t.join() + + # Small cache to trigger eviction on second set + cache = ByteLRUCache(max_bytes=100, on_evict=on_evict) + img = DecodedImage(buffer=np.zeros(60, dtype=np.uint8), width=60, height=1, bytes_per_line=60, format=1) + + cache["k1"] = img + cache["k2"] = img + + assert not lock_held_during_callback, "Lock was held during eviction callback!" diff --git a/faststack/tests/test_handle_failures_isolated.py b/faststack/tests/test_handle_failures_isolated.py index 09413fa..3738873 100644 --- a/faststack/tests/test_handle_failures_isolated.py +++ b/faststack/tests/test_handle_failures_isolated.py @@ -1,6 +1,6 @@ import sys -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from pathlib import Path from faststack.deletion_types import DeletionErrorCodes, DeleteResult, DeleteFailure, DeleteJob @@ -22,69 +22,14 @@ def __init__(self): def _key(self, p): return str(p) if p else None - # COPIED LOGIC FROM app.py _handle_delete_failures - def _handle_delete_failures(self, result: DeleteResult, job: DeleteJob) -> None: - """Handle items that failed to delete. Rollback UI or prompt for perm delete.""" - if not result.failures: - return + # Dynamically bind the real method we want to test + # This ensures we test the ACTUAL logic in app.py, not a copy + from faststack.app import AppController + _handle_delete_failures = AppController._handle_delete_failures - # Identify which UI items failed (map back using paths) - # Note: We use the _key() mapping to ensure we match robustly - failed_keys = {self._key(f.jpg) for f in result.failures if f.jpg} - - failed_indices_and_imgs = [] - for idx, img in job.removed_items: - if self._key(img.path) in failed_keys: - failed_indices_and_imgs.append((idx, img)) - - if not failed_indices_and_imgs: - return - - # Check if we should offer permanent delete (recycle bin error) - perm_candidates = [] # List of (idx, img) - - # Helper to find if a specific failure code warrants perm delete - recycle_codes = { - DeletionErrorCodes.RECYCLE_FAILED.value, - DeletionErrorCodes.PERMISSION_DENIED.value, - DeletionErrorCodes.TRASH_FULL.value - } - - # Map failure code by key for easy lookup - failure_map = {self._key(f.jpg): f for f in result.failures if f.jpg} - - for idx, img in failed_indices_and_imgs: - f = failure_map.get(self._key(img.path)) - if f and f.code in recycle_codes: - perm_candidates.append((idx, img)) - - if perm_candidates: - # Prompt user for permanent delete - - # 1. Rollback non-candidates first - candidate_keys = {self._key(img.path) for _, img in perm_candidates} - to_rollback = [(i, img) for i, img in failed_indices_and_imgs if self._key(img.path) not in candidate_keys] - - if to_rollback: - self._rollback_ui_items(to_rollback, job) - - # 2. Ask user - # candidate_imgs = [img for _, img in perm_candidates] - - # Using global mocks here instead of real imports - # confirmed = confirm_permanent_delete(...) - - # For test purpose, we just assert that we identified candidates correctly - pass - - else: - # Just rollback everything - self._rollback_ui_items(failed_indices_and_imgs, job) - - self._rebuild_path_to_index() - self.sync_ui_state() - -def test_handle_delete_failures_recycle_codes_isolation(): +@patch("faststack.app.confirm_permanent_delete", return_value=True) +@patch("faststack.app.confirm_batch_permanent_delete", return_value=True) +def test_handle_delete_failures_recycle_codes_isolation(mock_confirm, mock_batch_confirm): controller = MockController() # Create failure with RECYCLE_FAILED code diff --git a/faststack/tests/test_helicon_cleanup.py b/faststack/tests/test_helicon_cleanup.py new file mode 100644 index 0000000..6277ec5 --- /dev/null +++ b/faststack/tests/test_helicon_cleanup.py @@ -0,0 +1,66 @@ +import pytest +from unittest.mock import MagicMock, patch +from pathlib import Path +import tempfile +import os +from faststack.app import AppController +from faststack.models import ImageFile + +@pytest.fixture +def mock_controller(): + # Mock dependencies required by AppController init + engine = MagicMock() + with patch("faststack.app.Watcher"), \ + patch("faststack.app.SidecarManager"), \ + patch("faststack.app.ImageEditor"), \ + patch("faststack.app.ByteLRUCache"), \ + patch("faststack.app.Prefetcher"), \ + patch("faststack.app.ThumbnailCache"), \ + patch("faststack.app.PathResolver"), \ + patch("faststack.app.ThumbnailPrefetcher"), \ + patch("faststack.app.ThumbnailModel"), \ + patch("faststack.app.ThumbnailProvider"), \ + patch("faststack.app.concurrent.futures.ThreadPoolExecutor"), \ + patch("faststack.app.QTimer"), \ + patch("faststack.app.QApplication"): # Mock QApplication to avoid segfaults + + controller = AppController(image_dir=Path("c:/images"), engine=engine) + + # Setup dummy images for the controller + img1 = ImageFile(Path("c:/images/img1.jpg")) + controller.image_files = [img1] + + return controller + +def test_deferred_cleanup(mock_controller): + """Verify that temp files are tracked and deleted on shutdown.""" + + # Create a real temporary file to verify deletion + with tempfile.NamedTemporaryFile("w", delete=False) as tmp: + tmp.write("test_file_list") + tmp_path = Path(tmp.name) + + try: + # Mock launch_helicon_focus to return success and our real temp path + with patch("faststack.app.launch_helicon_focus", return_value=(True, tmp_path)): + + # Simulate launching helicon with some files + # We bypass the stack logic and call _launch_helicon_with_files directly or via launch_helicon if valid + # Let's call _launch_helicon_with_files directly for simplicity + files = [Path("c:/images/img1.jpg")] + success = mock_controller._launch_helicon_with_files(files) + + assert success is True + assert tmp_path in mock_controller._temp_files_to_clean + assert tmp_path.exists(), "File should still exist before shutdown" + + # Now simulate shutdown + mock_controller.shutdown_nonqt() + + assert not tmp_path.exists(), "File should be deleted after shutdown" + assert len(mock_controller._temp_files_to_clean) == 0 + + finally: + # Cleanup if test fails + if tmp_path.exists(): + os.remove(tmp_path) diff --git a/tests/test_prefetch_concurrency.py b/faststack/tests/test_prefetch_concurrency.py similarity index 100% rename from tests/test_prefetch_concurrency.py rename to faststack/tests/test_prefetch_concurrency.py diff --git a/faststack/tests/test_refresh_crash.py b/faststack/tests/test_refresh_crash.py index 6e828e3..a536848 100644 --- a/faststack/tests/test_refresh_crash.py +++ b/faststack/tests/test_refresh_crash.py @@ -16,7 +16,7 @@ def model(tmp_path): model.beginResetModel = Mock() model.endResetModel = Mock() model.selectionChanged = Mock() - return model + yield model def test_refresh_no_name_error(model): """Verify that refresh() doesn't raise NameError (fix for regression).""" diff --git a/faststack/tests/test_startup_opt.py b/faststack/tests/test_startup_opt.py new file mode 100644 index 0000000..774db52 --- /dev/null +++ b/faststack/tests/test_startup_opt.py @@ -0,0 +1,99 @@ +import pytest +from pathlib import Path +from unittest.mock import Mock, patch +from faststack.app import AppController + +@pytest.fixture(scope="session") +def qapp(): + from PySide6.QtWidgets import QApplication + app = QApplication.instance() + if app is None: + app = QApplication([]) + return app + +@pytest.fixture +def controller(tmp_path, qapp): + _ = qapp + with ( + patch('faststack.app.Watcher'), + patch('faststack.app.SidecarManager'), + patch('faststack.app.setup_logging'), + patch('faststack.app.QQmlApplicationEngine'), + ): + ctrl = AppController(tmp_path, Mock()) + ctrl._thumbnail_model = Mock() + ctrl._thumbnail_model.rowCount.return_value = 0 + ctrl._path_resolver = Mock() + return ctrl + +def test_startup_only_one_scan(controller): + """Verify that startup performs exactly one variant scan and zero simple scans.""" + with patch('faststack.app.find_images_with_variants', return_value=([], {})) as mock_scan: + controller.load() + + assert controller._scan_count_variant == 1 + assert controller._scan_count_simple == 0 + assert controller._grid_refreshes == 1 + assert mock_scan.call_count == 1 + + # refresh_from_controller should be used instead of refresh + assert controller._thumbnail_model.refresh_from_controller.called + assert not controller._thumbnail_model.refresh.called + +def test_toggle_grid_avoids_redundant_refresh(controller): + """Verify that toggling grid view does not trigger a refresh if the model is already current.""" + # Setup: simulate a state where model is already current + controller.image_files = [Mock()] + controller._grid_model_dirty = False + controller._is_grid_view_active = False + controller._thumbnail_model.rowCount.return_value = 1 + controller._grid_refreshes = 0 + + controller._thumbnail_model.find_image_index.return_value = -1 + + # Toggle to grid + controller._set_grid_view_active(True) + + # Should NOT have refreshed because it wasn't dirty and rowCount > 0 + assert not controller._thumbnail_model.refresh_from_controller.called + assert not controller._thumbnail_model.refresh.called + assert controller._grid_refreshes == 0 + +def test_apply_filter_uses_efficient_refresh(controller): + """Verify that apply_filter uses refresh_from_controller and only one refresh.""" + controller.image_files = [Mock()] + controller._thumbnail_model.refresh_from_controller.reset_mock() + controller._thumbnail_model.refresh.reset_mock() + + controller.apply_filter("test", []) + + assert controller._thumbnail_model.refresh_from_controller.called + assert not controller._thumbnail_model.refresh.called + # Check that it called set_filter with refresh=False + controller._thumbnail_model.set_filter.assert_called_with("test", refresh=False) + +def test_loupe_filter_handles_dirty_flag(controller): + """Verify that filtering in loupe mode doesn't refresh the grid but marks it dirty.""" + controller.image_files = [Mock()] + controller._is_grid_view_active = False + controller._grid_model_dirty = False + controller._grid_refreshes = 0 + controller._thumbnail_model.refresh_from_controller.reset_mock() + + print(f"DEBUG: before apply_filter: grid_active={controller._is_grid_view_active}, dirty={controller._grid_model_dirty}") + controller.apply_filter("test", []) + print(f"DEBUG: after apply_filter: grid_active={controller._is_grid_view_active}, dirty={controller._grid_model_dirty}") + + # Grid should NOT have refreshed + assert not controller._thumbnail_model.refresh_from_controller.called, "refresh_from_controller should NOT have been called" + assert controller._grid_refreshes == 0, f"grid_refreshes should be 0, got {controller._grid_refreshes}" + # Dirty flag should be set + assert controller._grid_model_dirty is True, "grid_model_dirty should be True after filtering in loupe mode" + + # Toggle to grid should now trigger the refresh + controller._thumbnail_model.find_image_index.return_value = -1 + controller._set_grid_view_active(True) + + assert controller._thumbnail_model.refresh_from_controller.called + assert controller._grid_refreshes == 1 + assert controller._grid_model_dirty is False diff --git a/faststack/tests/test_startup_optimization.py b/faststack/tests/test_startup_optimization.py new file mode 100644 index 0000000..824c5cc --- /dev/null +++ b/faststack/tests/test_startup_optimization.py @@ -0,0 +1,128 @@ +import sys +import os +from pathlib import Path +from unittest.mock import MagicMock, patch +import pytest + +from PySide6.QtWidgets import QApplication + +# We mock AppController dependencies here +# Assuming these tests are run in an environment where faststack is importable + +@pytest.fixture(scope="session", autouse=True) +def qapplication(): + if not QApplication.instance(): + app = QApplication(sys.argv) + yield QApplication.instance() + +@patch("faststack.app.UIState") +@patch("faststack.app.Keybinder") +@patch("faststack.app.ImageEditor") +@patch("faststack.app.ThumbnailProvider") +@patch("faststack.app.ThumbnailCache") +@patch("faststack.app.ThumbnailPrefetcher") +@patch("faststack.app.Prefetcher") +@patch("faststack.app.ByteLRUCache") +@patch("faststack.app.Watcher") +@patch("faststack.app.find_images_with_variants") +@patch("faststack.app.SidecarManager") +@patch("faststack.app.ThumbnailModel") +@patch("faststack.app.config") +def test_startup_optimization( + MockConfig, + MockThumbnailModel, MockSidecarManager, mock_find_images, MockWatcher, + MockByteLRUCache, MockPrefetcher, MockThumbnailPrefetcher, MockThumbnailCache, + MockThumbnailProvider, MockImageEditor, MockKeybinder, MockUIState +): + """Verify that startup only triggers one disk scan and one model refresh.""" + + # Delayed import to ensure patches are active if AppController is imported at top level + # (though typically patching modules works fine) + from faststack.app import AppController + + # Setup mocks + mock_find_images.return_value = ([], {}) # Empty list of images + + mock_model_instance = MockThumbnailModel.return_value + mock_model_instance.rowCount.return_value = 0 # Empty model initially + + mock_engine = MagicMock() + + controller = AppController(Path("."), mock_engine) + + # Simulate load() + controller.load() + + # Assertions + # 1. Exactly one scan variant (from refresh_image_list called by load) + assert controller._scan_count_variant == 1 + + # 2. Exactly one grid refresh (from refresh_image_list calling refresh_from_controller) + assert controller._grid_refreshes == 1 + + # 3. Check mock calls + mock_model_instance.refresh_from_controller.assert_called_once() + mock_model_instance.refresh.assert_not_called() + + +@patch("faststack.app.UIState") +@patch("faststack.app.Keybinder") +@patch("faststack.app.ImageEditor") +@patch("faststack.app.ThumbnailProvider") +@patch("faststack.app.ThumbnailCache") +@patch("faststack.app.ThumbnailPrefetcher") +@patch("faststack.app.Prefetcher") +@patch("faststack.app.ByteLRUCache") +@patch("faststack.app.Watcher") +@patch("faststack.app.find_images_with_variants") +@patch("faststack.app.SidecarManager") +@patch("faststack.app.ThumbnailModel") +@patch("faststack.app.config") +def test_filter_optimization( + MockConfig, + MockThumbnailModel, MockSidecarManager, mock_find_images, MockWatcher, + MockByteLRUCache, MockPrefetcher, MockThumbnailPrefetcher, MockThumbnailCache, + MockThumbnailProvider, MockImageEditor, MockKeybinder, MockUIState +): + """Verify that filtering uses optimized refresh logic.""" + from faststack.app import AppController + + # Setup mocks + mock_find_images.return_value = ([], {}) + + mock_model_instance = MockThumbnailModel.return_value + mock_model_instance.rowCount.return_value = 0 + + mock_engine = MagicMock() + controller = AppController(Path("."), mock_engine) + + # Initial load + controller.load() + + # Reset + mock_model_instance.reset_mock() + controller._grid_refreshes = 0 + controller._scan_count_variant = 0 + + # Apply filter + controller.apply_filter("test", []) + + # Verify behavior + mock_model_instance.set_filter.assert_called_with("test", refresh=False) + mock_model_instance.set_filter_flags.assert_called_with([], refresh=False) + + # refresh_from_controller called (grid active by default) + mock_model_instance.refresh_from_controller.assert_called_once() + mock_model_instance.refresh.assert_not_called() + assert controller._grid_refreshes == 1 + + # Test inactive grid view + controller._is_grid_view_active = False + mock_model_instance.reset_mock() + controller._grid_refreshes = 0 + controller._filter_enabled = True + + controller.clear_filter() + + mock_model_instance.refresh_from_controller.assert_not_called() + assert controller._grid_model_dirty is True diff --git a/faststack/tests/test_thumbnail_ready_emits_datachanged.py b/faststack/tests/test_thumbnail_ready_emits_datachanged.py index b969f11..7dcece9 100644 --- a/faststack/tests/test_thumbnail_ready_emits_datachanged.py +++ b/faststack/tests/test_thumbnail_ready_emits_datachanged.py @@ -6,11 +6,20 @@ import pytest # Minimal Qt imports needed for the model -from PySide6.QtCore import Qt, QModelIndex +from PySide6.QtCore import Qt, QModelIndex, QCoreApplication + + +@pytest.fixture(scope="session") +def qapp(): + """Ensure a QCoreApplication exists for the test session.""" + app = QCoreApplication.instance() + if app is None: + app = QCoreApplication([]) + yield app @pytest.fixture -def thumbnail_model(): +def thumbnail_model(qapp): """Create a ThumbnailModel with fake entries for testing.""" from faststack.thumbnail_view.model import ThumbnailModel, ThumbnailEntry diff --git a/faststack/tests/test_ui_prefetch_safety.py b/faststack/tests/test_ui_prefetch_safety.py new file mode 100644 index 0000000..0b9efc2 --- /dev/null +++ b/faststack/tests/test_ui_prefetch_safety.py @@ -0,0 +1,96 @@ +import unittest +from unittest.mock import MagicMock, patch +from pathlib import Path +from faststack.ui.provider import UIState + +class TestUIPrefetchSafety(unittest.TestCase): + def setUp(self): + self.app_controller = MagicMock() + self.model = MagicMock() + self.prefetcher = MagicMock() + + # Setup model and prefetcher on app_controller + self.app_controller._thumbnail_model = self.model + self.app_controller._thumbnail_prefetcher = self.prefetcher + + # Mock prefetcher constants + self.prefetcher.PRIO_MED = 1 + + # Fake clock state + self.current_time = 100.0 + + def fake_clock(): + return self.current_time + + self.ui_state = UIState(self.app_controller, clock_func=fake_clock) + + # Default mock behavior + self.model.rowCount.return_value = 5000 + self.model.thumbnail_size = (256, 256) + + def get_entry_mock(i): + entry = MagicMock() + entry.path = Path(f"image_{i}.jpg") + entry.mtime_ns = 123456789 + entry.is_folder = False + return entry + + self.model.get_entry.side_effect = get_entry_mock + + def test_budget_trimming(self): + """Verify that range is trimmed to maxCount.""" + self.ui_state.gridPrefetchRange(0, 4999, maxCount=200) + # Expected: 200 submissions (index 0 to 199) + self.assertEqual(self.prefetcher.submit.call_count, 200) + + def test_hard_cap(self): + """Verify that budget is capped at HARD_LIMIT (800).""" + self.ui_state.gridPrefetchRange(0, 4999, maxCount=2000) + # Expected: 800 submissions (index 0 to 799) + self.assertEqual(self.prefetcher.submit.call_count, 800) + + def test_duplicate_suppression(self): + """Verify identical requests within 30ms are suppressed.""" + # 1. First call - should succeed + self.ui_state.gridPrefetchRange(10, 50, maxCount=100) + self.assertEqual(self.prefetcher.submit.call_count, 41) + self.prefetcher.submit.reset_mock() + + # 2. Duplicate call at t+10ms - should be suppressed + self.current_time += 0.010 + self.ui_state.gridPrefetchRange(10, 50, maxCount=100) + self.assertEqual(self.prefetcher.submit.call_count, 0) + + # 3. Duplicate call at t+40ms (from start) - should succeed (cooldown expired) + self.current_time += 0.030 + self.ui_state.gridPrefetchRange(10, 50, maxCount=100) + self.assertEqual(self.prefetcher.submit.call_count, 41) + self.prefetcher.submit.reset_mock() + + # 4. Different range at t+10ms (from last) - should succeed (not duplicate) + self.current_time += 0.010 + self.ui_state.gridPrefetchRange(10, 51, maxCount=100) + self.assertEqual(self.prefetcher.submit.call_count, 42) + + def test_index_sanity(self): + """Verify handling of out-of-bounds, empty model, and invalid ranges.""" + # Case 1: Empty model + self.model.rowCount.return_value = 0 + self.prefetcher.submit.reset_mock() + self.ui_state.gridPrefetchRange(0, 10, 100) + self.assertEqual(self.prefetcher.submit.call_count, 0) + + # Case 2: startIndex > endIndex + self.model.rowCount.return_value = 5000 + self.ui_state.gridPrefetchRange(100, 50, 100) + self.assertEqual(self.prefetcher.submit.call_count, 0) + + # Case 3: Out-of-bounds clamping + self.prefetcher.submit.reset_mock() + self.ui_state.gridPrefetchRange(-10, 10000, 100) + # Should be clamped to [0, 4999], then budgeted to [0, 99] + self.ui_state.gridPrefetchRange(-10, 10000, 100) + self.assertEqual(self.prefetcher.submit.call_count, 100) + +if __name__ == "__main__": + unittest.main() diff --git a/faststack/tests/test_variants.py b/faststack/tests/test_variants.py new file mode 100644 index 0000000..c4f6696 --- /dev/null +++ b/faststack/tests/test_variants.py @@ -0,0 +1,357 @@ +"""Tests for variant (backup + developed) parsing, grouping, and integration.""" + +import os +import pytest +from pathlib import Path +from unittest.mock import MagicMock + +from faststack.io.variants import ( + parse_variant_stem, + build_variant_map, + build_badge_list, + norm_path, + VariantInfo, + VariantGroup, +) + + +# ── parse_variant_stem ────────────────────────────────────────────────────── + +class TestParseVariantStem: + """Tests for parse_variant_stem().""" + + def test_plain_stem(self): + key, dev, backup = parse_variant_stem("photo") + assert key == "photo" + assert dev is False + assert backup is None + + def test_developed(self): + key, dev, backup = parse_variant_stem("photo-developed") + assert key == "photo" + assert dev is True + assert backup is None + + def test_developed_case_insensitive(self): + key, dev, backup = parse_variant_stem("photo-Developed") + assert key == "photo" + assert dev is True + assert backup is None + + def test_backup_no_number(self): + key, dev, backup = parse_variant_stem("photo-backup") + assert key == "photo" + assert dev is False + assert backup == 1 + + def test_backup_with_number(self): + key, dev, backup = parse_variant_stem("photo-backup2") + assert key == "photo" + assert dev is False + assert backup == 2 + + def test_backup_high_number(self): + key, dev, backup = parse_variant_stem("photo-backup33") + assert key == "photo" + assert dev is False + assert backup == 33 + + def test_developed_and_backup(self): + """photo-developed-backup2 → developed + backup 2.""" + key, dev, backup = parse_variant_stem("photo-developed-backup2") + assert key == "photo" + assert dev is True + assert backup == 2 + + def test_backup_and_developed(self): + """photo-backup2-developed → developed + backup 2. + Note: -developed token comes after -backup, but -backup is at end of stripped stem.""" + key, dev, backup = parse_variant_stem("photo-backup2-developed") + assert key == "photo" + assert dev is True + assert backup == 2 + + def test_not_developed_substring(self): + """'photo-undeveloped' should NOT match -developed (substring).""" + key, dev, backup = parse_variant_stem("photo-undeveloped") + assert dev is False + assert key == "photo-undeveloped" + + def test_not_backup_prefix(self): + """'mybackup-photo' should NOT match -backup (not at end).""" + key, dev, backup = parse_variant_stem("mybackup-photo") + assert dev is False + assert backup is None + assert key == "mybackup-photo" + + def test_case_insensitive_mixed(self): + key, dev, backup = parse_variant_stem("photo-Developed-Backup2") + assert dev is True + assert backup == 2 + assert key == "photo" + + def test_backup_case_insensitive(self): + key, dev, backup = parse_variant_stem("photo-BACKUP3") + assert backup == 3 + assert key == "photo" + + def test_stem_with_dashes(self): + """Stems with dashes should work correctly.""" + key, dev, backup = parse_variant_stem("my-cool-photo-developed") + assert key == "my-cool-photo" + assert dev is True + assert backup is None + + def test_stem_with_dashes_and_backup(self): + key, dev, backup = parse_variant_stem("my-cool-photo-backup2") + assert key == "my-cool-photo" + assert dev is False + assert backup == 2 + + +# ── build_variant_map ──────────────────────────────────────────────────────── + +class TestBuildVariantMap: + """Tests for build_variant_map().""" + + def test_singleton_group(self): + """A single file creates a group with itself as main.""" + paths = [Path("/dir/photo.jpg")] + vmap = build_variant_map(paths) + assert "photo" in vmap + group = vmap["photo"] + # Paths are normalized in build_variant_map + assert group.main_path == Path(norm_path(paths[0])) + assert len(group.all_files) == 1 + + def test_main_selection_prefers_non_backup_non_developed(self): + paths = [ + Path("/dir/photo.jpg"), + Path("/dir/photo-developed.jpg"), + Path("/dir/photo-backup.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + assert group.main_path == Path(norm_path(Path("/dir/photo.jpg"))) + + def test_main_selection_fallback_to_developed(self): + """If no non-backup non-developed exists, main is non-backup developed.""" + paths = [ + Path("/dir/photo-developed.jpg"), + Path("/dir/photo-backup.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + assert group.main_path == Path(norm_path(Path("/dir/photo-developed.jpg"))) + + def test_developed_path_selection(self): + paths = [ + Path("/dir/photo.jpg"), + Path("/dir/photo-developed.jpg"), + Path("/dir/photo-backup.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + assert group.developed_path == Path(norm_path(Path("/dir/photo-developed.jpg"))) + + def test_backup_paths(self): + paths = [ + Path("/dir/photo.jpg"), + Path("/dir/photo-backup.jpg"), + Path("/dir/photo-backup2.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + assert 1 in group.backup_paths + assert 2 in group.backup_paths + assert group.backup_paths[1] == Path(norm_path(Path("/dir/photo-backup.jpg"))) + assert group.backup_paths[2] == Path(norm_path(Path("/dir/photo-backup2.jpg"))) + + def test_grouping_case_insensitive(self): + """Files with different case stems should group together.""" + paths = [ + Path("/dir/Photo.jpg"), + Path("/dir/photo-backup.jpg"), + ] + vmap = build_variant_map(paths) + # Should be grouped under casefolded key "photo" + assert "photo" in vmap + group = vmap["photo"] + assert len(group.all_files) == 2 + + def test_no_developed_when_only_main(self): + paths = [Path("/dir/photo.jpg")] + vmap = build_variant_map(paths) + group = vmap["photo"] + assert group.developed_path is None + + def test_complex_variant_set(self): + """Full set: main, developed, backup, backup2, developed-backup.""" + paths = [ + Path("/dir/photo.jpg"), + Path("/dir/photo-developed.jpg"), + Path("/dir/photo-backup.jpg"), + Path("/dir/photo-backup2.jpg"), + Path("/dir/photo-developed-backup.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + + assert group.main_path == Path(norm_path(Path("/dir/photo.jpg"))) + assert group.developed_path == Path(norm_path(Path("/dir/photo-developed.jpg"))) + assert group.backup_paths[1] == Path(norm_path(Path("/dir/photo-backup.jpg"))) + assert group.backup_paths[2] == Path(norm_path(Path("/dir/photo-backup2.jpg"))) + + +# ── build_badge_list ───────────────────────────────────────────────────────── + +class TestBuildBadgeList: + """Tests for build_badge_list().""" + + def test_badge_order_main_d_backups(self): + paths = [ + Path("/dir/photo.jpg"), + Path("/dir/photo-developed.jpg"), + Path("/dir/photo-backup.jpg"), + Path("/dir/photo-backup2.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + badges = build_badge_list(group) + + labels = [b["label"] for b in badges] + assert labels == ["Main", "D", "Bk", "Bk2"] + + def test_badge_no_duplicates(self): + """If developed IS main (no non-dev exists), D badge should not duplicate Main.""" + paths = [ + Path("/dir/photo-developed.jpg"), + Path("/dir/photo-backup.jpg"), + ] + vmap = build_variant_map(paths) + group = vmap["photo"] + badges = build_badge_list(group) + labels = [b["label"] for b in badges] + # photo-developed.jpg is main AND developed, so D badge is suppressed + assert "Main" in labels + assert "D" not in labels + + def test_singleton_no_badges(self): + paths = [Path("/dir/photo.jpg")] + vmap = build_variant_map(paths) + group = vmap["photo"] + badges = build_badge_list(group) + # Only Main badge + assert len(badges) == 1 + assert badges[0]["label"] == "Main" + + +# ── Integration: indexer find_images_with_variants ─────────────────────────── + +class TestFindImagesWithVariants: + """Tests for find_images_with_variants() filtering.""" + + def test_filters_developed_from_visible(self, tmp_path): + """Developed files should be removed from visible list when main exists.""" + from faststack.io.indexer import find_images_with_variants + + # Create test files + (tmp_path / "photo.jpg").write_bytes(b"\xff\xd8\xff\xe0") # minimal JPEG + (tmp_path / "photo-developed.jpg").write_bytes(b"\xff\xd8\xff\xe0") + + images, vmap = find_images_with_variants(tmp_path) + names = [img.path.name for img in images] + + assert "photo.jpg" in names + assert "photo-developed.jpg" not in names + + def test_keeps_orphan_developed(self, tmp_path): + """If no non-developed exists, developed IS main and stays visible.""" + from faststack.io.indexer import find_images_with_variants + + (tmp_path / "photo-developed.jpg").write_bytes(b"\xff\xd8\xff\xe0") + + images, vmap = find_images_with_variants(tmp_path) + names = [img.path.name for img in images] + assert "photo-developed.jpg" in names + + def test_backups_hidden(self, tmp_path): + """Backup files should be hidden from visible list.""" + from faststack.io.indexer import find_images_with_variants + + (tmp_path / "photo.jpg").write_bytes(b"\xff\xd8\xff\xe0") + (tmp_path / "photo-backup.jpg").write_bytes(b"\xff\xd8\xff\xe0") + (tmp_path / "photo-backup2.jpg").write_bytes(b"\xff\xd8\xff\xe0") + + images, vmap = find_images_with_variants(tmp_path) + names = [img.path.name for img in images] + + assert "photo.jpg" in names + assert "photo-backup.jpg" not in names + assert "photo-backup2.jpg" not in names + + def test_variant_flags_set(self, tmp_path): + """ImageFile should have has_backups/has_developed flags set.""" + from faststack.io.indexer import find_images_with_variants + + (tmp_path / "photo.jpg").write_bytes(b"\xff\xd8\xff\xe0") + (tmp_path / "photo-developed.jpg").write_bytes(b"\xff\xd8\xff\xe0") + (tmp_path / "photo-backup.jpg").write_bytes(b"\xff\xd8\xff\xe0") + + images, vmap = find_images_with_variants(tmp_path) + photo = [img for img in images if img.path.name == "photo.jpg"][0] + + assert photo.has_backups is True + assert photo.has_developed is True + + def test_no_flags_for_singles(self, tmp_path): + """Single images should not have variant flags.""" + from faststack.io.indexer import find_images_with_variants + + (tmp_path / "lonely.jpg").write_bytes(b"\xff\xd8\xff\xe0") + + images, vmap = find_images_with_variants(tmp_path) + lonely = [img for img in images if img.path.name == "lonely.jpg"][0] + + assert lonely.has_backups is False + assert lonely.has_developed is False + + def test_variant_map_built(self, tmp_path): + """Variant map should contain all files including backups.""" + from faststack.io.indexer import find_images_with_variants + + (tmp_path / "photo.jpg").write_bytes(b"\xff\xd8\xff\xe0") + (tmp_path / "photo-backup.jpg").write_bytes(b"\xff\xd8\xff\xe0") + + images, vmap = find_images_with_variants(tmp_path) + assert "photo" in vmap + group = vmap["photo"] + assert len(group.all_files) == 2 + + +# ── Thumbnail model roles ─────────────────────────────────────────────────── + +class TestThumbnailModelVariantRoles: + """Tests that ThumbnailModel propagates variant flags.""" + + def test_entry_has_variant_fields(self): + from faststack.thumbnail_view.model import ThumbnailEntry + entry = ThumbnailEntry( + path=Path("/dir/photo.jpg"), + name="photo.jpg", + is_folder=False, + has_backups=True, + has_developed=True, + ) + assert entry.has_backups is True + assert entry.has_developed is True + + def test_entry_defaults_false(self): + from faststack.thumbnail_view.model import ThumbnailEntry + entry = ThumbnailEntry( + path=Path("/dir/photo.jpg"), + name="photo.jpg", + is_folder=False, + ) + assert entry.has_backups is False + assert entry.has_developed is False diff --git a/faststack/tests/test_variants_logic.py b/faststack/tests/test_variants_logic.py new file mode 100644 index 0000000..fde69cf --- /dev/null +++ b/faststack/tests/test_variants_logic.py @@ -0,0 +1,92 @@ + +import unittest +from pathlib import Path +from faststack.io.variants import build_variant_map, VariantGroup + +class TestVariantsLogic(unittest.TestCase): + def test_main_selection_priority(self): + """Verify Main selection priority: Non-dev > Dev > Backup.""" + + # Cas 1: Pure Main exists + paths = [ + Path("img.jpg"), + Path("img-developed.jpg"), + Path("img-backup.jpg"), + ] + groups = build_variant_map(paths) + group = groups["img"] + + self.assertEqual(group.main_path.name, "img.jpg") + self.assertEqual(group.developed_path.name, "img-developed.jpg") + self.assertEqual(group.backup_paths[1].name, "img-backup.jpg") + + def test_main_selection_orphan_developed(self): + """Verify Developed is chosen as Main if no pure Main exists.""" + + paths = [ + Path("img-developed.jpg"), + Path("img-backup.jpg"), + ] + groups = build_variant_map(paths) + group = groups["img"] + + # developed becomes Main because it's Tier 1 (vs Backup Tier 2) + self.assertEqual(group.main_path.name, "img-developed.jpg") + # It is ALSO the developed path + self.assertEqual(group.developed_path.name, "img-developed.jpg") + + def test_main_selection_orphan_backup(self): + """Verify Backup is chosen as Main if nothing else exists.""" + + paths = [ + Path("img-backup.jpg"), + Path("img-backup2.jpg"), + ] + groups = build_variant_map(paths) + group = groups["img"] + + self.assertEqual(group.main_path.name, "img-backup.jpg") # Lowest backup + self.assertIsNone(group.developed_path) + + def test_backup_sorting(self): + """Verify backups are correctly identified and sorted.""" + paths = [ + Path("img-backup2.jpg"), + Path("img.jpg"), + Path("img-backup10.jpg"), + Path("img-backup.jpg"), # implies 1 + ] + groups = build_variant_map(paths) + group = groups["img"] + + self.assertEqual(len(group.backup_paths), 3) + self.assertEqual(group.backup_paths[1].name, "img-backup.jpg") + self.assertEqual(group.backup_paths[2].name, "img-backup2.jpg") + self.assertEqual(group.backup_paths[10].name, "img-backup10.jpg") + + def test_developed_backup_handling(self): + """Verify developed backups are handled correctly.""" + # Policy: developed-backup is "developed" candidate, AND "backup" candidate. + # But _select_developed prefers non-backup developed. + + paths = [ + Path("img.jpg"), + Path("img-developed.jpg"), + Path("img-backup-developed.jpg"), # This is a backup AND developed? + # Note: parse_variant_stem handles "-developed" then "-backup". + # "img-backup-developed" -> stem "img-backup". is_developed=True. + # Then "img-backup" -> group "img", backup=1. + ] + + groups = build_variant_map(paths) + group = groups["img"] + + self.assertEqual(group.main_path.name, "img.jpg") + self.assertEqual(group.developed_path.name, "img-developed.jpg") + + # Check that backup list contains the backup-developed file + # It has backup_number=1. + self.assertEqual(group.backup_paths[1].name, "img-backup-developed.jpg") + +if __name__ == '__main__': + unittest.main() diff --git a/faststack/tests/thumbnail_view/test_thumbnail_ready.py b/faststack/tests/thumbnail_view/test_thumbnail_ready.py new file mode 100644 index 0000000..f93347b --- /dev/null +++ b/faststack/tests/thumbnail_view/test_thumbnail_ready.py @@ -0,0 +1,82 @@ +"""Test that _on_thumbnail_ready only fires for matching size IDs.""" + +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from PySide6.QtCore import QCoreApplication + +from faststack.io.utils import compute_path_hash +from faststack.thumbnail_view.model import ThumbnailEntry, ThumbnailModel + + +@pytest.fixture(scope="module") +def qapp(): + """Ensure a QCoreApplication exists for the test module.""" + app = QCoreApplication.instance() + if app is None: + app = QCoreApplication(sys.argv) + yield app + + +@pytest.fixture() +def model_with_entry(tmp_path, qapp): + """Create a ThumbnailModel with one image entry.""" + img = tmp_path / "photo.jpg" + img.write_bytes(b"\xff\xd8") # minimal JPEG magic + mtime_ns = 1000000000 + + model = ThumbnailModel( + base_directory=tmp_path, + current_directory=tmp_path, + get_metadata_callback=None, + thumbnail_size=200, + ) + + # Manually insert a single entry and rebuild the id mapping + entry = ThumbnailEntry( + path=img, + name=img.name, + is_folder=False, + mtime_ns=mtime_ns, + ) + model._entries = [entry] + model._rebuild_id_mapping() + + path_hash = compute_path_hash(img) + return model, entry, path_hash, mtime_ns + + +def test_wrong_size_ignored(model_with_entry): + """thumbnailReady with wrong size should not bump thumb_rev.""" + model, entry, path_hash, mtime_ns = model_with_entry + + spy = MagicMock() + model.dataChanged.connect(spy) + + wrong_id = f"160/{path_hash}/{mtime_ns}" + model._on_thumbnail_ready(wrong_id) + + assert entry.thumb_rev == 0 + spy.assert_not_called() + + +def test_correct_size_bumps_rev(model_with_entry): + """thumbnailReady with correct size should bump thumb_rev and emit dataChanged.""" + model, entry, path_hash, mtime_ns = model_with_entry + + spy = MagicMock() + model.dataChanged.connect(spy) + + correct_id = f"200/{path_hash}/{mtime_ns}" + model._on_thumbnail_ready(correct_id) + + assert entry.thumb_rev == 1 + spy.assert_called_once() + + # Verify roles include ThumbnailSourceRole and ThumbRevRole + _, _, roles_arg = spy.call_args[0] + assert model.ThumbnailSourceRole in roles_arg + assert model.ThumbRevRole in roles_arg diff --git a/faststack/thumbnail_view/model.py b/faststack/thumbnail_view/model.py index 5b69fa5..ba35960 100644 --- a/faststack/thumbnail_view/model.py +++ b/faststack/thumbnail_view/model.py @@ -80,6 +80,8 @@ class ThumbnailEntry: is_restacked: bool = False is_favorite: bool = False folder_stats: Optional[FolderStats] = None + has_backups: bool = False + has_developed: bool = False mtime_ns: int = 0 thumb_rev: int = 0 # Bumped when thumbnail is ready, forces QML refresh @@ -111,6 +113,8 @@ class ThumbnailModel(QAbstractListModel): IsInBatchRole = Qt.ItemDataRole.UserRole + 15 IsCurrentRole = Qt.ItemDataRole.UserRole + 16 IsFavoriteRole = Qt.ItemDataRole.UserRole + 17 + HasBackupsRole = Qt.ItemDataRole.UserRole + 18 + HasDevelopedRole = Qt.ItemDataRole.UserRole + 19 # Signal emitted when a thumbnail is ready (id = "{size}/{path_hash}/{mtime_ns}") thumbnailReady = Signal(str) @@ -140,14 +144,25 @@ def __init__( self._last_selected_index: Optional[int] = None self._active_filter: str = "" # current filename filter (set by AppController) self._active_filter_flags: list = [] # current flag filters (e.g. ["uploaded", "stacked"]) + + # One-shot reason for logging + self._next_source_reason: Optional[str] = None # Mapping from thumbnail_id (without query params) to row index # id format: "{size}/{path_hash}/{mtime_ns}" self._id_to_row: Dict[str, int] = {} + + # One-shot reason for logging + self._next_source_reason: Optional[str] = None # Connect our own signal to handle thumbnail ready events self.thumbnailReady.connect(self._on_thumbnail_ready) + @property + def thumbnail_size(self) -> int: + """Target thumbnail size in pixels (read-only).""" + return self._thumbnail_size + @property def current_directory(self) -> Path: """Current directory being displayed.""" @@ -188,7 +203,10 @@ def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole): elif role == self.IsEditedRole: return entry.is_edited elif role == self.ThumbnailSourceRole: - return self._get_thumbnail_source(entry) + reason = self._next_source_reason or "scroll" + # Consume one-shot reason + self._next_source_reason = None + return self._get_thumbnail_source(entry, reason=reason) elif role == self.FolderStatsRole: if entry.folder_stats: return { @@ -233,6 +251,10 @@ def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole): loupe_idx = self._get_loupe_index_for_entry(entry) return loupe_idx is not None and loupe_idx == current_idx return False + elif role == self.HasBackupsRole: + return entry.has_backups + elif role == self.HasDevelopedRole: + return entry.has_developed return None @@ -265,12 +287,14 @@ def roleNames(self) -> Dict[int, bytes]: self.IsInBatchRole: b"isInBatch", self.IsCurrentRole: b"isCurrent", self.IsFavoriteRole: b"isFavorite", + self.HasBackupsRole: b"hasBackups", + self.HasDevelopedRole: b"hasDeveloped", } - def _get_thumbnail_source(self, entry: ThumbnailEntry) -> str: + def _get_thumbnail_source(self, entry: ThumbnailEntry, reason: str = "scroll") -> str: """Build thumbnail URL for QML Image source. - Format: image://thumbnail/{size}/{path_hash}/{mtime_ns}?r={rev} + Format: image://thumbnail/{size}/{path_hash}/{mtime_ns}?r={rev}&reason={reason} Folders use: image://thumbnail/folder/{path_hash}/{mtime_ns}?r={rev} """ path_hash = compute_path_hash(entry.path) @@ -280,28 +304,21 @@ def _get_thumbnail_source(self, entry: ThumbnailEntry) -> str: if entry.is_folder: return f"image://thumbnail/folder/{path_hash}/{mtime_ns}?r={rev}" else: - return f"image://thumbnail/{self._thumbnail_size}/{path_hash}/{mtime_ns}?r={rev}" + return f"image://thumbnail/{self._thumbnail_size}/{path_hash}/{mtime_ns}?r={rev}&reason={reason}" - def set_filter(self, filter_string: str) -> None: - """Set the active filename filter and refresh the model. - - Args: - filter_string: Filter to apply (case-insensitive substring match on stem). - Pass empty string to clear the filter. - """ + def set_filter(self, filter_string: str, refresh: bool = True) -> None: + """Set the active filename filter and optionally refresh the model.""" self._active_filter = filter_string - self.refresh() + self._next_source_reason = "filter" + if refresh: + self.refresh() - def set_filter_flags(self, flags: list) -> None: - """Set the active flag filters and refresh the model. - - Args: - flags: List of flag names to filter by (e.g. ["uploaded", "stacked"]). - Images must have ALL listed flags set to True (AND logic). - Pass empty list to clear flag filters. - """ + def set_filter_flags(self, flags: list, refresh: bool = True) -> None: + """Set the active flag filters and optionally refresh the model.""" self._active_filter_flags = list(flags) - self.refresh() + self._next_source_reason = "filter" + if refresh: + self.refresh() def _add_folders_to_entries(self): """Scan for folders and add them to self._entries.""" @@ -381,10 +398,16 @@ def refresh(self): for img in images: try: meta = self._get_metadata(img.path.stem) + if not isinstance(meta, dict): + # Ensure it's a dict before .get() + log.debug("Metadata for %s is not a dict: %r", img.path.stem, meta) + continue + if all(meta.get(flag, False) for flag in flags): filtered.append(img) - except Exception: - pass # Skip images with metadata errors + except Exception as e: + log.debug("Error filtering image %s: %s", img.path, e) + # Skip images with metadata errors images = filtered self._add_images_to_entries(images) @@ -472,6 +495,7 @@ def refresh_from_controller(self, images: List, metadata_map: Optional[Dict[str, Folders are still scanned, but image entries are built from the provided objects. """ + self._next_source_reason = "refresh" cur, own = QThread.currentThread(), self.thread() assert cur == own, f"ThumbnailModel refresh thread mismatch" @@ -503,10 +527,16 @@ def refresh_from_controller(self, images: List, metadata_map: Optional[Dict[str, meta = self._get_metadata(img.path.stem) else: continue + + if not isinstance(meta, dict): + log.debug("Metadata for %s is not a dict: %r", img.path.stem, meta) + continue + if all(meta.get(flag, False) for flag in flags): filtered.append(img) - except Exception: - pass # Skip images with metadata errors + except Exception as e: + log.debug("Error filtering image %s: %s", img.path, e) + # Skip images with metadata errors images = filtered self._add_images_to_entries(images, metadata_map) @@ -552,13 +582,19 @@ def _add_images_to_entries(self, images: List, metadata_map: Optional[Dict[str, elif self._get_metadata: try: meta = self._get_metadata(img.path.stem) - is_stacked = meta.get("stacked", False) - is_uploaded = meta.get("uploaded", False) - is_edited = meta.get("edited", False) - is_restacked = meta.get("restacked", False) - is_favorite = meta.get("favorite", False) - except Exception: - pass + if isinstance(meta, dict): + is_stacked = meta.get("stacked", False) + is_uploaded = meta.get("uploaded", False) + is_edited = meta.get("edited", False) + is_restacked = meta.get("restacked", False) + is_favorite = meta.get("favorite", False) + else: + log.debug("Metadata for %s is not a dict: %r", img.path.stem, meta) + except Exception as e: + log.debug("Error getting metadata for %s: %s", img.path, e) + + has_backups = getattr(img, 'has_backups', False) + has_developed = getattr(img, 'has_developed', False) self._entries.append( ThumbnailEntry( @@ -570,6 +606,8 @@ def _add_images_to_entries(self, images: List, metadata_map: Optional[Dict[str, is_edited=is_edited, is_restacked=is_restacked, is_favorite=is_favorite, + has_backups=has_backups, + has_developed=has_developed, mtime_ns=mtime_ns, ) ) @@ -597,6 +635,12 @@ def _make_thumbnail_id(self, entry: ThumbnailEntry) -> str: def _on_thumbnail_ready(self, thumbnail_id: str): """Handle thumbnail ready signal - bump revision and emit dataChanged.""" if thumbnail_id not in self._id_to_row: + if log.isEnabledFor(logging.DEBUG): + actual_size = thumbnail_id.split("/", 1)[0] if "/" in thumbnail_id else "?" + log.debug( + "thumbnail ready id not in model map: id=%s expected_size=%s actual_size=%s", + thumbnail_id, self._thumbnail_size, actual_size, + ) return row = self._id_to_row[thumbnail_id] @@ -660,6 +704,7 @@ def navigate_to(self, path: Path, update_base_if_above: bool = False): self._current_directory = resolved self._selected_indices.clear() self._last_selected_index = None + self._next_source_reason = "jump" self.refresh() # Selection methods diff --git a/faststack/thumbnail_view/prefetcher.py b/faststack/thumbnail_view/prefetcher.py index 08e9e0a..dfc66c3 100644 --- a/faststack/thumbnail_view/prefetcher.py +++ b/faststack/thumbnail_view/prefetcher.py @@ -2,6 +2,7 @@ import logging import os +import time from concurrent.futures import Future from pathlib import Path from threading import Lock @@ -16,6 +17,7 @@ from faststack.util.executors import create_priority_executor from faststack.imaging.orientation import get_exif_orientation, apply_orientation_to_np from faststack.io.utils import compute_path_hash +import faststack.util.thumb_debug as thumb_debug log = logging.getLogger(__name__) @@ -70,6 +72,8 @@ def __init__( on_ready_callback: Optional[Callable[[str], None]] = None, max_workers: int = None, target_size: int = 200, + debug_timing: bool = False, + debug_trace: bool = False, ): """Initialize the prefetcher. @@ -78,6 +82,8 @@ def __init__( on_ready_callback: Called with thumbnail_id when decode completes max_workers: Number of worker threads (default: min(4, cpu_count//2)) target_size: Target thumbnail size in pixels + debug_timing: Enable [THUMB-TIMING] log lines + debug_trace: Enable verbose trace logs """ if max_workers is None: max_workers = min(4, max(1, (os.cpu_count() or 4) // 2)) @@ -92,8 +98,10 @@ def __init__( # Track in-flight jobs to avoid duplicates # Key: (size, path_hash, mtime_ns) - self._inflight: Set[Tuple[int, str, int]] = set() + # Value: (rid, ThumbTimer) + self._inflight: Dict[Tuple[int, str, int], Tuple[int, "thumb_debug.ThumbTimer"]] = {} self._inflight_lock = Lock() + self._debug_trace = debug_trace # Track futures for potential cancellation self._futures: Dict[Tuple[int, str, int], Future] = {} @@ -109,6 +117,10 @@ def __init__( except Exception: self._ready_emitter = None + # Timing stats + self._debug_timing = debug_timing + self._submit_count = 0 + log.info( "ThumbnailPrefetcher initialized with %d workers, target size %dpx", max_workers, @@ -116,7 +128,9 @@ def __init__( ) def submit( - self, path: Path, mtime_ns: int, size: int = None, priority: int = PRIO_MED + self, path: Path, mtime_ns: int, size: int = None, + priority: int = PRIO_MED, + timer: Optional["thumb_debug.ThumbTimer"] = None ) -> bool: """Submit a thumbnail decode job. @@ -125,6 +139,7 @@ def submit( mtime_ns: File modification time in nanoseconds size: Target size (default: self._target_size) priority: Job priority (PRIO_HIGH, PRIO_MED) + timer: Pre-existing timer from provider (optional) Returns: True if job was submitted, False if already in-flight or cached @@ -144,26 +159,50 @@ def submit( if self._cache.get(cache_key) is not None: return False + if timer is None: + timer = thumb_debug.ThumbTimer(key=cache_key, path=path, reason="prefetch") + # Check/add to inflight set with self._inflight_lock: # If already in flight, check if we want to upgrade priority if job_key in self._inflight: - # ThreadPoolExecutor doesn't support priority upgrade easily, - # but our PriorityExecutor pulls from a queue. - # However, once a task is pulled, it stays at that priority. - # For now, we don't re-submit. If it's already in flight, it's either - # running or waiting in the queue. + existing_rid, existing_timer = self._inflight[job_key] + if existing_timer: + # Capture where this request originated + existing_timer.coalesced_from = timer.reason + # Update effective priority if this requested one is higher + if existing_timer.prio_effective is not None and priority < existing_timer.prio_effective: + existing_timer.prio_effective = priority + if timer: + thumb_debug.log_trace("prio_bump", rid=existing_timer.rid, new_prio=priority, triggered_by_rid=timer.rid) + + if timer: + thumb_debug.inc("decode_coalesced") + thumb_debug.log_trace("coalesced", rid=timer.rid, existing_rid=existing_rid) return False - self._inflight.add(job_key) + + if timer: + timer.t_queued = time.perf_counter() + timer.prio_submitted = priority + timer.prio_effective = priority + thumb_debug.inc("decode_submitted") + thumb_debug.log_trace("queued", rid=timer.rid, prio=priority, qdepth=len(self._inflight)+1) + + self._inflight[job_key] = (timer.rid if timer else 0, timer) + thumb_debug.gauge("inflight", len(self._inflight)) + thumb_debug.gauge("qdepth", len(self._inflight)) # Submit decode job try: + self._submit_count += 1 + future = self._executor.submit( self._decode_worker, path, path_hash, mtime_ns, size, + timer=timer, priority=priority, ) @@ -173,14 +212,15 @@ def submit( # Add callback *after* registering future. If already done, add_done_callback # may invoke immediately in this thread, so we want state initialized first. future.add_done_callback( - lambda f: self._on_decode_done(f, job_key, cache_key) + lambda f: self._on_decode_done(f, job_key, cache_key, timer) ) return True except RuntimeError: # Executor shutdown with self._inflight_lock: - self._inflight.discard(job_key) + self._inflight.pop(job_key, None) + thumb_debug.gauge("inflight", len(self._inflight)) return False def prefetch_batch(self, entries: list, margin: int = 2): @@ -200,36 +240,62 @@ def _decode_worker( path_hash: str, mtime_ns: int, size: int, + timer: "thumb_debug.ThumbTimer", ) -> Optional[bytes]: """Worker function to decode a thumbnail. Returns JPEG bytes or None on error. """ + if timer: + timer.t_worker_start = time.perf_counter() + timer.started = True + thumb_debug.inc("decode_started") + thumb_debug.log_trace("worker_start", rid=timer.rid) + try: # Read and decode - rgb_array = self._decode_image(path, size) + rgb_array = self._decode_image(path, size, timer=timer) + if rgb_array is None: return None + # Check cancellation early if possible + if timer.cancelled or self._stop_event.is_set(): + thumb_debug.log_trace("cancel_honored", rid=timer.rid, stage="after_decode") + return None + # Get EXIF orientation and apply (single point of orientation) - orientation = get_exif_orientation(path) - rgb_array = apply_orientation_to_np(rgb_array, orientation) + with timer.stage("orientation"): + orientation = get_exif_orientation(path) + rgb_array = apply_orientation_to_np(rgb_array, orientation) + + # Check cancellation again + if timer.cancelled or self._stop_event.is_set(): + thumb_debug.log_trace("cancel_honored", rid=timer.rid, stage="after_orientation") + return None # Encode to JPEG bytes for storage - pil_image = Image.fromarray(rgb_array, mode="RGB") + with timer.stage("encode"): + pil_image = Image.fromarray(rgb_array, mode="RGB") - # Use BytesIO to encode to JPEG - import io + # Use BytesIO to encode to JPEG + import io - buf = io.BytesIO() - pil_image.save(buf, format="JPEG", quality=85) - return buf.getvalue() + buf = io.BytesIO() + pil_image.save(buf, format="JPEG", quality=85) + result = buf.getvalue() + + if timer.cancelled: + thumb_debug.log_trace("cancel_too_late", rid=timer.rid) + + return result except Exception as e: + thumb_debug.log_trace("worker_error", rid=timer.rid, error=str(e)) log.debug("Failed to decode thumbnail for %s: %s", path, e) return None - def _decode_image(self, path: Path, target_size: int) -> Optional[np.ndarray]: + def _decode_image(self, path: Path, target_size: int, timer: "thumb_debug.ThumbTimer") -> Optional[np.ndarray]: """Decode image to numpy array at target size. Uses TurboJPEG if available for faster decoding. @@ -240,35 +306,38 @@ def _decode_image(self, path: Path, target_size: int) -> Optional[np.ndarray]: # Try TurboJPEG for JPEG files if HAS_TURBOJPEG and suffix in (".jpg", ".jpeg"): try: - with open(path, "rb") as f: - jpeg_data = f.read() - - # Get dimensions first - width, height, _, _ = _tj.decode_header(jpeg_data) - - # Calculate scale factor for turbojpeg (powers of 2: 1, 2, 4, 8) - scale_factor = 1 - while ( - width // (scale_factor * 2) >= target_size - and height // (scale_factor * 2) >= target_size - and scale_factor < 8 - ): - scale_factor *= 2 - - # Decode with scaling - scaling_factor = (1, scale_factor) - rgb = _tj.decode( - jpeg_data, pixel_format=TJPF_RGB, scaling_factor=scaling_factor - ) + with timer.stage("io"): + with open(path, "rb") as f: + jpeg_data = f.read() + + with timer.stage("decode"): + # Get dimensions first + width, height, _, _ = _tj.decode_header(jpeg_data) + + # Calculate scale factor for turbojpeg (powers of 2: 1, 2, 4, 8) + scale_factor = 1 + while ( + width // (scale_factor * 2) >= target_size + and height // (scale_factor * 2) >= target_size + and scale_factor < 8 + ): + scale_factor *= 2 + + # Decode with scaling + scaling_factor = (1, scale_factor) + rgb = _tj.decode( + jpeg_data, pixel_format=TJPF_RGB, scaling_factor=scaling_factor + ) # Further resize with PIL if needed h, w = rgb.shape[:2] if w > target_size or h > target_size: - pil_img = Image.fromarray(rgb) - pil_img.thumbnail( - (target_size, target_size), Image.Resampling.LANCZOS - ) - rgb = np.array(pil_img) + with timer.stage("decode"): + pil_img = Image.fromarray(rgb) + pil_img.thumbnail( + (target_size, target_size), Image.Resampling.LANCZOS + ) + rgb = np.array(pil_img) return rgb @@ -279,42 +348,60 @@ def _decode_image(self, path: Path, target_size: int) -> Optional[np.ndarray]: # Fallback to PIL try: - with Image.open(path) as img: + with timer.stage("decode"): + pil_img = Image.open(path) # Convert to RGB if needed - if img.mode != "RGB": - img = img.convert("RGB") + if pil_img.mode != "RGB": + pil_img = pil_img.convert("RGB") # Resize - img.thumbnail((target_size, target_size), Image.Resampling.LANCZOS) - return np.array(img) + pil_img.thumbnail((target_size, target_size), Image.Resampling.LANCZOS) + return np.array(pil_img) except Exception as e: log.debug("PIL decode failed for %s: %s", path, e) return None def _on_decode_done( - self, future: Future, job_key: Tuple[int, str, int], cache_key: str + self, future: Future, job_key: Tuple[int, str, int], cache_key: str, timer: "thumb_debug.ThumbTimer" ): """Callback when decode completes.""" # Always remove bookkeeping first to avoid stranding entries with self._inflight_lock: - self._inflight.discard(job_key) + self._inflight.pop(job_key, None) + thumb_debug.gauge("inflight", len(self._inflight)) + thumb_debug.gauge("qdepth", len(self._inflight)) if self._futures.get(job_key) is future: del self._futures[job_key] + if timer: + timer.t_done = time.perf_counter() + # Then bail if shutting down if self._stop_event.is_set(): + if timer: + thumb_debug.inc("decode_cancelled") + thumb_debug.log_trace("cancelled", rid=timer.rid, factor="shutdown") return try: # If cancelled, don't call result() - if future.cancelled(): + if future.cancelled() or (timer and timer.cancelled): + if timer: + thumb_debug.inc("decode_cancelled") + event = "cancelled_midflight" if timer.started else "cancelled_before_start" + thumb_debug.log_trace(event, rid=timer.rid) return jpeg_bytes = future.result() if jpeg_bytes: # Store in cache self._cache.put(cache_key, jpeg_bytes) + + if timer: + thumb_debug.inc("decode_done_ok") + thumb_debug.log_trace("completed", rid=timer.rid) + timer.log_timing(cache="miss") # Notify ready if self._on_ready: @@ -333,8 +420,15 @@ def cancel_all(self): # Future.cancel() can synchronously run callbacks. with self._inflight_lock: futures = list(self._futures.values()) + inflight_timers = [t for _, t in self._inflight.values()] self._futures.clear() self._inflight.clear() + thumb_debug.gauge("qdepth", 0) + thumb_debug.gauge("inflight", 0) + + for timer in inflight_timers: + timer.cancelled = True + thumb_debug.log_trace("cancel_requested", rid=timer.rid) for f in futures: try: diff --git a/faststack/thumbnail_view/provider.py b/faststack/thumbnail_view/provider.py index 57b6749..8e9b148 100644 --- a/faststack/thumbnail_view/provider.py +++ b/faststack/thumbnail_view/provider.py @@ -5,6 +5,8 @@ from pathlib import Path from typing import TYPE_CHECKING, Optional +import faststack.util.thumb_debug as thumb_debug + from PySide6.QtCore import QSize from PySide6.QtGui import QImage, QPixmap, QColor from PySide6.QtQuick import QQuickImageProvider @@ -43,6 +45,8 @@ def __init__( prefetcher: "ThumbnailPrefetcher", path_resolver: callable = None, default_size: int = 200, + debug_timing: bool = False, + debug_trace: bool = False, ): """Initialize the provider. @@ -51,18 +55,28 @@ def __init__( prefetcher: Prefetcher to schedule decodes path_resolver: Function to resolve path_hash to actual Path default_size: Default thumbnail size + debug_timing: Enable [THUMB-TIMING] log lines + debug_trace: Enable verbose trace logs """ super().__init__(QQuickImageProvider.ImageType.Pixmap) self._cache = cache self._prefetcher = prefetcher self._path_resolver = path_resolver self._default_size = default_size + self._debug_timing = debug_timing + self._debug_trace = debug_trace # Pre-create placeholder pixmaps self._placeholder = self._create_placeholder(default_size, PLACEHOLDER_COLOR) self._folder_placeholder = self._create_folder_placeholder(default_size) self._error_placeholder = self._create_placeholder(default_size, ERROR_COLOR) + # Timing stats for requestPixmap + self._first_request_time: Optional[float] = None + self._request_count = 0 + self._first_second_logged = False + self._first_hit_logged = False + log.debug("ThumbnailProvider initialized with default size %d", default_size) def _create_placeholder(self, size: int, color: QColor) -> QPixmap: @@ -127,51 +141,114 @@ def requestPixmap(self, id_str: str, size: QSize, requestedSize: QSize) -> QPixm # Strip query params id_clean = id_str.split("?")[0] - parts = id_clean.split("/") + + # Track total requests if logging enabled + if thumb_debug.timing_enabled or thumb_debug.trace_enabled: + thumb_debug.inc_request_count() + + # Deferred logging setup + timer = None + if thumb_debug.timing_enabled or thumb_debug.trace_enabled: + # Parse reason + reason = "unknown" + parts_query = id_str.split("?") + if len(parts_query) > 1: + query = parts_query[1] + for param in query.split("&"): + if param.startswith("reason="): + reason = param.split("=")[1] + + # Key/ID parts + # Key format: {size}/{path_hash}/{mtime_ns} + parts = id_clean.split("/") + if len(parts) < 3: + log.debug("Invalid thumbnail ID format: %s", id_str) + return self._error_placeholder - if len(parts) < 3: - log.debug("Invalid thumbnail ID format: %s", id_str) - return self._error_placeholder + # Determine if folder (early exit for folders) + if parts[0] == "folder": + return self._folder_placeholder - # Determine if folder - if parts[0] == "folder": - # Folder thumbnail - path_hash = parts[1] try: + thumb_size = int(parts[0]) + path_hash = parts[1] mtime_ns = int(parts[2]) - except ValueError: + except (ValueError, IndexError): + log.debug("Invalid thumbnail ID: %s", id_str) return self._error_placeholder - # Folders just get folder icon - return self._folder_placeholder - - # File thumbnail - try: - thumb_size = int(parts[0]) - path_hash = parts[1] - mtime_ns = int(parts[2]) - except (ValueError, IndexError): - log.debug("Invalid thumbnail ID: %s", id_str) - return self._error_placeholder - - # Build cache key (same as id_clean) - cache_key = f"{thumb_size}/{path_hash}/{mtime_ns}" + cache_key = f"{thumb_size}/{path_hash}/{mtime_ns}" + # Resolve path only if needed for trace + path = self._path_resolver(path_hash) if self._path_resolver else None + timer = thumb_debug.ThumbTimer(key=cache_key, path=path, reason=reason) + thumb_debug.log_trace("requested", rid=timer.rid, key=timer.key, src=timer.src, reason=reason) + else: + # Normal fast path — already have id_clean + cache_key = id_clean # Check cache (O(1) lookup) + t_cache_get_start = time.perf_counter() cached_bytes = self._cache.get(cache_key) + dt_cache_get = (time.perf_counter() - t_cache_get_start) * 1000 + if cached_bytes: + 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 pixmap + t_pixmap_start = time.perf_counter() pixmap = self._bytes_to_pixmap(cached_bytes) + dt_pixmap = (time.perf_counter() - t_pixmap_start) * 1000 + if pixmap and not pixmap.isNull(): + if timer: + thumb_debug.log_trace("delivered", rid=timer.rid, pixmap_ms=f"{dt_pixmap:.3f}") + timer.log_timing( + cache="hit", + cache_get_ms=f"{dt_cache_get:.3f}", + pixmap_ms=f"{dt_pixmap:.3f}" + ) return pixmap + + if timer: + thumb_debug.inc("req_cache_miss") + thumb_debug.log_trace("cache_miss", rid=timer.rid, ms=f"{dt_cache_get:.3f}") + + # Not in cache - parse parts if we haven't already + # If timer was created, parts, thumb_size, path_hash, mtime_ns are already set. + # If not, we need to parse them now. + if not timer: + parts = id_clean.split("/") + if len(parts) < 3: + log.debug("Invalid thumbnail ID format: %s", id_str) + return self._error_placeholder + + # Determine if folder (early exit for folders) + if parts[0] == "folder": + path_hash = parts[1] + try: + mtime_ns = int(parts[2]) + except ValueError: + return self._error_placeholder + return self._folder_placeholder + + try: + thumb_size = int(parts[0]) + path_hash = parts[1] + mtime_ns = int(parts[2]) + except (ValueError, IndexError): + log.debug("Invalid thumbnail ID: %s", id_str) + return self._error_placeholder - # Not in cache - schedule decode if we can resolve the path - if self._path_resolver: - path = self._path_resolver(path_hash) - if path: - self._prefetcher.submit( - path, mtime_ns, thumb_size, priority=self._prefetcher.PRIO_HIGH - ) + # Resolve path + path = self._path_resolver(path_hash) if self._path_resolver else None + if path: + self._prefetcher.submit( + path, mtime_ns, thumb_size, + priority=self._prefetcher.PRIO_HIGH, + timer=timer + ) # Return placeholder immediately (non-blocking) return self._placeholder diff --git a/faststack/ui/provider.py b/faststack/ui/provider.py index ba55583..00c048b 100644 --- a/faststack/ui/provider.py +++ b/faststack/ui/provider.py @@ -225,6 +225,7 @@ class UIState(QObject): cacheStatsChanged = Signal(str) isDecodingChanged = Signal(bool) debugModeChanged = Signal(bool) # General debug mode signal + debugThumbTimingChanged = Signal(bool) # Thumbnail pipeline timing isDialogOpenChanged = Signal(bool) # New signal for dialog state editSourceModeChanged = Signal(str) # Notify when JPEG/RAW mode changes saveBehaviorMessageChanged = Signal() # Signal for save behavior message updates @@ -232,10 +233,17 @@ class UIState(QObject): batchAutoLevelsProgressChanged = Signal() batchAutoLevelsActiveChanged = Signal() - def __init__(self, app_controller): + # Variant badges + variantBadgesChanged = Signal() + variantSaveHintChanged = Signal() + + def __init__(self, app_controller, clock_func=None): super().__init__() self.app_controller = app_controller self._app_controller = app_controller # Backward compatibility alias + self._clock = clock_func or (lambda: __import__("time").monotonic()) + self._last_prefetch_data = None # (startIndex, endIndex, maxCount) + self._last_prefetch_time = 0 self._is_preloading = False self._preload_progress = 0 # 1 = light, 0 = dark (controller will overwrite this on startup) @@ -282,6 +290,7 @@ def __init__(self, app_controller): self._is_decoding = False self._is_dialog_open = False self._is_saving = False # Save operation in progress + self._debug_thumb_timing = False self._batch_al_current = 0 self._batch_al_total = 0 self._batch_al_active = False @@ -311,6 +320,9 @@ def __init__(self, app_controller): self.app_controller.batchAutoLevelsFinished.connect( self._on_batch_al_finished ) + + # Ensure image source updates when switching grid/loupe + self.isGridViewActiveChanged.connect(lambda _: self.currentImageSourceChanged.emit()) def _on_batch_al_progress(self, current: int, total: int): self._batch_al_current = current @@ -389,6 +401,9 @@ def imageCount(self): @Property(str, notify=currentImageSourceChanged) def currentImageSource(self): + # Prevent QML from requesting full-res images when in grid view + if self.isGridViewActive: + return "" return f"image://provider/{self.app_controller.current_index}/{self.app_controller.ui_refresh_generation}" @Property(str, notify=metadataChanged) @@ -541,6 +556,11 @@ def statusMessage(self, value: str): self._status_message = value self.statusMessageChanged.emit() + @Property(str, notify=variantSaveHintChanged) + def variantSaveHint(self): + """Returns a hint message when saving from a variant.""" + return self.app_controller.get_variant_save_hint() + @Property(str, notify=filterStringChanged) def filterString(self): """Returns the current filter string (empty if no filter active).""" @@ -1291,8 +1311,39 @@ def debugMode(self, value: bool): self._debug_mode = value self.debugModeChanged.emit(value) + @Property(bool, notify=debugThumbTimingChanged) + def debugThumbTiming(self) -> bool: + return self._debug_thumb_timing + + @debugThumbTiming.setter + def debugThumbTiming(self, value: bool): + if self._debug_thumb_timing != value: + self._debug_thumb_timing = value + self.debugThumbTimingChanged.emit(value) + # --- RAW / Editor Source Logic --- + # --- Variant Badge Properties --- + + @Property(list, notify=variantBadgesChanged) + def variantBadges(self) -> list: + """Returns the badge list for the current image's variant group.""" + if hasattr(self.app_controller, "get_variant_badges"): + return self.app_controller.get_variant_badges() + return [] + + @Property(str, notify=variantBadgesChanged) + def activeVariantKind(self) -> str: + """Returns 'main', 'developed', 'backup', or '' for current view.""" + kind = getattr(self.app_controller, "view_override_kind", None) + return kind if kind else "main" + + @Slot(str) + def setVariantOverride(self, path_str: str): + """Switch loupe view to a different variant file.""" + if hasattr(self.app_controller, "set_variant_override"): + self.app_controller.set_variant_override(path_str) + # --- Grid View Properties --- # Signals for grid view @@ -1394,16 +1445,7 @@ def gridGetSelectedPaths(self) -> list: @Slot() def gridRefresh(self): """Refresh the grid view.""" - if ( - hasattr(self.app_controller, "_thumbnail_model") - and self.app_controller._thumbnail_model - ): - self.app_controller._thumbnail_model.refresh() - # Also update path resolver - if hasattr(self.app_controller, "_path_resolver"): - self.app_controller._path_resolver.update_from_model( - self.app_controller._thumbnail_model - ) + self.app_controller.refresh_grid() @Property(bool, notify=gridCanGoBackChanged) def gridCanGoBack(self) -> bool: @@ -1430,9 +1472,18 @@ def gridDeleteAtCursor(self, cursorIndex: int): if hasattr(self.app_controller, "grid_delete_at_cursor"): self.app_controller.grid_delete_at_cursor(cursorIndex) - @Slot(int, int) - def gridPrefetchRange(self, startIndex: int, endIndex: int): - """Prefetch thumbnails for the given index range.""" + @Slot() + def cancelThumbnailPrefetch(self): + """Cancels all pending thumbnail prefetch requests.""" + if ( + hasattr(self.app_controller, "_thumbnail_prefetcher") + and self.app_controller._thumbnail_prefetcher + ): + self.app_controller._thumbnail_prefetcher.cancel_all() + + @Slot(int, int, int) + def gridPrefetchRange(self, startIndex: int, endIndex: int, maxCount: int = 800): + """Prefetch thumbnails for the given index range with budget and duplicate suppression.""" if ( not hasattr(self.app_controller, "_thumbnail_model") or not self.app_controller._thumbnail_model @@ -1447,16 +1498,43 @@ def gridPrefetchRange(self, startIndex: int, endIndex: int): model = self.app_controller._thumbnail_model prefetcher = self.app_controller._thumbnail_prefetcher - # Clamp indices - startIndex = max(0, startIndex) - endIndex = min(model.rowCount() - 1, endIndex) + # 1. Index Validation + rowCount = model.rowCount() + if rowCount <= 0: + return + + # Clamp indices to valid boundaries + startIndex = max(0, min(startIndex, rowCount - 1)) + endIndex = max(0, min(endIndex, rowCount - 1)) + + if startIndex > endIndex: + return + + # 2. Duplicate Suppression + now = self._clock() + current_req = (startIndex, endIndex, maxCount) + if current_req == self._last_prefetch_data and (now - self._last_prefetch_time) < 0.030: + return + + self._last_prefetch_data = current_req + self._last_prefetch_time = now + + # 3. Budgeting / Hard Cap + HARD_LIMIT = 800 + budget = max(1, min(maxCount, HARD_LIMIT)) + + # Trim endIndex if the requested range exceeds the budget + if (endIndex - startIndex + 1) > budget: + endIndex = startIndex + budget - 1 # Submit prefetch jobs for visible range + # Defensive fallback if thumbnail_size is refactored away + size = getattr(model, "thumbnail_size", None) or getattr(prefetcher, "_target_size", None) for i in range(startIndex, endIndex + 1): entry = model.get_entry(i) if entry and not entry.is_folder: prefetcher.submit( - entry.path, entry.mtime_ns, priority=prefetcher.PRIO_MED + entry.path, entry.mtime_ns, size=size, priority=prefetcher.PRIO_MED ) @Property(str, notify=recycleBinStatsTextChanged) diff --git a/faststack/util/executors.py b/faststack/util/executors.py index e196c02..1cac583 100644 --- a/faststack/util/executors.py +++ b/faststack/util/executors.py @@ -11,51 +11,65 @@ log = logging.getLogger(__name__) -def create_daemon_threadpool_executor( - max_workers: int, thread_name_prefix: str = "" -) -> ThreadPoolExecutor: - """ - Create a standard ThreadPoolExecutor whose worker threads are daemon. - - ThreadPoolExecutor worker threads are created with daemon=None, so they inherit - the daemon status of the thread that creates them. This helper creates the executor - from a short-lived daemon thread and forces worker creation so the workers become - daemon threads (suitable for expendable background work like prefetching/previews). +import weakref +from concurrent.futures.thread import _worker, _threads_queues - NOTE: For critical tasks (saving/deleting), prefer a normal non-daemon executor. - """ - if max_workers < 1: - raise ValueError("max_workers must be >= 1") +class DaemonThreadPoolExecutor(ThreadPoolExecutor): + """ThreadPoolExecutor whose worker threads are daemon threads. - executor_container: dict[str, ThreadPoolExecutor] = {} - error_container: list[BaseException] = [] + Near-literal copy of CPython 3.12.2 ``_adjust_thread_count`` + (Lib/concurrent/futures/thread.py) with the sole change: + ``t.daemon = True`` before ``t.start()``. - def creator() -> None: - try: - executor = ThreadPoolExecutor( - max_workers=max_workers, thread_name_prefix=thread_name_prefix - ) - executor_container["executor"] = executor + No hasattr guard — if CPython internals change, this will raise + AttributeError immediately rather than silently falling back to + non-daemon threads. - # Pre-spawn workers so they inherit daemon status from this daemon creator thread. - futs = [executor.submit(lambda: None) for _ in range(max_workers)] - for f in futs: - f.result() - except BaseException as e: - error_container.append(e) + Thread-safety note: ``_adjust_thread_count`` is only called from + ``submit()``, which already holds ``_global_shutdown_lock``, so the + mutation of ``_threads_queues`` is safe without acquiring it again. + """ - t = threading.Thread(target=creator, name=f"{thread_name_prefix}_Creator", daemon=True) - t.start() - t.join() + def _adjust_thread_count(self) -> None: + # if idle threads are available, don't spin new threads + if self._idle_semaphore.acquire(timeout=0): + return + + # When the executor gets lost, the weakref callback will wake up + # the worker threads. + def weakref_cb(_, q=self._work_queue): + q.put(None) + + num_threads = len(self._threads) + if num_threads < self._max_workers: + thread_name = '%s_%d' % (self._thread_name_prefix or self, + num_threads) + t = threading.Thread(name=thread_name, target=_worker, + args=(weakref.ref(self, weakref_cb), + self._work_queue, + self._initializer, + self._initargs)) + t.daemon = True + t.start() + self._threads.add(t) + # Safe without explicit locking: submit() already holds + # _global_shutdown_lock when calling _adjust_thread_count(). + _threads_queues[t] = self._work_queue - if error_container: - raise error_container[0] - executor = executor_container.get("executor") - if executor is None: - raise RuntimeError("Failed to create daemon ThreadPoolExecutor") +def create_daemon_threadpool_executor( + max_workers: int, thread_name_prefix: str = "" +) -> ThreadPoolExecutor: + """ + Create a ThreadPoolExecutor whose worker threads are daemon threads. + Returns a DaemonThreadPoolExecutor instance which is a subclass of ThreadPoolExecutor. + """ + if max_workers < 1: + raise ValueError("max_workers must be >= 1") - return executor + return DaemonThreadPoolExecutor( + max_workers=max_workers, thread_name_prefix=thread_name_prefix + ) def create_priority_executor( diff --git a/faststack/util/thumb_debug.py b/faststack/util/thumb_debug.py new file mode 100644 index 0000000..a8628f3 --- /dev/null +++ b/faststack/util/thumb_debug.py @@ -0,0 +1,257 @@ +"""Thumbnail pipeline debug logging and performance tracing.""" + +import logging +import time +import threading +import itertools +from contextlib import contextmanager +from typing import Dict, Optional, Any +from pathlib import Path + +log = logging.getLogger(__name__) + +# Global configuration +timing_enabled = False +trace_enabled = False + +# Global RID counter +_rid_counter = itertools.count(1) + +# Interval statistics +_stats_lock = threading.Lock() +_interval_stats = { + # Provider stats + "req_total": 0, + "req_cache_hit": 0, + "req_cache_miss": 0, + + # Decode stats + "decode_submitted": 0, + "decode_coalesced": 0, + "decode_started": 0, + "decode_done_ok": 0, + "decode_done_fail": 0, + "decode_cancelled": 0, + + # Summary helpers + "total_ms": 0.0, + "max_ms": 0.0, + "inflight": 0, + "qdepth": 0, + "qdepth_max": 0, +} + +_last_summary_time = time.monotonic() +_summary_interval = 5.0 + + +def init(timing: bool = False, trace: bool = False): + """Initialize thumbnail debug logging globals.""" + global timing_enabled, trace_enabled + timing_enabled = timing + trace_enabled = trace + if timing_enabled: + log.info("Thumbnail timing logs ENABLED") + if trace_enabled: + log.info("Thumbnail trace logs ENABLED") + + +def log_trace(event: str, **kwargs): + """Log a verbose trace event if enabled.""" + if not trace_enabled: + return + + parts = [f"thumbtrace event={event}"] + for k, v in kwargs.items(): + parts.append(f"{k}={v}") + + log.info(" ".join(parts)) + + +def inc(name: str, delta: Any = 1): + """Increment a counter statistic.""" + if not (timing_enabled or trace_enabled): + return + with _stats_lock: + if name in _interval_stats: + _interval_stats[name] += delta + if name == "total_ms" and delta > _interval_stats["max_ms"]: + _interval_stats["max_ms"] = delta + + +def gauge(name: str, value: Any): + """Set a gauge statistic.""" + if not (timing_enabled or trace_enabled): + return + with _stats_lock: + if name in _interval_stats: + _interval_stats[name] = value + if name == "qdepth": + _interval_stats["qdepth_max"] = max(_interval_stats["qdepth_max"], value) + + +def inc_request_count(): + """Thread-safe increment for the global request counter.""" + if not (timing_enabled or trace_enabled): + return + with _stats_lock: + _interval_stats["req_total"] += 1 + return _interval_stats["req_total"] + + +def record_stat(name: str, value: Any): + """Deprecated: use inc() or gauge() instead.""" + if name == "done": + inc("decode_done_ok") + elif name == "hit": + inc("req_cache_hit") + elif name == "miss": + inc("req_cache_miss") + elif name == "cancel": + inc("decode_cancelled") + elif isinstance(value, (int, float)) and name in ("inflight", "qdepth"): + gauge(name, int(value)) + else: + inc(name, 1 if value is None else int(value)) + + +class ThumbTimer: + """Timer and tracer for a single thumbnail request.""" + + def __init__(self, key: str, path: Optional[Path] = None, reason: str = "unknown"): + self.rid = next(_rid_counter) + self.key = key + self.path = path + self.reason = reason + self.stages: Dict[str, float] = {} + self._current_stage: Optional[str] = None + self._stage_start: float = 0.0 + self.cancelled = False + self.started = False + + # Timestamps (perf_counter) + self.t_requested = time.perf_counter() + self.t_queued: Optional[float] = None + self.t_worker_start: Optional[float] = None + self.t_done: Optional[float] = None + + # Priority info + self.prio_submitted: Optional[int] = None + self.prio_effective: Optional[int] = None + self.coalesced_from: Optional[str] = None + + # Short path for logging + self.src = str(path.name) if path else "none" + + @contextmanager + def stage(self, name: str): + """Context manager to time a pipeline stage.""" + if not timing_enabled and not trace_enabled: + yield + return + + t0 = time.perf_counter() + log_trace("stage_start", rid=self.rid, stage=name) + try: + yield + finally: + dt = (time.perf_counter() - t0) * 1000 + self.stages[name] = self.stages.get(name, 0.0) + dt + log_trace("stage_end", rid=self.rid, stage=name, ms=f"{dt:.2f}") + + def log_timing(self, **kwargs): + """Log the final timing summary for this request.""" + if not timing_enabled: + return + + now = time.perf_counter() + total_ms = (now - self.t_requested) * 1000 + inc("total_ms", total_ms) + + # Base parts + parts = [ + "thumbtiming", + f"rid={self.rid}", + f"key={self.key}", + f"src={self.src}", + f"reason={self.reason}", + ] + + # Priority info + if self.prio_submitted is not None: + parts.append(f"prio={self.prio_submitted}") + if self.prio_effective is not None and self.prio_effective != self.prio_submitted: + parts.append(f"prio_eff={self.prio_effective}") + if self.coalesced_from: + parts.append(f"coalesced_from={self.coalesced_from}") + + # Overall timing + parts.append(f"total_ms={total_ms:.1f}") + + # Phase timings + if self.t_queued: + sched_ms = (self.t_queued - self.t_requested) * 1000 + parts.append(f"sched_ms={sched_ms:.1f}") + + if self.t_worker_start: + wait_ms = (self.t_worker_start - self.t_queued) * 1000 + parts.append(f"wait_ms={wait_ms:.1f}") + + if self.t_done: + worker_ms = (self.t_done - self.t_worker_start) * 1000 + parts.append(f"worker_ms={worker_ms:.1f}") + + # Stage breakdown + for stage, ms in self.stages.items(): + parts.append(f"{stage}_ms={ms:.1f}") + + # Extra tags + for k, v in kwargs.items(): + parts.append(f"{k}={v}") + + log.info(" ".join(parts)) + + +def check_periodic_summary(): + """Print the periodic summary if the interval has passed.""" + if not (timing_enabled or trace_enabled): + return + + global _last_summary_time + now = time.monotonic() + if now - _last_summary_time < _summary_interval: + return + + with _stats_lock: + stats = _interval_stats.copy() + # Reset interval stats + # NOTE: inflight and qdepth are persistent states, do NOT reset them to 0. + # qdepth_max is reset so we see the max for the NEW interval. + for k in [ + "req_total", "req_cache_hit", "req_cache_miss", + "decode_submitted", "decode_coalesced", "decode_started", + "decode_done_ok", "decode_done_fail", "decode_cancelled", + "total_ms", "max_ms", "qdepth_max" + ]: + if k in _interval_stats: + _interval_stats[k] = 0 + _last_summary_time = now + + # Summary is useful if ANYTHING happened or if there is work in flight + activity = ( + stats["req_total"] + stats["decode_submitted"] + + stats["decode_done_ok"] + stats["decode_cancelled"] + + stats["inflight"] + ) + if activity == 0: + return + + avg_ms = stats["total_ms"] / stats["decode_done_ok"] if stats["decode_done_ok"] > 0 else 0 + + log.info( + f"thumbtiming-summary " + f"REQ[tot={stats['req_total']} hit={stats['req_cache_hit']} miss={stats['req_cache_miss']}] " + f"DEC[sub={stats['decode_submitted']} coal={stats['decode_coalesced']} start={stats['decode_started']} ok={stats['decode_done_ok']} fail={stats['decode_done_fail']} can={stats['decode_cancelled']}] " + f"avg_ms={avg_ms:.1f} max_ms={stats['max_ms']:.1f} " + f"inflight={stats['inflight']} qdepth={stats['qdepth']} qdepth_max={stats['qdepth_max']}" + ) diff --git a/faststack/verify_cache_fix.py b/faststack/verify_cache_fix.py new file mode 100644 index 0000000..5135428 --- /dev/null +++ b/faststack/verify_cache_fix.py @@ -0,0 +1,44 @@ +import sys +import os +from pathlib import Path + +# Add current dir to path +sys.path.append(os.getcwd()) + +from imaging.cache import ByteLRUCache +from models import DecodedImage +import numpy as np + +def test_cache(): + evicted = [] + def on_evict(k, v): + evicted.append((k, v)) + print(f"Evicted: {k}") + + cache = ByteLRUCache(max_bytes=100, size_of=sys.getsizeof, on_evict=on_evict) + img1 = DecodedImage(buffer=memoryview(np.zeros(60, dtype=np.uint8)), width=60, height=1, bytes_per_line=60, format="dummy_format") + img2 = DecodedImage(buffer=memoryview(np.zeros(60, dtype=np.uint8)), width=60, height=1, bytes_per_line=60, format="dummy_format") + + cache["k1"] = img1 + print("Added k1") + cache["k2"] = img2 + print("Added k2") + + assert len(evicted) == 1 + assert evicted[0][0] == "k1" + print("Eviction verified!") + + cache.popitem() + assert len(evicted) == 2 + assert evicted[1][0] == "k2" + print("Popitem verification passed!") + +if __name__ == "__main__": + try: + test_cache() + print("STANDALONE TEST PASSED") + except Exception as e: + print(f"TEST FAILED: {e}") + import traceback + traceback.print_exc() + sys.exit(1) diff --git a/implementation_summary.md b/implementation_summary.md new file mode 100644 index 0000000..5ccff6c --- /dev/null +++ b/implementation_summary.md @@ -0,0 +1,20 @@ +The "no full-res decode in grid view" gating cleanup has been completed with the requested 5 fixes and additional safety improvements. + +**Summary of Changes:** + +1. **`faststack/app.py`**: + * **Fix 1**: Moved the `_loupe_decode_allowed()` check to the very top of `_do_prefetch`. This ensures no resize handling or other side effects occur when prefetch is blocked (e.g. in grid view). + * **Fix 2**: In `_set_grid_view_active`, explicitly cleared `self.pending_prefetch_index = None` when entering grid view to prevent stale deferred prefetches from firing upon return to loupe. + * **Fix 3**: Simplified `_loupe_decode_allowed` to access `self._folder_loaded` directly, removing `getattr`. + +2. **`faststack/ui/provider.py`**: + * **Fix 4**: Connected `isGridViewActiveChanged` to `currentImageSourceChanged.emit` in `UIState.__init__`. This fixes a bug where the signal was being re-connected repeatedly in `_on_dialog_state_changed`, causing potential leaks and performance issues. + +3. **`faststack/qml/ThumbnailGridView.qml`** & **`faststack/qml/Main.qml`**: + * **Fix 5**: Centralized prefetch gating ownership in `Main.qml` and improved robustness. + * **Main.qml**: Updated `onIsGridViewActiveChanged` and `gridViewLoader.onLoaded` to safely check for item existence and validity inside `Qt.callLater` callbacks, preventing crashes if the loader state changes quickly. + * **ThumbnailGridView.qml**: Removed the auto-enable logic from `Component.onCompleted`. Gated `onWidthChanged` and `onHeightChanged` timer restarts to only run when `prefetchEnabled` is true, avoiding unnecessary wakeups. + +**Verification:** +* Re-ran `pytest faststack/tests/test_startup_optimization.py` to ensure no regressions in existing tests. +* Confirmed logical consistency: grid view operations now strictly avoid full-res decode paths, and view switching handles state cleanup and QML property updates deterministically and safely. diff --git a/pyproject.toml b/pyproject.toml index 788599a..2162bce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,11 +1,10 @@ - [build-system] requires = ["setuptools>=61.0"] build-backend = "setuptools.build_meta" [project] name = "faststack" -version = "1.5.8" +version = "1.5.9" authors = [ { name="Alan Rockefeller"}, ] @@ -26,11 +25,11 @@ dependencies = [ "cachetools>=5.0,<6.0", "watchdog>=4.0,<5.0", "Pillow>=10.0,<11.0", - "opencv-python>=4.0,<5.0", + # CRITICAL FIX: OpenCV 4.10+ is required for NumPy 2.0 binary compatibility + "opencv-python>=4.10.0,<5.0", ] [project.optional-dependencies] - dev = [ "pytest>=8.0,<9.0", "build", @@ -50,11 +49,13 @@ include = ["faststack*"] [tool.setuptools.package-data] faststack = ["qml/**/*"] - [tool.pytest.ini_options] +minversion = "8.0" testpaths = ["faststack/tests"] python_files = ["test_*.py"] -addopts = "-p no:cacheprovider -p no:doctest --basetemp=./var/pytest-temp" + +# FIX: Add import-mode=importlib to fix resolution errors +addopts = "--import-mode=importlib -p no:cacheprovider -p no:doctest --basetemp=./var/pytest-temp" + norecursedirs = ["var", ".venv", "cache", "faststack.egg-info", "__pycache__"] pythonpath = ["."] - diff --git a/tests/debug_import.py b/tests/debug_import.py deleted file mode 100644 index c822e41..0000000 --- a/tests/debug_import.py +++ /dev/null @@ -1,27 +0,0 @@ -import os -import sys -import traceback - -# Add project root to path -# tests/ is at project_root/tests, so project_root is .. -PROJECT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) -sys.path.insert(0, PROJECT_ROOT) -print(f"Sys Path[0]: {sys.path[0]}") - -try: - print("Attempting import faststack.app...") - import faststack.app as app - - print(f"Import faststack.app success! ({app.__file__})") - - print("Attempting import AppController...") - from faststack.app import AppController - - print("Import AppController success!") - - print("Attributes:") - print(f"get_active_edit_path: {hasattr(AppController, 'get_active_edit_path')}") -except Exception: - print("Import FAILED:") - traceback.print_exc() - raise # optional: makes CI fail loud diff --git a/tests/repro_exif_fix.py b/tests/repro_exif_fix.py deleted file mode 100644 index a36f9a5..0000000 --- a/tests/repro_exif_fix.py +++ /dev/null @@ -1,60 +0,0 @@ -from PIL import Image, ExifTags -import io - - -def test_exif_sanitization(): - # 1. Create a dummy image with EXIF orientation = 6 (Rotated 90 CW) - # We can't easily "create" raw EXIF bytes without saving, - # so we'll save a temp, change it, then load it. - - img = Image.new("RGB", (100, 100), color="red") - exif = img.getexif() - exif[ExifTags.Base.Orientation] = 6 # Simulate rotated - - buf = io.BytesIO() - img.save(buf, format="JPEG", exif=exif.tobytes()) - buf.seek(0) - - # 2. Load it back (this simulates self.original_image) - original_image = Image.open(buf) - print( - f"Original Orientation: {original_image.getexif().get(ExifTags.Base.Orientation)}" - ) - - # 3. Simulate processing (we have a new image to save, but want metadata from original) - # In Editor code: existing logic takes original_image.info.get('exif') - # Proposed logic: take original_image.getexif(), mod it, tobytes() - - new_img = Image.new("RGB", (100, 100), color="blue") # The "edited" image - - # Proposed Fix Logic: - exif_obj = original_image.getexif() - if exif_obj: - exif_obj[ExifTags.Base.Orientation] = 1 - try: - exif_bytes = exif_obj.tobytes() - print("Successfully serialized modified EXIF.") - except Exception as e: - print(f"Failed to serialize: {e}") - exif_bytes = original_image.info.get("exif") # Fallback? - else: - exif_bytes = original_image.info.get("exif") - - # Save - out_buf = io.BytesIO() - new_img.save(out_buf, format="JPEG", exif=exif_bytes) - out_buf.seek(0) - - # 4. Verify result - result_img = Image.open(out_buf) - res_orientation = result_img.getexif().get(ExifTags.Base.Orientation) - print(f"Result Orientation: {res_orientation}") - - if res_orientation == 1: - print("PASS: Orientation sanitized.") - else: - print("FAIL: Orientation NOT sanitized.") - - -if __name__ == "__main__": - test_exif_sanitization() diff --git a/tests/test_highlights_v2.py b/tests/test_highlights_v2.py deleted file mode 100644 index f52091a..0000000 --- a/tests/test_highlights_v2.py +++ /dev/null @@ -1,71 +0,0 @@ -import unittest -import numpy as np -from faststack.imaging.editor import ImageEditor -from faststack.imaging.math_utils import _apply_headroom_shoulder -from PySide6.QtCore import QObject - - -class MockAppController(QObject): - def __init__(self): - super().__init__() - self.image_editor = ImageEditor() - self.ui_state = None # Circular ref handle manually - - -class TestHighlightsV2(unittest.TestCase): - def test_shoulder_asymptote(self): - """Verify the new shoulder asymptotes to 1.0 + max_overshoot.""" - x = np.array([1.0, 2.0, 10.0, 100.0], dtype=np.float32) - max_overshoot = 0.05 - out = _apply_headroom_shoulder(x, max_overshoot=max_overshoot) - - # At 1.0, should be 1.0 - self.assertAlmostEqual(out[0], 1.0, places=5) - - # Above 1.0, should be < 1.0 + max_overshoot - self.assertTrue(np.all(out[1:] < 1.0 + max_overshoot)) - - # Monotonicity - self.assertTrue(out[1] > out[0]) - self.assertTrue(out[2] > out[1]) - - # Asymptote check: at very large x, should be close to 1.05 - self.assertAlmostEqual(out[-1], 1.0 + max_overshoot, delta=0.001) - - def test_analysis_decoupling(self): - """Verify analysis runs before adjustments and is cached.""" - editor = ImageEditor() - # Create a linear image with some headroom - linear = np.ones((100, 100, 3), dtype=np.float32) * 1.2 - # sRGB mock indicating some clipping (e.g. 255) - srgb = np.ones((100, 100, 3), dtype=np.uint8) * 255 - - # Run with highlights=-0.5 - editor._apply_highlights_shadows( - linear, highlights=-0.5, shadows=0.0, srgb_u8=srgb - ) - - # Check cache - self.assertIsNotNone(editor._last_highlight_state) - self.assertGreater(editor._last_highlight_state["headroom_pct"], 0.9) - self.assertGreater(editor._last_highlight_state["clipped_pct"], 0.9) - - def test_robust_ceiling(self): - """Verify headroom ceiling handles hot pixels.""" - editor = ImageEditor() - linear = np.ones((100, 100, 3), dtype=np.float32) * 1.1 # Moderate headroom - # Add a single hot pixel - linear[50, 50, :] = 1000.0 - - # Use highlights recovery - # This triggers the robust percentile logic - out = editor._apply_highlights_shadows(linear, highlights=-1.0, shadows=0.0) - - # Check that we didn't explode or crash - self.assertTrue(np.isfinite(out).all()) - # The hot pixel should be compressed but not NaN - self.assertLess(out[50, 50, 0], 1000.0) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/verify_manual.py b/tests/verify_manual.py deleted file mode 100644 index 18a1f7a..0000000 --- a/tests/verify_manual.py +++ /dev/null @@ -1,127 +0,0 @@ -import sys -import os -from pathlib import Path -from unittest.mock import MagicMock, patch - -sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) - -try: - from faststack.app import AppController - - print("Imported AppController") -except Exception as e: - print(f"Failed to import AppController: {e}") - sys.exit(1) - - -class DummyController: - def __init__(self): - self.current_edit_source_mode = "jpeg" - self.image_files = [] - self.current_index = 0 - self.ui_state = MagicMock() - self.ui_state.isHistogramVisible = False - self.editSourceModeChanged = MagicMock() - - # Copy methods - get_active_edit_path = AppController.get_active_edit_path - is_valid_working_tif = AppController.is_valid_working_tif - _set_current_index = AppController._set_current_index - enable_raw_editing = AppController.enable_raw_editing - - def sync_ui_state(self): - pass - - def _reset_crop_settings(self): - pass - - def _do_prefetch(self, *args, **kwargs): - pass - - def update_histogram(self): - pass - - def load_image_for_editing(self): - pass - - def _develop_raw_backend(self): - pass - - -def log(msg): - with open("verify_result.txt", "a") as f: - f.write(msg + "\n") - - -def run_checks(): - # Clear log - with open("verify_result.txt", "w") as f: - f.write("Starting Verification\n") - - controller = DummyController() - - # Setup data - img_jpg = MagicMock() - img_jpg.path = Path("test.jpg") # suffix is derived from Path, not assigned - img_jpg.raw_pair = Path("test.CR2") - img_jpg.working_tif_path = Path("test.tif") - img_jpg.has_working_tif = False - - img_raw = MagicMock() - img_raw.path = Path("orphan.CR2") # suffix is derived from Path, not assigned - img_raw.raw_pair = None - - controller.image_files = [img_jpg, img_raw] - - log("--- Test 1: Default Mode ---") - controller.current_index = 0 - path = controller.get_active_edit_path(0) - if path == Path("test.jpg") and controller.current_edit_source_mode == "jpeg": - log("PASS") - else: - log(f"FAIL: path={path}, mode={controller.current_edit_source_mode}") - - log("--- Test 2: Enable RAW (trigger dev) ---") - controller._develop_raw_backend = MagicMock() - controller.enable_raw_editing() - if controller.current_edit_source_mode == "raw": - log("PASS: Mode switched") - else: - log("FAIL: Mode not switched") - controller._develop_raw_backend.assert_called_once() - log("PASS: Dev triggered") - - log("--- Test 3: Valid TIFF ---") - img_jpg.has_working_tif = True - with patch.object(controller, "is_valid_working_tif", return_value=True): - controller.load_image_for_editing = MagicMock() - controller._develop_raw_backend = MagicMock() - controller.current_edit_source_mode = "jpeg" # Reset - controller.enable_raw_editing() - - if ( - controller.current_edit_source_mode == "raw" - and controller.get_active_edit_path(0) == Path("test.tif") - ): - log("PASS: Mode raw, Returns TIFF") - else: - log(f"FAIL: returns {controller.get_active_edit_path(0)}") - - controller._develop_raw_backend.assert_not_called() - log("PASS: No dev triggered") - - log("--- Test 4: RAW Only ---") - # Mock RAW_EXTENSIONS import - # Note: Logic in app.py uses local import: from faststack.io.indexer import RAW_EXTENSIONS - # Patching faststack.io.indexer.RAW_EXTENSIONS works if module is already loaded or loads fresh. - # Since we imported AppController (which imports indexer), it is loaded. - with patch("faststack.io.indexer.RAW_EXTENSIONS", {".CR2"}): - # We also need to patch JPG_EXTENSIONS maybe? No, defaults are fine. - controller._set_current_index(1) - if controller.current_edit_source_mode == "raw": - log("PASS: Auto raw mode") - else: - log(f"FAIL: Mode is {controller.current_edit_source_mode}") - - -run_checks() diff --git a/tests/verify_raw_mode.py b/tests/verify_raw_mode.py deleted file mode 100644 index fa656f0..0000000 --- a/tests/verify_raw_mode.py +++ /dev/null @@ -1,175 +0,0 @@ -import unittest -from pathlib import Path -from unittest.mock import MagicMock, patch -import sys -import os - -# Add project root to path -sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) - -# Mock things we don't want to import fully or that need QObject -# We need to test AppController methods, but AppController inherits QObject. -# To test logic without full Qt app, we can either: -# 1. Use QTest (requires PySide6 installed and GUI context) -# 2. Extract logic or subclass AppController with mocked QObject? -# Let's try to minimal import. - -# Assuming we can instantiate AppController or minimal subclass -# But AppController creates threads and QObjects in __init__. -# Better to mock AppController's state and just test the methods if possible. -# But methods are bound to 'self'. -# We can create a dummy class that looks like AppController for these methods. - - -class DummyController: - def __init__(self): - self.current_edit_source_mode = "jpeg" - self.image_files = [] - self.current_index = 0 - self.ui_state = MagicMock() - self.ui_state.isHistogramVisible = False - self.editSourceModeChanged = MagicMock() # Signal stub - - # Copy methods to test - try: - from faststack.app import AppController - - get_active_edit_path = AppController.get_active_edit_path - is_valid_working_tif = AppController.is_valid_working_tif - _set_current_index = AppController._set_current_index - enable_raw_editing = AppController.enable_raw_editing - except Exception as e: - print(f"CRITICAL ERROR importing AppController: {e}") - import traceback - - traceback.print_exc() - # Define dummy placeholders to prevent AttributeError during test collection/execution - get_active_edit_path = lambda *args: None - is_valid_working_tif = lambda *args: False - _set_current_index = lambda *args: None - enable_raw_editing = lambda *args: None - - def sync_ui_state(self): - pass - - def _reset_crop_settings(self): - pass - - def _do_prefetch(self, *args, **kwargs): - pass - - def update_histogram(self): - pass - - def load_image_for_editing(self): - pass - - def _develop_raw_backend(self): - pass - - -class TestRawMode(unittest.TestCase): - def setUp(self): - self.controller = DummyController() - - # Create mock image files - self.img_jpg = MagicMock() - self.img_jpg.path = Path( - "test.jpg" - ) # suffix is derived from Path, not assigned - self.img_jpg.raw_pair = Path("test.CR2") - self.img_jpg.working_tif_path = Path("test.tif") - self.img_jpg.has_working_tif = False # Initially false - - self.img_raw_only = MagicMock() - self.img_raw_only.path = Path( - "orphan.CR2" - ) # suffix is derived from Path, not assigned - self.img_raw_only.raw_pair = None - - self.controller.image_files = [self.img_jpg, self.img_raw_only] - - def test_default_mode(self): - """Test 1: Default mode should be JPEG.""" - self.controller.current_index = 0 - path = self.controller.get_active_edit_path(0) - self.assertEqual(path, Path("test.jpg")) - self.assertEqual(self.controller.current_edit_source_mode, "jpeg") - - def test_switch_to_raw_with_development(self): - """Test 2: Enabling RAW should switch mode and trigger develop if no TIFF.""" - self.controller.current_index = 0 - - # Mock _develop_raw_backend - self.controller._develop_raw_backend = MagicMock() - - self.controller.enable_raw_editing() - - self.assertEqual(self.controller.current_edit_source_mode, "raw") - self.controller._develop_raw_backend.assert_called_once() - - # Path check: even if we switch mode, if TIFF is invalid, get_active_edit_path might return RAW path? - # Logic says: if mode=raw, check valid TIFF, else return raw_pair. - # So it should return the RAW file if TIFF not ready. - path = self.controller.get_active_edit_path(0) - self.assertEqual(path, Path("test.CR2")) - - def test_switch_to_raw_with_existing_tiff(self): - """Test 3: Enabling RAW should load TIFF if valid.""" - self.controller.current_index = 0 - self.img_jpg.has_working_tif = True - - # Mock is_valid_working_tif to return True - with patch.object(self.controller, "is_valid_working_tif", return_value=True): - # Create mocks BEFORE calling enable_raw_editing - self.controller.load_image_for_editing = MagicMock() - self.controller._develop_raw_backend = MagicMock() - - self.controller.enable_raw_editing() - - self.assertEqual(self.controller.current_edit_source_mode, "raw") - # Should NOT develop (mock was set up before the call) - self.controller._develop_raw_backend.assert_not_called() - # Should load immediately - self.controller.load_image_for_editing.assert_called_once() - - # Helper should return TIF - path = self.controller.get_active_edit_path(0) - self.assertEqual(path, Path("test.tif")) - - def test_raw_only_case(self): - """Test 4: Opening RAW-only files should force RAW mode.""" - # Navigate to index 1 (RAW only) - # Using _set_current_index logic - - # Need to mock the logic in _set_current_index... - # Wait, I copied _set_current_index to DummyController. - # But it requires `from faststack.io.indexer import RAW_EXTENSIONS`. - # I need to mock that import or ensure it works. - - with patch("faststack.io.indexer.RAW_EXTENSIONS", {".CR2", ".ARW"}): - self.controller._set_current_index(1) - - self.assertEqual(self.controller.current_index, 1) - self.assertEqual(self.controller.current_edit_source_mode, "raw") - - path = self.controller.get_active_edit_path(1) - self.assertEqual(path, Path("orphan.CR2")) - - def test_navigation_resets_mode(self): - """Test 5: Navigating to a normal pair should reset mode to JPEG.""" - # First set raw mode on index 0 - self.controller.current_index = 0 - self.controller.current_edit_source_mode = "raw" - - # Navigate to index 0 again via _set_current_index (like jumping or reloading) - # Or pretend we have another image. Let's make index 0 a normal pair. - - with patch("faststack.io.indexer.RAW_EXTENSIONS", {".CR2", ".ARW"}): - self.controller._set_current_index(0) - - self.assertEqual(self.controller.current_edit_source_mode, "jpeg") - - -if __name__ == "__main__": - unittest.main()