Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ The application is structured into two main packages: `core` and `ui`.
- Contains background worker classes for core business logic and application-level operations
- **`update_worker.py`**: Background worker for checking application updates without blocking the UI
- **`rating_loader_worker.py`**: Background worker for loading metadata and ratings for images
- **`rotation_application_worker.py`**: Background worker for applying batch image rotations with parallel processing support
- Workers should inherit from QObject and use Qt signals for communication
- Managed by the `WorkerManager` in the UI layer
- UI-specific workers (like preview preloading, blur detection) remain in `src/ui/ui_components.py` due to tight coupling with UI operations
Expand Down Expand Up @@ -126,6 +127,8 @@ The application is structured into two main packages: `core` and `ui`.
- **Separation of Concerns**: Keep UI logic separate from business logic. The `core` package should not depend on the `ui` package. The `ui` package, specifically `MainWindow`, should be as "dumb" as possible, delegating all logic to the `AppController`.
- **File Operations**: All file system operations (move, rename, delete) MUST be handled by the `ImageFileOperations` class in `src/core/image_file_ops.py`. This ensures that file manipulations are centralized and handled consistently.
- **Threading**: All long-running tasks MUST be executed in a background thread using the `WorkerManager`. This ensures the UI remains responsive. Workers should communicate with the main thread via Qt signals.
- **Parallel Processing**: Workers that process multiple items can use `ThreadPoolExecutor` for parallel execution. The number of workers should be determined by `calculate_max_workers()` from `app_settings.py`, which respects the user's performance mode preference (Balanced/Performance/Custom).
- **Example**: `RotationApplicationWorker` uses parallel processing for batch rotation operations when `max_workers > 1` and multiple images are being rotated. Single-image operations always use sequential processing to avoid overhead.
- **PyExiv2 Usage**: **ALL PyExiv2 operations must use the abstraction layer in `src/core/pyexiv2_wrapper.py`**. Never import or use pyexiv2 directly in application code. This prevents DLL conflicts and ensures proper initialization:

```python
Expand Down
2 changes: 1 addition & 1 deletion src/core/image_processing/image_rotator.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class ImageRotator:
".tiff",
".tif",
".bmp",
} # Removed .heif, .heic
}
_XMP_UPDATE_SUPPORTED_EXTENSIONS = {
".jpg",
".jpeg",
Expand Down
33 changes: 26 additions & 7 deletions src/ui/advanced_image_viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,26 @@ def _create_viewer(self) -> IndividualViewer:
self.viewer_splitter.addWidget(viewer)
return viewer

def _resize_viewer_pool(self, target_count: int):
"""Ensure the viewer pool matches the desired size."""
target_count = max(1, target_count)
current_count = len(self.image_viewers)

if current_count != target_count:
logger.debug(f"Resizing viewer pool from {current_count} to {target_count}")

while len(self.image_viewers) < target_count:
self._create_viewer()

while len(self.image_viewers) > target_count:
viewer = self.image_viewers.pop()
if viewer.parent() is self.viewer_splitter:
viewer.setParent(None)
viewer.deleteLater()

if self._focused_index >= len(self.image_viewers):
self._focused_index = max(0, len(self.image_viewers) - 1)

def _get_current_view_mode(self):
return "side_by_side" if self.side_by_side_btn.isChecked() else "single"

Expand Down Expand Up @@ -1089,10 +1109,9 @@ def set_image_data(
f"set image data {image_data} called with viewer_index={viewer_index}, preserve_view_mode={preserve_view_mode}"
)

# Ensure we have at least one viewer
if not self.image_viewers:
logger.debug("Creating first viewer")
self._create_viewer()
desired_viewer_count = max(viewer_index + 1, 1)
self._resize_viewer_pool(desired_viewer_count)
viewer_index = min(viewer_index, len(self.image_viewers) - 1)

# Update all viewers: set data for the target, clear others
for i, viewer in enumerate(self.image_viewers):
Expand Down Expand Up @@ -1138,9 +1157,7 @@ def set_images_data(self, images_data: List[Dict[str, Any]]):
self.set_image_data(images_data[0], 0)
return

# Ensure we have enough viewer widgets for all images
while len(self.image_viewers) < num_images:
self._create_viewer()
self._resize_viewer_pool(num_images)

# Update all viewers, then hide unused ones
for i, viewer in enumerate(self.image_viewers):
Expand All @@ -1162,6 +1179,7 @@ def set_images_data(self, images_data: List[Dict[str, Any]]):

def clear(self):
logger.debug("clear called")
self._resize_viewer_pool(1)
for i, viewer in enumerate(self.image_viewers):
logger.debug(f"Clearing viewer {i}")
viewer.clear()
Expand All @@ -1172,6 +1190,7 @@ def setText(self, text: str):
"""Clears all viewers, resets to single view, and displays the given text centrally."""
logger.debug(f"setText called with: {text}")
# Clear all viewers to remove any existing images or text.
self._resize_viewer_pool(1)
for i, viewer in enumerate(self.image_viewers):
logger.debug(f"Clearing viewer {i}")
viewer.clear()
Expand Down
80 changes: 68 additions & 12 deletions src/ui/app_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ def __init__(
self.app_state = app_state
self.worker_manager = worker_manager

# Track pending rotations for batch preview regeneration
self._pending_rotated_paths: List[str] = []

def connect_signals(self):
"""Connects signals from the WorkerManager to the controller's slots."""
# File Scan Worker
self.worker_manager.file_scan_found_files.connect(self.handle_files_found)
self.worker_manager.file_scan_finished.connect(self.handle_scan_finished)
self.worker_manager.file_scan_error.connect(self.handle_scan_error)
self.worker_manager.file_scan_thumbnail_preload_finished.connect(
self.handle_thumbnail_preload_finished
)

# Similarity Worker
self.worker_manager.similarity_progress.connect(self.handle_similarity_progress)
Expand Down Expand Up @@ -655,10 +655,6 @@ def handle_blur_detection_error(self, message: str):
bool(self.app_state.image_files_data)
)

def handle_thumbnail_preload_finished(self, all_file_data: List[Dict[str, any]]):
logger.debug("Thumbnail preload finished signal received (deprecated, no-op).")
pass

# --- Rotation Detection Handlers ---

def handle_rotation_detection_progress(
Expand Down Expand Up @@ -907,17 +903,77 @@ def handle_rotation_applied(
message: str,
is_lossy: bool,
):
"""Handle completion of a single rotation."""
"""Handle completion of a single rotation.

Defers UI-heavy updates (preview regeneration, selection refresh) until batch completion.
Only performs lightweight cache invalidation here.
"""
if success:
self.main_window._handle_successful_rotation(
file_path, direction, message, is_lossy=is_lossy
# Track for batch processing
self._pending_rotated_paths.append(file_path)

# Perform only lightweight cache invalidation (no preview regeneration yet)
filename = os.path.basename(file_path)
logger.info(f"Rotation completed for '{filename}' (Lossy: {is_lossy})")
self.main_window.image_pipeline.preview_cache.delete_all_for_path(file_path)
self.main_window.image_pipeline.thumbnail_cache.delete_all_for_path(
file_path
)

def handle_rotation_application_finished(
self, successful_count: int, failed_count: int
):
"""Handle completion of all rotation applications."""
self.main_window.hide_loading_overlay()
"""Handle completion of all rotation applications.

Performs batch preview regeneration in parallel, then updates UI.
"""
logger.info(
f"Rotation batch finished: {successful_count} successful, {failed_count} failed. "
f"Processing {len(self._pending_rotated_paths)} rotated images..."
)

try:
if self._pending_rotated_paths:
# Update loading overlay for preview regeneration
self.main_window.update_loading_text(
f"Regenerating previews for {len(self._pending_rotated_paths)} images..."
)

# Batch regenerate previews in parallel using existing infrastructure
t1 = time.perf_counter()
self.main_window.image_pipeline.preload_previews(
self._pending_rotated_paths
)
t2 = time.perf_counter()
logger.info(
f"Batch preview regeneration for {len(self._pending_rotated_paths)} images "
f"completed in {t2 - t1:.2f}s"
)

# Batch update thumbnails in UI
t3 = time.perf_counter()
self.main_window._batch_update_rotated_thumbnails(
self._pending_rotated_paths
)
t4 = time.perf_counter()
logger.info(f"Batch thumbnail update completed in {t4 - t3:.2f}s")

# Single selection refresh if any rotated images are in current selection
selected_paths = self.main_window._get_selected_file_paths_from_view()
if any(path in selected_paths for path in self._pending_rotated_paths):
t5 = time.perf_counter()
self.main_window._handle_file_selection_changed()
t6 = time.perf_counter()
logger.info(f"Selection refresh completed in {t6 - t5:.2f}s")

except Exception as e:
logger.error(
f"Error during batch post-rotation processing: {e}", exc_info=True
)
finally:
# Clear pending list
self._pending_rotated_paths.clear()
self.main_window.hide_loading_overlay()

# Show summary message
if successful_count > 0 and failed_count == 0:
Expand Down
5 changes: 0 additions & 5 deletions src/ui/dark_theme.qss
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ QTreeView::item {

QListView::item { /* Specifically for Grid View items */
background-color: transparent;
/* color property removed to allow item.setForeground() to take effect,
will inherit default text color from QWidget if not set by item. */
padding: 4px;
border-radius: 3px;
margin: 0px;
Expand Down Expand Up @@ -314,9 +312,6 @@ QWidget#search_container QPushButton:checked {
border-color: #005A9E;
}

/* Bottom bar removed - using status bar only */


/* Splitter Handle */
QSplitter#main_splitter::handle {
background-color: #2B2B2B; /* Match main background */
Expand Down
Loading