From d7786ede805f8264a5b746ceb635cc3775906424 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 27 Nov 2025 21:33:53 +0100 Subject: [PATCH] centralised image loading logic --- pyPhotoAlbum/async_backend.py | 41 +++++++++++++ pyPhotoAlbum/mixins/asset_drop.py | 22 +++---- pyPhotoAlbum/mixins/operations/element_ops.py | 21 +++---- pyPhotoAlbum/models.py | 27 +++------ tests/test_element_ops_mixin.py | 58 ++++++++----------- 5 files changed, 91 insertions(+), 78 deletions(-) diff --git a/pyPhotoAlbum/async_backend.py b/pyPhotoAlbum/async_backend.py index 0e19099..55d50fc 100644 --- a/pyPhotoAlbum/async_backend.py +++ b/pyPhotoAlbum/async_backend.py @@ -32,6 +32,47 @@ class LoadPriority(Enum): URGENT = 3 # User is actively interacting with +def get_image_dimensions( + image_path: str, + max_size: Optional[int] = None +) -> Optional[Tuple[int, int]]: + """ + Extract image dimensions without loading the full image. + + Uses PIL's lazy loading to read only the header, making this a fast + operation suitable for UI code that needs dimensions before async loading. + + Args: + image_path: Path to the image file (absolute or relative) + max_size: Optional maximum dimension - if provided, dimensions are + scaled down proportionally to fit within this limit + + Returns: + Tuple of (width, height) or None if the image cannot be read + + Example: + # Get raw dimensions + dims = get_image_dimensions("/path/to/image.jpg") + + # Get dimensions scaled to fit within 300px + dims = get_image_dimensions("/path/to/image.jpg", max_size=300) + """ + try: + with Image.open(image_path) as img: + width, height = img.size + + if max_size and (width > max_size or height > max_size): + scale = min(max_size / width, max_size / height) + width = int(width * scale) + height = int(height * scale) + + return (width, height) + + except Exception as e: + logger.warning(f"Could not extract dimensions for {image_path}: {e}") + return None + + @dataclass(order=True) class LoadRequest: """Request to load and process an image.""" diff --git a/pyPhotoAlbum/mixins/asset_drop.py b/pyPhotoAlbum/mixins/asset_drop.py index 96a1b6d..c0e0bfa 100644 --- a/pyPhotoAlbum/mixins/asset_drop.py +++ b/pyPhotoAlbum/mixins/asset_drop.py @@ -128,22 +128,16 @@ class AssetDropMixin: print(f"Error importing dropped image: {e}") def _calculate_image_dimensions(self, image_path): - """Calculate scaled image dimensions for new image""" - try: - from PIL import Image - img = Image.open(image_path) - img_width, img_height = img.size + """Calculate scaled image dimensions for new image using centralized utility.""" + from pyPhotoAlbum.async_backend import get_image_dimensions - max_size = 300 - if img_width > max_size or img_height > max_size: - scale = min(max_size / img_width, max_size / img_height) - img_width = int(img_width * scale) - img_height = int(img_height * scale) + # Use centralized utility (max 300px for UI display) + dimensions = get_image_dimensions(image_path, max_size=300) + if dimensions: + return dimensions - return img_width, img_height - except Exception as e: - print(f"Error loading image dimensions: {e}") - return 200, 150 + # Fallback dimensions if image cannot be read + return 200, 150 def _add_new_image_to_page(self, asset_path, target_page, page_index, page_renderer, x, y, img_width, img_height, main_window): diff --git a/pyPhotoAlbum/mixins/operations/element_ops.py b/pyPhotoAlbum/mixins/operations/element_ops.py index c7cc499..a8d7ac4 100644 --- a/pyPhotoAlbum/mixins/operations/element_ops.py +++ b/pyPhotoAlbum/mixins/operations/element_ops.py @@ -3,10 +3,10 @@ Element operations mixin for pyPhotoAlbum """ from PyQt6.QtWidgets import QFileDialog -from PIL import Image from pyPhotoAlbum.decorators import ribbon_action from pyPhotoAlbum.models import ImageData, TextBoxData, PlaceholderData from pyPhotoAlbum.commands import AddElementCommand +from pyPhotoAlbum.async_backend import get_image_dimensions class ElementOperationsMixin: @@ -43,18 +43,15 @@ class ElementOperationsMixin: # Import asset to project asset_path = self.project.asset_manager.import_asset(file_path) - # Load image from imported asset (not from original source) + # Get dimensions using centralized utility (max 300px for UI display) full_asset_path = os.path.join(self.project.folder_path, asset_path) - img = Image.open(full_asset_path) - img_width, img_height = img.size - - # Scale to reasonable size (max 300px) - max_size = 300 - if img_width > max_size or img_height > max_size: - scale = min(max_size / img_width, max_size / img_height) - img_width = int(img_width * scale) - img_height = int(img_height * scale) - + dimensions = get_image_dimensions(full_asset_path, max_size=300) + if dimensions: + img_width, img_height = dimensions + else: + # Fallback dimensions if image cannot be read + img_width, img_height = 200, 150 + # Create image element at center of page page_width_mm = current_page.layout.size[0] page_height_mm = current_page.layout.size[1] diff --git a/pyPhotoAlbum/models.py b/pyPhotoAlbum/models.py index 4b70430..d81d1b8 100644 --- a/pyPhotoAlbum/models.py +++ b/pyPhotoAlbum/models.py @@ -158,27 +158,16 @@ class ImageData(BaseLayoutElement): def _extract_dimensions_metadata(self): """ Extract image dimensions without loading the full image. - Uses PIL's lazy loading to just read the header. + Uses the centralized get_image_dimensions() utility. """ - try: - image_path = self.resolve_image_path() - if image_path: - # Use PIL to just read dimensions (fast, doesn't load pixel data) - with Image.open(image_path) as img: - width, height = img.width, img.height + from pyPhotoAlbum.async_backend import get_image_dimensions - # Apply same downsampling logic as the old sync code (max 2048px) - max_size = 2048 - if width > max_size or height > max_size: - scale = min(max_size / width, max_size / height) - width = int(width * scale) - height = int(height * scale) - - self.image_dimensions = (width, height) - print(f"ImageData: Extracted dimensions {self.image_dimensions} for {self.image_path}") - except Exception as e: - print(f"ImageData: Could not extract dimensions for {self.image_path}: {e}") - self.image_dimensions = None + image_path = self.resolve_image_path() + if image_path: + # Use centralized utility (max 2048px for texture loading) + self.image_dimensions = get_image_dimensions(image_path, max_size=2048) + if self.image_dimensions: + print(f"ImageData: Extracted dimensions {self.image_dimensions} for {self.image_path}") def render(self): """Render the image using OpenGL""" diff --git a/tests/test_element_ops_mixin.py b/tests/test_element_ops_mixin.py index e89557b..35dd3e2 100755 --- a/tests/test_element_ops_mixin.py +++ b/tests/test_element_ops_mixin.py @@ -10,8 +10,6 @@ from pyPhotoAlbum.models import ImageData, TextBoxData, PlaceholderData from pyPhotoAlbum.project import Project, Page from pyPhotoAlbum.page_layout import PageLayout from pyPhotoAlbum.commands import CommandHistory -from PIL import Image -import io # Create test window with ElementOperationsMixin @@ -69,8 +67,8 @@ class TestAddImage: """Test add_image method""" @patch('pyPhotoAlbum.mixins.operations.element_ops.QFileDialog.getOpenFileName') - @patch('pyPhotoAlbum.mixins.operations.element_ops.Image.open') - def test_add_image_success(self, mock_image_open, mock_file_dialog, qtbot): + @patch('pyPhotoAlbum.mixins.operations.element_ops.get_image_dimensions') + def test_add_image_success(self, mock_get_dims, mock_file_dialog, qtbot): """Test successfully adding an image""" window = TestElementWindow() qtbot.addWidget(window) @@ -85,10 +83,8 @@ class TestAddImage: # Mock file dialog mock_file_dialog.return_value = ("/path/to/image.jpg", "Image Files (*.jpg)") - # Mock PIL Image - mock_img = Mock() - mock_img.size = (800, 600) - mock_image_open.return_value = mock_img + # Mock get_image_dimensions (returns scaled dimensions) + mock_get_dims.return_value = (300, 225) # 800x600 scaled to max 300 # Mock asset manager window.project.asset_manager.import_asset.return_value = "assets/image.jpg" @@ -139,8 +135,8 @@ class TestAddImage: assert not window._update_view_called @patch('pyPhotoAlbum.mixins.operations.element_ops.QFileDialog.getOpenFileName') - @patch('pyPhotoAlbum.mixins.operations.element_ops.Image.open') - def test_add_image_scales_large_image(self, mock_image_open, mock_file_dialog, qtbot): + @patch('pyPhotoAlbum.mixins.operations.element_ops.get_image_dimensions') + def test_add_image_scales_large_image(self, mock_get_dims, mock_file_dialog, qtbot): """Test that large images are scaled down""" window = TestElementWindow() qtbot.addWidget(window) @@ -153,22 +149,20 @@ class TestAddImage: mock_file_dialog.return_value = ("/path/to/large.jpg", "Image Files (*.jpg)") - # Mock very large image - mock_img = Mock() - mock_img.size = (3000, 2000) # Much larger than max_size=300 - mock_image_open.return_value = mock_img + # Mock get_image_dimensions returning scaled dimensions (3000x2000 -> 300x200) + mock_get_dims.return_value = (300, 200) window.project.asset_manager.import_asset.return_value = "assets/large.jpg" window.add_image() - # Image should be added (scaled down internally) + # Image should be added (scaled down by get_image_dimensions) assert window._update_view_called @patch('pyPhotoAlbum.mixins.operations.element_ops.QFileDialog.getOpenFileName') - @patch('pyPhotoAlbum.mixins.operations.element_ops.Image.open') - def test_add_image_error_handling(self, mock_image_open, mock_file_dialog, qtbot): - """Test error handling when adding image fails""" + @patch('pyPhotoAlbum.mixins.operations.element_ops.get_image_dimensions') + def test_add_image_fallback_dimensions(self, mock_get_dims, mock_file_dialog, qtbot): + """Test fallback dimensions when get_image_dimensions returns None""" window = TestElementWindow() qtbot.addWidget(window) @@ -180,14 +174,16 @@ class TestAddImage: mock_file_dialog.return_value = ("/path/to/broken.jpg", "Image Files (*.jpg)") - # Mock error - mock_image_open.side_effect = Exception("Cannot open image") + # Mock get_image_dimensions returning None (image unreadable) + mock_get_dims.return_value = None + + window.project.asset_manager.import_asset.return_value = "assets/broken.jpg" window.add_image() - # Should show error - assert window._error_message is not None - assert "failed to add image" in window._error_message.lower() + # Should still add image with fallback dimensions (200x150) + assert window._update_view_called + assert window.project.history.can_undo() class TestAddText: @@ -294,8 +290,8 @@ class TestElementOperationsIntegration: """Test integration between element operations""" @patch('pyPhotoAlbum.mixins.operations.element_ops.QFileDialog.getOpenFileName') - @patch('pyPhotoAlbum.mixins.operations.element_ops.Image.open') - def test_add_multiple_elements(self, mock_image_open, mock_file_dialog, qtbot): + @patch('pyPhotoAlbum.mixins.operations.element_ops.get_image_dimensions') + def test_add_multiple_elements(self, mock_get_dims, mock_file_dialog, qtbot): """Test adding multiple different element types""" window = TestElementWindow() qtbot.addWidget(window) @@ -317,9 +313,7 @@ class TestElementOperationsIntegration: # Add image mock_file_dialog.return_value = ("/test.jpg", "Image Files") - mock_img = Mock() - mock_img.size = (100, 100) - mock_image_open.return_value = mock_img + mock_get_dims.return_value = (100, 100) window.project.asset_manager.import_asset.return_value = "assets/test.jpg" window.add_image() @@ -328,8 +322,8 @@ class TestElementOperationsIntegration: assert window._update_view_called @patch('pyPhotoAlbum.mixins.operations.element_ops.QFileDialog.getOpenFileName') - @patch('pyPhotoAlbum.mixins.operations.element_ops.Image.open') - def test_add_image_with_undo(self, mock_image_open, mock_file_dialog, qtbot): + @patch('pyPhotoAlbum.mixins.operations.element_ops.get_image_dimensions') + def test_add_image_with_undo(self, mock_get_dims, mock_file_dialog, qtbot): """Test that adding image can be undone""" window = TestElementWindow() qtbot.addWidget(window) @@ -341,9 +335,7 @@ class TestElementOperationsIntegration: window._current_page = page mock_file_dialog.return_value = ("/test.jpg", "Image Files") - mock_img = Mock() - mock_img.size = (200, 150) - mock_image_open.return_value = mock_img + mock_get_dims.return_value = (200, 150) window.project.asset_manager.import_asset.return_value = "assets/test.jpg" # Should have no commands initially