diff --git a/.gitignore b/.gitignore index edbff18..f5a9bb9 100644 --- a/.gitignore +++ b/.gitignore @@ -74,6 +74,8 @@ test_report*.log test_results.txt smoke_test_output.txt verify_result.txt +faststack/*fail*.txt +faststack/*final*.txt # Same junk when produced inside the package dir faststack/debug_*.txt diff --git a/ChangeLog.md b/ChangeLog.md index 5532fc1..07e8d4c 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,6 +1,59 @@ # ChangeLog -Todo: Make it work on Linux / Mac. Create Windows .exe. Write better documentation / help. Add splash screen / icon. +Todo: Make it work on Linux / Mac. Create Windows .exe. Write better documentation / help. Add splash screen / icon. Fix raw image support. + +# Changelog + +## 1.5.5 (2026-02-07) + +### Changed +- Image save behavior in the editor is now navigation-aware: + - Only clear editor state / close editor UI when the user is still on the same image. + - Only perform a full list refresh + re-select logic when the user is still on the same image. + - If the user navigated away, preserve their selection and only invalidate the saved image’s cache entry. + +- Recycle/delete of JPG+RAW pairs is now more atomic and robust: + - Check RAW existence **before** any moves to avoid post-move existence ambiguity. + - Move JPG first; only attempt RAW move if JPG succeeds and RAW existed. + - If RAW move fails after JPG succeeds, roll back the JPG move to keep pairs consistent. + - Track `raw_moved` based on whether RAW existed and whether it was moved successfully. + +- Cache invalidation after edits is now targeted instead of global: + - Replace multiple `image_cache.clear()` calls after save/export with `image_cache.pop_path(saved_path)` to invalidate only the edited file. + +- Keep internal path→index lookup consistent: + - Rebuild the path-to-index map after operations that mutate the image list, including after recycle/rollback flows. + +### Fixed +- Rotation/autocrop and straighten edge handling: + - Use `floor()` instead of `round()` in inscribed-rectangle and crop coordinate math to reduce off-by-one drift. + - Skip inset trimming for exact 90° rotations to preserve full dimensions and avoid unnecessary cropping. + +## 1.5.4 (2026-02-04) + +### Fixed +- Image rotation fixed - no more black wedges on the edges of the image. +- Prevented “undo delete” from resurrecting files when recycle/rollback fails: if a JPG can’t be restored after a partial delete, it’s marked as deleted (`jpg_moved=True`), a warning is shown, and a `recycled_jpg_path` breadcrumb is stored for potential cleanup. +- Improved crop behavior when straightening/rotating with `expand=True` by transforming crop coordinates from original image space into the expanded canvas. +- Prevented exporting with stale preview-resolution blur caches by validating cached array shapes against the current Y channel dimensions. +- Improved highlight recovery by switching to an adaptive rational compression shoulder (new `k` parameter) and added tests for identity-at-zero, pivot invariance, and increasing compression with higher amount. +- Fixed QML empty-state message timing by only showing “No images in this folder” after the folder has been scanned at least once. +- Improved Escape key reliability during crop/rotation by explicitly re-focusing the loupe view. + +### Changed +- Refactored deletion into a unified core deletion engine (`_delete_indices`) shared by loupe, grid cursor, grid selection, and batch deletion paths. +- Deletion now uses an optimistic UI update for instant feedback, with deferred/coalesced disk refresh to avoid flicker and “deleted items reappear” issues. +- Grid deletion now supports multi-selection and cursor deletion through a single entry point, rebuilding the path→index mapping for reliable lookup. +- Image saving is now offloaded to a background thread to keep the UI responsive: + - Added an `isSaving` state to disable Save actions and show “Saving…” feedback. + - Prevented “surprise close” by only auto-closing the editor if the user is still on the same image when the save completes. +- Improved recycle-bin cleanup on quit: + - Replaced the simple message dialog with a richer dialog showing per-bin counts (JPG/RAW/other) and an optional detailed file list. + +### UI +- Resized the Image Editor dialog to accommodate the saving state/controls. +- Enhanced recycle bin cleanup dialog layout and interaction (expandable detailed list, clearer button actions). + ## 1.5.3 (2026-01-27) diff --git a/README.md b/README.md index 8551109..99aa10b 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # FastStack -# Version 1.5 - January 1, 2026 +# Version 1.5.4 - February 4, 2026 # By Alan Rockefeller Ultra-fast, caching JPG viewer designed for culling and selecting RAW or JPG files for focus stacking and website upload. diff --git a/faststack/all_verification_results.txt b/faststack/all_verification_results.txt new file mode 100644 index 0000000..0948837 Binary files /dev/null and b/faststack/all_verification_results.txt differ diff --git a/faststack/all_verification_results_utf8.txt b/faststack/all_verification_results_utf8.txt new file mode 100644 index 0000000..2f22428 --- /dev/null +++ b/faststack/all_verification_results_utf8.txt @@ -0,0 +1,19 @@ +============================= 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 14 items + +tests\test_editor_rotation.py::test_rotated_rect_edge_cases PASSED [ 7%] +tests\test_editor_rotation.py::test_rotated_rect_calculation_branches[100-100-0] PASSED [ 14%] +tests\test_editor_rotation.py::test_rotated_rect_calculation_branches[200-100-45] PASSED [ 21%] +tests\test_editor_rotation.py::test_rotated_rect_calculation_branches[1000-500-15] PASSED [ 28%] +tests\test_editor_rotation.py::test_rotated_rect_calculation_branches[500-1000-15] PASSED [ 35%] +tests\test_editor_rotation.py::test_rotate_autocrop_rgb_behavior PASSED [ 42%] +tests\test_editor_rotation.py::test_boundary_clamping PASSED [ 50%] +tests\test_editor_rotation.py::test_integration_straighten_modes FAILED [ 57%] +tests\test_editor_rotation.py::test_rotate_cw PASSED [ 64%] +tests\test_editor_rotation.py::test_rotate_ccw PASSED [ 71%] +tests\test_rotation_unittest.py::TestEditorRotation::test_rotate_cw PASSED [ 78%] +tests\test_rotation_unittest.py::TestEditorRotation::test_straighten_angle PASSED [ 85%] +tests\test_editor_integration.py::TestEditorIntegration::test_missing_methods diff --git a/faststack/app.py b/faststack/app.py index 62381f1..13e9e8d 100644 --- a/faststack/app.py +++ b/faststack/app.py @@ -77,6 +77,11 @@ ) import numpy as np from faststack.io.indexer import RAW_EXTENSIONS +from faststack.io.deletion import ( + confirm_permanent_delete, + confirm_batch_permanent_delete, + permanently_delete_image_files, +) def make_hdrop(paths): @@ -121,6 +126,9 @@ class ProgressReporter(QObject): finished = Signal() editSourceModeChanged = Signal(str) # Notify when JPEG/RAW mode changes + _saveFinished = Signal( + object + ) # Signal for save completion (result or error from background) def __init__( self, image_dir: Path, engine: QQmlApplicationEngine, debug_cache: bool = False @@ -135,6 +143,10 @@ def __init__( self.histogramReady.connect(self._apply_histogram_result) self.previewReady.connect(self._apply_preview_result) + # Save Offloading Setup (runs save_image in background thread) + self._save_executor = concurrent.futures.ThreadPoolExecutor(max_workers=1) + self._saveFinished.connect(self._on_save_finished) + # Preview Offloading Setup self._preview_executor = concurrent.futures.ThreadPoolExecutor(max_workers=1) self._preview_inflight = False @@ -143,6 +155,7 @@ def __init__( self._preview_lock = threading.Lock() self._last_rendered_preview = None # Store latest valid render self._shutting_down = False # Flag to gate async callbacks during shutdown + self._refresh_scheduled = False # Coalesce guard for deferred disk refresh self._opencv_warning_shown = False # Only show OpenCV warning once per session self.image_dir = image_dir @@ -158,7 +171,9 @@ def __init__( self.debug_cache = debug_cache # New debug_cache flag # Ensure clean shutdown of background threads - QCoreApplication.instance().aboutToQuit.connect(self._shutdown_executors) + inst = QCoreApplication.instance() + if inst: + inst.aboutToQuit.connect(self._shutdown_executors) self.display_width = 0 self.display_height = 0 @@ -319,21 +334,7 @@ def __init__( # Connect editor open/close signal for memory cleanup self.ui_state.is_editor_open_changed.connect(self._on_editor_open_changed) - def _move_to_recycle(self, src_path: Path) -> Path: - """Moves a file to the recycle bin in the same directory. - - Raises: - OSError/PermissionError if move fails. - """ - recycle_bin = src_path.parent / "image recycle bin" - recycle_bin.mkdir(parents=True, exist_ok=True) - - dst_path = recycle_bin / src_path.name - shutil.move(str(src_path), str(dst_path)) - - self.active_recycle_bins.add(recycle_bin) - return dst_path - + # _move_to_recycle robust version is defined below in the deletion section @Slot(bool) def _on_editor_open_changed(self, is_open: bool): """Handle necessary setup/cleanup when editor opens or closes.""" @@ -567,13 +568,25 @@ def eventFilter(self, watched, event) -> bool: self.ui_state.isHistogramVisible = False return True # Consume event, histogram closed - # When cropping (or editing), let QML handle Enter/Esc and related keys. - # Otherwise keybinder can swallow them before QML sees them. - if getattr(self.ui_state, "isCropping", False) or getattr( - self.ui_state, "isEditorOpen", False + # Esc cancels crop mode if active (priority: before grid handling) + # Handled here because QML focus issues can prevent Keys.onEscapePressed from firing + if event.key() == Qt.Key_Escape and getattr( + self.ui_state, "isCropping", False ): + self.cancel_crop_mode() + return True # Consume event, crop mode cancelled + + # When editing, let QML handle Enter/Esc and related keys. + # Otherwise keybinder can swallow them before QML sees them. + if getattr(self.ui_state, "isEditorOpen", False): return False + # When cropping, let QML handle Enter/Return for crop execution + if getattr(self.ui_state, "isCropping", False): + key = event.key() + if key in (Qt.Key_Enter, Qt.Key_Return): + return False # Let QML handle crop execution + # When in grid view, let QML handle navigation and action keys if self._is_grid_view_active: key = event.key() @@ -647,6 +660,9 @@ def load(self, skip_thumbnail_refresh: bool = False): 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() def refresh_image_list(self): """Rescans the directory for images from disk and updates cache. @@ -929,78 +945,138 @@ def sync_ui_state(self): @Slot() def save_edited_image(self): - """Saves functionality delegating to ImageEditor. - - Restores "Old" behavior: - - Save image - - Close Editor - - Clear Editor State - - Refresh List - - Re-select saved image + """Saves the edited image in a background thread to keep UI responsive. + + Sets isSaving=True, spawns background worker, returns immediately. + On completion, _on_save_finished is called via signal to perform cleanup. """ if not self.image_editor.original_image: return - # Only write developed sidecar when editing from RAW source + # Prevent double-saves + if self.ui_state.isSaving: + return + + # Capture state needed for save before we start write_sidecar = self.current_edit_source_mode == "raw" dev_path = None if write_sidecar and 0 <= self.current_index < len(self.image_files): dev_path = self.image_files[self.current_index].developed_jpg_path - try: - result = self.image_editor.save_image( - write_developed_jpg=write_sidecar, developed_path=dev_path - ) - except RuntimeError as e: - self.update_status_message(str(e)) + # Store save token to prevent "surprise close" if user navigates away during save + self._save_initiated_path = self.image_editor.current_filepath + + # Show saving indicator + self.ui_state.isSaving = True + self.update_status_message("Saving...") + + # Submit save work to background thread + 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 + ) + return {"success": True, "result": result} + except RuntimeError as e: + return {"success": False, "error": str(e)} + except Exception as e: + log.exception(f"Unexpected error during save: {e}") + return {"success": False, "error": "Failed to save image"} + + def on_done(future): + """Callback when background save completes - emits signal to hop to main thread.""" + # Guard emit during shutdown to prevent signal to deleted QObject + if self._shutting_down: + return + try: + result = future.result() + except Exception as e: + result = {"success": False, "error": str(e)} + # Emit signal to process result on main thread + self._saveFinished.emit(result) + + future = self._save_executor.submit(do_save) + future.add_done_callback(on_done) + + @Slot(object) + def _on_save_finished(self, save_result: dict): + """Handle save completion on main thread (called via signal from background).""" + # Guard against callbacks during/after shutdown + if self._shutting_down: return - except Exception as e: - log.exception(f"Unexpected error during save: {e}") - self.update_status_message("Failed to save image") + + # Always clear saving indicator + self.ui_state.isSaving = False + + if not save_result.get("success"): + self.update_status_message(save_result.get("error", "Save failed")) return + result = save_result.get("result") if result: saved_path, _ = result # backup_path unused - # --- Restore Old Behavior --- + # --- Post-Save Cleanup --- - # 1. Close Editor UI - self.ui_state.isEditorOpen = False + # Only auto-close editor if still on the same image that initiated the save + # Prevents "surprise close" if user navigated away during save + initiated_path = getattr(self, "_save_initiated_path", None) + editor_still_on_same_image = ( + self.ui_state.isEditorOpen + and self.image_editor.current_filepath + and initiated_path + and self.image_editor.current_filepath == initiated_path + ) - # 2. Clear Editor State (release memory) - self.image_editor.clear() + # 1. Close Editor UI (only if still on same image) + if editor_still_on_same_image: + self.ui_state.isEditorOpen = False - # 3. Refresh List (to see new file or updated timestamp) - self.refresh_image_list() + # 2. Clear Editor State (release memory) - only if still on same image + if editor_still_on_same_image: + self.image_editor.clear() - # 4. Find and Select the saved image - new_index = self.current_index # Default to keeping selection if not found + # 3. Refresh List and Handle Selection + if editor_still_on_same_image: + # Full refresh to see new file or updated timestamp + self.refresh_image_list() - # Try to find by exact path match - if saved_path: - try: - target_resolve = saved_path.resolve() - for i, img in enumerate(self.image_files): - try: - # Robust path comparison - if img.path.resolve() == target_resolve: - new_index = i - break - except (OSError, RuntimeError): - # Fallback to string compare - if str(img.path) == str(saved_path): - new_index = i - break - except (OSError, RuntimeError): - pass # Keep current selection if resolution fails - - self.current_index = new_index - - # 5. Force UI Sync / Prefetch - self.image_cache.clear() # Clear cache to ensure we reload valid image - self.prefetcher.cancel_all() - self.prefetcher.update_prefetch(self.current_index) - self.sync_ui_state() + # 4. Find and re-select the saved image + new_index = ( + self.current_index + ) # Default to keeping selection if not found + + # Try to find by exact path match + if saved_path: + try: + target_resolve = saved_path.resolve() + for i, img in enumerate(self.image_files): + try: + # Robust path comparison + if img.path.resolve() == target_resolve: + new_index = i + break + except (OSError, RuntimeError): + # Fallback to string compare + if str(img.path) == str(saved_path): + new_index = i + break + except (OSError, RuntimeError): + pass # Keep current selection if resolution fails + + self.current_index = new_index + + # 5. Force UI Sync / Prefetch + self.image_cache.clear() # Clear cache to ensure we reload valid image + self.prefetcher.cancel_all() + self.prefetcher.update_prefetch(self.current_index) + self.sync_ui_state() + else: + # User navigated away - skip full refresh to preserve their selection + # Just clear stale cache entry for the saved image + if saved_path: + self.image_cache.pop_path(saved_path) self.update_status_message("Image saved") else: @@ -1253,34 +1329,65 @@ def grid_open_index(self, index: int): log.info("Opened image from grid: %s", entry.path) - def grid_delete_at_cursor(self, cursor_index: int): - """Delete images from grid view - either selection or cursor image. + @Slot() + def delete_current_image(self): + """Standard entry point for Loupe deletion. + Triggers batch dialog if current image is part of a multi-image batch. + """ + # 1. Check if we're in a multi-image batch + batch_count = self.get_batch_count_for_current_image() + if batch_count > 1 and self.main_window: + # Trigger QML batch deletion dialog (user confirms there) + self.main_window.show_delete_batch_dialog(batch_count) + return + + # 2. Otherwise default to single image deletion + self._delete_indices([self.current_index], "loupe") - If there are selected images, deletes all selected images. - If no selection, deletes the image at the cursor position. + @Slot(int) + def grid_delete_at_cursor(self, cursor_index: int): + """Unified grid deletion entry point. + Handles both multi-selection and single-cursor deletion. """ if not self._thumbnail_model: return - # Check if there are selections + # 1. Rebuild index mapping once for reliable lookup + self._rebuild_path_to_index() + + # 2. Prefer selection if it exists selected_paths = self._thumbnail_model.get_selected_paths() if selected_paths: - # Delete all selected images - self._delete_grid_selected_images(selected_paths) - return + indices = [] + for path in selected_paths: + idx = self._path_to_index.get(path.resolve()) + if idx is not None: + indices.append(idx) + + if not indices: + self.update_status_message("Selected images not found in current list.") + return - # No selection - delete the cursor image - entry = self._thumbnail_model.get_entry(cursor_index) - if not entry: - log.warning("grid_delete_at_cursor: no entry at index %d", cursor_index) + summary = self._delete_indices(indices, "grid_selection") + if summary["all_deleted"]: + self._thumbnail_model.clear_selection() return - if entry.is_folder: - self.update_status_message("Cannot delete folders") - return + # 3. Fallback to cursor index if no selection + if cursor_index >= 0: + entry = self._thumbnail_model.get_entry(cursor_index) + if not entry: + return + if entry.is_folder: + self.update_status_message("Cannot delete folders in grid view.") + return + + idx = self._path_to_index.get(entry.path.resolve()) + if idx is None: + self.update_status_message("Image not found in current list.") + return - # Delete this single image using its path - self._delete_grid_selected_images([entry.path]) + self._delete_indices([idx], "grid_cursor") def _on_thumbnail_ready(self, thumbnail_id: str): """Callback when a thumbnail finishes decoding (called from worker thread). @@ -2632,33 +2739,6 @@ def get_batch_count_for_current_image(self) -> int: return 0 - @Slot() - def delete_current_image(self): - """Moves current JPG and RAW to recycle bin. Shows dialog if multiple images in batch.""" - # Check if in grid view with selections - delete grid selection instead - if self._is_grid_view_active and self._thumbnail_model: - selected_paths = self._thumbnail_model.get_selected_paths() - if selected_paths: - self._delete_grid_selected_images(selected_paths) - return - - if not self.image_files: - self.update_status_message("No image to delete.") - return - - # Check if current image is in a batch with multiple images - batch_count = self.get_batch_count_for_current_image() - - if batch_count > 1: - # Show dialog asking what to delete - if hasattr(self, "main_window") and self.main_window: - # Set batch count in dialog and open it - self.main_window.show_delete_batch_dialog(batch_count) - return - - # Single image deletion - proceed normally - self._delete_single_image(self.current_index) - def _move_to_recycle(self, src: Path) -> Optional[Path]: """Moves a file to the recycle bin safely, handling collisions and cross-device moves.""" if not src.exists() or not src.is_file(): @@ -2695,135 +2775,303 @@ def _move_to_recycle(self, src: Path) -> Optional[Path]: log.error("Failed to recycle %s: %s", src.name, e) return None - def _ensure_recycle_bin_dir(self) -> bool: - """Try to create the recycle bin directory. + def _delete_indices(self, indices: List[int], action_type: str) -> dict: + """Unified core deletion engine for FastStack. - Returns: - True if recycle bin exists or was created successfully. - False if creation failed (e.g., permission denied). - """ - from faststack.io.deletion import ensure_recycle_bin_dir - - return ensure_recycle_bin_dir(self.recycle_bin_dir) - - def _confirm_permanent_delete(self, image_file, reason: str = "") -> bool: - """Show a confirmation dialog for permanent deletion of a single image. + Uses optimistic UI pattern: updates in-memory list and UI immediately + for instant visual feedback, then performs file I/O synchronously. + If deletion fails or is cancelled, state is rolled back. + Heavy disk-scan refresh is deferred to after UI paint. Args: - image_file: The ImageFile to delete permanently. - reason: Reason for permanent deletion (e.g., "Recycle bin unavailable"). + indices: List of indices into self.image_files to delete. + action_type: String for logging (e.g. 'loupe', 'grid_selection', 'grid_cursor', 'batch'). Returns: - True if user confirms deletion, False if cancelled. + dict: { + "total_deleted": int, + "recycled": int, + "permanent": int, + "failed_recycles": list[ImageFile], + "cancelled": bool + } """ - from faststack.io.deletion import confirm_permanent_delete + from PySide6.QtCore import QTimer + + summary = { + "total_deleted": 0, + "recycled": 0, + "permanent": 0, + "failed_recycles": [], + "cancelled": False, + "requested_count": 0, # Updated after validation + "all_deleted": False, + } - return confirm_permanent_delete(image_file, reason) + if not self.image_files or not indices: + log.debug(f"[_delete_indices] Nothing to delete: action={action_type}") + return summary + + # 1. Collect ImageFile objects and sort indices in reverse to prevent shifting + sorted_indices = sorted(list(set(indices)), reverse=True) + images_to_delete = [] + for idx in sorted_indices: + if 0 <= idx < len(self.image_files): + images_to_delete.append(self.image_files[idx]) + + if not images_to_delete: + log.warning(f"[_delete_indices] No valid indices found in {indices}") + return summary + + # Update requested_count from validated list (not raw indices) + summary["requested_count"] = len(images_to_delete) + + # --- PHASE 1: OPTIMISTIC UI UPDATE (instant, no I/O) --- + # Snapshot for potential rollback (store in ascending order for proper restoration) + removed_items = [ + (idx, self.image_files[idx]) + for idx in sorted(sorted_indices) + if 0 <= idx < len(self.image_files) + ] + previous_index = self.current_index - def _confirm_batch_permanent_delete(self, images: list, reason: str = "") -> bool: - """Show a confirmation dialog for permanent deletion of multiple images. + # Remove from in-memory list immediately for instant visual feedback + for idx in sorted_indices: + if 0 <= idx < len(self.image_files): + del self.image_files[idx] - Args: - images: List of ImageFile objects to delete permanently. - reason: Reason for permanent deletion. - - Returns: - True if user confirms deletion, False if cancelled. - """ - from faststack.io.deletion import confirm_batch_permanent_delete + # Reposition current_index immediately (fast, in-memory only) + if not self.image_files: + self.current_index = 0 + else: + self.current_index = min(previous_index, len(self.image_files) - 1) - return confirm_batch_permanent_delete(images, reason) + # Update UI immediately - this is fast since it just reads from memory + self.display_generation += 1 + self.image_cache.clear() + self.prefetcher.cancel_all() + if self.image_files: + self.prefetcher.update_prefetch(self.current_index) + self._rebuild_path_to_index() # Keep path->index map in sync + self.sync_ui_state() - def _permanently_delete_image_files(self, image_file) -> bool: - """Permanently delete an image and its RAW pair from disk. + # NOTE: Thumbnail model refresh is deferred to Phase 4 to avoid disk rescan + # while files are still in transit (prevents "deleted items reappear" flicker) + + # --- PHASE 2: SYNCHRONOUS FILE I/O (for correct undo/summary) --- + recycled_count = 0 + permanent_count = 0 + partial_fail_count = 0 + failed_recycles = [] + # Track per-image deletion status (resolved path -> {jpg_moved, raw_moved}) + # Use resolved paths for robustness against symbolic links or path variations + successfully_deleted = {} # resolved_path -> deletion status dict + timestamp = time.time() - This does NOT add to undo history since deletion is permanent. + for img in images_to_delete: + jpg_path = img.path + raw_path = img.raw_pair - Args: - image_file: The ImageFile to delete. + try: + # Check RAW existence BEFORE any moves (existence changes after move) + raw_exists = raw_path and raw_path.exists() - Returns: - True if at least one file was deleted, False otherwise. - """ - from faststack.io.deletion import permanently_delete_image_files + # Step 1: Move JPG first + recycled_jpg = self._move_to_recycle(jpg_path) - return permanently_delete_image_files(image_file) + if not recycled_jpg: + # JPG failed to move - don't attempt RAW, add to failed list + log.error(f"Failed to recycle JPG: {jpg_path.name}") + failed_recycles.append(img) + continue - def _delete_single_image(self, index: int): - """Internal method to delete a single image by index.""" - if not self.image_files or index < 0 or index >= len(self.image_files): - self.update_status_message("No image to delete.") - return + # Step 2: Only move RAW if JPG succeeded and RAW exists + recycled_raw = None + if raw_exists: + recycled_raw = self._move_to_recycle(raw_path) - previous_index = self.current_index - image_file = self.image_files[index] - jpg_path = image_file.path - raw_path = image_file.raw_pair - - # Try to ensure recycle bin is available - recycle_bin_available = self._ensure_recycle_bin_dir() - - if recycle_bin_available: - # Normal path: move to recycle bin - recycled_jpg = self._move_to_recycle(jpg_path) - recycled_raw = ( - self._move_to_recycle(raw_path) - if (raw_path and raw_path.exists()) - else None - ) + if not recycled_raw: + # RAW failed but JPG succeeded - atomic rollback + log.warning( + f"Partial recycle for {img.path.name}: JPG ok, RAW failed. " + "Undoing JPG move to keep pair consistent." + ) + undo_succeeded = False + try: + # Move JPG back from recycle bin + import shutil + + shutil.move(str(recycled_jpg), str(jpg_path)) + log.info(f"Restored {jpg_path.name} from recycle bin") + undo_succeeded = True + except (OSError, shutil.Error) as undo_err: + log.exception( + f"Failed to undo JPG move for {jpg_path.name}: {undo_err}" + ) + # Mark as deleted to prevent rollback from resurrecting missing image + resolved_key = img.path.resolve() + successfully_deleted[resolved_key] = { + "jpg_moved": True, # JPG is not in folder anymore + "raw_moved": False, # RAW still present + "undo_failed": True, + "recycled_jpg_path": recycled_jpg, # Breadcrumb for cleanup + } + self.update_status_message( + f"Warning: couldn't restore {jpg_path.name}; " + "file may be locked. RAW not deleted." + ) - # Add to delete history if anything was moved - if recycled_jpg or recycled_raw: - import time + partial_fail_count += 1 + # Only add to failed_recycles if undo succeeded (JPG is back in folder) + # If undo failed, permanent delete can't act on it properly + if undo_succeeded: + failed_recycles.append(img) + continue - timestamp = time.time() - # Store tuple of (src, bin_path) for each file - # Format: ( (jpg_src, jpg_bin), (raw_src, raw_bin) ) + # Full success (JPG moved, and RAW either moved or didn't exist) record = ((jpg_path, recycled_jpg), (raw_path, recycled_raw)) - self.delete_history.append(record) self.undo_history.append(("delete", record, timestamp)) - status_msg = f"Deleted {jpg_path.name}" + recycled_count += 1 + # Use resolved path as key for robustness + resolved_key = img.path.resolve() + successfully_deleted[resolved_key] = { + "jpg_moved": True, + "raw_moved": recycled_raw is not None or not raw_exists, + } + except (OSError, PermissionError) as e: + log.warning(f"Recycle exception for {jpg_path.name}: {e}") + failed_recycles.append(img) + + # Handle failed recycles with permanent delete fallback + if failed_recycles: + reason = "Recycle bin failure or insufficient permissions." + confirmed = False + if len(failed_recycles) == 1: + confirmed = confirm_permanent_delete(failed_recycles[0], reason=reason) else: - self.update_status_message("Delete failed") - return - else: - # Fallback: permanent delete with confirmation - if not self._confirm_permanent_delete( - image_file, - reason="Recycle bin could not be created due to permissions.", - ): - self.update_status_message("Deletion cancelled") - return - - if self._permanently_delete_image_files(image_file): - # Do NOT add to undo_history - permanent deletion is not undoable - status_msg = ( - f"Permanently deleted {jpg_path.name} (recycle bin unavailable)" + confirmed = confirm_batch_permanent_delete( + failed_recycles, reason=reason ) + + if confirmed: + for img in failed_recycles: + if permanently_delete_image_files(img): + permanent_count += 1 + successfully_deleted[img.path.resolve()] = { + "jpg_moved": True, + "raw_moved": True, # Permanent delete removes both + } else: - self.update_status_message("Delete failed") - return + summary["cancelled"] = True + log.info( + f"Permanent deletion of {len(failed_recycles)} files cancelled by user." + ) - # Clear folder stats cache so recycle bin count updates - clear_raw_count_cache() + # Build summary + deleted_count = recycled_count + permanent_count + summary["total_deleted"] = deleted_count + summary["recycled"] = recycled_count + summary["permanent"] = permanent_count + summary["failed_recycles"] = failed_recycles + summary["all_deleted"] = deleted_count == summary["requested_count"] + + # --- ROLLBACK if deletion incomplete --- + # If cancelled or some files failed to delete, restore those items to the list + if summary["cancelled"] or deleted_count < summary["requested_count"]: + # Identify items to restore: only if JPG wasn't successfully deleted + # (prevents restoring ImageFile whose RAW is orphaned in recycle) + items_to_restore = [ + (idx, img) + for idx, img in removed_items + if img.path.resolve() not in successfully_deleted + or not successfully_deleted[img.path.resolve()].get("jpg_moved", False) + ] - # Refresh image list and move to next image - self.refresh_image_list() + if items_to_restore: + log.info( + f"Rolling back {len(items_to_restore)} items after incomplete deletion" + ) + # Restore items in descending index order + # Restore in descending order to preserve index validity + items_to_restore.sort(key=lambda x: x[0], reverse=True) + for idx, img in items_to_restore: + # Clamp insertion index to valid range + insert_idx = min(idx, len(self.image_files)) + self.image_files.insert(insert_idx, img) + + # Restore previous index position + self.current_index = min(previous_index, len(self.image_files) - 1) + + # Refresh UI to reflect rollback + self.display_generation += 1 + self.image_cache.clear() + self.prefetcher.cancel_all() + if self.image_files: + self.prefetcher.update_prefetch(self.current_index) + self._rebuild_path_to_index() # Keep path->index map in sync after rollback + self.sync_ui_state() - # Refresh thumbnail model so folder stats (e.g., recycle bin count) update - if self._thumbnail_model: - self._thumbnail_model.refresh() - if self.image_files: - self._reposition_after_delete(None, previous_index) - # Clear cache and invalidate display generation to force image reload - self.display_generation += 1 - self.image_cache.clear() - self.prefetcher.cancel_all() # Cancel stale tasks since image list changed - self.prefetcher.update_prefetch(self.current_index) - self.sync_ui_state() + # --- PHASE 3: Status messages (immediate feedback) --- + if deleted_count > 0: + if permanent_count > 0: + msg = f"Permanently deleted {permanent_count} image(s)" + if recycled_count > 0: + msg += f" ({recycled_count} moved to recycle bin)" + self.update_status_message(msg) + elif recycled_count > 0: + if summary["cancelled"] and failed_recycles: + msg = f"Deleted {recycled_count} image(s); {len(failed_recycles)} could not be deleted (cancelled)" + elif partial_fail_count > 0: + msg = f"Deleted {recycled_count} images (some RAW pairs failed to recycle)" + else: + msg = ( + "Image moved to recycle bin" + if recycled_count == 1 + else f"Deleted {recycled_count} images" + ) + self.update_status_message(msg) - self.update_status_message(status_msg) + # Log completion + log.info( + f"Deletion complete: type='{action_type}', total_deleted={deleted_count}, " + f"recycled={recycled_count}, permanent={permanent_count}, " + f"partial_fails={partial_fail_count}, " + f"final_index={self.current_index}, list_len={len(self.image_files)}" + ) + + # --- PHASE 4: DEFERRED DISK REFRESH (after UI paint) --- + # Schedule heavy disk operations for next event loop iteration + # Use coalescing guard to prevent multiple refreshes on rapid deletes + if not self._refresh_scheduled: + self._refresh_scheduled = True + + def do_deferred_refresh(): + self._refresh_scheduled = False + clear_raw_count_cache() + self.refresh_image_list() + self._rebuild_path_to_index() + # Now safe to refresh thumbnail model after disk state is consistent + if self._thumbnail_model: + self._thumbnail_model.refresh() + if hasattr(self, "_path_resolver"): + self._path_resolver.update_from_model(self._thumbnail_model) + + QTimer.singleShot(0, do_deferred_refresh) + + else: + if failed_recycles: + if summary["cancelled"]: + self.update_status_message("Deletion cancelled") + else: + self.update_status_message("Delete failed") + log.info( + f"Deletion Action '{action_type}' resulted in no changes (cancelled/failed)." + ) + else: + log.debug(f"Deletion Action '{action_type}' - nothing processed.") + + return summary def _reposition_after_delete( self, preserved_path: Optional[Path], previous_index: int @@ -2844,248 +3092,38 @@ def _reposition_after_delete( @Slot() def delete_current_image_only(self): """Delete only the current image, ignoring batch selection.""" - if not self.image_files: - self.update_status_message("No image to delete.") - return - self._delete_single_image(self.current_index) + self._delete_indices([self.current_index], "loupe_single_only") @Slot() def delete_batch_images(self): - """Delete all images in the current batch.""" - if not self.image_files: - self.update_status_message("No images to delete.") + """Standard entry point for batch deletion. + Deletes all images currently in batches. + """ + if not self.batches: + self.update_status_message("No images in batch to delete.") return - # Collect all indices in batches + # 1. Collect all indices from batches (filter to valid range) + max_index = len(self.image_files) - 1 indices_to_delete = set() for start, end in self.batches: for i in range(start, end + 1): - if 0 <= i < len(self.image_files): + if 0 <= i <= max_index: indices_to_delete.add(i) - if not indices_to_delete: - self.update_status_message("No images in batch to delete.") - return - - # Sort indices in reverse order so we delete from end to start - # This way indices don't shift as we delete - sorted_indices = sorted(indices_to_delete, reverse=True) - - # Determine where to land after deletion - min_deleted_index = min(sorted_indices) - - # Try to ensure recycle bin is available - recycle_bin_available = self._ensure_recycle_bin_dir() - - deleted_count = 0 - permanent_delete_mode = not recycle_bin_available - import time - - timestamp = time.time() - - # Collect images to delete first - images_to_delete = [ - self.image_files[i] for i in sorted_indices if i < len(self.image_files) - ] - - if permanent_delete_mode and images_to_delete: - # Show single batch confirmation for all images - if not self._confirm_batch_permanent_delete( - images_to_delete, - reason="Recycle bin could not be created due to permissions.", - ): - self.update_status_message("Deletion cancelled.") - return - - # Delete all confirmed images - for image_file in images_to_delete: - if self._permanently_delete_image_files(image_file): - deleted_count += 1 - else: - # Normal path: move to recycle bin - for image_file in images_to_delete: - jpg_path = image_file.path - raw_path = image_file.raw_pair - try: - recycled_jpg = self._move_to_recycle(jpg_path) - recycled_raw = ( - self._move_to_recycle(raw_path) - if (raw_path and raw_path.exists()) - else None - ) - - if recycled_jpg or recycled_raw: - record = ((jpg_path, recycled_jpg), (raw_path, recycled_raw)) - self.delete_history.append(record) - self.undo_history.append(("delete", record, timestamp)) - deleted_count += 1 - - except OSError as e: - log.exception("Failed to delete image %s: %s", jpg_path.name, e) + # 2. Call unified engine + summary = self._delete_indices(list(indices_to_delete), "batch") - if deleted_count > 0: - # Clear all batches after deletion + # 3. Clear batches only if all intended images were deleted + if summary["all_deleted"]: self.batches = [] self.batch_start_index = None self._invalidate_batch_cache() - - # Clear folder stats cache so recycle bin count updates - clear_raw_count_cache() - - # Refresh image list - self.refresh_image_list() - - if self.image_files: - # Calculate new index - new_index = min_deleted_index - new_index = max(0, min(new_index, len(self.image_files) - 1)) - - self.current_index = new_index - - # Clear cache and invalidate display generation to force image reload - self.display_generation += 1 - self.image_cache.clear() - self.prefetcher.cancel_all() # Cancel stale tasks since image list changed - self.prefetcher.update_prefetch(self.current_index) - self.sync_ui_state() - - if permanent_delete_mode: - self.update_status_message( - f"Permanently deleted {deleted_count} image(s) (recycle bin unavailable)" - ) - else: - self.update_status_message(f"Deleted {deleted_count} image(s)") - log.info("Deleted %d image(s) from batch", deleted_count) + log.info("Batch state cleared after successful deletion.") + elif summary["cancelled"]: + log.info("Batches retained after user cancelled deletion.") else: - self.update_status_message("No images were deleted.") - - def _delete_grid_selected_images(self, selected_paths: list): - """Delete images selected in grid view.""" - if not selected_paths: - self.update_status_message("No images selected.") - return - - # Build a path -> index map for the main image list - path_to_index = {} - for i, img in enumerate(self.image_files): - path_to_index[img.path.resolve()] = i - - # Find indices for selected paths - indices_to_delete = set() - for path in selected_paths: - resolved = path.resolve() - if resolved in path_to_index: - indices_to_delete.add(path_to_index[resolved]) - - if not indices_to_delete: - self.update_status_message("Selected images not found in current list.") - return - - # Sort indices in reverse order so we delete from end to start - sorted_indices = sorted(indices_to_delete, reverse=True) - min_deleted_index = min(sorted_indices) - - # Try to ensure recycle bin is available - NO LONGER NEEDED, we try per-file - # recycle_bin_available = self._ensure_recycle_bin_dir() - - deleted_count = 0 - permanent_delete_mode = ( - False # Default to recycle bin, fallback if needed per file - ) - import time - - timestamp = time.time() - - # Collect images to delete first - images_to_delete = [ - self.image_files[i] for i in sorted_indices if i < len(self.image_files) - ] - - # Normal path: move to recycle bin - failed_deletes = [] # List of (ImageFile, exception) - - for image_file in images_to_delete: - jpg_path = image_file.path - raw_path = image_file.raw_pair - - try: - # Use new per-folder move - recycled_jpg = self._move_to_recycle(jpg_path) - # Only try to recycle RAW if it exists - recycled_raw = ( - self._move_to_recycle(raw_path) - if (raw_path and raw_path.exists()) - else None - ) - - if recycled_jpg or recycled_raw: - record = ((jpg_path, recycled_jpg), (raw_path, recycled_raw)) - self.delete_history.append(record) - self.undo_history.append(("delete", record, timestamp)) - deleted_count += 1 - - except OSError as e: - # Move failed - likely permission or lock issue - failed_deletes.append((image_file, e)) - - # Handle failures reactively - if failed_deletes: - log.warning( - "%d files failed to recycle, prompting for permanent delete", - len(failed_deletes), - ) - # Prompt to permanent delete the ones that failed - failed_images = [err[0] for err in failed_deletes] - - if self._confirm_batch_permanent_delete( - failed_images, - reason="Recycle bin partial failure (files failed to move).", - ): - for img in failed_images: - if self._permanently_delete_image_files(img): - deleted_count += 1 - else: - self.update_status_message( - f"Deleted {deleted_count} files. {len(failed_deletes)} failed." - ) - - if deleted_count > 0: - # Clear grid selection - self._thumbnail_model.clear_selection() - - # Clear folder stats cache so recycle bin count updates - clear_raw_count_cache() - - # Refresh image list - self.refresh_image_list() - - # Refresh the grid model to remove deleted images - self._thumbnail_model.refresh() - self._path_resolver.update_from_model(self._thumbnail_model) - - if self.image_files: - # Calculate new index for loupe view - new_index = min_deleted_index - new_index = max(0, min(new_index, len(self.image_files) - 1)) - self.current_index = new_index - - # Clear cache and invalidate display generation - self.display_generation += 1 - self.image_cache.clear() - self.prefetcher.cancel_all() - # Restart prefetch for the new current image - self.prefetcher.update_prefetch(self.current_index) - - self.sync_ui_state() - if permanent_delete_mode: - self.update_status_message( - f"Permanently deleted {deleted_count} image(s) (recycle bin unavailable)" - ) - else: - self.update_status_message(f"Deleted {deleted_count} image(s)") - log.info("Deleted %d image(s) from grid selection", deleted_count) - else: - self.update_status_message("No images were deleted.") + log.info("Batches retained after failed/empty deletion.") def _restore_backup_safe(self, saved_path_str: str, backup_path_str: str) -> bool: """ @@ -3427,6 +3465,7 @@ def _shutdown_executors(self): log.info("Shutting down background executors...") self._hist_executor.shutdown(wait=False, cancel_futures=True) self._preview_executor.shutdown(wait=False, cancel_futures=True) + self._save_executor.shutdown(wait=False, cancel_futures=True) def empty_recycle_bin(self): """Permanently deletes all files in all tracked recycle bins.""" @@ -4748,7 +4787,7 @@ def execute_crop(self): # Invalidate cache and refresh display self.display_generation += 1 - self.image_cache.clear() + self.image_cache.pop_path(saved_path) self.prefetcher.cancel_all() self.prefetcher.update_prefetch(self.current_index) self.sync_ui_state() @@ -4956,7 +4995,7 @@ def quick_auto_levels(self): ) self.display_generation += 1 - self.image_cache.clear() + self.image_cache.pop_path(saved_path) self.prefetcher.cancel_all() self.prefetcher.update_prefetch(self.current_index) self.sync_ui_state() @@ -5032,7 +5071,7 @@ def quick_auto_white_balance(self): # Invalidate cache for the edited image so it's reloaded from disk # This ensures the Image Editor will see the updated version self.display_generation += 1 - self.image_cache.clear() + self.image_cache.pop_path(saved_path) self.prefetcher.cancel_all() self.prefetcher.update_prefetch(self.current_index) self.sync_ui_state() @@ -5277,7 +5316,14 @@ def get_recycle_bin_stats(self) -> List[Dict[str, Any]]: """Get stats for all tracked recycle bins. Returns: - List of dicts: [{"path": absolute_path, "count": num_files}, ...] + List of dicts: [{ + "path": absolute_path, + "count": total_count, + "jpg_count": num_jpg, + "raw_count": num_raw, + "other_count": num_other, + "file_paths": [list of file names] + }, ...] """ stats = [] # Filter out bins that don't exist anymore @@ -5286,10 +5332,33 @@ def get_recycle_bin_stats(self) -> List[Dict[str, Any]]: for bin_path in self.active_recycle_bins: try: - # Count files - count = sum(1 for p in bin_path.iterdir() if p.is_file()) - if count > 0: - stats.append({"path": str(bin_path), "count": count}) + jpg_count = 0 + raw_count = 0 + other_count = 0 + file_names = [] + + for p in bin_path.iterdir(): + if p.is_file(): + file_names.append(p.name) + ext = p.suffix.lower() + if ext in [".jpg", ".jpeg"]: + jpg_count += 1 + elif ext in RAW_EXTENSIONS: + raw_count += 1 + else: + other_count += 1 + + if file_names: + stats.append( + { + "path": str(bin_path), + "count": len(file_names), + "jpg_count": jpg_count, + "raw_count": raw_count, + "other_count": other_count, + "file_paths": sorted(file_names), + } + ) except OSError: continue diff --git a/faststack/imaging/cache.py b/faststack/imaging/cache.py index 40b6128..dde13ee 100644 --- a/faststack/imaging/cache.py +++ b/faststack/imaging/cache.py @@ -66,6 +66,40 @@ def clear(self): finally: self.on_evict = callback + def pop_path(self, path: Union[Path, str]): + """Targeted invalidation of all generations for a given path. + + Hardened to handle both Path objects and string keys, and resolved paths. + Expected type: Union[Path, str]. + """ + targets = {path, str(path), str(path).replace("\\", "/")} + try: + # Handle Path objects and ensure we check the resolved variant + p = Path(path) + resolved = p.resolve() + targets.update({resolved, str(resolved), resolved.as_posix()}) + except (OSError, ValueError, TypeError): + pass + + keys_to_remove = [] + # Use list(self.keys()) to avoid mutation during iteration + for key in list(self.keys()): + key_str = str(key) + # Match exact path or path::generation pattern + for t in targets: + t_str = str(t) + if key_str == t_str or key_str.startswith(f"{t_str}::"): + keys_to_remove.append(key) + break + + for k in keys_to_remove: + self.pop(k, None) + + if keys_to_remove: + log.debug( + f"Invalidated {len(keys_to_remove)} cache entries for path: {path}" + ) + def get_decoded_image_size(item) -> int: """Calculates the size of a decoded image tuple (buffer, qimage).""" diff --git a/faststack/imaging/editor.py b/faststack/imaging/editor.py index 3b0e83c..f10cf6d 100644 --- a/faststack/imaging/editor.py +++ b/faststack/imaging/editor.py @@ -228,8 +228,8 @@ def _rotated_rect_with_max_area(w: int, h: int, angle_rad: float) -> tuple[int, wr = (w * cos_a - h * sin_a) / cos_2a hr = (h * cos_a - w * sin_a) / cos_2a - cw = round(abs(wr)) - ch = round(abs(hr)) + cw = math.floor(abs(wr)) + ch = math.floor(abs(hr)) cw = max(1, min(w, cw)) ch = max(1, min(h, ch)) return cw, ch @@ -270,17 +270,25 @@ def rotate_autocrop_rgb( # Center-crop to the inscribed rectangle cx = rot.width / 2.0 cy = rot.height / 2.0 - left = round(cx - crop_w / 2.0) - top = round(cy - crop_h / 2.0) + left = math.floor(cx - crop_w / 2.0) + top = math.floor(cy - crop_h / 2.0) right = left + crop_w bottom = top + crop_h # Small inset to remove any bicubic edge contamination - if inset > 0 and (right - left) > 2 * inset and (bottom - top) > 2 * inset: - left += inset - top += inset - right -= inset - bottom -= inset + # We skip this for exact 90-degree increments as there is no edge contamination. + is_exact_90 = abs(angle_deg % 90.0) < 0.01 + actual_inset = 0 if is_exact_90 else inset + + if ( + actual_inset > 0 + and (right - left) > 2 * actual_inset + and (bottom - top) > 2 * actual_inset + ): + left += actual_inset + top += actual_inset + right -= actual_inset + bottom -= actual_inset # Clamp defensively left = max(0, min(rot.width - 1, left)) @@ -615,6 +623,29 @@ def _apply_edits( # Alias arr = img_arr + # ENSURE we are working with a float32 numpy array + if isinstance(arr, Image.Image): + arr = np.array(arr.convert("RGB")).astype(np.float32) / 255.0 + elif not isinstance(arr, np.ndarray): + arr = np.array(arr) + if arr.dtype == np.uint8: + arr = arr.astype(np.float32) / 255.0 + elif arr.dtype == np.uint16: + arr = arr.astype(np.float32) / 65535.0 + else: + arr = arr.astype(np.float32) + # Heuristic: only scan for max if necessary, or use a sample for speed + # If the first few thousand pixels are > 1.0, it's likely 8-bit data. + if arr.size > 0: + sample = arr.reshape(-1)[:2000] + s_max = sample.max() + if s_max > 1.0 and s_max <= 255.0: + arr /= 255.0 + elif s_max <= 1.0: + # Double check full array only if sample was small or ambiguous + # but typically 0.0-1.0 images stay 0.0-1.0. + pass + # NOTE: For UI analysis, we want to capture the state AFTER White Balance and Exposure # but BEFORE Highlights/Shadows/ToneMapping, so the indicators reflect the # "available headroom" and "current clipping" accurately for the recovery tools. @@ -644,6 +675,9 @@ def _apply_edits( apply_rotation = abs(straighten_angle) > 0.001 and (for_export or has_crop_box) + # Capture original dimensions BEFORE rotation for crop coordinate transformation + orig_h, orig_w = arr.shape[:2] + if apply_rotation: # Use the float rotation helper # Note: rotate_autocrop_rgb logic was complex. @@ -676,8 +710,11 @@ def _apply_edits( right = left + cw bottom = top + ch - # Apply inset (2px) to match legacy behavior and avoid edge artifacts - inset = 2 + # Apply inset (2px) to match legacy behavior and avoid edge artifacts. + # Skip for exact 90-degree increments to preserve full dimensions. + is_exact_90 = abs(straighten_angle % 90.0) < 0.01 + inset = 0 if is_exact_90 else 2 + if (right - left) > 2 * inset and (bottom - top) > 2 * inset: left += inset top += inset @@ -696,17 +733,70 @@ def _apply_edits( if has_crop_box: crop_box = edits.get("crop_box", 0.0) if len(crop_box) == 4: - # 0-1000 relative to current size - h, w = arr.shape[:2] - left = int(crop_box[0] * w / 1000) - t = int(crop_box[1] * h / 1000) - r = int(crop_box[2] * w / 1000) - b = int(crop_box[3] * h / 1000) - - left = max(0, left) - t = max(0, t) - r = min(w, r) - b = min(h, b) + # The crop_box is in 0-1000 normalized coordinates relative to the + # ORIGINAL (un-rotated) image. After rotation with expand=True, + # the original image is centered within a larger canvas. + # We need to transform the coordinates from original image space + # to the expanded canvas space. + + if apply_rotation and abs(straighten_angle) > 0.001: + # Transform crop box through rotation: + # 1. Convert 0-1000 to pixel coords in original image + # 2. Rotate corners around original center + # 3. Translate to expanded canvas + new_h, new_w = arr.shape[:2] + orig_cx, orig_cy = orig_w / 2.0, orig_h / 2.0 + canvas_cx, canvas_cy = new_w / 2.0, new_h / 2.0 + + # Get crop corners in original pixel space + c_left = crop_box[0] * orig_w / 1000 + c_top = crop_box[1] * orig_h / 1000 + c_right = crop_box[2] * orig_w / 1000 + c_bottom = crop_box[3] * orig_h / 1000 + + # Define the 4 corners, rotate each around original center + corners = [ + (c_left, c_top), + (c_right, c_top), + (c_right, c_bottom), + (c_left, c_bottom), + ] + angle_rad = math.radians(-straighten_angle) + cos_a, sin_a = math.cos(angle_rad), math.sin(angle_rad) + + rotated_corners = [] + for px, py in corners: + # Rotate around original center + dx, dy = px - orig_cx, py - orig_cy + rx = dx * cos_a - dy * sin_a + ry = dx * sin_a + dy * cos_a + # Translate to canvas center + rotated_corners.append((rx + canvas_cx, ry + canvas_cy)) + + # Get axis-aligned bounding box of rotated corners + xs = [c[0] for c in rotated_corners] + ys = [c[1] for c in rotated_corners] + left = int(min(xs)) + t = int(min(ys)) + r = int(max(xs)) + b = int(max(ys)) + + left = max(0, left) + t = max(0, t) + r = min(new_w, r) + b = min(new_h, b) + else: + # No rotation - use current dimensions directly + h, w = arr.shape[:2] + left = int(crop_box[0] * w / 1000) + t = int(crop_box[1] * h / 1000) + r = int(crop_box[2] * w / 1000) + b = int(crop_box[3] * h / 1000) + + left = max(0, left) + t = max(0, t) + r = min(w, r) + b = min(h, b) if r > left and b > t: arr = arr[t:b, left:r, :] @@ -870,6 +960,16 @@ def _apply_edits( cached_exp_gain = cached.get("exp_gain", 1.0) cache_hit = True + # Validate cached array shapes match current Y dimensions + # This prevents reusing preview-resolution blurs during export + y_shape = Y.shape + for cached_arr in (Y20_cached, Y3_cached, Y1_cached): + if cached_arr is not None and cached_arr.shape != y_shape: + # Shape mismatch - invalidate cache + Y20_cached = Y3_cached = Y1_cached = None + cache_hit = False + break + # Compute exposure scale factor for reusing cached blurs # blur(k*Y) = k*blur(Y) is exact only if Y scales linearly with exposure. # Since highlights/shadows recovery (step 7) is non-linear and sits between diff --git a/faststack/imaging/math_utils.py b/faststack/imaging/math_utils.py index 4b52a98..1cac080 100644 --- a/faststack/imaging/math_utils.py +++ b/faststack/imaging/math_utils.py @@ -166,6 +166,7 @@ def _highlight_recover_linear( amount: float, *, pivot: float = 0.7, + k: float = 8.0, chroma_rolloff: float = 0.15, headroom_ceiling: float = 1.0, ) -> np.ndarray: @@ -184,6 +185,7 @@ def _highlight_recover_linear( rgb_linear: Float32 RGB array (H, W, 3) in linear light, may have values > 1.0 amount: Recovery strength 0.0-1.0 (mapped from slider -100 to 0) pivot: Brightness threshold below which no recovery occurs + k: Compression factor (adaptive). Higher k = stronger shoulder. chroma_rolloff: Desaturation amount in extreme highlights (0-1) headroom_ceiling: Maximum output brightness (> 1.0 preserves headroom detail) @@ -202,11 +204,25 @@ def _highlight_recover_linear( # Use headroom_ceiling instead of 1.0 for the normalization range mask = _smoothstep01((brightness - pivot) / (headroom_ceiling - pivot + eps)) - # Highlights recovery should DIM bright areas to reveal detail/contrast. - # We use a gain-based approach that preserves the pivot and pull down highlights. - # strength of 0.3 means max 30% darkening at pure white. - recovery_strength = 0.3 - target_brightness = brightness * (1.0 - amount * recovery_strength * mask) + # Highlights recovery: we want to pull down highlights to reveal detail. + # Rational compression formula: y = x / (1 + kx). + # We apply this relative to the pivot. + # normalization: brightness is already linear. + x_norm = (brightness - pivot) / (headroom_ceiling - pivot + eps) + x_norm = np.clip(x_norm, 0.0, None) + + # Compressed value (normalized context) + # At amount=1, we use full rational compression. + # At amount=0, we use identity. + compressed_norm = x_norm / (1.0 + k * amount * x_norm) + + # Map back to brightness scale + target_brightness = pivot + compressed_norm * (headroom_ceiling - pivot) + # Clamp to headroom_ceiling to satisfy docstring contract (small amount can cause overshoot) + target_brightness = np.minimum(target_brightness, headroom_ceiling) + + # If brightness was below pivot, keep it as is + target_brightness = np.where(brightness > pivot, target_brightness, brightness) # Rescale RGB to preserve hue/chroma # Protect against div-by-zero or huge scale factors for near-black pixels diff --git a/faststack/imaging/metadata.py b/faststack/imaging/metadata.py index baf82ac..a1c06e8 100644 --- a/faststack/imaging/metadata.py +++ b/faststack/imaging/metadata.py @@ -58,9 +58,7 @@ def get_exif_data(path: Union[str, Path]) -> Dict[str, Any]: if not exif: return {"summary": {}, "full": {}} - except ( - Exception - ) as e: + except Exception as e: log.warning(f"Failed to extract EXIF from {path}: {e}") return {"summary": {}, "full": {}} diff --git a/faststack/integration_results.txt b/faststack/integration_results.txt new file mode 100644 index 0000000..53f7d04 Binary files /dev/null and b/faststack/integration_results.txt differ diff --git a/faststack/integration_traceback.txt b/faststack/integration_traceback.txt new file mode 100644 index 0000000..efb9cdb Binary files /dev/null and b/faststack/integration_traceback.txt differ diff --git a/faststack/io/deletion.py b/faststack/io/deletion.py index 9ccb599..64ac168 100644 --- a/faststack/io/deletion.py +++ b/faststack/io/deletion.py @@ -4,6 +4,7 @@ from pathlib import Path from PySide6.QtWidgets import QMessageBox + log = logging.getLogger(__name__) diff --git a/faststack/path_check.txt b/faststack/path_check.txt new file mode 100644 index 0000000..e253897 Binary files /dev/null and b/faststack/path_check.txt differ diff --git a/faststack/qml/Components.qml b/faststack/qml/Components.qml index 9031654..28160ee 100644 --- a/faststack/qml/Components.qml +++ b/faststack/qml/Components.qml @@ -609,6 +609,8 @@ Item { isCropDragging = true } + // Ensure loupeView has active focus so Escape key works + loupeView.forceActiveFocus() return } @@ -855,6 +857,8 @@ Item { cropDragMode = "none" // Settle zoom/pan after rotation ends (Force recompute) if (mainMouseArea.isRotating) imageRotator.recomputeFitScale(true) + // Ensure loupeView has active focus so Escape key works + loupeView.forceActiveFocus() } } diff --git a/faststack/qml/ImageEditorDialog.qml b/faststack/qml/ImageEditorDialog.qml index df7c73c..5b940da 100644 --- a/faststack/qml/ImageEditorDialog.qml +++ b/faststack/qml/ImageEditorDialog.qml @@ -7,7 +7,7 @@ import QtQuick.Window 2.15 Window { id: imageEditorDialog width: 800 - height: 750 + height: 820 title: uiState && uiState.editorFilename ? "Image Editor - " + uiState.editorFilename + " (" + uiState.editorBitDepth + "-bit)" : "Image Editor" visible: uiState ? uiState.isEditorOpen : false flags: Qt.Window | Qt.WindowTitleHint | Qt.WindowCloseButtonHint @@ -67,9 +67,10 @@ Window { Shortcut { sequence: "S" context: Qt.WindowShortcut + enabled: uiState ? !uiState.isSaving : true onActivated: { controller.save_edited_image() - uiState.isEditorOpen = false + // Note: Editor closes automatically via _on_save_finished callback } } @@ -396,16 +397,17 @@ Window { // Save (Primary) Button { - text: "Save" + text: uiState && uiState.isSaving ? "Saving..." : "Save" Layout.preferredWidth: 100 highlighted: true + enabled: uiState ? !uiState.isSaving : true Material.background: imageEditorDialog.accentColor onClicked: { controller.save_edited_image() - uiState.isEditorOpen = false + // Note: Editor closes automatically via _on_save_finished callback } background: Rectangle { - color: parent.pressed ? Qt.darker(imageEditorDialog.accentColor, 1.1) : imageEditorDialog.accentColor + color: parent.enabled ? (parent.pressed ? Qt.darker(imageEditorDialog.accentColor, 1.1) : imageEditorDialog.accentColor) : Qt.darker(imageEditorDialog.accentColor, 1.5) radius: 4 // Subtle shadow simulation layer.enabled: true diff --git a/faststack/qml/Main.qml b/faststack/qml/Main.qml index e261cc0..f9df1a2 100644 --- a/faststack/qml/Main.qml +++ b/faststack/qml/Main.qml @@ -24,7 +24,6 @@ ApplicationWindow { } if (uiState && uiState.hasRecycleBinItems) { close.accepted = false - recycleBinCleanupDialog.text = uiState.recycleBinStatsText recycleBinCleanupDialog.open() } else { close.accepted = true @@ -1248,29 +1247,130 @@ ApplicationWindow { color: "black" } } - MessageDialog { + Dialog { id: recycleBinCleanupDialog title: "Clean up Recycle Bins?" - buttons: MessageDialog.Yes | MessageDialog.No | MessageDialog.Cancel - - // Custom button text isn't directly supported in standard MessageDialog - // So we interpret: - // Yes -> Delete and Quit - // No -> Quit (Keep Files) - // Cancel -> Don't Quit + x: (parent.width - width) / 2 + y: (parent.height - height) / 2 + width: Math.min(550, parent.width * 0.85) + modal: true + standardButtons: Dialog.NoButton - detailedText: "Select 'Yes' to permanently delete these files and quit.\nSelect 'No' to quit but keep files in the recycle bins." + background: Rectangle { + color: root.isDarkTheme ? "#2d2d2d" : "#ffffff" + border.color: root.isDarkTheme ? "#555555" : "#cccccc" + radius: 8 + } - onButtonClicked: function(button, role) { - if (button === MessageDialog.Yes) { - uiState.cleanupRecycleBins() - allowCloseWithRecycleBins = true - Qt.quit() - } else if (button === MessageDialog.No) { - allowCloseWithRecycleBins = true - Qt.quit() + header: Rectangle { + implicitHeight: 50 + color: root.isDarkTheme ? "#333333" : "#f0f0f0" + radius: 8 + Rectangle { + anchors.bottom: parent.bottom + width: parent.width + height: 8 + color: parent.color + } + Text { + anchors.centerIn: parent + text: "Clean up Recycle Bins?" + color: root.currentTextColor + font.bold: true + font.pixelSize: 18 + } + } + + // Use Column inside the default content area + Column { + id: dialogContent + anchors.fill: parent + anchors.margins: 20 + spacing: 12 + + Text { + width: parent.width + text: uiState ? uiState.recycleBinStatsText : "Loading..." + color: root.currentTextColor + wrapMode: Text.WordWrap + font.pixelSize: 14 + lineHeight: 1.4 + } + + Text { + text: detailedSection.visible ? "▼ Hide File List" : "▶ Show File List" + color: "#4fb360" + font.pixelSize: 13 + MouseArea { + anchors.fill: parent + cursorShape: Qt.PointingHandCursor + onClicked: detailedSection.visible = !detailedSection.visible + } + } + + Rectangle { + id: detailedSection + width: parent.width + height: visible ? 180 : 0 + visible: false + color: root.isDarkTheme ? "#1a1a1a" : "#f5f5f5" + border.color: root.isDarkTheme ? "#444444" : "#cccccc" + border.width: 1 + radius: 4 + clip: true + + Behavior on height { NumberAnimation { duration: 150 } } + + Flickable { + anchors.fill: parent + anchors.margins: 8 + contentWidth: detailsText.width + contentHeight: detailsText.height + clip: true + + Text { + id: detailsText + text: uiState ? uiState.recycleBinDetailedText : "" + color: root.currentTextColor + font.family: "Consolas" + font.pixelSize: 12 + } + } + } + + // Spacer + Item { width: 1; height: 10 } + + // Buttons row + Row { + anchors.horizontalCenter: parent.horizontalCenter + spacing: 15 + + Button { + text: "Cancel" + flat: true + onClicked: recycleBinCleanupDialog.close() + } + Button { + text: "Keep and Quit" + onClicked: { + allowCloseWithRecycleBins = true + recycleBinCleanupDialog.close() + Qt.quit() + } + } + Button { + text: "Delete and Quit" + highlighted: true + Material.accent: "#e57373" + onClicked: { + if (uiState) uiState.cleanupRecycleBins() + allowCloseWithRecycleBins = true + recycleBinCleanupDialog.close() + Qt.quit() + } + } } - // Cancel does nothing, just closes dialog } } } diff --git a/faststack/qml/ThumbnailGridView.qml b/faststack/qml/ThumbnailGridView.qml index 6829b7f..e696fb3 100644 --- a/faststack/qml/ThumbnailGridView.qml +++ b/faststack/qml/ThumbnailGridView.qml @@ -130,7 +130,7 @@ Item { // Empty state Text { anchors.centerIn: parent - visible: thumbnailGrid.count === 0 + visible: thumbnailGrid.count === 0 && uiState && uiState.isFolderLoaded text: "No images in this folder" color: gridViewRoot.isDarkTheme ? "#888888" : "#666666" font.pixelSize: 16 diff --git a/faststack/repro_type_error.py b/faststack/repro_type_error.py new file mode 100644 index 0000000..a536809 --- /dev/null +++ b/faststack/repro_type_error.py @@ -0,0 +1,30 @@ +import sys +from pathlib import Path + +# Ensure we can import faststack +repo_root = str(Path(__file__).resolve().parent.parent) +sys.path.insert(0, repo_root) + +from faststack.imaging.editor import ImageEditor +from PIL import Image +import numpy as np + +editor = ImageEditor() +img = Image.new("RGB", (100, 100), (255, 0, 0)) +editor.original_image = img + +print("Calling _apply_edits...") +try: + res = editor._apply_edits(img) + print(f"Result type: {type(res)}") + if res is not None: + print( + f"Result shape/size: {getattr(res, 'shape', 'N/A')} / {getattr(res, 'size', 'N/A')}" + ) + else: + print("Result is None!") +except Exception as e: # noqa: BLE001 + print(f"Caught exception: {type(e).__name__}: {e}") + import traceback + + traceback.print_exc() diff --git a/faststack/rotation_error.txt b/faststack/rotation_error.txt new file mode 100644 index 0000000..ab0537e Binary files /dev/null and b/faststack/rotation_error.txt differ diff --git a/faststack/test_err.txt b/faststack/test_err.txt deleted file mode 100644 index e09c301..0000000 Binary files a/faststack/test_err.txt and /dev/null differ diff --git a/faststack/test_manual_output.txt b/faststack/test_manual_output.txt deleted file mode 100644 index 021eb2a..0000000 Binary files a/faststack/test_manual_output.txt and /dev/null differ diff --git a/faststack/test_pil_blur.py b/faststack/test_pil_blur.py deleted file mode 100644 index 2a424f9..0000000 --- a/faststack/test_pil_blur.py +++ /dev/null @@ -1,29 +0,0 @@ -from PIL import Image, ImageFilter -import numpy as np -import time - - -def test_blur(): - try: - # Create a dummy float image - data = np.random.rand(100, 100).astype(np.float32) - img = Image.fromarray(data, mode="F") - - print("Attempting blur on mode 'F'...") - start = time.time() - blurred = img.filter(ImageFilter.GaussianBlur(radius=5)) - print(f"Blur took {time.time() - start:.4f}s") - - result = np.array(blurred) - print(f"Result shape: {result.shape}, dtype: {result.dtype}") - - # Check if it actually blurred (simple check: std dev should decrease) - print(f"Original std: {np.std(data):.4f}") - print(f"Blurred std: {np.std(result):.4f}") - - except Exception as e: - print(f"Failed: {e}") - - -if __name__ == "__main__": - test_blur() diff --git a/faststack/test_traceback.txt b/faststack/test_traceback.txt deleted file mode 100644 index f221197..0000000 Binary files a/faststack/test_traceback.txt and /dev/null differ diff --git a/faststack/tests/test_deletion_unification.py b/faststack/tests/test_deletion_unification.py new file mode 100644 index 0000000..1094fa2 --- /dev/null +++ b/faststack/tests/test_deletion_unification.py @@ -0,0 +1,330 @@ +import pytest +from unittest.mock import Mock, patch +from pathlib import Path +from faststack.app import AppController +from faststack.models import ImageFile + + +@pytest.fixture(scope="session") +def qapp(): + """Ensure a QApplication exists for tests that might touch UI elements.""" + from PySide6.QtWidgets import QApplication + + app = QApplication.instance() + if app is None: + app = QApplication([]) + return app + + +@pytest.fixture +def mock_controller(tmp_path, qapp): + """Creates an AppController with mocked dependencies.""" + _ = qapp # Keep QApplication active for UI-touching code + engine = Mock() + 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.ThumbnailPrefetcher"), + patch("faststack.app.ThumbnailModel"), + patch("faststack.app.ThumbnailProvider"), + patch("faststack.app.UIState"), + patch("faststack.app.QCoreApplication"), + patch("faststack.app.Keybinder"), + ): + controller = AppController(tmp_path, engine) + + # Mock signals and methods for verification + controller.dataChanged = Mock() + controller.sync_ui_state = Mock() + controller.update_status_message = Mock() + controller.refresh_image_list = Mock() + controller.image_cache = Mock() + controller.prefetcher = Mock() + controller._thumbnail_model = Mock() + controller._thumbnail_model.rowCount.return_value = 0 + + return controller + + +def test_delete_batch_images_success(mock_controller): + """Test deleting a batch of images to recycle bin.""" + # Setup state + img1 = ImageFile(Path("test1.jpg")) + img2 = ImageFile(Path("test2.jpg")) + img3 = ImageFile(Path("test3.jpg")) + mock_controller.image_files = [img1, img2, img3] + mock_controller.batches = [[0, 1]] # Delete test1 and test2 + mock_controller.undo_history = [] + + # Mock _move_to_recycle + mock_controller._move_to_recycle = Mock( + side_effect=lambda p: Path("recycle") / p.name + ) + + with patch("faststack.app.log.info") as mock_log: + mock_controller.delete_batch_images() + + # Verify standardized action used + found_log = any( + "type='batch'" in call.args[0] + for call in mock_log.call_args_list + if "Deletion complete" in call.args[0] + ) + assert found_log + + # Verifications + assert mock_controller._move_to_recycle.call_count == 2 + # Note: refresh_image_list is now deferred via QTimer.singleShot for faster UI + # We verify sync_ui_state was called (immediate UI update) instead + mock_controller.sync_ui_state.assert_called_once() + assert mock_controller.batches == [] + mock_controller.update_status_message.assert_called_with("Deleted 2 images") + + +def test_grid_delete_selection(mock_controller): + """Test deleting images selected in grid view.""" + # Setup state + img1 = ImageFile(Path("test1.jpg")) + img2 = ImageFile(Path("test2.jpg")) + mock_controller.image_files = [img1, img2] + mock_controller._path_to_index = {img1.path.resolve(): 0, img2.path.resolve(): 1} + + # Mock selection in thumbnail model + mock_controller._thumbnail_model.get_selected_paths.return_value = [img1.path] + mock_controller._move_to_recycle = Mock(return_value=Path("recycle/test1.jpg")) + + with patch("faststack.app.log.info") as mock_log: + mock_controller.grid_delete_at_cursor(0) + found_log = any( + "type='grid_selection'" in call.args[0] + for call in mock_log.call_args_list + if "Deletion complete" in call.args[0] + ) + assert found_log + + mock_controller._thumbnail_model.clear_selection.assert_called_once() + mock_controller.update_status_message.assert_called_with( + "Image moved to recycle bin" + ) + + +def test_grid_cursor_correct_mapping(mock_controller): + """CRITICAL: Test that grid delete at cursor uses path mapping, NOT raw index.""" + # Setup: Application order is 0:A, 1:B + # Grid order is 0:B, 1:A (reversed sort) + imgA = ImageFile(Path("A.jpg")) + imgB = ImageFile(Path("B.jpg")) + mock_controller.image_files = [imgA, imgB] + mock_controller._path_to_index = {imgA.path.resolve(): 0, imgB.path.resolve(): 1} + + # User clicks 'Delete' on Grid Index 0 (which is image B) + mock_controller._thumbnail_model.get_selected_paths.return_value = [] + # Mock entry at index 0 returns path B + mock_entry = Mock() + mock_entry.path = imgB.path + mock_entry.is_folder = False + mock_controller._thumbnail_model.get_entry.return_value = mock_entry + + mock_controller._move_to_recycle = Mock(return_value=Path("recycle/B.jpg")) + + # Call delete at grid index 0 + mock_controller.grid_delete_at_cursor(0) + + # VERIFY: Image B (app index 1) was sent to deletion engine + # We check _move_to_recycle was called with B's path + mock_controller._move_to_recycle.assert_called_once_with(imgB.path) + + +def test_partial_recycle_feedback(mock_controller): + """Test behavior when JPG recycles but RAW fails and undo also fails. + + With atomic pair behavior, if RAW exists and fails to move, we try to undo + the JPG move. If undo also fails (common in tests), the image is marked as + deleted to prevent UI resurrection of a missing file. + """ + img = ImageFile(Path("test.jpg")) + img.raw_pair = Path("test.DNG") + mock_controller.image_files = [img] + + # Mock RAW exists but fails to recycle + with patch("faststack.models.Path.exists", return_value=True): + mock_controller._move_to_recycle = Mock( + side_effect=[Path("recycle/test.jpg"), None] + ) + + mock_controller.delete_current_image() + + # Undo failed (paths don't exist in test), so: + # - Image is marked as deleted (jpg_moved=True) + # - No fallback dialog (can't act on it) + # - Image removed from list (not resurrected) + assert len(mock_controller.image_files) == 0 + # Warning message shown to user + mock_controller.update_status_message.assert_called() + + +def test_permanent_delete_fallback_cancelled(mock_controller): + """Test that batches are NOT cleared if user cancels permanent delete fallback.""" + img1 = ImageFile(Path("test1.jpg")) + mock_controller.image_files = [img1] + mock_controller.batches = [[0, 0]] + + mock_controller._move_to_recycle = Mock(return_value=None) + + with patch("faststack.app.confirm_permanent_delete", return_value=False): + mock_controller.delete_batch_images() + + assert mock_controller.batches == [[0, 0]] + mock_controller.update_status_message.assert_called_with("Deletion cancelled") + + +def test_delete_current_image_triggers_batch_dialog(mock_controller): + """Test that delete_current_image triggers the multi-image dialog if a batch exists.""" + img1 = ImageFile(Path("test1.jpg")) + mock_controller.image_files = [img1] + mock_controller.current_index = 0 + + # Mock a batch containing the current image + mock_controller.get_batch_count_for_current_image = Mock(return_value=5) + mock_controller.main_window = Mock() + mock_controller._delete_indices = Mock() + + mock_controller.delete_current_image() + + # Verify dialog was opened instead of immediate deletion + mock_controller.main_window.show_delete_batch_dialog.assert_called_once_with(5) + # Ensure _delete_indices was NOT called (deletion is deferred to dialog) + assert mock_controller._delete_indices.call_count == 0 + + +def test_grid_cursor_not_found_feedback(mock_controller): + """Test standardized feedback for grid cursor delete when image not found.""" + mock_controller._thumbnail_model.get_selected_paths.return_value = [] + mock_entry = Mock() + mock_entry.path = Path("missing.jpg") + mock_entry.is_folder = False + mock_controller._thumbnail_model.get_entry.return_value = mock_entry + + mock_controller._path_to_index = {} # Image not in list + + mock_controller.grid_delete_at_cursor(0) + + mock_controller.update_status_message.assert_called_with( + "Image not found in current list." + ) + + +def test_delete_indices_summary_return(mock_controller): + """Test that _delete_indices returns the expected summary dictionary.""" + img1 = ImageFile(Path("test1.jpg")) + mock_controller.image_files = [img1] + mock_controller._move_to_recycle = Mock(return_value=Path("recycle/test1.jpg")) + + result = mock_controller._delete_indices([0], "test") + + assert result["total_deleted"] == 1 + assert result["recycled"] == 1 + assert result["permanent"] == 0 + assert result["cancelled"] is False + + +def test_grid_cursor_mapping_regression(mock_controller): + """Locked-in regression test: Ensure grid delete at index 0 maps to correct app index. + + Setup: + - App internal list: [B, A] (A is at index 1) + - Grid view (sorted): [A, B] (A is at index 0) + + User presses Delete on Grid index 0. We must delete A (app index 1). + """ + imgA = ImageFile(Path("A.jpg")) + imgB = ImageFile(Path("B.jpg")) + mock_controller.image_files = [imgB, imgA] + mock_controller._path_to_index = {imgB.path.resolve(): 0, imgA.path.resolve(): 1} + + # User on Grid Index 0 (A.jpg) + mock_controller._thumbnail_model.get_selected_paths.return_value = [] + mock_entry = Mock() + mock_entry.path = imgA.path + mock_entry.is_folder = False + mock_controller._thumbnail_model.get_entry.return_value = mock_entry + + mock_controller._move_to_recycle = Mock(return_value=Path("recycle/A.jpg")) + + # EXECUTE: Delete at grid index 0 + mock_controller.grid_delete_at_cursor(0) + + # VERIFY: Image A (application index 1) was deleted + mock_controller._move_to_recycle.assert_called_once_with(imgA.path) + + +def test_grid_delete_folder_feedback(mock_controller): + """Test feedback when attempting to delete a folder in grid.""" + mock_controller._thumbnail_model.get_selected_paths.return_value = [] + mock_entry = Mock() + mock_entry.is_folder = True + mock_controller._thumbnail_model.get_entry.return_value = mock_entry + + mock_controller.grid_delete_at_cursor(0) + + mock_controller.update_status_message.assert_called_with( + "Cannot delete folders in grid view." + ) + + +def test_delete_updates_path_resolver(mock_controller): + """Test that deletion schedules a path resolver update via deferred refresh. + + Note: The actual path resolver update happens in a deferred QTimer callback, + so we verify the _refresh_scheduled flag is set (scheduling happened). + """ + + img1 = ImageFile(Path("test1.jpg")) + mock_controller.image_files = [img1] + mock_controller._move_to_recycle = Mock(return_value=Path("recycle/test1.jpg")) + mock_controller._path_resolver = Mock() + mock_controller._refresh_scheduled = False # Initialize the flag + + # Configure shared mock for the model in both calls + mock_controller._thumbnail_model.rowCount.return_value = 1 + mock_controller._thumbnail_model.get_entry.return_value = Mock( + path=img1.path, is_folder=False + ) + + # 1. Selection path + mock_controller._thumbnail_model.get_selected_paths.return_value = [img1.path] + mock_controller.grid_delete_at_cursor(0) + + # Verify deferred refresh was scheduled (path resolver update happens there) + assert mock_controller._refresh_scheduled is True + + +def test_partial_delete_cancel_preserves_batch(mock_controller): + """Test that if some images in a batch fail to delete and user cancels, batch is NOT cleared.""" + img1 = ImageFile(Path("test1.jpg")) + img2 = ImageFile(Path("test2.jpg")) + mock_controller.image_files = [img1, img2] + mock_controller.batches = [[0, 1]] + + # img1 recycles successfully, img2 fails + def mock_recycle(p): + if p == img1.path: + return Path("recycle/test1.jpg") + raise PermissionError("Fail img2") + + mock_controller._move_to_recycle = Mock(side_effect=mock_recycle) + + # User cancels permanent delete for img2 + with patch("faststack.app.confirm_permanent_delete", return_value=False): + # We need to mock rowCount for the resolver update that happens during refresh + mock_controller._thumbnail_model.rowCount.return_value = 1 + mock_controller.delete_batch_images() + + # Verify: + # 1. batches were NOT cleared because all_deleted was False + assert len(mock_controller.batches) == 1 + assert mock_controller.batches == [[0, 1]] diff --git a/faststack/tests/test_editor_integration.py b/faststack/tests/test_editor_integration.py index 9dc4525..ebba82d 100644 --- a/faststack/tests/test_editor_integration.py +++ b/faststack/tests/test_editor_integration.py @@ -26,15 +26,44 @@ def setUp(self): patch("faststack.app.SidecarManager"), patch("faststack.app.Prefetcher"), patch("faststack.app.ByteLRUCache"), + patch("faststack.app.ThumbnailProvider"), ): self.controller = AppController(Path("."), self.mock_engine) # Mock the internal image_editor to verify delegation self.controller.image_editor = MagicMock() + self.controller.image_editor.current_edits = {} self.controller.image_editor.current_filepath = Path("test.jpg") self.controller.image_editor.float_image = MagicMock() self.controller.image_editor.original_image = MagicMock() + # Initialize state for delegation tests + self.controller.image_files = [MagicMock(path=Path("test.jpg"))] + self.controller.current_index = 0 + self.controller.auto_level_threshold = 0.001 + + # Mock returns for methods that unpack results + self.controller.image_editor.auto_levels.return_value = (0, 255, 0, 255) + self.controller.image_editor.save_image.return_value = (Path("test.jpg"), None) + + # Mock _save_executor to be synchronous to avoid race conditions + self.controller._save_executor = MagicMock() + + def mock_submit(fn, *args, **kwargs): + # Execute synchronously + result = fn(*args, **kwargs) + # Return a mock future that triggers callbacks immediately + mock_future = MagicMock() + mock_future.result.return_value = result + + def mock_add_done_callback(callback): + callback(mock_future) + + mock_future.add_done_callback.side_effect = mock_add_done_callback + return mock_future + + self.controller._save_executor.submit.side_effect = mock_submit + def tearDown(self): self.config_patcher.stop() @@ -54,14 +83,16 @@ def test_missing_methods(self): # 2. rotate_image_cw try: self.controller.rotate_image_cw() - self.controller.image_editor.rotate_image_cw.assert_called_once() + # AppController delegates rotation via set_edit_param("rotation", ...) + self.controller.image_editor.set_edit_param.assert_any_call("rotation", 270) except AttributeError: self.fail("AppController is missing method 'rotate_image_cw'") # 3. rotate_image_ccw try: self.controller.rotate_image_ccw() - self.controller.image_editor.rotate_image_ccw.assert_called_once() + # AppController delegates rotation via set_edit_param("rotation", ...) + self.controller.image_editor.set_edit_param.assert_any_call("rotation", 90) except AttributeError: self.fail("AppController is missing method 'rotate_image_ccw'") diff --git a/faststack/tests/test_editor_rotation.py b/faststack/tests/test_editor_rotation.py index 6994d1a..3756778 100644 --- a/faststack/tests/test_editor_rotation.py +++ b/faststack/tests/test_editor_rotation.py @@ -1,5 +1,6 @@ import pytest import math +import numpy as np from PIL import Image from faststack.imaging.editor import ( _rotated_rect_with_max_area, @@ -18,8 +19,8 @@ def test_rotated_rect_edge_cases(): # Near zero angle (should be close to original dimensions) w, h = 100, 50 cw, ch = _rotated_rect_with_max_area(w, h, 0.0000001) - assert cw == w - assert ch == h + assert w - 1 <= cw <= w + assert h - 1 <= ch <= h # Near 90 degree angle (should swap Dimensions roughly) # The function expects radians. pi/2 is 90 degrees. @@ -99,8 +100,12 @@ def test_rotate_autocrop_rgb_behavior(): assert res.getpixel((cx, cy)) == (255, 0, 0) # Corner pixels should also be red if cropped correctly - assert res.getpixel((0, 0)) == (255, 0, 0) - assert res.getpixel((res.width - 1, res.height - 1)) == (255, 0, 0) + # Allow small tolerance for interpolation/quantization (254 instead of 255) + def assert_red(p): + assert p[0] >= 254 and p[1] < 2 and p[2] < 2 + + assert_red(res.getpixel((0, 0))) + assert_red(res.getpixel((res.width - 1, res.height - 1))) def test_boundary_clamping(): @@ -141,59 +146,61 @@ def test_integration_straighten_modes(): res_b = editor._apply_edits(img.copy(), for_export=True) # Should define a specific size based on autocrop - w_b, h_b = res_b.size + h_b, w_b = res_b.shape[:2] # --- Scenario A: Manual Crop --- # We want to simulate the logic where we replicate what autocrop would have done, # but manually via crop_box. - # 1. Calculate what the autocrop rect would be relative to the *rotated* canvas. - # Note: _rotated_rect yields dims in *original* pixel space generally, - # but let's look at how app.py handles normalization or how editor applies it. - - # Actually, let's just assert that if we manually crop to the SAME pixels - # that autocrop found, we get the same result. - - # Re-use the helper to find the crop box - angle_rad = math.radians(angle) - cw, ch = _rotated_rect_with_max_area(w, h, angle_rad) - - # rotate_autocrop_rgb logic: - # It rotates with expand=True. The new center is center of rotated image. - # It crops centered rect of size (cw, ch). - - # So if we emulate this in editor: - editor.current_edits["straighten_angle"] = angle + # Instead of re-deriving the inscribed rect, we simply take the *actual* + # dimensions that Scenario B produced (w_b, h_b) and create a manual crop + # of that exact size, centered on the rotated canvas. - # We need to compute the 'crop_box' (normalized 0-1000) that corresponds - # to that center crop on the ROTATED image. + # NOTE: The editor implementation applies 'crop_box' BEFORE 'straighten_angle' + # (Crop-then-Rotate) if both are present. This makes it impossible to define + # a precise axis-aligned crop on the *rotated* canvas using the standard parameters. + # To simulate a "User cropping the rotated image" correctly in this test, + # we feed the editor a pre-rotated image and set straighten_angle=0. - # Get rotated size + # 1) Compute the rotated canvas size using PIL rot_temp = img.rotate(-angle, expand=True) rw, rh = rot_temp.size - cx, cy = rw / 2.0, rh / 2.0 - left = cx - cw / 2.0 - top = cy - ch / 2.0 - right = left + cw - bottom = top + ch - - # Normalize to 0-1000 relative to rotated size - # (Editor applies crop_box relative to the current (rotated) image size) - n_left = int(left / rw * 1000) - n_top = int(top / rh * 1000) - n_right = int(right / rw * 1000) - n_bottom = int(bottom / rh * 1000) + # Update editor to use the rotated image as 'original' for this scenario + editor.original_image = rot_temp + editor.current_edits["straighten_angle"] = 0.0 + # 2) Create a centered crop rectangle with width=w_b and height=h_b + cx, cy = rw / 2.0, rh / 2.0 + left = cx - w_b / 2.0 + top = cy - h_b / 2.0 + right = left + w_b + bottom = top + h_b + + # 3) Convert to normalized 0-1000 relative to (rw, rh) + # 4) Use round() rather than int() to reduce systematic flooring error + # 5) Clamp to [0, 1000] + def clamp(val): + return max(0, min(1000, val)) + + n_left = clamp(round(left / rw * 1000)) + n_top = clamp(round(top / rh * 1000)) + n_right = clamp(round(right / rw * 1000)) + n_bottom = clamp(round(bottom / rh * 1000)) + + # 6) Set editor.current_edits["crop_box"] editor.current_edits["crop_box"] = (n_left, n_top, n_right, n_bottom) - res_a = editor._apply_edits(img.copy(), for_export=True) + # Use the pre-rotated image + res_a = editor._apply_edits(rot_temp.copy(), for_export=True) - # Allow for 1-2 pixel differences due to int/round conversions in normalization - assert abs(res_a.width - w_b) < 5 - assert abs(res_a.height - h_b) < 5 + # Allow for a few pixels difference due to floor/round in rotation math + assert abs(res_a.shape[1] - w_b) < 10 + assert abs(res_a.shape[0] - h_b) < 10 # Verify both are Green (center pixel) - assert res_a.getpixel((res_a.width // 2, res_a.height // 2)) == (0, 255, 0) + # Scale from 0-1 to 0-255 for comparison + pixel = np.round(res_a[res_a.shape[0] // 2, res_a.shape[1] // 2] * 255).astype(int) + assert tuple(pixel) == (0, 255, 0) # ------------------------------------------------------------------------- @@ -250,28 +257,30 @@ def test_rotate_cw(): res = editor._apply_edits(editor.original_image.copy()) - # Check pixels - # Original TL (Red) -> New TR - # Original TR (Green) -> New BR - # Original BL (Blue) -> New TL - # Original BR (White) -> New BL - - w, h = res.size + h, w = res.shape[:2] # Sample center of quadrants q_w, q_h = w // 4, h // 4 + # Helper to get pixel as 0-255 tuple + def get_p(arr, x, y): + return tuple(np.round(arr[y, x] * 255).astype(int)) + + # Helper for tolerant comparison + def assert_color(c1, c2, msg=""): + assert all(abs(a - b) <= 1 for a, b in zip(c1, c2)), f"{msg}: {c1} != {c2}" + # New TL (Should be Blue) - assert res.getpixel((q_w, q_h)) == (0, 0, 255), "TL should be Blue (was Red)" + assert_color(get_p(res, q_w, q_h), (0, 0, 255), "TL should be Blue (was Red)") # New TR (Should be Red) - assert res.getpixel((w - q_w, q_h)) == (255, 0, 0), "TR should be Red" + assert_color(get_p(res, w - q_w, q_h), (255, 0, 0), "TR should be Red") # New BL (Should be White) - assert res.getpixel((q_w, h - q_h)) == (255, 255, 255), "BL should be White" + assert_color(get_p(res, q_w, h - q_h), (255, 255, 255), "BL should be White") # New BR (Should be Green) - assert res.getpixel((w - q_w, h - q_h)) == (0, 255, 0), "BR should be Green" + assert_color(get_p(res, w - q_w, h - q_h), (0, 255, 0), "BR should be Green") def test_rotate_ccw(): @@ -287,23 +296,31 @@ def test_rotate_ccw(): res = editor._apply_edits(editor.original_image.copy()) - w, h = res.size + h, w = res.shape[:2] q_w, q_h = w // 4, h // 4 + # Helper to get pixel as 0-255 tuple + def get_p(arr, x, y): + return tuple(np.round(arr[y, x] * 255).astype(int)) + # CCW Rotation: # TL (Red) -> BL # TR (Green) -> TL # BL (Blue) -> BR # BR (White) -> TR + # Helper for tolerant comparison + def assert_color(c1, c2, msg=""): + assert all(abs(a - b) <= 1 for a, b in zip(c1, c2)), f"{msg}: {c1} != {c2}" + # New TL (Should be Green) - assert res.getpixel((q_w, q_h)) == (0, 255, 0), "TL should be Green" + assert_color(get_p(res, q_w, q_h), (0, 255, 0), "TL should be Green") # New TR (Should be White) - assert res.getpixel((w - q_w, q_h)) == (255, 255, 255), "TR should be White" + assert_color(get_p(res, w - q_w, q_h), (255, 255, 255), "TR should be White") # New BL (Should be Red) - assert res.getpixel((q_w, h - q_h)) == (255, 0, 0), "BL should be Red" + assert_color(get_p(res, q_w, h - q_h), (255, 0, 0), "BL should be Red") # New BR (Should be Blue) - assert res.getpixel((w - q_w, h - q_h)) == (0, 0, 255), "BR should be Blue" + assert_color(get_p(res, w - q_w, h - q_h), (0, 0, 255), "BR should be Blue") diff --git a/faststack/tests/test_highlight_recovery.py b/faststack/tests/test_highlight_recovery.py index 942aa8c..f01a548 100644 --- a/faststack/tests/test_highlight_recovery.py +++ b/faststack/tests/test_highlight_recovery.py @@ -210,6 +210,56 @@ def test_benchmark(): # Informational only - no hard assertion for CI stability +def test_amount_zero_identity(): + """amount=0 should return input unchanged (identity transform).""" + arr = np.random.rand(20, 20, 3).astype(np.float32) * 1.5 # Include headroom + + recovered = _highlight_recover_linear(arr.copy(), amount=0.0, pivot=0.5) + + # Should be extremely close to identity + diff = np.abs(recovered - arr).max() + assert diff < 1e-6, f"amount=0 should be identity, but max diff = {diff}" + + print("test_amount_zero_identity passed") + + +def test_pivot_behavior(): + """Values <= pivot should remain unchanged regardless of amount.""" + np.random.seed(123) + low_values = np.random.rand(15, 15, 3).astype(np.float32) * 0.4 # All below 0.5 + + for amount in [0.0, 0.5, 1.0]: + recovered = _highlight_recover_linear( + low_values.copy(), amount=amount, pivot=0.5 + ) + diff = np.abs(recovered - low_values).max() + assert ( + diff < 1e-5 + ), f"Values below pivot changed with amount={amount}: max_diff={diff}" + + print("test_pivot_behavior passed") + + +def test_increasing_amount_increases_compression(): + """Higher amount should result in more compression of highlights.""" + # Create bright image with headroom + bright = np.ones((10, 10, 3), dtype=np.float32) * 1.5 + + recovered_low = _highlight_recover_linear(bright, amount=0.3, pivot=0.5) + recovered_high = _highlight_recover_linear(bright, amount=0.9, pivot=0.5) + + # Higher amount should compress more (result closer to 1.0) + avg_low = recovered_low.mean() + avg_high = recovered_high.mean() + + # avg_high should be lower (more compressed toward 1.0) than avg_low + assert ( + avg_high <= avg_low + ), f"Higher amount should compress more: avg_low={avg_low:.4f}, avg_high={avg_high:.4f}" + + print("test_increasing_amount_increases_compression passed") + + if __name__ == "__main__": try: test_monotonicity() @@ -220,6 +270,9 @@ def test_benchmark(): test_headroom_shoulder() test_analyze_highlight_state() test_source_clipping_detection() + test_amount_zero_identity() + test_pivot_behavior() + test_increasing_amount_increases_compression() test_benchmark() print("\nALL TESTS PASSED") except Exception as e: diff --git a/faststack/tests/test_loupe_delete.py b/faststack/tests/test_loupe_delete.py new file mode 100644 index 0000000..9df9574 --- /dev/null +++ b/faststack/tests/test_loupe_delete.py @@ -0,0 +1,144 @@ +import pytest +from unittest.mock import Mock, patch +from pathlib import Path +from faststack.app import AppController +from faststack.models import ImageFile + + +@pytest.fixture(scope="session") +def qapp(): + """Ensure a QApplication exists for tests that might touch UI elements.""" + from PySide6.QtWidgets import QApplication + + app = QApplication.instance() + if app is None: + app = QApplication([]) + return app + + +@pytest.fixture +def mock_controller(tmp_path, qapp): + """Creates an AppController with mocked dependencies.""" + # Mock dependencies + engine = Mock() + + # Mock internal components heavily to avoid initializing the full app + 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.ThumbnailPrefetcher"), + patch("faststack.app.ThumbnailModel"), + patch("faststack.app.ThumbnailProvider"), + patch("faststack.app.UIState"), + patch("faststack.app.QCoreApplication"), + patch("faststack.app.Keybinder"), + ): + controller = AppController(tmp_path, engine) + + # Manually mock signals that might be emitted + controller.dataChanged = Mock() + controller.dataChanged.emit = Mock() + controller.sync_ui_state = Mock() + controller._do_prefetch = Mock() + controller.update_status_message = Mock() + controller._thumbnail_model = Mock() + controller._thumbnail_model.rowCount.return_value = 0 + + return controller + + +def test_delete_current_image_recycle_success(mock_controller): + """Test successful deletion to recycle bin.""" + # Setup state + img1 = ImageFile(Path("test1.jpg")) + img2 = ImageFile(Path("test2.jpg")) + mock_controller.image_files = [img1, img2] + mock_controller.current_index = 0 + mock_controller.undo_history = [] + mock_controller.refresh_image_list = Mock() + mock_controller.image_cache = Mock() + mock_controller.prefetcher = Mock() + + # Mock _move_to_recycle to return a path (success) + mock_controller._move_to_recycle = Mock(return_value=Path("recycle/test1.jpg")) + + # Call delete + mock_controller.delete_current_image() + + # Verification + mock_controller._move_to_recycle.assert_called_with(img1.path) + # Note: refresh_image_list is now deferred via QTimer for faster UI + mock_controller.sync_ui_state.assert_called_once() + + # Verify undo history + assert len(mock_controller.undo_history) == 1 + action, record, ts = mock_controller.undo_history[0] + assert action == "delete" + assert record[0][0] == img1.path + assert record[0][1] == Path("recycle/test1.jpg") + + mock_controller.update_status_message.assert_called_with( + "Image moved to recycle bin" + ) + + # Verify cache/prefetch cleanup + mock_controller.image_cache.clear.assert_called_once() + mock_controller.prefetcher.cancel_all.assert_called_once() + + +def test_delete_current_image_recycle_fail_fallback_success(mock_controller): + """Test recycle bin failure falling back to permanent delete (confirmed).""" + # Setup state + img1 = ImageFile(Path("test1.jpg")) + mock_controller.image_files = [img1] + mock_controller.current_index = 0 + + # Mock _move_to_recycle to fail + mock_controller._move_to_recycle = Mock( + side_effect=PermissionError("Mock perm error") + ) + + # Mock external deletion module + with ( + patch( + "faststack.app.confirm_permanent_delete", return_value=True + ) as mock_confirm, + patch( + "faststack.app.permanently_delete_image_files", return_value=True + ) as mock_perm_delete, + ): + mock_controller.delete_current_image() + + mock_confirm.assert_called_once() + mock_perm_delete.assert_called_once_with(img1) + + mock_controller.update_status_message.assert_called_with( + "Permanently deleted 1 image(s)" + ) + + +def test_delete_current_image_cancel(mock_controller): + """Test user canceling permanent delete fallback.""" + # Setup state + img1 = ImageFile(Path("test1.jpg")) + mock_controller.image_files = [img1] + mock_controller.current_index = 0 + + # Mock _move_to_recycle to fail + mock_controller._move_to_recycle = Mock( + side_effect=PermissionError("Mock perm error") + ) + + # Mock external deletion module - user says NO + with patch( + "faststack.app.confirm_permanent_delete", return_value=False + ) as mock_confirm: + mock_controller.delete_current_image() + + mock_confirm.assert_called_once() + # verify no refresh or cache clear occurred + mock_controller.update_status_message.assert_called_with("Deletion cancelled") diff --git a/faststack/tests/test_reactive_delete.py b/faststack/tests/test_reactive_delete.py index d3e4f4a..7157d25 100644 --- a/faststack/tests/test_reactive_delete.py +++ b/faststack/tests/test_reactive_delete.py @@ -1,5 +1,6 @@ import pytest from unittest.mock import MagicMock, patch +from faststack.models import ImageFile @pytest.fixture @@ -27,8 +28,19 @@ def app_controller(tmp_path): patch("faststack.app.ThumbnailModel"), patch("faststack.app.ThumbnailPrefetcher"), patch("faststack.app.ThumbnailCache"), + patch("faststack.app.Keybinder"), + patch("faststack.app.UIState"), ): controller = AppController(image_dir, mock_engine, debug_cache=False) + # Mock depth + controller.refresh_image_list = MagicMock() + controller.update_status_message = MagicMock() + controller.sync_ui_state = MagicMock() + controller.image_cache = MagicMock() + controller.prefetcher = MagicMock() + controller._thumbnail_model = MagicMock() + controller._thumbnail_model.rowCount.return_value = 0 + return controller @@ -38,59 +50,65 @@ def test_reactive_delete_fallback(app_controller, tmp_path): img_path = app_controller.image_dir / "test.jpg" img_path.write_text("content") - img_file = MagicMock() - img_file.path = img_path - img_file.raw_pair = None - + img_file = ImageFile(img_path) app_controller.image_files = [img_file] + app_controller.current_index = 0 # Mock _move_to_recycle to raise OSError with patch.object( app_controller, "_move_to_recycle", side_effect=OSError("Permission denied") ): - # Mock confirmation dialogs - # First one is "Recycle bin partial failure..." -> say YES - with patch.object( - app_controller, "_confirm_batch_permanent_delete", return_value=True + # Mock confirmation dialogs in app (where they are patched by tests normally) + with patch( + "faststack.app.confirm_permanent_delete", return_value=True ) as mock_confirm: # Mock permanent delete execution - with patch.object( - app_controller, "_permanently_delete_image_files", return_value=True + with patch( + "faststack.app.permanently_delete_image_files", return_value=True ) as mock_perm_delete: - app_controller._delete_grid_selected_images([img_path]) + app_controller.delete_current_image() # Verify fallback triggered mock_confirm.assert_called_once() - assert ( - "Recycle bin partial failure" in mock_confirm.call_args[1]["reason"] - ) - - # Verify permanent delete called mock_perm_delete.assert_called_with(img_file) + # Verify standard Refreshes/Cleanup + # With optimistic deletion, cache is cleared immediately before file I/O + app_controller.image_cache.clear.assert_called_once() + app_controller.prefetcher.cancel_all.assert_called_once() + # Note: refresh_image_list is now deferred via QTimer + app_controller.sync_ui_state.assert_called_once() + def test_reactive_delete_fallback_cancelled(app_controller, tmp_path): - """Test that user can cancel the fallback permanent delete.""" + """Test that user can cancel the fallback permanent delete and UI rolls back.""" img_path = app_controller.image_dir / "test.jpg" img_path.write_text("content") - img_file = MagicMock() - img_file.path = img_path - img_file.raw_pair = None - + img_file = ImageFile(img_path) app_controller.image_files = [img_file] + app_controller.current_index = 0 with patch.object( app_controller, "_move_to_recycle", side_effect=OSError("Permission denied") ): # User says NO to permanent delete - with patch.object( - app_controller, "_confirm_batch_permanent_delete", return_value=False + with patch( + "faststack.app.confirm_permanent_delete", return_value=False ) as mock_confirm: - with patch.object( - app_controller, "_permanently_delete_image_files" + with patch( + "faststack.app.permanently_delete_image_files" ) as mock_perm_delete: - app_controller._delete_grid_selected_images([img_path]) + app_controller.delete_current_image() mock_confirm.assert_called_once() mock_perm_delete.assert_not_called() + + # With rollback on cancelled deletion: + # 1. sync_ui_state called for optimistic UI update + # 2. sync_ui_state called again after rollback restores the list + assert app_controller.sync_ui_state.call_count == 2 + + # Verify the image was restored (rollback worked) + assert len(app_controller.image_files) == 1 + assert app_controller.image_files[0] == img_file diff --git a/faststack/traceback.txt b/faststack/traceback.txt new file mode 100644 index 0000000..b12d998 Binary files /dev/null and b/faststack/traceback.txt differ diff --git a/faststack/ui/provider.py b/faststack/ui/provider.py index d0fc8b5..258819d 100644 --- a/faststack/ui/provider.py +++ b/faststack/ui/provider.py @@ -142,6 +142,7 @@ class UIState(QObject): # Recycle Bin Signals recycleBinStatsTextChanged = Signal() + recycleBinDetailedTextChanged = Signal() hasRecycleBinItemsChanged = Signal() isZoomedChanged = Signal() @@ -210,6 +211,7 @@ class UIState(QObject): 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 + isSavingChanged = Signal(bool) # Signal for save operation in progress def __init__(self, app_controller): super().__init__() @@ -260,6 +262,7 @@ def __init__(self, app_controller): self._cache_stats = "" self._is_decoding = False self._is_dialog_open = False + self._is_saving = False # Save operation in progress # Connect to controller's dialog state signal self.app_controller.dialogStateChanged.connect(self._on_dialog_state_changed) @@ -785,6 +788,16 @@ def isDialogOpen(self, new_value: bool): self._is_dialog_open = new_value self.isDialogOpenChanged.emit(new_value) + @Property(bool, notify=isSavingChanged) + def isSaving(self) -> bool: + return self._is_saving + + @isSaving.setter + def isSaving(self, new_value: bool): + if self._is_saving != new_value: + self._is_saving = new_value + self.isSavingChanged.emit(new_value) + @Property(bool, notify=anySliderPressedChanged) def anySliderPressed(self): return self._any_slider_pressed @@ -1199,6 +1212,15 @@ def debugMode(self, value: bool): gridSelectedCountChanged = Signal() # No args - QML property notify pattern gridScrollToIndex = Signal(int) # Scroll grid view to show this index gridCanGoBackChanged = Signal() # Emitted when back history changes + isFolderLoadedChanged = Signal() # Emitted after first model refresh + + @Property(bool, notify=isFolderLoadedChanged) + def isFolderLoaded(self) -> bool: + """Returns True after the folder has been scanned at least once. + + Used by QML to avoid showing 'No images' message during initial load. + """ + return getattr(self.app_controller, "_folder_loaded", False) @Property(bool, notify=isGridViewActiveChanged) def isGridViewActive(self) -> bool: @@ -1348,18 +1370,43 @@ def gridPrefetchRange(self, startIndex: int, endIndex: int): @Property(str, notify=recycleBinStatsTextChanged) def recycleBinStatsText(self): - """Returns a formatted string of recycle bin stats.""" + """Returns a formatted string of recycle bin stats summary.""" stats = self.app_controller.get_recycle_bin_stats() if not stats: return "" - summary = "The following recycle bins contain items:\n\n" + summary = "The following recycle bins contain items:\n" for item in stats: - summary += f"• {item['path']}: {item['count']} files\n" - - summary += "\nDo you want to delete them before quitting?" + counts = [] + if item.get("jpg_count", 0) > 0: + counts.append(f"{item['jpg_count']} JPG") + if item.get("raw_count", 0) > 0: + counts.append(f"{item['raw_count']} RAW") + if item.get("other_count", 0) > 0: + counts.append(f"{item['other_count']} other") + + count_str = f" ({', '.join(counts)})" if counts else "" + summary += f"\n• {item['path']}:\n {item['count']} files{count_str}\n" + + summary += "\nDo you want to permanently delete them before quitting?" return summary + @Property(str, notify=recycleBinDetailedTextChanged) + def recycleBinDetailedText(self): + """Returns a detailed list of all file paths in recycle bins.""" + stats = self.app_controller.get_recycle_bin_stats() + if not stats: + return "" + + lines = [] + for item in stats: + lines.append(f"Directory: {item['path']}") + for fname in item.get("file_paths", []): + lines.append(f" - {fname}") + lines.append("") + + return "\n".join(lines) + @Property(bool, notify=hasRecycleBinItemsChanged) def hasRecycleBinItems(self): """Returns True if there are items in any recycle bin.""" @@ -1372,4 +1419,5 @@ def cleanupRecycleBins(self): self.app_controller.cleanup_recycle_bins() self.recycleBinStatsTextChanged.emit() + self.recycleBinDetailedTextChanged.emit() self.hasRecycleBinItemsChanged.emit() diff --git a/pyproject.toml b/pyproject.toml index 39afe2c..b08fd09 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [project] name = "faststack" -version = "1.5.3" +version = "1.5.5" authors = [ { name="Alan Rockefeller"}, ]