From fef9f4b04f1c5b1b0d62bb1fc8c19efde7be0c87 Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Sun, 7 Dec 2025 20:46:41 -0800 Subject: [PATCH 1/2] minor bug fixes --- faststack/faststack/app.py | 41 +++++++++++++++++++ faststack/faststack/imaging/cache.py | 5 ++- faststack/faststack/imaging/editor.py | 10 ++--- faststack/faststack/imaging/jpeg.py | 2 +- faststack/faststack/imaging/metadata.py | 6 +-- faststack/faststack/imaging/prefetch.py | 2 - faststack/faststack/qml/Components.qml | 4 +- faststack/faststack/qml/ExifDialog.qml | 8 +++- .../faststack/tests/test_new_features.py | 10 ++++- faststack/faststack/tests/test_sidecar.py | 10 ++--- faststack/faststack/ui/provider.py | 2 +- faststack/faststack/verify_wb.py | 10 ++--- 12 files changed, 79 insertions(+), 31 deletions(-) diff --git a/faststack/faststack/app.py b/faststack/faststack/app.py index 4e9b406..bfa4817 100644 --- a/faststack/faststack/app.py +++ b/faststack/faststack/app.py @@ -1763,6 +1763,47 @@ def undo_delete(self): log.exception("Failed to undo auto white balance") # Put it back in history if it failed self.undo_history.append(("auto_white_balance", (saved_path, backup_path), timestamp)) + + elif action_type == "auto_levels": + saved_path, backup_path = action_data + filepath_obj = Path(saved_path) + + try: + backup_path_obj = Path(backup_path) + if backup_path_obj.exists(): + # Restore the backup + filepath_obj.unlink() # Remove the edited version + backup_path_obj.rename(filepath_obj) # Restore backup + log.info("Restored backup %s for %s", backup_path_obj.name, saved_path) + + # Refresh the view + self.refresh_image_list() + + # Find the restored image + for i, img_file in enumerate(self.image_files): + if img_file.path == filepath_obj: + self.current_index = i + break + + self.display_generation += 1 + self.image_cache.clear() + self.prefetcher.cancel_all() + self.prefetcher.update_prefetch(self.current_index) + self.sync_ui_state() + + if self.ui_state.isHistogramVisible: + self.update_histogram() + + self.update_status_message("Undid auto levels") + else: + self.update_status_message("Backup not found") + log.warning("Backup %s disappeared before it could be restored.", backup_path) + self.undo_history.append(("auto_levels", (saved_path, backup_path), timestamp)) + except OSError as e: + self.update_status_message(f"Undo failed: {e}") + log.exception("Failed to undo auto levels") + # Put it back in history if it failed + self.undo_history.append(("auto_levels", (saved_path, backup_path), timestamp)) elif action_type == "crop": saved_path, backup_path = action_data diff --git a/faststack/faststack/imaging/cache.py b/faststack/faststack/imaging/cache.py index c552e9c..2aa5f0f 100644 --- a/faststack/faststack/imaging/cache.py +++ b/faststack/faststack/imaging/cache.py @@ -2,7 +2,7 @@ import logging from pathlib import Path -from typing import Any, Callable, Union +from typing import Any, Callable, Optional, Union from cachetools import LRUCache @@ -16,7 +16,7 @@ def __init__( self, max_bytes: int, size_of: Callable[[Any], int] = len, - on_evict: Callable[[], None] = None, + on_evict: Optional[Callable[[], None]] = None, ): super().__init__(maxsize=max_bytes, getsizeof=size_of) self.on_evict = on_evict @@ -66,6 +66,7 @@ def get_decoded_image_size(item) -> int: bytes_per_pixel = getattr(item, "channels", 4) # Default to RGBA return item.width * item.height * bytes_per_pixel + log.warning(f"Unexpected item type in cache: {type(item)}. Returning estimated size of 1.") return 1 # Should not happen diff --git a/faststack/faststack/imaging/editor.py b/faststack/faststack/imaging/editor.py index 216a28f..4ce3db3 100644 --- a/faststack/faststack/imaging/editor.py +++ b/faststack/faststack/imaging/editor.py @@ -465,16 +465,14 @@ def _restore_file_times(self, path: Path, original_stat: os.stat_result) -> None print(f"Warning: Unable to restore timestamps for {path}: {e}") def rotate_image_cw(self): - """Decreases the rotation edit parameter by 90° modulo 360.""" + """Increases the rotation edit parameter by 90° modulo 360 (clockwise).""" current = self.current_edits.get('rotation', 0) - self.current_edits['rotation'] = (current - 90) % 360 - if self.current_edits['rotation'] < 0: - self.current_edits['rotation'] += 360 + self.current_edits['rotation'] = (current + 90) % 360 def rotate_image_ccw(self): - """Increases the rotation edit parameter by 90° modulo 360.""" + """Decreases the rotation edit parameter by 90° modulo 360 (counter-clockwise).""" current = self.current_edits.get('rotation', 0) - self.current_edits['rotation'] = (current + 90) % 360 + self.current_edits['rotation'] = (current - 90) % 360 # Dictionary of ratios for QML dropdown ASPECT_RATIOS = [{"name": name, "ratio": ratio} for name, ratio in INSTAGRAM_RATIOS.items()] diff --git a/faststack/faststack/imaging/jpeg.py b/faststack/faststack/imaging/jpeg.py index 44b3eda..262ad12 100644 --- a/faststack/faststack/imaging/jpeg.py +++ b/faststack/faststack/imaging/jpeg.py @@ -168,7 +168,7 @@ def decode_jpeg_resized( if scale_factor_ratio > 4: resampling = Image.Resampling.BILINEAR # Much faster else: - resampling = Image.Resampling.BILINEAR # Changed from LANCZOS to BILINEAR for speed + resampling = Image.Resampling.LANCZOS # Higher quality for smaller downscales img.thumbnail((width, height), resampling) return np.array(img.convert("RGB")) diff --git a/faststack/faststack/imaging/metadata.py b/faststack/faststack/imaging/metadata.py index faf5699..811e396 100644 --- a/faststack/faststack/imaging/metadata.py +++ b/faststack/faststack/imaging/metadata.py @@ -49,11 +49,11 @@ def get_exif_data(path: Union[str, Path]) -> Dict[str, Any]: return {"summary": {}, "full": {}} try: - img = Image.open(path) - exif = img._getexif() + with Image.open(path) as img: + exif = img._getexif() if not exif: return {"summary": {}, "full": {}} - except Exception as e: + except Exception as e: # noqa: BLE001 - defensive catch for arbitrary EXIF parsing issues log.warning(f"Failed to extract EXIF from {path}: {e}") return {"summary": {}, "full": {}} diff --git a/faststack/faststack/imaging/prefetch.py b/faststack/faststack/imaging/prefetch.py index 77408d4..ed6476e 100644 --- a/faststack/faststack/imaging/prefetch.py +++ b/faststack/faststack/imaging/prefetch.py @@ -259,8 +259,6 @@ def submit_task(self, index: int, generation: int, priority: bool = False) -> Op if index in self.futures and not self.futures[index].done(): return self.futures[index] # Already submitted - # For high-priority tasks (current image), cancel pending prefetch tasks - # to free up worker threads and reduce blocking time # For high-priority tasks (current image), cancel pending prefetch tasks # to free up worker threads and reduce blocking time if priority: diff --git a/faststack/faststack/qml/Components.qml b/faststack/faststack/qml/Components.qml index 1090c4b..34c30e4 100644 --- a/faststack/faststack/qml/Components.qml +++ b/faststack/faststack/qml/Components.qml @@ -832,7 +832,7 @@ Item { anchors.leftMargin: 10 anchors.verticalCenter: parent.verticalCenter text: uiState && uiState.aspectRatioNames ? uiState.aspectRatioNames[index] : "" - color: "white" + color: aspectRatioWindow.isDark ? "white" : "black" font.pixelSize: 11 } @@ -862,7 +862,7 @@ Item { anchors.leftMargin: 10 anchors.verticalCenter: parent.verticalCenter text: "Rotate" - color: "white" + color: aspectRatioWindow.isDark ? "white" : "black" font.pixelSize: 11 } diff --git a/faststack/faststack/qml/ExifDialog.qml b/faststack/faststack/qml/ExifDialog.qml index 926a3e7..ec4ab99 100644 --- a/faststack/faststack/qml/ExifDialog.qml +++ b/faststack/faststack/qml/ExifDialog.qml @@ -29,11 +29,15 @@ Dialog { // Reset to summary view when opened showFull = false // Notify Python that a dialog is open - controller.dialog_opened() + if (controller) { + controller.dialog_opened() + } } onClosed: { - controller.dialog_closed() + if (controller) { + controller.dialog_closed() + } } contentItem: ColumnLayout { diff --git a/faststack/faststack/tests/test_new_features.py b/faststack/faststack/tests/test_new_features.py index ffe12bb..e1e6e5d 100644 --- a/faststack/faststack/tests/test_new_features.py +++ b/faststack/faststack/tests/test_new_features.py @@ -36,8 +36,14 @@ def test_auto_levels_strength(self): b_scaled = blacks * strength w_scaled = whites * strength - self.assertEqual(b_scaled, blacks * 0.5) - self.assertEqual(w_scaled, whites * 0.5) + # Verify auto_levels returned meaningful values for the test image + # With 10% black pixels and 10% white pixels at 0.1 threshold, + # we expect blacks and whites to be near zero (histogram endpoints are at 0 and 255) + self.assertIsInstance(blacks, float) + self.assertIsInstance(whites, float) + # Verify scaling works correctly + self.assertAlmostEqual(b_scaled, blacks * strength) + self.assertAlmostEqual(w_scaled, whites * strength) def test_highlights_recovery(self): # Set highlights to -1.0 (Recovery) diff --git a/faststack/faststack/tests/test_sidecar.py b/faststack/faststack/tests/test_sidecar.py index d2792d5..3069b28 100644 --- a/faststack/faststack/tests/test_sidecar.py +++ b/faststack/faststack/tests/test_sidecar.py @@ -20,7 +20,7 @@ def _create(content: dict = None): def test_sidecar_load_non_existent(mock_sidecar_dir): """Tests loading when no sidecar file exists.""" d = mock_sidecar_dir() - sm = SidecarManager(d) + sm = SidecarManager(d, None) assert sm.data.version == 2 assert sm.data.last_index == 0 assert not sm.data.entries @@ -28,7 +28,7 @@ def test_sidecar_load_non_existent(mock_sidecar_dir): def test_sidecar_load_existing(mock_sidecar_dir): """Tests loading a valid, existing sidecar file.""" content = { - "version": 1, + "version": 2, "last_index": 42, "entries": { "IMG_0001": { "flag": True, "reject": False, "stack_id": 1 }, @@ -36,7 +36,7 @@ def test_sidecar_load_existing(mock_sidecar_dir): } } d = mock_sidecar_dir(content) - sm = SidecarManager(d) + sm = SidecarManager(d, None) assert sm.data.last_index == 42 assert len(sm.data.entries) == 2 @@ -47,7 +47,7 @@ def test_sidecar_load_existing(mock_sidecar_dir): def test_sidecar_save(mock_sidecar_dir): """Tests saving data back to the JSON file.""" d = mock_sidecar_dir() - sm = SidecarManager(d) + sm = SidecarManager(d, None) # Modify data sm.set_last_index(10) @@ -67,7 +67,7 @@ def test_sidecar_save(mock_sidecar_dir): def test_sidecar_get_metadata_creates_new(mock_sidecar_dir): """Tests that get_metadata creates a new entry if one doesn't exist.""" d = mock_sidecar_dir() - sm = SidecarManager(d) + sm = SidecarManager(d, None) assert "NEW_IMG" not in sm.data.entries meta = sm.get_metadata("NEW_IMG") assert isinstance(meta, EntryMetadata) diff --git a/faststack/faststack/ui/provider.py b/faststack/faststack/ui/provider.py index 5eff04d..04e72fe 100644 --- a/faststack/faststack/ui/provider.py +++ b/faststack/faststack/ui/provider.py @@ -702,7 +702,7 @@ def currentCropBox(self, new_value): e, ) - # only accept 4‑element tuples + # only accept 4-element tuples if not isinstance(new_value, tuple) or len(new_value) != 4: log.warning("UIState.currentCropBox: ignoring invalid crop box %r", new_value) return diff --git a/faststack/faststack/verify_wb.py b/faststack/faststack/verify_wb.py index ea1d6b0..866925e 100644 --- a/faststack/faststack/verify_wb.py +++ b/faststack/faststack/verify_wb.py @@ -56,11 +56,11 @@ def test_white_balance(): print("FAIL: Grey did not shift as expected.") # Cleanup - try: - os.remove(black_path) - os.remove(grey_path) - except: - pass + for path in [black_path, grey_path]: + try: + os.remove(path) + except OSError: + pass # File may not exist or be locked if __name__ == "__main__": test_white_balance() From 49d3e0439e133d3d1d918eef4e303d484286b177 Mon Sep 17 00:00:00 2001 From: AlanRockefeller Date: Sun, 7 Dec 2025 20:58:13 -0800 Subject: [PATCH 2/2] minor bug fixes --- faststack/faststack/app.py | 6 +-- faststack/faststack/imaging/editor.py | 8 ++-- .../faststack/tests/test_new_features.py | 37 +++++++++---------- faststack/faststack/ui/provider.py | 6 ++- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/faststack/faststack/app.py b/faststack/faststack/app.py index bfa4817..5041d73 100644 --- a/faststack/faststack/app.py +++ b/faststack/faststack/app.py @@ -2274,16 +2274,14 @@ def save_edited_image(self): def rotate_image_cw(self): """Rotate the edited image 90 degrees clockwise.""" current = self.image_editor.current_edits.get('rotation', 0) - new_rotation = (current + 90) % 360 + new_rotation = (current - 90) % 360 self.set_edit_parameter('rotation', new_rotation) @Slot() def rotate_image_ccw(self): """Rotate the edited image 90 degrees counter-clockwise.""" current = self.image_editor.current_edits.get('rotation', 0) - new_rotation = (current - 90) % 360 - if new_rotation < 0: - new_rotation += 360 + new_rotation = (current + 90) % 360 self.set_edit_parameter('rotation', new_rotation) @Slot() diff --git a/faststack/faststack/imaging/editor.py b/faststack/faststack/imaging/editor.py index 4ce3db3..bf59b40 100644 --- a/faststack/faststack/imaging/editor.py +++ b/faststack/faststack/imaging/editor.py @@ -465,14 +465,14 @@ def _restore_file_times(self, path: Path, original_stat: os.stat_result) -> None print(f"Warning: Unable to restore timestamps for {path}: {e}") def rotate_image_cw(self): - """Increases the rotation edit parameter by 90° modulo 360 (clockwise).""" + """Decreases the rotation edit parameter by 90° modulo 360 (clockwise).""" current = self.current_edits.get('rotation', 0) - self.current_edits['rotation'] = (current + 90) % 360 + self.current_edits['rotation'] = (current - 90) % 360 def rotate_image_ccw(self): - """Decreases the rotation edit parameter by 90° modulo 360 (counter-clockwise).""" + """Increases the rotation edit parameter by 90° modulo 360 (counter-clockwise).""" current = self.current_edits.get('rotation', 0) - self.current_edits['rotation'] = (current - 90) % 360 + self.current_edits['rotation'] = (current + 90) % 360 # Dictionary of ratios for QML dropdown ASPECT_RATIOS = [{"name": name, "ratio": ratio} for name, ratio in INSTAGRAM_RATIOS.items()] diff --git a/faststack/faststack/tests/test_new_features.py b/faststack/faststack/tests/test_new_features.py index e1e6e5d..dc6be7d 100644 --- a/faststack/faststack/tests/test_new_features.py +++ b/faststack/faststack/tests/test_new_features.py @@ -13,37 +13,36 @@ def setUp(self): self.editor._preview_image = self.img def test_auto_levels_strength(self): - # Create an image capable of clipping - # 10% black (0), 80% gray (128), 10% white (255) - arr = np.zeros((100, 100), dtype=np.uint8) + 128 - arr[0:10, :] = 0 - arr[90:100, :] = 255 + # Create an image capable of clipping but with limited range to force non-zero adjustments + # Range 50-200. Auto-levels should expand this to 0-255. + arr = np.linspace(50, 200, 10000).reshape(100, 100).astype(np.uint8) img = Image.fromarray(arr) self.editor.original_image = img self.editor._preview_image = img - # Calculate auto levels with 10% threshold (should clip the 0s and 255s) - # Percentiles: 10% is 0, 90% is 255? - # Let's use a threshold that clips inside the gray area to force a stretch if possible, - # or just ensure it returns non-zero. - - # Actually, simpler: just check valid return + # Calculate auto levels blacks, whites = self.editor.auto_levels(0.1) + # With range [50, 200], we expect: + # blacks approx -50/40 = -1.25 + # whites approx (200-255)/40 = -1.375 + self.assertNotEqual(blacks, 0.0) + self.assertNotEqual(whites, 0.0) + self.assertLess(blacks, 0.0) + self.assertLess(whites, 0.0) + # Mock strength application matching app.py logic strength = 0.5 b_scaled = blacks * strength w_scaled = whites * strength - # Verify auto_levels returned meaningful values for the test image - # With 10% black pixels and 10% white pixels at 0.1 threshold, - # we expect blacks and whites to be near zero (histogram endpoints are at 0 and 255) - self.assertIsInstance(blacks, float) - self.assertIsInstance(whites, float) - # Verify scaling works correctly - self.assertAlmostEqual(b_scaled, blacks * strength) - self.assertAlmostEqual(w_scaled, whites * strength) + # Verify scaling works correctly and produces expected intermediate values + self.assertAlmostEqual(b_scaled, blacks * 0.5) + self.assertAlmostEqual(w_scaled, whites * 0.5) + # Verify magnitude is reduced + self.assertLess(abs(b_scaled), abs(blacks)) + self.assertLess(abs(w_scaled), abs(whites)) def test_highlights_recovery(self): # Set highlights to -1.0 (Recovery) diff --git a/faststack/faststack/ui/provider.py b/faststack/faststack/ui/provider.py index 04e72fe..61fbb07 100644 --- a/faststack/faststack/ui/provider.py +++ b/faststack/faststack/ui/provider.py @@ -703,7 +703,11 @@ def currentCropBox(self, new_value): ) # only accept 4-element tuples - if not isinstance(new_value, tuple) or len(new_value) != 4: + if ( + not isinstance(new_value, tuple) + or len(new_value) != 4 + or not all(isinstance(v, (int, float)) for v in new_value) + ): log.warning("UIState.currentCropBox: ignoring invalid crop box %r", new_value) return if self._current_crop_box != new_value: