From f0aa005d8c15c90dd986e9fa8fad92182adba6fd Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 1 Jan 2026 18:42:58 +0100 Subject: [PATCH] refactor to reduce complexity of nesting --- pyPhotoAlbum/mixins/mouse_interaction.py | 377 +++++++++--------- .../mixins/operations/alignment_ops.py | 85 ++-- .../mixins/operations/distribution_ops.py | 63 ++- pyPhotoAlbum/mixins/operations/size_ops.py | 149 +++---- pyPhotoAlbum/mixins/viewport.py | 200 +++++----- pyPhotoAlbum/snapping.py | 68 ---- tests/test_snapping_system.py | 45 --- 7 files changed, 393 insertions(+), 594 deletions(-) diff --git a/pyPhotoAlbum/mixins/mouse_interaction.py b/pyPhotoAlbum/mixins/mouse_interaction.py index eeb69a8..a47aab4 100644 --- a/pyPhotoAlbum/mixins/mouse_interaction.py +++ b/pyPhotoAlbum/mixins/mouse_interaction.py @@ -25,210 +25,227 @@ class MouseInteractionMixin: self.is_dragging = False self.is_panning = False + def _handle_rotation_start(self, x: float, y: float): + """Start rotation interaction for selected element.""" + self._begin_rotate(self.selected_element) + self.drag_start_pos = (x, y) + self.rotation_start_angle = self.selected_element.rotation + self.is_dragging = True + + def _handle_resize_start(self, x: float, y: float, handle): + """Start resize interaction for selected element.""" + self._begin_resize(self.selected_element) + self.resize_handle = handle + self.drag_start_pos = (x, y) + self.resize_start_pos = self.selected_element.position + self.resize_start_size = self.selected_element.size + self.is_dragging = True + + def _handle_image_pan_start(self, x: float, y: float, element): + """Start image pan mode for an ImageData element.""" + self.selected_elements = {element} + self.drag_start_pos = (x, y) + self.image_pan_mode = True + self.image_pan_start_crop = element.crop_info + self._begin_image_pan(element) + self.is_dragging = True + self.setCursor(Qt.CursorShape.SizeAllCursor) + + def _handle_multi_select(self, element): + """Toggle element in multi-selection.""" + if element in self.selected_elements: + self.selected_elements.remove(element) + else: + self.selected_elements.add(element) + + def _handle_element_drag_start(self, x: float, y: float, element): + """Start dragging an element.""" + self.selected_elements = {element} + self.drag_start_pos = (x, y) + self.drag_start_element_pos = element.position + if not self.rotation_mode: + self._begin_move(element) + self.is_dragging = True + def mousePressEvent(self, event): """Handle mouse press events""" - # Ensure widget has focus for keyboard events self.setFocus() if event.button() == Qt.MouseButton.LeftButton: - x, y = event.position().x(), event.position().y() - ctrl_pressed = event.modifiers() & Qt.KeyboardModifier.ControlModifier - shift_pressed = event.modifiers() & Qt.KeyboardModifier.ShiftModifier - - # Check if clicking on ghost page button - if self._check_ghost_page_click(x, y): - return - - # Update current_page_index based on where user clicked - page, page_index, renderer = self._get_page_at(x, y) - if page_index >= 0: - self.current_page_index = page_index - - if len(self.selected_elements) == 1 and self.selected_element: - if self.rotation_mode: - # In rotation mode, start rotation tracking - self._begin_rotate(self.selected_element) - self.drag_start_pos = (x, y) - self.rotation_start_angle = self.selected_element.rotation - self.is_dragging = True - return - else: - # In normal mode, check for resize handles - handle = self._get_resize_handle_at(x, y) - if handle: - self._begin_resize(self.selected_element) - self.resize_handle = handle - self.drag_start_pos = (x, y) - self.resize_start_pos = self.selected_element.position - self.resize_start_size = self.selected_element.size - self.is_dragging = True - return - - element = self._get_element_at(x, y) - if element: - print( - f"DEBUG: Clicked on element: {element}, ctrl_pressed: {ctrl_pressed}, shift_pressed: {shift_pressed}" - ) - # Check if Ctrl is pressed and element is ImageData - enter image pan mode - if ctrl_pressed and isinstance(element, ImageData) and not self.rotation_mode: - # Enter image pan mode - pan image within frame - self.selected_elements = {element} - self.drag_start_pos = (x, y) - self.image_pan_mode = True - self.image_pan_start_crop = element.crop_info - self._begin_image_pan(element) - self.is_dragging = True - self.setCursor(Qt.CursorShape.SizeAllCursor) - print(f"Entered image pan mode for {element}") - elif ctrl_pressed: - # Multi-select mode (for non-ImageData elements or when Ctrl is pressed) - print(f"DEBUG: Multi-select mode triggered") - if element in self.selected_elements: - print(f"DEBUG: Removing element from selection") - self.selected_elements.remove(element) - else: - print(f"DEBUG: Adding element to selection. Current count: {len(self.selected_elements)}") - self.selected_elements.add(element) - print(f"DEBUG: Total selected elements: {len(self.selected_elements)}") - elif shift_pressed: - # Shift can be used for multi-select as well - if element in self.selected_elements: - self.selected_elements.remove(element) - else: - self.selected_elements.add(element) - else: - # Normal drag mode - print(f"DEBUG: Normal drag mode - single selection") - self.selected_elements = {element} - self.drag_start_pos = (x, y) - self.drag_start_element_pos = element.position - if not self.rotation_mode: - self._begin_move(element) - self.is_dragging = True - else: - if not ctrl_pressed: - self.selected_elements.clear() - - self.update() - + self._handle_left_click(event) elif event.button() == Qt.MouseButton.MiddleButton: self.is_panning = True self.drag_start_pos = (event.position().x(), event.position().y()) self.setCursor(Qt.CursorShape.ClosedHandCursor) + def _handle_left_click(self, event): + """Handle left mouse button click.""" + x, y = event.position().x(), event.position().y() + ctrl_pressed = event.modifiers() & Qt.KeyboardModifier.ControlModifier + shift_pressed = event.modifiers() & Qt.KeyboardModifier.ShiftModifier + + # Check if clicking on ghost page button + if self._check_ghost_page_click(x, y): + return + + # Update current_page_index based on where user clicked + page, page_index, renderer = self._get_page_at(x, y) + if page_index >= 0: + self.current_page_index = page_index + + # Handle interaction with already-selected element + if len(self.selected_elements) == 1 and self.selected_element: + if self.rotation_mode: + self._handle_rotation_start(x, y) + return + else: + handle = self._get_resize_handle_at(x, y) + if handle: + self._handle_resize_start(x, y, handle) + return + + # Handle click on element + element = self._get_element_at(x, y) + if element: + if ctrl_pressed and isinstance(element, ImageData) and not self.rotation_mode: + self._handle_image_pan_start(x, y, element) + elif ctrl_pressed or shift_pressed: + self._handle_multi_select(element) + else: + self._handle_element_drag_start(x, y, element) + else: + if not ctrl_pressed: + self.selected_elements.clear() + + self.update() + + def _handle_canvas_pan(self, x: float, y: float): + """Handle canvas panning with middle mouse button.""" + dx = x - self.drag_start_pos[0] + dy = y - self.drag_start_pos[1] + + self.pan_offset[0] += dx + self.pan_offset[1] += dy + + if hasattr(self, "clamp_pan_offset"): + self.clamp_pan_offset() + + self.drag_start_pos = (x, y) + self.update() + + main_window = self.window() + if hasattr(main_window, "update_scrollbars"): + main_window.update_scrollbars() + + def _handle_rotation_move(self, x: float, y: float): + """Handle element rotation during drag.""" + if not hasattr(self.selected_element, "_page_renderer"): + return + + renderer = self.selected_element._page_renderer + elem_x, elem_y = self.selected_element.position + elem_w, elem_h = self.selected_element.size + + center_page_x = elem_x + elem_w / 2 + center_page_y = elem_y + elem_h / 2 + screen_center_x, screen_center_y = renderer.page_to_screen(center_page_x, center_page_y) + + dx = x - screen_center_x + dy = y - screen_center_y + angle = math.degrees(math.atan2(dy, dx)) + angle = round(angle / self.rotation_snap_angle) * self.rotation_snap_angle + angle = angle % 360 + + self.selected_element.rotation = angle + + main_window = self.window() + if hasattr(main_window, "show_status"): + main_window.show_status(f"Rotation: {angle:.1f}°", 100) + + def _handle_resize_move(self, x: float, y: float): + """Handle element resize during drag.""" + screen_dx = x - self.drag_start_pos[0] + screen_dy = y - self.drag_start_pos[1] + + total_dx = screen_dx / self.zoom_level + total_dy = screen_dy / self.zoom_level + + self._resize_element(total_dx, total_dy) + + def _handle_element_move(self, x: float, y: float): + """Handle element movement during drag, including page transfer.""" + current_page, current_page_index, current_renderer = self._get_page_at(x, y) + + if current_page and hasattr(self.selected_element, "_parent_page"): + source_page = self.selected_element._parent_page + + if current_page is not source_page: + self._transfer_element_to_page( + self.selected_element, source_page, current_page, x, y, current_renderer + ) + else: + self._move_element_within_page(x, y, source_page) + else: + # No page context - simple move without snapping + total_dx = (x - self.drag_start_pos[0]) / self.zoom_level + total_dy = (y - self.drag_start_pos[1]) / self.zoom_level + + new_x = self.drag_start_element_pos[0] + total_dx + new_y = self.drag_start_element_pos[1] + total_dy + + self.selected_element.position = (new_x, new_y) + + def _move_element_within_page(self, x: float, y: float, page): + """Move element within its current page with snapping.""" + total_dx = (x - self.drag_start_pos[0]) / self.zoom_level + total_dy = (y - self.drag_start_pos[1]) / self.zoom_level + + new_x = self.drag_start_element_pos[0] + total_dx + new_y = self.drag_start_element_pos[1] + total_dy + + main_window = self.window() + snap_sys = page.layout.snapping_system + page_size = page.layout.size + dpi = main_window.project.working_dpi + + snapped_pos = snap_sys.snap_position( + position=(new_x, new_y), + size=self.selected_element.size, + page_size=page_size, + dpi=dpi, + project=main_window.project, + ) + + self.selected_element.position = snapped_pos + def mouseMoveEvent(self, event): """Handle mouse move events""" x, y = event.position().x(), event.position().y() - # Update status bar with page information self._update_page_status(x, y) + # Canvas panning (middle mouse button) if self.is_panning and self.drag_start_pos: - dx = x - self.drag_start_pos[0] - dy = y - self.drag_start_pos[1] - - self.pan_offset[0] += dx - self.pan_offset[1] += dy - - # Clamp pan offset to content bounds - if hasattr(self, "clamp_pan_offset"): - self.clamp_pan_offset() - - self.drag_start_pos = (x, y) - self.update() - - # Update scrollbars if available - main_window = self.window() - if hasattr(main_window, "update_scrollbars"): - main_window.update_scrollbars() - + self._handle_canvas_pan(x, y) return if not self.is_dragging or not self.drag_start_pos: return - if self.selected_element: - if self.image_pan_mode: - # Image pan mode - delegate to ImagePanMixin - self._handle_image_pan_move(x, y, self.selected_element) + if not self.selected_element: + return - elif self.rotation_mode: - # Rotation mode - if not hasattr(self.selected_element, "_page_renderer"): - return + # Dispatch to appropriate handler based on interaction mode + if self.image_pan_mode: + self._handle_image_pan_move(x, y, self.selected_element) + elif self.rotation_mode: + self._handle_rotation_move(x, y) + elif self.resize_handle: + self._handle_resize_move(x, y) + else: + self._handle_element_move(x, y) - renderer = self.selected_element._page_renderer - elem_x, elem_y = self.selected_element.position - elem_w, elem_h = self.selected_element.size - - center_page_x = elem_x + elem_w / 2 - center_page_y = elem_y + elem_h / 2 - screen_center_x, screen_center_y = renderer.page_to_screen(center_page_x, center_page_y) - - dx = x - screen_center_x - dy = y - screen_center_y - angle = math.degrees(math.atan2(dy, dx)) - - angle = round(angle / self.rotation_snap_angle) * self.rotation_snap_angle - angle = angle % 360 - - self.selected_element.rotation = angle - - main_window = self.window() - if hasattr(main_window, "show_status"): - main_window.show_status(f"Rotation: {angle:.1f}°", 100) - - elif self.resize_handle: - # Resize mode - screen_dx = x - self.drag_start_pos[0] - screen_dy = y - self.drag_start_pos[1] - - total_dx = screen_dx / self.zoom_level - total_dy = screen_dy / self.zoom_level - - self._resize_element(total_dx, total_dy) - else: - # Move mode - current_page, current_page_index, current_renderer = self._get_page_at(x, y) - - if current_page and hasattr(self.selected_element, "_parent_page"): - source_page = self.selected_element._parent_page - - if current_page is not source_page: - self._transfer_element_to_page( - self.selected_element, source_page, current_page, x, y, current_renderer - ) - else: - total_dx = (x - self.drag_start_pos[0]) / self.zoom_level - total_dy = (y - self.drag_start_pos[1]) / self.zoom_level - - new_x = self.drag_start_element_pos[0] + total_dx - new_y = self.drag_start_element_pos[1] + total_dy - - main_window = self.window() - snap_sys = source_page.layout.snapping_system - page_size = source_page.layout.size - dpi = main_window.project.working_dpi - - snapped_pos = snap_sys.snap_position( - position=(new_x, new_y), - size=self.selected_element.size, - page_size=page_size, - dpi=dpi, - project=main_window.project, - ) - - self.selected_element.position = snapped_pos - else: - total_dx = (x - self.drag_start_pos[0]) / self.zoom_level - total_dy = (y - self.drag_start_pos[1]) / self.zoom_level - - new_x = self.drag_start_element_pos[0] + total_dx - new_y = self.drag_start_element_pos[1] + total_dy - - self.selected_element.position = (new_x, new_y) - - self.update() + self.update() def mouseReleaseEvent(self, event): """Handle mouse release events""" diff --git a/pyPhotoAlbum/mixins/operations/alignment_ops.py b/pyPhotoAlbum/mixins/operations/alignment_ops.py index ce4c744..e899abd 100644 --- a/pyPhotoAlbum/mixins/operations/alignment_ops.py +++ b/pyPhotoAlbum/mixins/operations/alignment_ops.py @@ -14,6 +14,25 @@ class AlignmentOperationsMixin: """Get list of selected elements for alignment operations""" return list(self.gl_widget.selected_elements) if self.gl_widget.selected_elements else [] + def _execute_alignment(self, alignment_func, status_msg: str): + """ + Execute an alignment operation with common boilerplate. + + Args: + alignment_func: AlignmentManager method to call with elements + status_msg: Status message format string (will receive element count) + """ + elements = self._get_selected_elements_list() + if not self.require_selection(min_count=2): + return + + changes = alignment_func(elements) + if changes: + cmd = AlignElementsCommand(changes) + self.project.history.execute(cmd) + self.update_view() + self.show_status(status_msg.format(len(elements)), 2000) + @ribbon_action( label="Align Left", tooltip="Align selected elements to the left", @@ -24,16 +43,7 @@ class AlignmentOperationsMixin: ) def align_left(self): """Align selected elements to the left""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.align_left(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Aligned {len(elements)} elements to left", 2000) + self._execute_alignment(AlignmentManager.align_left, "Aligned {} elements to left") @ribbon_action( label="Align Right", @@ -45,16 +55,7 @@ class AlignmentOperationsMixin: ) def align_right(self): """Align selected elements to the right""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.align_right(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Aligned {len(elements)} elements to right", 2000) + self._execute_alignment(AlignmentManager.align_right, "Aligned {} elements to right") @ribbon_action( label="Align Top", @@ -66,16 +67,7 @@ class AlignmentOperationsMixin: ) def align_top(self): """Align selected elements to the top""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.align_top(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Aligned {len(elements)} elements to top", 2000) + self._execute_alignment(AlignmentManager.align_top, "Aligned {} elements to top") @ribbon_action( label="Align Bottom", @@ -87,16 +79,7 @@ class AlignmentOperationsMixin: ) def align_bottom(self): """Align selected elements to the bottom""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.align_bottom(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Aligned {len(elements)} elements to bottom", 2000) + self._execute_alignment(AlignmentManager.align_bottom, "Aligned {} elements to bottom") @ribbon_action( label="Align H-Center", @@ -108,16 +91,7 @@ class AlignmentOperationsMixin: ) def align_horizontal_center(self): """Align selected elements to horizontal center""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.align_horizontal_center(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Aligned {len(elements)} elements to horizontal center", 2000) + self._execute_alignment(AlignmentManager.align_horizontal_center, "Aligned {} elements to horizontal center") @ribbon_action( label="Align V-Center", @@ -129,16 +103,7 @@ class AlignmentOperationsMixin: ) def align_vertical_center(self): """Align selected elements to vertical center""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.align_vertical_center(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Aligned {len(elements)} elements to vertical center", 2000) + self._execute_alignment(AlignmentManager.align_vertical_center, "Aligned {} elements to vertical center") @ribbon_action( label="Maximize Pattern", diff --git a/pyPhotoAlbum/mixins/operations/distribution_ops.py b/pyPhotoAlbum/mixins/operations/distribution_ops.py index 7a07da2..757456c 100644 --- a/pyPhotoAlbum/mixins/operations/distribution_ops.py +++ b/pyPhotoAlbum/mixins/operations/distribution_ops.py @@ -14,6 +14,25 @@ class DistributionOperationsMixin: """Get list of selected elements for distribution operations""" return list(self.gl_widget.selected_elements) if self.gl_widget.selected_elements else [] + def _execute_distribution(self, distribution_func, status_msg: str): + """ + Execute a distribution operation with common boilerplate. + + Args: + distribution_func: AlignmentManager method to call with elements + status_msg: Status message format string (will receive element count) + """ + elements = self._get_selected_elements_list() + if not self.require_selection(min_count=3): + return + + changes = distribution_func(elements) + if changes: + cmd = AlignElementsCommand(changes) + self.project.history.execute(cmd) + self.update_view() + self.show_status(status_msg.format(len(elements)), 2000) + @ribbon_action( label="Distribute H", tooltip="Distribute selected elements evenly horizontally", @@ -24,16 +43,7 @@ class DistributionOperationsMixin: ) def distribute_horizontally(self): """Distribute selected elements evenly horizontally""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=3): - return - - changes = AlignmentManager.distribute_horizontally(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Distributed {len(elements)} elements horizontally", 2000) + self._execute_distribution(AlignmentManager.distribute_horizontally, "Distributed {} elements horizontally") @ribbon_action( label="Distribute V", @@ -45,16 +55,7 @@ class DistributionOperationsMixin: ) def distribute_vertically(self): """Distribute selected elements evenly vertically""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=3): - return - - changes = AlignmentManager.distribute_vertically(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Distributed {len(elements)} elements vertically", 2000) + self._execute_distribution(AlignmentManager.distribute_vertically, "Distributed {} elements vertically") @ribbon_action( label="Space H", @@ -66,16 +67,7 @@ class DistributionOperationsMixin: ) def space_horizontally(self): """Space selected elements equally horizontally""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=3): - return - - changes = AlignmentManager.space_horizontally(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Spaced {len(elements)} elements horizontally", 2000) + self._execute_distribution(AlignmentManager.space_horizontally, "Spaced {} elements horizontally") @ribbon_action( label="Space V", @@ -87,13 +79,4 @@ class DistributionOperationsMixin: ) def space_vertically(self): """Space selected elements equally vertically""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=3): - return - - changes = AlignmentManager.space_vertically(elements) - if changes: - cmd = AlignElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Spaced {len(elements)} elements vertically", 2000) + self._execute_distribution(AlignmentManager.space_vertically, "Spaced {} elements vertically") diff --git a/pyPhotoAlbum/mixins/operations/size_ops.py b/pyPhotoAlbum/mixins/operations/size_ops.py index 39bf4e6..7dd1ba3 100644 --- a/pyPhotoAlbum/mixins/operations/size_ops.py +++ b/pyPhotoAlbum/mixins/operations/size_ops.py @@ -14,6 +14,50 @@ class SizeOperationsMixin: """Get list of selected elements for size operations""" return list(self.gl_widget.selected_elements) if self.gl_widget.selected_elements else [] + def _execute_resize(self, resize_func, status_msg: str): + """ + Execute a resize operation on multiple elements. + + Args: + resize_func: AlignmentManager method to call with elements + status_msg: Status message format string (will receive element count) + """ + elements = self._get_selected_elements_list() + if not self.require_selection(min_count=2): + return + + changes = resize_func(elements) + if changes: + cmd = ResizeElementsCommand(changes) + self.project.history.execute(cmd) + self.update_view() + self.show_status(status_msg.format(len(elements)), 2000) + + def _execute_fit_to_page(self, fit_func, status_msg: str): + """ + Execute a fit-to-page operation on a single element. + + Args: + fit_func: Function that takes (element, page) and returns a change tuple + status_msg: Status message to display on success + """ + if not self.require_selection(min_count=1): + return + + page = self.get_current_page() + if not page: + self.show_warning("No Page", "Please create a page first.") + return + + element = next(iter(self.gl_widget.selected_elements)) + change = fit_func(element, page) + + if change: + cmd = ResizeElementsCommand([change]) + self.project.history.execute(cmd) + self.update_view() + self.show_status(status_msg, 2000) + @ribbon_action( label="Same Size", tooltip="Make all selected elements the same size", @@ -24,16 +68,7 @@ class SizeOperationsMixin: ) def make_same_size(self): """Make all selected elements the same size""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.make_same_size(elements) - if changes: - cmd = ResizeElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Resized {len(elements)} elements to same size", 2000) + self._execute_resize(AlignmentManager.make_same_size, "Resized {} elements to same size") @ribbon_action( label="Same Width", @@ -45,16 +80,7 @@ class SizeOperationsMixin: ) def make_same_width(self): """Make all selected elements the same width""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.make_same_width(elements) - if changes: - cmd = ResizeElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Resized {len(elements)} elements to same width", 2000) + self._execute_resize(AlignmentManager.make_same_width, "Resized {} elements to same width") @ribbon_action( label="Same Height", @@ -66,16 +92,7 @@ class SizeOperationsMixin: ) def make_same_height(self): """Make all selected elements the same height""" - elements = self._get_selected_elements_list() - if not self.require_selection(min_count=2): - return - - changes = AlignmentManager.make_same_height(elements) - if changes: - cmd = ResizeElementsCommand(changes) - self.project.history.execute(cmd) - self.update_view() - self.show_status(f"Resized {len(elements)} elements to same height", 2000) + self._execute_resize(AlignmentManager.make_same_height, "Resized {} elements to same height") @ribbon_action( label="Fit Width", @@ -87,26 +104,10 @@ class SizeOperationsMixin: ) def fit_to_width(self): """Fit selected element to page width""" - if not self.require_selection(min_count=1): - return - - page = self.get_current_page() - if not page: - self.show_warning("No Page", "Please create a page first.") - return - - # Get the first selected element - element = next(iter(self.gl_widget.selected_elements)) - - # Fit to page width - page_width = page.layout.size[0] - change = AlignmentManager.fit_to_page_width(element, page_width) - - if change: - cmd = ResizeElementsCommand([change]) - self.project.history.execute(cmd) - self.update_view() - self.show_status("Fitted element to page width", 2000) + self._execute_fit_to_page( + lambda elem, page: AlignmentManager.fit_to_page_width(elem, page.layout.size[0]), + "Fitted element to page width", + ) @ribbon_action( label="Fit Height", @@ -118,26 +119,10 @@ class SizeOperationsMixin: ) def fit_to_height(self): """Fit selected element to page height""" - if not self.require_selection(min_count=1): - return - - page = self.get_current_page() - if not page: - self.show_warning("No Page", "Please create a page first.") - return - - # Get the first selected element - element = next(iter(self.gl_widget.selected_elements)) - - # Fit to page height - page_height = page.layout.size[1] - change = AlignmentManager.fit_to_page_height(element, page_height) - - if change: - cmd = ResizeElementsCommand([change]) - self.project.history.execute(cmd) - self.update_view() - self.show_status("Fitted element to page height", 2000) + self._execute_fit_to_page( + lambda elem, page: AlignmentManager.fit_to_page_height(elem, page.layout.size[1]), + "Fitted element to page height", + ) @ribbon_action( label="Fit to Page", @@ -149,26 +134,10 @@ class SizeOperationsMixin: ) def fit_to_page(self): """Fit selected element to page dimensions""" - if not self.require_selection(min_count=1): - return - - page = self.get_current_page() - if not page: - self.show_warning("No Page", "Please create a page first.") - return - - # Get the first selected element - element = next(iter(self.gl_widget.selected_elements)) - - # Fit to page - page_width, page_height = page.layout.size - change = AlignmentManager.fit_to_page(element, page_width, page_height) - - if change: - cmd = ResizeElementsCommand([change]) - self.project.history.execute(cmd) - self.update_view() - self.show_status("Fitted element to page", 2000) + self._execute_fit_to_page( + lambda elem, page: AlignmentManager.fit_to_page(elem, page.layout.size[0], page.layout.size[1]), + "Fitted element to page", + ) @ribbon_action( label="Expand Image", diff --git a/pyPhotoAlbum/mixins/viewport.py b/pyPhotoAlbum/mixins/viewport.py index 80bb88d..204bfaa 100644 --- a/pyPhotoAlbum/mixins/viewport.py +++ b/pyPhotoAlbum/mixins/viewport.py @@ -175,6 +175,84 @@ class ViewportMixin: "height": total_height, } + def _clamp_vertical_pan(self, viewport_height: float) -> float: + """Clamp vertical pan offset and return the original value before clamping.""" + bounds = self.get_content_bounds() + content_height = bounds["height"] + + # Save original for page selection (prevents clamping from changing which page we target) + original_pan_y = self.pan_offset[1] + + if content_height > viewport_height: + max_pan_up = 0 # Can't pan beyond top edge + min_pan_up = -(content_height - viewport_height) # Can't pan beyond bottom edge + self.pan_offset[1] = max(min_pan_up, min(max_pan_up, self.pan_offset[1])) + + return original_pan_y + + def _build_page_centerlines(self, pages, dpi: float) -> list: + """Build list of (center_y, center_x, width) tuples for each page.""" + PAGE_MARGIN = 50 + PAGE_SPACING = 50 + + centerlines = [] + current_y = PAGE_MARGIN + + for page in pages: + page_width_mm, page_height_mm = page.layout.size + screen_page_width = page_width_mm * dpi / 25.4 * self.zoom_level + screen_page_height = page_height_mm * dpi / 25.4 * self.zoom_level + + page_center_y = current_y + screen_page_height / 2 + page_center_x = PAGE_MARGIN + screen_page_width / 2 + + centerlines.append((page_center_y, page_center_x, screen_page_width)) + current_y += screen_page_height + PAGE_SPACING + + return centerlines + + def _interpolate_target_centerline(self, centerlines: list, viewport_center_y: float) -> tuple: + """Find target centerline by interpolating between pages based on viewport position.""" + if not centerlines: + return 0, 0 + + # Find the page index we're at or past + page_idx = self._find_page_at_viewport_y(centerlines, viewport_center_y) + + if page_idx == 0: + return centerlines[0][1], centerlines[0][2] + + # Interpolate between previous and current page + prev_y, prev_x, prev_w = centerlines[page_idx - 1] + curr_y, curr_x, curr_w = centerlines[page_idx] + + if curr_y == prev_y: + return curr_x, curr_w + + t = max(0, min(1, (viewport_center_y - prev_y) / (curr_y - prev_y))) + return prev_x + t * (curr_x - prev_x), prev_w + t * (curr_w - prev_w) + + def _find_page_at_viewport_y(self, centerlines: list, viewport_center_y: float) -> int: + """Find index of page at or after viewport Y position.""" + for i, (page_y, _, _) in enumerate(centerlines): + if viewport_center_y <= page_y: + return i + return len(centerlines) - 1 # Below all pages - use last + + def _clamp_horizontal_pan(self, viewport_width: float, target_centerline_x: float, target_page_width: float): + """Clamp horizontal pan to keep viewport centered on target page.""" + ideal_pan_x = viewport_width / 2 - target_centerline_x + + if target_page_width > viewport_width: + max_deviation = (target_page_width / 2) + (viewport_width / 4) + else: + max_deviation = 100 # Small margin to avoid jitter + + min_pan_x = ideal_pan_x - max_deviation + max_pan_x = ideal_pan_x + max_deviation + + self.pan_offset[0] = max(min_pan_x, min(max_pan_x, self.pan_offset[0])) + def clamp_pan_offset(self): """ Clamp pan offset to prevent scrolling beyond content bounds. @@ -194,118 +272,18 @@ class ViewportMixin: viewport_width = self.width() viewport_height = self.height() + # Vertical clamping (returns original pan_y for page selection) + original_pan_y = self._clamp_vertical_pan(viewport_height) + + # Build page centerline data dpi = main_window.project.working_dpi - PAGE_MARGIN = 50 - PAGE_SPACING = 50 - - # Vertical clamping - bounds = self.get_content_bounds() - content_height = bounds["height"] - - # Save original pan_offset[1] BEFORE clamping for page selection - # This prevents clamping from changing which page we think we're on - original_pan_y = self.pan_offset[1] - - if content_height > viewport_height: - # Content is taller than viewport - restrict panning - max_pan_up = 0 # Can't pan beyond top edge - min_pan_up = -(content_height - viewport_height) # Can't pan beyond bottom edge - self.pan_offset[1] = max(min_pan_up, min(max_pan_up, self.pan_offset[1])) - # Don't force centering when content fits - preserve scroll position - # This prevents jumping when zooming in/out across the content_height == viewport_height boundary - - # Horizontal clamping - centerline-based approach - # Calculate the centerline position for each page and interpolate - - # Build list of page centerlines and their Y positions - page_centerlines = [] - current_y = PAGE_MARGIN - - for page in main_window.project.pages: - page_width_mm, page_height_mm = page.layout.size - page_width_px = page_width_mm * dpi / 25.4 - page_height_px = page_height_mm * dpi / 25.4 - - screen_page_width = page_width_px * self.zoom_level - screen_page_height = page_height_px * self.zoom_level - - # Calculate page center Y position (in world coordinates) - page_center_y = current_y + screen_page_height / 2 - - # Calculate the centerline X position (center of the page) - # Pages are left-aligned at PAGE_MARGIN, so center is at PAGE_MARGIN + width/2 - page_center_x = PAGE_MARGIN + screen_page_width / 2 - - page_centerlines.append((page_center_y, page_center_x, screen_page_width)) - - current_y += screen_page_height + PAGE_SPACING - - if not page_centerlines: + centerlines = self._build_page_centerlines(main_window.project.pages, dpi) + if not centerlines: return - # Determine current viewport center Y in world coordinates using ORIGINAL pan_y - # This prevents vertical clamping from changing which page we're targeting - # viewport_center_y in screen coords = viewport_height / 2 - # Convert to world coords: world_y = (screen_y - pan_offset[1]) / zoom_level - # But we want screen position, so we use pan_offset directly - viewport_center_y_world = -original_pan_y + viewport_height / 2 + # Find target centerline by interpolating based on viewport position + viewport_center_y = -original_pan_y + viewport_height / 2 + target_x, target_width = self._interpolate_target_centerline(centerlines, viewport_center_y) - # Find which pages we're between and interpolate - target_centerline_x = page_centerlines[0][1] # Default to first page - target_page_width = page_centerlines[0][2] - selected_page_index = 0 - - for i in range(len(page_centerlines)): - page_y, page_x, page_w = page_centerlines[i] - - if viewport_center_y_world <= page_y: - # We're above or at this page's center - if i == 0: - # First page - target_centerline_x = page_x - target_page_width = page_w - selected_page_index = 0 - else: - # Interpolate between previous and current page - prev_y, prev_x, prev_w = page_centerlines[i - 1] - - # Linear interpolation factor - if page_y != prev_y: - t = (viewport_center_y_world - prev_y) / (page_y - prev_y) - t = max(0, min(1, t)) # Clamp to [0, 1] - - target_centerline_x = prev_x + t * (page_x - prev_x) - target_page_width = prev_w + t * (page_w - prev_w) - selected_page_index = i - 1 if t < 0.5 else i - else: - target_centerline_x = page_x - target_page_width = page_w - selected_page_index = i - break - else: - # We're below all pages - use last page - target_centerline_x = page_centerlines[-1][1] - target_page_width = page_centerlines[-1][2] - selected_page_index = len(page_centerlines) - 1 - - # Horizontal clamping to keep viewport reasonably centered on the page - # The centerline should ideally be at viewport_width / 2 - ideal_pan_x = viewport_width / 2 - target_centerline_x - - # Calculate how far we need to allow panning to see the full width of the page - # If page is wider than viewport, allow panning to see left and right edges - # If page is narrower, keep it centered - if target_page_width > viewport_width: - # Page wider than viewport - allow panning to see edges plus some margin - # Allow user to pan to see any part of the page, with reasonable overshoot - max_deviation = (target_page_width / 2) + (viewport_width / 4) - else: - # Page narrower than viewport - keep centered with small margin for stability - max_deviation = 100 # Small margin to avoid jitter - - # Calculate bounds - min_pan_x = ideal_pan_x - max_deviation - max_pan_x = ideal_pan_x + max_deviation - - old_pan_x = self.pan_offset[0] - self.pan_offset[0] = max(min_pan_x, min(max_pan_x, self.pan_offset[0])) + # Horizontal clamping + self._clamp_horizontal_pan(viewport_width, target_x, target_width) diff --git a/pyPhotoAlbum/snapping.py b/pyPhotoAlbum/snapping.py index d45a8e7..e751648 100644 --- a/pyPhotoAlbum/snapping.py +++ b/pyPhotoAlbum/snapping.py @@ -370,74 +370,6 @@ class SnappingSystem: return best_snap - def _snap_axis( - self, position: float, size: float, page_size_mm: float, dpi: int, snap_threshold_px: float, orientation: str - ) -> float: - """ - Snap along a single axis - - Args: - position: Current position along axis in pixels - size: Element size along axis in pixels - page_size_mm: Page size along axis in mm - dpi: DPI for conversion - snap_threshold_px: Snap threshold in pixels - orientation: 'vertical' for x-axis, 'horizontal' for y-axis - - Returns: - Snapped position in pixels - """ - snap_candidates: List[Tuple[float, float]] = [] - - # 1. Page edge snapping - if self.snap_to_edges: - # Snap to start edge (0) - snap_candidates.append((0.0, abs(position - 0))) - - # Snap to end edge - page_size_px = page_size_mm * dpi / 25.4 - snap_candidates.append((page_size_px - size, abs(position - (page_size_px - size)))) - - # Also snap element's far edge to page edge - snap_candidates.append((page_size_px - size, abs((position + size) - page_size_px))) - - # 2. Grid snapping - if self.snap_to_grid: - grid_size_px = self.grid_size_mm * dpi / 25.4 - - # Snap to nearest grid line - nearest_grid = round(position / grid_size_px) * grid_size_px - snap_candidates.append((nearest_grid, abs(position - nearest_grid))) - - # Also try snapping element's far edge to grid - element_end = position + size - nearest_grid_end = round(element_end / grid_size_px) * grid_size_px - snap_candidates.append((nearest_grid_end - size, abs(element_end - nearest_grid_end))) - - # 3. Guide snapping - if self.snap_to_guides: - for guide in self.guides: - if guide.orientation == orientation: - guide_pos_px = guide.position * dpi / 25.4 - - # Snap start edge to guide - snap_candidates.append((guide_pos_px, abs(position - guide_pos_px))) - - # Snap end edge to guide - element_end = position + size - snap_candidates.append((guide_pos_px - size, abs(element_end - guide_pos_px))) - - # Find the best snap candidate within threshold - best_snap = None - best_distance = snap_threshold_px - - for snap_pos, distance in snap_candidates: - if distance < best_distance: - best_snap = snap_pos - best_distance = distance - - return best_snap if best_snap is not None else position - def get_snap_lines(self, page_size: Tuple[float, float], dpi: int = 300) -> dict: """ Get all snap lines for visualization diff --git a/tests/test_snapping_system.py b/tests/test_snapping_system.py index 615a5bd..320baec 100644 --- a/tests/test_snapping_system.py +++ b/tests/test_snapping_system.py @@ -462,51 +462,6 @@ class TestSnapEdgeToTargets: assert result is None -class TestSnapAxis: - """Tests for _snap_axis method""" - - def test_snap_axis_to_start(self): - """Test snapping axis to start edge""" - snap = SnappingSystem() - snap.snap_to_edges = True - snap.snap_to_grid = False - snap.snap_to_guides = False - - dpi = 300 - threshold_px = 50 - - result = snap._snap_axis( - position=10, size=50, page_size_mm=210, dpi=dpi, snap_threshold_px=threshold_px, orientation="vertical" - ) - - assert result == 0 - - def test_snap_axis_to_end(self): - """Test snapping axis so element end aligns with page end""" - snap = SnappingSystem() - snap.snap_to_edges = True - snap.snap_to_grid = False - snap.snap_to_guides = False - - dpi = 300 - page_size_mm = 210 - page_size_px = page_size_mm * dpi / 25.4 - element_size = 50 - threshold_px = 50 - - result = snap._snap_axis( - position=page_size_px - element_size - 10, - size=element_size, - page_size_mm=page_size_mm, - dpi=dpi, - snap_threshold_px=threshold_px, - orientation="vertical", - ) - - expected = page_size_px - element_size - assert abs(result - expected) < 1 - - class TestGetSnapLines: """Tests for get_snap_lines method"""