From fae9e5bd2bdabf796cbe62b22269472528c19e72 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 27 Nov 2025 21:57:57 +0100 Subject: [PATCH] Additional refactoring --- pyPhotoAlbum/asset_heal_dialog.py | 2 +- pyPhotoAlbum/async_backend.py | 22 +-- pyPhotoAlbum/autosave_manager.py | 4 +- pyPhotoAlbum/commands.py | 5 +- pyPhotoAlbum/decorators.py | 2 +- pyPhotoAlbum/dialogs/page_setup_dialog.py | 2 +- pyPhotoAlbum/gl_imports.py | 115 ++++++++++++ pyPhotoAlbum/gl_widget.py | 11 +- pyPhotoAlbum/image_utils.py | 164 ++++++++++++++++++ pyPhotoAlbum/main.py | 4 +- pyPhotoAlbum/merge_manager.py | 7 +- pyPhotoAlbum/mixins/asset_drop.py | 3 +- pyPhotoAlbum/mixins/asset_path.py | 68 ++++++++ pyPhotoAlbum/mixins/async_loading.py | 27 +-- pyPhotoAlbum/mixins/mouse_interaction.py | 4 +- pyPhotoAlbum/mixins/operations/element_ops.py | 3 +- pyPhotoAlbum/mixins/operations/file_ops.py | 8 +- pyPhotoAlbum/mixins/rendering.py | 5 +- pyPhotoAlbum/mixins/viewport.py | 2 +- pyPhotoAlbum/models.py | 82 +++------ pyPhotoAlbum/page_layout.py | 12 +- pyPhotoAlbum/page_renderer.py | 2 +- pyPhotoAlbum/pdf_exporter.py | 85 +++------ pyPhotoAlbum/snapping.py | 3 +- pyPhotoAlbum/version_manager.py | 7 +- tests/test_asset_drop_mixin.py | 12 +- tests/test_element_ops_mixin.py | 15 +- 27 files changed, 472 insertions(+), 204 deletions(-) create mode 100644 pyPhotoAlbum/gl_imports.py create mode 100644 pyPhotoAlbum/image_utils.py create mode 100644 pyPhotoAlbum/mixins/asset_path.py diff --git a/pyPhotoAlbum/asset_heal_dialog.py b/pyPhotoAlbum/asset_heal_dialog.py index e43535a..cdeb506 100644 --- a/pyPhotoAlbum/asset_heal_dialog.py +++ b/pyPhotoAlbum/asset_heal_dialog.py @@ -3,6 +3,7 @@ Asset healing dialog for reconnecting missing images """ import os +import shutil from typing import List, Dict, Set from PyQt6.QtWidgets import ( QDialog, QVBoxLayout, QHBoxLayout, QLabel, QPushButton, @@ -207,7 +208,6 @@ class AssetHealDialog(QDialog): # Copy it to the correct location dest_path = os.path.join(self.project.folder_path, asset_path) os.makedirs(os.path.dirname(dest_path), exist_ok=True) - import shutil shutil.copy2(found_path, dest_path) print(f"Restored: {asset_path} from {found_path}") else: diff --git a/pyPhotoAlbum/async_backend.py b/pyPhotoAlbum/async_backend.py index 55d50fc..898e55f 100644 --- a/pyPhotoAlbum/async_backend.py +++ b/pyPhotoAlbum/async_backend.py @@ -21,6 +21,8 @@ import threading from PIL import Image from PyQt6.QtCore import QObject, pyqtSignal +from pyPhotoAlbum.image_utils import convert_to_rgba, resize_to_fit + logger = logging.getLogger(__name__) @@ -388,23 +390,16 @@ class AsyncImageLoader(QObject): Returns: Processed PIL Image """ - # Load image img = Image.open(path) - - # Convert to RGBA for consistency - if img.mode != 'RGBA': - img = img.convert('RGBA') + img = convert_to_rgba(img) # Downsample if target size specified (preserving aspect ratio) if target_size: max_size = target_size[0] # Assume square target (2048, 2048) - if img.width > max_size or img.height > max_size: - # Calculate scale to fit within max_size while preserving aspect ratio - scale = min(max_size / img.width, max_size / img.height) - new_width = int(img.width * scale) - new_height = int(img.height * scale) - img = img.resize((new_width, new_height), Image.Resampling.LANCZOS) - logger.debug(f"Downsampled {path}: {img.size} -> ({new_width}, {new_height})") + original_size = img.size + img = resize_to_fit(img, max_size) + if img.size != original_size: + logger.debug(f"Downsampled {path}: {original_size} -> {img.size}") return img @@ -721,8 +716,7 @@ class AsyncPDFGenerator(QObject): # Load and cache (unrotated - rotation is applied per-element) img = original_open(path, *args, **kwargs) - if img.mode != 'RGBA': - img = img.convert('RGBA') + img = convert_to_rgba(img) self.image_cache.put(Path(path), img) return img diff --git a/pyPhotoAlbum/autosave_manager.py b/pyPhotoAlbum/autosave_manager.py index f643c9b..b964914 100644 --- a/pyPhotoAlbum/autosave_manager.py +++ b/pyPhotoAlbum/autosave_manager.py @@ -9,7 +9,7 @@ import os import json import shutil from pathlib import Path -from datetime import datetime +from datetime import datetime, timedelta from typing import Optional, List, Tuple from pyPhotoAlbum.project_serializer import save_to_zip, load_from_zip @@ -185,8 +185,6 @@ class AutosaveManager: max_age_hours: Maximum age in hours (default: 7 days) max_count: Maximum number of checkpoints to keep per project """ - from datetime import timedelta - now = datetime.now() checkpoints_by_project = {} diff --git a/pyPhotoAlbum/commands.py b/pyPhotoAlbum/commands.py index 52b37da..e6c5b0f 100644 --- a/pyPhotoAlbum/commands.py +++ b/pyPhotoAlbum/commands.py @@ -2,6 +2,7 @@ Command pattern implementation for undo/redo functionality """ +import os from abc import ABC, abstractmethod from typing import Dict, Any, List, Optional from pyPhotoAlbum.models import BaseLayoutElement, ImageData, PlaceholderData, TextBoxData @@ -20,8 +21,7 @@ def _normalize_asset_path(image_path: str, asset_manager) -> str: """ if not asset_manager or not image_path: return image_path - - import os + if os.path.isabs(image_path): return os.path.relpath(image_path, asset_manager.project_folder) return image_path @@ -742,7 +742,6 @@ class CommandHistory: if isinstance(command, (AddElementCommand, DeleteElementCommand)): if isinstance(command.element, ImageData) and command.element.image_path: # Convert absolute path to relative for asset manager - import os asset_path = command.element.image_path if os.path.isabs(asset_path): asset_path = os.path.relpath(asset_path, self.asset_manager.project_folder) diff --git a/pyPhotoAlbum/decorators.py b/pyPhotoAlbum/decorators.py index 6dcaa48..884c3b8 100644 --- a/pyPhotoAlbum/decorators.py +++ b/pyPhotoAlbum/decorators.py @@ -2,6 +2,7 @@ Decorator system for pyPhotoAlbum ribbon UI """ +import copy from functools import wraps from typing import Optional, Callable @@ -280,7 +281,6 @@ class UndoableOperation: current_page = instance.get_current_page() if hasattr(instance, 'get_current_page') else None if current_page: # Deep copy elements - import copy return [copy.deepcopy(elem.serialize()) for elem in current_page.layout.elements] return [] diff --git a/pyPhotoAlbum/dialogs/page_setup_dialog.py b/pyPhotoAlbum/dialogs/page_setup_dialog.py index e5b121a..65f170f 100644 --- a/pyPhotoAlbum/dialogs/page_setup_dialog.py +++ b/pyPhotoAlbum/dialogs/page_setup_dialog.py @@ -5,6 +5,7 @@ Encapsulates all UI logic for page setup configuration, separating presentation from business logic. """ +import math from typing import Optional, Dict, Any from PyQt6.QtWidgets import ( QDialog, QVBoxLayout, QHBoxLayout, QLabel, @@ -286,7 +287,6 @@ class PageSetupDialog(QDialog): content_pages = sum( p.get_page_count() for p in self.project.pages if not p.is_cover ) - import math sheets = math.ceil(content_pages / 4) spine_width = sheets * self.thickness_spinbox.value() * 2 diff --git a/pyPhotoAlbum/gl_imports.py b/pyPhotoAlbum/gl_imports.py new file mode 100644 index 0000000..9081c77 --- /dev/null +++ b/pyPhotoAlbum/gl_imports.py @@ -0,0 +1,115 @@ +""" +Centralized OpenGL imports for pyPhotoAlbum. + +Provides a single point of import for all OpenGL functions used throughout +the application. This centralizes GL dependency management and provides +graceful handling when OpenGL is not available (e.g., during testing). + +Usage: + from pyPhotoAlbum.gl_imports import glBegin, glEnd, GL_QUADS, GL_AVAILABLE + + if GL_AVAILABLE: + # Safe to use GL functions + glBegin(GL_QUADS) + ... +""" + +try: + from OpenGL.GL import ( + # Drawing primitives + glBegin, + glEnd, + glVertex2f, + GL_QUADS, + GL_LINE_LOOP, + GL_LINES, + GL_TRIANGLE_FAN, + + # Colors + glColor3f, + glColor4f, + + # Line state + glLineWidth, + glLineStipple, + GL_LINE_STIPPLE, + + # General state + glEnable, + glDisable, + GL_DEPTH_TEST, + GL_BLEND, + GL_SRC_ALPHA, + GL_ONE_MINUS_SRC_ALPHA, + glBlendFunc, + + # Textures + glGenTextures, + glBindTexture, + glTexImage2D, + glTexParameteri, + glDeleteTextures, + GL_TEXTURE_2D, + GL_RGBA, + GL_UNSIGNED_BYTE, + GL_TEXTURE_MIN_FILTER, + GL_TEXTURE_MAG_FILTER, + GL_LINEAR, + glTexCoord2f, + + # Matrix operations + glPushMatrix, + glPopMatrix, + glScalef, + glTranslatef, + glLoadIdentity, + glRotatef, + + # Clear operations + glClear, + glClearColor, + GL_COLOR_BUFFER_BIT, + GL_DEPTH_BUFFER_BIT, + + # Viewport + glViewport, + glMatrixMode, + glOrtho, + GL_PROJECTION, + GL_MODELVIEW, + + # Info/debug + glGetString, + GL_VERSION, + ) + GL_AVAILABLE = True + +except ImportError: + GL_AVAILABLE = False + + # Define dummy functions/constants for when OpenGL is not available + # This allows the code to be imported without OpenGL for testing + def _gl_stub(*args, **kwargs): + pass + + glBegin = glEnd = glVertex2f = _gl_stub + glColor3f = glColor4f = _gl_stub + glLineWidth = glLineStipple = _gl_stub + glEnable = glDisable = glBlendFunc = _gl_stub + glGenTextures = glBindTexture = glTexImage2D = _gl_stub + glTexParameteri = glDeleteTextures = glTexCoord2f = _gl_stub + glPushMatrix = glPopMatrix = glScalef = glTranslatef = _gl_stub + glLoadIdentity = glRotatef = _gl_stub + glClear = glClearColor = _gl_stub + glViewport = glMatrixMode = glOrtho = _gl_stub + glGetString = _gl_stub + + # Constants + GL_QUADS = GL_LINE_LOOP = GL_LINES = GL_TRIANGLE_FAN = 0 + GL_LINE_STIPPLE = GL_DEPTH_TEST = GL_BLEND = 0 + GL_SRC_ALPHA = GL_ONE_MINUS_SRC_ALPHA = 0 + GL_TEXTURE_2D = GL_RGBA = GL_UNSIGNED_BYTE = 0 + GL_TEXTURE_MIN_FILTER = GL_TEXTURE_MAG_FILTER = GL_LINEAR = 0 + GL_COLOR_BUFFER_BIT = GL_DEPTH_BUFFER_BIT = 0 + GL_PROJECTION = GL_MODELVIEW = 0 + GL_VERSION = 0 diff --git a/pyPhotoAlbum/gl_widget.py b/pyPhotoAlbum/gl_widget.py index 862475f..4c137af 100644 --- a/pyPhotoAlbum/gl_widget.py +++ b/pyPhotoAlbum/gl_widget.py @@ -4,11 +4,12 @@ OpenGL widget for pyPhotoAlbum rendering - refactored with mixins from PyQt6.QtOpenGLWidgets import QOpenGLWidget from PyQt6.QtCore import Qt -from OpenGL.GL import * +from pyPhotoAlbum.gl_imports import * # Import all mixins from pyPhotoAlbum.mixins.viewport import ViewportMixin from pyPhotoAlbum.mixins.rendering import RenderingMixin +from pyPhotoAlbum.mixins.asset_path import AssetPathMixin from pyPhotoAlbum.mixins.asset_drop import AssetDropMixin from pyPhotoAlbum.mixins.page_navigation import PageNavigationMixin from pyPhotoAlbum.mixins.image_pan import ImagePanMixin @@ -24,6 +25,7 @@ class GLWidget( AsyncLoadingMixin, ViewportMixin, RenderingMixin, + AssetPathMixin, AssetDropMixin, PageNavigationMixin, ImagePanMixin, @@ -72,6 +74,13 @@ class GLWidget( self._cleanup_async_loading() super().closeEvent(event) + def _get_project_folder(self): + """Override AssetPathMixin to access project via main window.""" + main_window = self.window() + if hasattr(main_window, 'project') and main_window.project: + return getattr(main_window.project, 'folder_path', None) + return None + def keyPressEvent(self, event): """Handle key press events""" if event.key() == Qt.Key.Key_Delete or event.key() == Qt.Key.Key_Backspace: diff --git a/pyPhotoAlbum/image_utils.py b/pyPhotoAlbum/image_utils.py new file mode 100644 index 0000000..c863d2d --- /dev/null +++ b/pyPhotoAlbum/image_utils.py @@ -0,0 +1,164 @@ +""" +Centralized image processing utilities for pyPhotoAlbum. + +This module consolidates common image operations to avoid code duplication +across models.py, pdf_exporter.py, and async_backend.py. +""" + +from typing import Tuple +from PIL import Image + + +# ============================================================================= +# Image Processing Utilities +# ============================================================================= + +def apply_pil_rotation(image: Image.Image, pil_rotation_90: int) -> Image.Image: + """ + Apply 90-degree rotation increments to a PIL image. + + Args: + image: PIL Image to rotate + pil_rotation_90: Number of 90-degree rotations (0, 1, 2, or 3) + + Returns: + Rotated PIL Image (or original if no rotation needed) + """ + if pil_rotation_90 <= 0: + return image + + angle = pil_rotation_90 * 90 + if angle == 90: + return image.transpose(Image.ROTATE_270) # CCW 90 = rotate right + elif angle == 180: + return image.transpose(Image.ROTATE_180) + elif angle == 270: + return image.transpose(Image.ROTATE_90) # CCW 270 = rotate left + + return image + + +def convert_to_rgba(image: Image.Image) -> Image.Image: + """ + Convert image to RGBA mode if not already. + + Args: + image: PIL Image in any mode + + Returns: + PIL Image in RGBA mode + """ + if image.mode != 'RGBA': + return image.convert('RGBA') + return image + + +def calculate_center_crop_coords( + img_width: int, + img_height: int, + target_width: float, + target_height: float, + crop_info: Tuple[float, float, float, float] = (0, 0, 1, 1) +) -> Tuple[float, float, float, float]: + """ + Calculate texture/crop coordinates for center-crop fitting an image to a target aspect ratio. + + This implements the center-crop algorithm used for fitting images into frames + while preserving aspect ratio. The image is scaled to cover the target area, + then the excess is cropped equally from both sides. + + Args: + img_width: Source image width in pixels + img_height: Source image height in pixels + target_width: Target frame width (any unit, only ratio matters) + target_height: Target frame height (any unit, only ratio matters) + crop_info: Additional crop range as (x_min, y_min, x_max, y_max) in 0-1 range + Default (0, 0, 1, 1) means no additional cropping + + Returns: + Tuple of (tx_min, ty_min, tx_max, ty_max) texture coordinates in 0-1 range + """ + crop_x_min, crop_y_min, crop_x_max, crop_y_max = crop_info + + img_aspect = img_width / img_height + target_aspect = target_width / target_height + + # Calculate base texture coordinates for center crop + if img_aspect > target_aspect: + # Image is wider than target - crop horizontally + scale = target_aspect / img_aspect + tx_offset = (1.0 - scale) / 2.0 + tx_min_base = tx_offset + tx_max_base = 1.0 - tx_offset + ty_min_base = 0.0 + ty_max_base = 1.0 + else: + # Image is taller than target - crop vertically + scale = img_aspect / target_aspect + ty_offset = (1.0 - scale) / 2.0 + tx_min_base = 0.0 + tx_max_base = 1.0 + ty_min_base = ty_offset + ty_max_base = 1.0 - ty_offset + + # Apply additional crop from crop_info (for spanning elements, user crops, etc.) + tx_range = tx_max_base - tx_min_base + ty_range = ty_max_base - ty_min_base + + tx_min = tx_min_base + crop_x_min * tx_range + tx_max = tx_min_base + crop_x_max * tx_range + ty_min = ty_min_base + crop_y_min * ty_range + ty_max = ty_min_base + crop_y_max * ty_range + + return (tx_min, ty_min, tx_max, ty_max) + + +def crop_image_to_coords( + image: Image.Image, + coords: Tuple[float, float, float, float] +) -> Image.Image: + """ + Crop an image using normalized texture coordinates. + + Args: + image: PIL Image to crop + coords: Tuple of (tx_min, ty_min, tx_max, ty_max) in 0-1 range + + Returns: + Cropped PIL Image + """ + tx_min, ty_min, tx_max, ty_max = coords + img_width, img_height = image.size + + crop_left_px = int(tx_min * img_width) + crop_right_px = int(tx_max * img_width) + crop_top_px = int(ty_min * img_height) + crop_bottom_px = int(ty_max * img_height) + + return image.crop((crop_left_px, crop_top_px, crop_right_px, crop_bottom_px)) + + +def resize_to_fit( + image: Image.Image, + max_size: int, + resample: Image.Resampling = Image.Resampling.LANCZOS +) -> Image.Image: + """ + Resize image to fit within max_size while preserving aspect ratio. + + Args: + image: PIL Image to resize + max_size: Maximum dimension (width or height) + resample: Resampling filter (default LANCZOS for quality) + + Returns: + Resized PIL Image, or original if already smaller + """ + if image.width <= max_size and image.height <= max_size: + return image + + scale = min(max_size / image.width, max_size / image.height) + new_width = int(image.width * scale) + new_height = int(image.height * scale) + + return image.resize((new_width, new_height), resample) diff --git a/pyPhotoAlbum/main.py b/pyPhotoAlbum/main.py index 5832685..7da212e 100644 --- a/pyPhotoAlbum/main.py +++ b/pyPhotoAlbum/main.py @@ -6,6 +6,7 @@ This version uses the mixin architecture with auto-generated ribbon configuratio """ import sys +from datetime import datetime from pathlib import Path from PyQt6.QtWidgets import ( QApplication, QMainWindow, QVBoxLayout, QWidget, @@ -23,6 +24,7 @@ from pyPhotoAlbum.autosave_manager import AutosaveManager # Import mixins from pyPhotoAlbum.mixins.base import ApplicationStateMixin +from pyPhotoAlbum.mixins.asset_path import AssetPathMixin from pyPhotoAlbum.mixins.operations import ( FileOperationsMixin, EditOperationsMixin, @@ -41,6 +43,7 @@ from pyPhotoAlbum.mixins.operations import ( class MainWindow( QMainWindow, ApplicationStateMixin, + AssetPathMixin, FileOperationsMixin, EditOperationsMixin, ElementOperationsMixin, @@ -317,7 +320,6 @@ class MainWindow( # Parse timestamp for better display try: - from datetime import datetime timestamp = datetime.fromisoformat(timestamp_str) time_display = timestamp.strftime("%Y-%m-%d %H:%M:%S") except: diff --git a/pyPhotoAlbum/merge_manager.py b/pyPhotoAlbum/merge_manager.py index 83be239..24ae9dc 100644 --- a/pyPhotoAlbum/merge_manager.py +++ b/pyPhotoAlbum/merge_manager.py @@ -7,10 +7,11 @@ This module provides functionality for: - Resolving conflicts based on user input or automatic strategies """ +import copy from typing import Dict, Any, List, Optional, Tuple from enum import Enum from dataclasses import dataclass -from datetime import datetime +from datetime import datetime, timezone class ConflictType(Enum): @@ -369,7 +370,6 @@ class MergeManager: Merged project data """ # Start with a copy of our project - import copy merged_data = copy.deepcopy(our_project_data) # Apply resolutions @@ -529,8 +529,6 @@ def concatenate_projects( Returns: Combined project data """ - import copy - # Start with project A as base merged_data = copy.deepcopy(project_a_data) @@ -542,7 +540,6 @@ def concatenate_projects( # Keep project A's ID and settings # Update last_modified to now - from datetime import datetime, timezone merged_data["last_modified"] = datetime.now(timezone.utc).isoformat() print(f"Concatenated projects: {len(project_a_data.get('pages', []))} + {len(project_b_data.get('pages', []))} = {len(merged_data['pages'])} pages") diff --git a/pyPhotoAlbum/mixins/asset_drop.py b/pyPhotoAlbum/mixins/asset_drop.py index c0e0bfa..3408103 100644 --- a/pyPhotoAlbum/mixins/asset_drop.py +++ b/pyPhotoAlbum/mixins/asset_drop.py @@ -103,7 +103,6 @@ class AssetDropMixin: def _handle_drop_on_empty_space(self, image_path, x, y): """Handle dropping an image onto empty space""" - import os main_window = self.window() if not (hasattr(main_window, 'project') and main_window.project and main_window.project.pages): return @@ -117,7 +116,7 @@ class AssetDropMixin: try: # Import asset first, then calculate dimensions from imported asset asset_path = main_window.project.asset_manager.import_asset(image_path) - full_asset_path = os.path.join(main_window.project.folder_path, asset_path) + full_asset_path = self.get_asset_full_path(asset_path) img_width, img_height = self._calculate_image_dimensions(full_asset_path) self._add_new_image_to_page( diff --git a/pyPhotoAlbum/mixins/asset_path.py b/pyPhotoAlbum/mixins/asset_path.py new file mode 100644 index 0000000..6a082e1 --- /dev/null +++ b/pyPhotoAlbum/mixins/asset_path.py @@ -0,0 +1,68 @@ +""" +Asset path resolution mixin for components that need to resolve asset paths. +""" + +import os +from typing import Optional + + +class AssetPathMixin: + """ + Mixin providing asset path resolution functionality. + + Requires access to self.project (typically via ApplicationStateMixin). + """ + + def resolve_asset_path(self, asset_path: str) -> Optional[str]: + """ + Resolve a relative asset path to an absolute path. + + Args: + asset_path: Relative path (e.g., "assets/photo.jpg") or absolute path + + Returns: + Absolute path if the asset exists, None otherwise + """ + if not asset_path: + return None + + # Handle absolute paths + if os.path.isabs(asset_path): + if os.path.exists(asset_path): + return asset_path + return None + + # Resolve relative path using project folder + project_folder = self._get_project_folder() + if project_folder: + full_path = os.path.join(project_folder, asset_path) + if os.path.exists(full_path): + return full_path + + return None + + def get_asset_full_path(self, relative_path: str) -> Optional[str]: + """ + Get the full path for a relative asset path (without existence check). + + Args: + relative_path: Relative path from project folder + + Returns: + Full absolute path, or None if no project folder + """ + project_folder = self._get_project_folder() + if project_folder and relative_path: + return os.path.join(project_folder, relative_path) + return None + + def _get_project_folder(self) -> Optional[str]: + """ + Get the current project folder. + + Override this method if the project is accessed differently. + Default implementation uses self.project.folder_path. + """ + if hasattr(self, 'project') and self.project: + return getattr(self.project, 'folder_path', None) + return None diff --git a/pyPhotoAlbum/mixins/async_loading.py b/pyPhotoAlbum/mixins/async_loading.py index fb4da81..e03f929 100644 --- a/pyPhotoAlbum/mixins/async_loading.py +++ b/pyPhotoAlbum/mixins/async_loading.py @@ -2,6 +2,7 @@ Async loading mixin for non-blocking image loading and PDF generation. """ +import os from pathlib import Path from typing import Optional import logging @@ -168,33 +169,17 @@ class AsyncLoadingMixin: if not image_data.image_path: return - # Resolve path - only load from project's assets folder - # Search paths are only used for healing, not for async loading - from pyPhotoAlbum.models import get_asset_search_paths - import os - - # Only load images that are properly in the assets folder - # Paths must be relative and start with "assets/" - if os.path.isabs(image_data.image_path): - logger.warning(f"Skipping absolute path (needs healing): {image_data.image_path}") - return - + # Security: only load images from the assets folder if not image_data.image_path.startswith("assets/"): logger.warning(f"Skipping path not in assets folder (needs healing): {image_data.image_path}") return - project_folder, _ = get_asset_search_paths() - if not project_folder: - logger.warning("No project folder set, cannot load image") + # Use ImageData's path resolution (delegates to project layer) + image_full_path = image_data.resolve_image_path() + if not image_full_path: + logger.warning(f"Image not found (needs healing): {image_data.image_path}") return - full_path = os.path.join(project_folder, image_data.image_path) - if not os.path.exists(full_path): - logger.warning(f"Image not found in assets (needs healing): {image_data.image_path}") - return - - image_full_path = full_path - # Calculate target size (max 2048px like original) target_size = (2048, 2048) # Will be downsampled if larger diff --git a/pyPhotoAlbum/mixins/mouse_interaction.py b/pyPhotoAlbum/mixins/mouse_interaction.py index 0ce75a3..dc3128a 100644 --- a/pyPhotoAlbum/mixins/mouse_interaction.py +++ b/pyPhotoAlbum/mixins/mouse_interaction.py @@ -2,6 +2,8 @@ Mouse interaction mixin for GLWidget - coordinates all mouse events """ +import math + from PyQt6.QtCore import Qt from pyPhotoAlbum.models import ImageData @@ -150,8 +152,6 @@ class MouseInteractionMixin: elif self.rotation_mode: # Rotation mode - import math - if not hasattr(self.selected_element, '_page_renderer'): return diff --git a/pyPhotoAlbum/mixins/operations/element_ops.py b/pyPhotoAlbum/mixins/operations/element_ops.py index a8d7ac4..f38622e 100644 --- a/pyPhotoAlbum/mixins/operations/element_ops.py +++ b/pyPhotoAlbum/mixins/operations/element_ops.py @@ -39,12 +39,11 @@ class ElementOperationsMixin: return try: - import os # Import asset to project asset_path = self.project.asset_manager.import_asset(file_path) # Get dimensions using centralized utility (max 300px for UI display) - full_asset_path = os.path.join(self.project.folder_path, asset_path) + full_asset_path = self.get_asset_full_path(asset_path) dimensions = get_image_dimensions(full_asset_path, max_size=300) if dimensions: img_width, img_height = dimensions diff --git a/pyPhotoAlbum/mixins/operations/file_ops.py b/pyPhotoAlbum/mixins/operations/file_ops.py index 51b1cbc..96bfcb8 100644 --- a/pyPhotoAlbum/mixins/operations/file_ops.py +++ b/pyPhotoAlbum/mixins/operations/file_ops.py @@ -2,6 +2,8 @@ File operations mixin for pyPhotoAlbum """ +import os + from PyQt6.QtWidgets import ( QFileDialog, QDialog, QVBoxLayout, QHBoxLayout, QLabel, QDoubleSpinBox, QSpinBox, QPushButton, QGroupBox, QRadioButton, @@ -312,7 +314,6 @@ class FileOperationsMixin: def _check_missing_assets(self) -> list: """Check for missing assets in the project - returns list of missing paths""" - import os from pyPhotoAlbum.models import ImageData missing = [] @@ -326,9 +327,8 @@ class FileOperationsMixin: elif not element.image_path.startswith("assets/"): missing.append(element.image_path) else: - # Check if file exists in assets - full_path = os.path.join(self.project.folder_path, element.image_path) - if not os.path.exists(full_path): + # Check if file exists in assets using mixin + if not self.resolve_asset_path(element.image_path): missing.append(element.image_path) return list(set(missing)) # Remove duplicates diff --git a/pyPhotoAlbum/mixins/rendering.py b/pyPhotoAlbum/mixins/rendering.py index 62b0475..184c2ea 100644 --- a/pyPhotoAlbum/mixins/rendering.py +++ b/pyPhotoAlbum/mixins/rendering.py @@ -2,7 +2,9 @@ Rendering mixin for GLWidget - handles OpenGL rendering """ -from OpenGL.GL import * +import math + +from pyPhotoAlbum.gl_imports import * from PyQt6.QtGui import QPainter, QFont, QColor, QPen from PyQt6.QtCore import Qt, QRectF from pyPhotoAlbum.models import TextBoxData @@ -152,7 +154,6 @@ class RenderingMixin: glLineWidth(1.0) if self.rotation_mode: - import math handle_radius = 6 handles = [(x, y), (x + w, y), (x, y + h), (x + w, y + h)] diff --git a/pyPhotoAlbum/mixins/viewport.py b/pyPhotoAlbum/mixins/viewport.py index 9d16520..99d44e7 100644 --- a/pyPhotoAlbum/mixins/viewport.py +++ b/pyPhotoAlbum/mixins/viewport.py @@ -2,7 +2,7 @@ Viewport mixin for GLWidget - handles zoom and pan """ -from OpenGL.GL import * +from pyPhotoAlbum.gl_imports import * class ViewportMixin: diff --git a/pyPhotoAlbum/models.py b/pyPhotoAlbum/models.py index d81d1b8..13450b4 100644 --- a/pyPhotoAlbum/models.py +++ b/pyPhotoAlbum/models.py @@ -11,6 +11,18 @@ import uuid from datetime import datetime, timezone from PIL import Image +from pyPhotoAlbum.image_utils import apply_pil_rotation, calculate_center_crop_coords +from pyPhotoAlbum.gl_imports import ( + GL_AVAILABLE, glBegin, glEnd, glVertex2f, glColor3f, glColor4f, + GL_QUADS, GL_LINE_LOOP, glEnable, glDisable, GL_TEXTURE_2D, + glBindTexture, glTexCoord2f, glTexParameteri, GL_TEXTURE_MIN_FILTER, + GL_TEXTURE_MAG_FILTER, GL_LINEAR, glGenTextures, glTexImage2D, + GL_RGBA, GL_UNSIGNED_BYTE, glDeleteTextures, glGetString, GL_VERSION, + glLineStipple, GL_LINE_STIPPLE, glPushMatrix, glPopMatrix, + glTranslatef, glRotatef, GL_BLEND, glBlendFunc, GL_SRC_ALPHA, + GL_ONE_MINUS_SRC_ALPHA, +) + logger = logging.getLogger(__name__) # Global configuration for asset path resolution @@ -171,9 +183,6 @@ class ImageData(BaseLayoutElement): def render(self): """Render the image using OpenGL""" - from OpenGL.GL import (glBegin, glEnd, glVertex2f, glColor3f, glColor4f, GL_QUADS, GL_LINE_LOOP, - glEnable, glDisable, GL_TEXTURE_2D, glBindTexture, glTexCoord2f, - glTexParameteri, GL_TEXTURE_MIN_FILTER, GL_TEXTURE_MAG_FILTER, GL_LINEAR) x, y = self.position w, h = self.size @@ -197,39 +206,10 @@ class ImageData(BaseLayoutElement): # No dimensions available, render without aspect ratio correction img_width, img_height = int(w), int(h) - # Get crop info - crop_x_min, crop_y_min, crop_x_max, crop_y_max = self.crop_info - - # Calculate aspect ratios for center crop - img_aspect = img_width / img_height - target_aspect = w / h - - # Calculate texture coordinates for center crop - if img_aspect > target_aspect: - # Image is wider - crop horizontally - scale = target_aspect / img_aspect - tx_offset = (1.0 - scale) / 2.0 - tx_min_base = tx_offset - tx_max_base = 1.0 - tx_offset - ty_min_base = 0.0 - ty_max_base = 1.0 - else: - # Image is taller - crop vertically - scale = img_aspect / target_aspect - ty_offset = (1.0 - scale) / 2.0 - tx_min_base = 0.0 - tx_max_base = 1.0 - ty_min_base = ty_offset - ty_max_base = 1.0 - ty_offset - - # Apply additional crop from crop_info (for spanning elements) - tx_range = tx_max_base - tx_min_base - ty_range = ty_max_base - ty_min_base - - tx_min = tx_min_base + crop_x_min * tx_range - tx_max = tx_min_base + crop_x_max * tx_range - ty_min = ty_min_base + crop_y_min * ty_range - ty_max = ty_min_base + crop_y_max * ty_range + # Calculate texture coordinates for center crop with element's crop_info + tx_min, ty_min, tx_max, ty_max = calculate_center_crop_coords( + img_width, img_height, w, h, self.crop_info + ) # Enable texturing and draw with crop glEnable(GL_TEXTURE_2D) @@ -334,16 +314,8 @@ class ImageData(BaseLayoutElement): # Apply PIL-level rotation if needed if hasattr(self, 'pil_rotation_90') and self.pil_rotation_90 > 0: - # Rotate counter-clockwise by 90° * pil_rotation_90 - # PIL.Image.ROTATE_90 rotates counter-clockwise - angle = self.pil_rotation_90 * 90 - if angle == 90: - pil_image = pil_image.transpose(Image.ROTATE_270) # CCW 90 = rotate right - elif angle == 180: - pil_image = pil_image.transpose(Image.ROTATE_180) - elif angle == 270: - pil_image = pil_image.transpose(Image.ROTATE_90) # CCW 270 = rotate left - logger.debug(f"ImageData: Applied PIL rotation {angle}° to {self.image_path}") + pil_image = apply_pil_rotation(pil_image, self.pil_rotation_90) + logger.debug(f"ImageData: Applied PIL rotation {self.pil_rotation_90 * 90}° to {self.image_path}") # Store the image for texture creation during next render() # This avoids GL context issues when callback runs on wrong thread/timing @@ -370,11 +342,6 @@ class ImageData(BaseLayoutElement): if not hasattr(self, '_pending_pil_image') or self._pending_pil_image is None: return False - from OpenGL.GL import (glGenTextures, glBindTexture, glTexImage2D, GL_TEXTURE_2D, - glTexParameteri, GL_TEXTURE_MIN_FILTER, GL_TEXTURE_MAG_FILTER, - GL_LINEAR, GL_RGBA, GL_UNSIGNED_BYTE, glDeleteTextures, - glGetString, GL_VERSION) - try: # Verify GL context is actually current before creating textures # glGetString returns None if no context is active @@ -454,9 +421,7 @@ class PlaceholderData(BaseLayoutElement): def render(self): """Render the placeholder using OpenGL""" - from OpenGL.GL import (glBegin, glEnd, glVertex2f, glColor3f, GL_QUADS, GL_LINE_LOOP, glLineStipple, - glEnable, glDisable, GL_LINE_STIPPLE, glPushMatrix, glPopMatrix, glTranslatef, glRotatef) - + x, y = self.position w, h = self.size @@ -535,10 +500,6 @@ class TextBoxData(BaseLayoutElement): def render(self): """Render the text box using OpenGL""" - from OpenGL.GL import (glBegin, glEnd, glVertex2f, glColor3f, glColor4f, GL_QUADS, GL_LINE_LOOP, - glEnable, glDisable, GL_BLEND, glBlendFunc, GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA, - glPushMatrix, glPopMatrix, glTranslatef, glRotatef) - x, y = self.position w, h = self.size @@ -623,10 +584,7 @@ class GhostPageData(BaseLayoutElement): def render(self): """Render the ghost page with 'Add Page' button in page-local coordinates""" - from OpenGL.GL import (glBegin, glEnd, glVertex2f, glColor3f, glColor4f, GL_QUADS, GL_LINE_LOOP, - glEnable, glDisable, GL_BLEND, glBlendFunc, GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA, - glLineStipple, GL_LINE_STIPPLE) - + # Render at page origin (0,0) in page-local coordinates # PageRenderer will handle transformation to screen coordinates x, y = 0, 0 diff --git a/pyPhotoAlbum/page_layout.py b/pyPhotoAlbum/page_layout.py index 839c6e8..bc6d2a5 100644 --- a/pyPhotoAlbum/page_layout.py +++ b/pyPhotoAlbum/page_layout.py @@ -5,6 +5,12 @@ Page layout and template system for pyPhotoAlbum from typing import List, Dict, Any, Optional, Tuple from pyPhotoAlbum.models import BaseLayoutElement, ImageData, PlaceholderData, TextBoxData from pyPhotoAlbum.snapping import SnappingSystem +from pyPhotoAlbum.gl_imports import ( + glBegin, glEnd, glVertex2f, glColor3f, glColor4f, + GL_QUADS, GL_LINE_LOOP, GL_LINES, glLineWidth, + glEnable, glDisable, GL_DEPTH_TEST, GL_BLEND, + glBlendFunc, GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA, +) class PageLayout: """Class to manage page layout and templates""" @@ -50,8 +56,6 @@ class PageLayout: dpi: Working DPI for converting mm to pixels project: Optional project instance for global snapping settings """ - from OpenGL.GL import glBegin, glEnd, glVertex2f, glColor3f, GL_QUADS, GL_LINE_LOOP, GL_LINES, glLineWidth, glDisable, glEnable, GL_DEPTH_TEST - # Disable depth testing for 2D rendering glDisable(GL_DEPTH_TEST) @@ -143,10 +147,6 @@ class PageLayout: def _render_snap_lines(self, dpi: int, page_x: float, page_y: float, project=None): """Render snap lines (grid, edges, guides)""" - from OpenGL.GL import (glColor3f, glColor4f, glLineWidth, glBegin, glEnd, - glVertex2f, GL_LINES, glEnable, glDisable, GL_BLEND, - glBlendFunc, GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA) - # Use project settings if available, otherwise fall back to local snapping_system if project: # Use project-level global settings diff --git a/pyPhotoAlbum/page_renderer.py b/pyPhotoAlbum/page_renderer.py index 7273de4..25fba26 100644 --- a/pyPhotoAlbum/page_renderer.py +++ b/pyPhotoAlbum/page_renderer.py @@ -11,7 +11,7 @@ Coordinate Systems: """ from typing import Tuple, Optional -from OpenGL.GL import glPushMatrix, glPopMatrix, glScalef, glTranslatef +from pyPhotoAlbum.gl_imports import glPushMatrix, glPopMatrix, glScalef, glTranslatef class PageRenderer: diff --git a/pyPhotoAlbum/pdf_exporter.py b/pyPhotoAlbum/pdf_exporter.py index c9757d6..64b6855 100644 --- a/pyPhotoAlbum/pdf_exporter.py +++ b/pyPhotoAlbum/pdf_exporter.py @@ -14,6 +14,12 @@ from reportlab.lib.enums import TA_LEFT, TA_CENTER, TA_RIGHT from PIL import Image import math from pyPhotoAlbum.models import ImageData, TextBoxData, PlaceholderData +from pyPhotoAlbum.image_utils import ( + apply_pil_rotation, + convert_to_rgba, + calculate_center_crop_coords, + crop_image_to_coords, +) @dataclass @@ -429,73 +435,36 @@ class PDFExporter: try: # Load image using resolved path img = Image.open(image_full_path) - img = img.convert('RGBA') + img = convert_to_rgba(img) - # Apply PIL-level rotation if needed (same logic as _on_async_image_loaded in models.py) + # Apply PIL-level rotation if needed if hasattr(ctx.image_element, 'pil_rotation_90') and ctx.image_element.pil_rotation_90 > 0: - # Rotate counter-clockwise by 90° * pil_rotation_90 - # PIL.Image.ROTATE_90 rotates counter-clockwise - angle = ctx.image_element.pil_rotation_90 * 90 - if angle == 90: - img = img.transpose(Image.ROTATE_270) # CCW 90 = rotate right - elif angle == 180: - img = img.transpose(Image.ROTATE_180) - elif angle == 270: - img = img.transpose(Image.ROTATE_90) # CCW 270 = rotate left + img = apply_pil_rotation(img, ctx.image_element.pil_rotation_90) - # Apply element's crop_info (from the element's own cropping) + # Get element's crop_info and combine with split cropping if applicable crop_x_min, crop_y_min, crop_x_max, crop_y_max = ctx.image_element.crop_info - - # Combine with split cropping if applicable final_crop_x_min = crop_x_min + (crop_x_max - crop_x_min) * ctx.crop_left final_crop_x_max = crop_x_min + (crop_x_max - crop_x_min) * ctx.crop_right - - # Calculate pixel crop coordinates - img_width, img_height = img.size - - # Apply center crop first (matching the render logic in models.py) - img_aspect = img_width / img_height - # Use original dimensions for aspect ratio if provided (for split images) - # This prevents stretching when splitting an image across pages + + # Determine target dimensions for aspect ratio + # Use original dimensions for split images to prevent stretching if ctx.original_width_pt is not None and ctx.original_height_pt is not None: - target_aspect = ctx.original_width_pt / ctx.original_height_pt + target_width = ctx.original_width_pt + target_height = ctx.original_height_pt else: - target_aspect = ctx.width_pt / ctx.height_pt - - if img_aspect > target_aspect: - # Image is wider - crop horizontally - scale = target_aspect / img_aspect - tx_offset = (1.0 - scale) / 2.0 - tx_min_base = tx_offset - tx_max_base = 1.0 - tx_offset - ty_min_base = 0.0 - ty_max_base = 1.0 - else: - # Image is taller - crop vertically - scale = img_aspect / target_aspect - ty_offset = (1.0 - scale) / 2.0 - tx_min_base = 0.0 - tx_max_base = 1.0 - ty_min_base = ty_offset - ty_max_base = 1.0 - ty_offset - - # Apply element crop_info range - tx_range = tx_max_base - tx_min_base - ty_range = ty_max_base - ty_min_base - - tx_min = tx_min_base + final_crop_x_min * tx_range - tx_max = tx_min_base + final_crop_x_max * tx_range - ty_min = ty_min_base + crop_y_min * ty_range - ty_max = ty_min_base + crop_y_max * ty_range - - # Convert to pixel coordinates - crop_left_px = int(tx_min * img_width) - crop_right_px = int(tx_max * img_width) - crop_top_px = int(ty_min * img_height) - crop_bottom_px = int(ty_max * img_height) - + target_width = ctx.width_pt + target_height = ctx.height_pt + + # Calculate center crop coordinates + img_width, img_height = img.size + crop_coords = calculate_center_crop_coords( + img_width, img_height, + target_width, target_height, + (final_crop_x_min, crop_y_min, final_crop_x_max, crop_y_max) + ) + # Crop the image - cropped_img = img.crop((crop_left_px, crop_top_px, crop_right_px, crop_bottom_px)) + cropped_img = crop_image_to_coords(img, crop_coords) # Downsample image to target resolution based on export DPI # This prevents embedding huge images and reduces PDF file size diff --git a/pyPhotoAlbum/snapping.py b/pyPhotoAlbum/snapping.py index a72dba1..5f37be3 100644 --- a/pyPhotoAlbum/snapping.py +++ b/pyPhotoAlbum/snapping.py @@ -3,6 +3,7 @@ Snapping system for pyPhotoAlbum Provides grid snapping, edge snapping, and custom guide snapping """ +import math from typing import List, Tuple, Optional from dataclasses import dataclass @@ -93,8 +94,6 @@ class SnappingSystem: Returns: Snapped position (x, y) in pixels """ - import math - x, y = position width, height = size page_width_mm, page_height_mm = page_size diff --git a/pyPhotoAlbum/version_manager.py b/pyPhotoAlbum/version_manager.py index b65cfe0..84b073e 100644 --- a/pyPhotoAlbum/version_manager.py +++ b/pyPhotoAlbum/version_manager.py @@ -2,8 +2,10 @@ Version management and migration system for pyPhotoAlbum projects """ -from typing import Dict, Any, Optional, Callable, List import os +import uuid +from datetime import datetime, timezone +from typing import Dict, Any, Optional, Callable, List # Current data version - increment when making breaking changes to data format @@ -193,9 +195,6 @@ def migrate_2_0_to_3_0(data: Dict[str, Any]) -> Dict[str, Any]: - Add project_id to project - Add deletion tracking (deleted, deleted_at) to pages and elements """ - import uuid - from datetime import datetime, timezone - print("Migration 2.0 → 3.0: Adding UUIDs, timestamps, and project_id") # Get current timestamp for migration diff --git a/tests/test_asset_drop_mixin.py b/tests/test_asset_drop_mixin.py index 2225a4c..83e3f66 100755 --- a/tests/test_asset_drop_mixin.py +++ b/tests/test_asset_drop_mixin.py @@ -8,6 +8,7 @@ from PyQt6.QtCore import QMimeData, QUrl, QPoint from PyQt6.QtGui import QDragEnterEvent, QDragMoveEvent, QDropEvent from PyQt6.QtOpenGLWidgets import QOpenGLWidget from pyPhotoAlbum.mixins.asset_drop import AssetDropMixin +from pyPhotoAlbum.mixins.asset_path import AssetPathMixin from pyPhotoAlbum.mixins.viewport import ViewportMixin from pyPhotoAlbum.mixins.page_navigation import PageNavigationMixin from pyPhotoAlbum.project import Project, Page @@ -16,14 +17,21 @@ from pyPhotoAlbum.models import ImageData # Create test widget combining necessary mixins -class TestAssetDropWidget(AssetDropMixin, PageNavigationMixin, ViewportMixin, QOpenGLWidget): - """Test widget combining asset drop, page navigation, and viewport mixins""" +class TestAssetDropWidget(AssetDropMixin, AssetPathMixin, PageNavigationMixin, ViewportMixin, QOpenGLWidget): + """Test widget combining asset drop, asset path, page navigation, and viewport mixins""" def _get_element_at(self, x, y): """Mock implementation for testing""" # Will be overridden in tests that need it return None + def _get_project_folder(self): + """Override to access project via window mock""" + main_window = self.window() + if hasattr(main_window, 'project') and main_window.project: + return getattr(main_window.project, 'folder_path', None) + return None + class TestAssetDropInitialization: """Test AssetDropMixin initialization""" diff --git a/tests/test_element_ops_mixin.py b/tests/test_element_ops_mixin.py index 35dd3e2..dcae6b4 100755 --- a/tests/test_element_ops_mixin.py +++ b/tests/test_element_ops_mixin.py @@ -6,6 +6,7 @@ import pytest from unittest.mock import Mock, MagicMock, patch, mock_open from PyQt6.QtWidgets import QMainWindow, QFileDialog from pyPhotoAlbum.mixins.operations.element_ops import ElementOperationsMixin +from pyPhotoAlbum.mixins.asset_path import AssetPathMixin from pyPhotoAlbum.models import ImageData, TextBoxData, PlaceholderData from pyPhotoAlbum.project import Project, Page from pyPhotoAlbum.page_layout import PageLayout @@ -13,7 +14,7 @@ from pyPhotoAlbum.commands import CommandHistory # Create test window with ElementOperationsMixin -class TestElementWindow(ElementOperationsMixin, QMainWindow): +class TestElementWindow(ElementOperationsMixin, AssetPathMixin, QMainWindow): """Test window with element operations mixin""" def __init__(self): @@ -23,10 +24,10 @@ class TestElementWindow(ElementOperationsMixin, QMainWindow): self.gl_widget = Mock() # Mock project - self.project = Mock() - self.project.history = CommandHistory() - self.project.asset_manager = Mock() - self.project.folder_path = "/tmp/test_project" + self._project = Mock() + self._project.history = CommandHistory() + self._project.asset_manager = Mock() + self._project.folder_path = "/tmp/test_project" # Track method calls self._update_view_called = False @@ -35,6 +36,10 @@ class TestElementWindow(ElementOperationsMixin, QMainWindow): self._require_page_called = False self._current_page_index = 0 + @property + def project(self): + return self._project + def require_page(self): """Track require_page calls""" self._require_page_called = True