From 2fc99a3292ea1a2329e3044d0e0b2e3c44fab470 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 23 Nov 2025 21:32:58 +0100 Subject: [PATCH] Fixed bug where zooming could cause viewport to moce to page 1 --- pyPhotoAlbum/main.py | 27 +++- pyPhotoAlbum/mixins/mouse_interaction.py | 1 + pyPhotoAlbum/mixins/viewport.py | 149 ++++++++++++++++++++--- pyPhotoAlbum/models.py | 4 +- 4 files changed, 164 insertions(+), 17 deletions(-) diff --git a/pyPhotoAlbum/main.py b/pyPhotoAlbum/main.py index 5a96317..ccd630b 100644 --- a/pyPhotoAlbum/main.py +++ b/pyPhotoAlbum/main.py @@ -173,12 +173,16 @@ class MainWindow( # Track scrollbar updates to prevent feedback loops self._updating_scrollbars = False + # Track scrollbar visibility changes to prevent resize-triggered recentering + self._updating_scrollbar_visibility = False def _on_vertical_scroll(self, value): """Handle vertical scrollbar changes""" if not self._updating_scrollbars: # Invert scrollbar value to pan offset (scrolling down = negative pan) + old_pan_y = self._gl_widget.pan_offset[1] self._gl_widget.pan_offset[1] = -value + print(f"SCROLLBAR V: changed pan_y from {old_pan_y:.1f} to {self._gl_widget.pan_offset[1]:.1f} (scrollbar value={value})") self._gl_widget.update() def _on_horizontal_scroll(self, value): @@ -190,8 +194,16 @@ class MainWindow( def update_scrollbars(self): """Update scrollbar positions and ranges based on current content and pan offset""" + print(f"UPDATE_SCROLLBARS: START - pan_offset = [{self._gl_widget.pan_offset[0]:.1f}, {self._gl_widget.pan_offset[1]:.1f}]") self._updating_scrollbars = True + # Block signals to prevent feedback loop + signals_were_blocked_v = self._v_scrollbar.signalsBlocked() + signals_were_blocked_h = self._h_scrollbar.signalsBlocked() + self._v_scrollbar.blockSignals(True) + self._h_scrollbar.blockSignals(True) + print(f"UPDATE_SCROLLBARS: Blocked signals (v was {signals_were_blocked_v}, h was {signals_were_blocked_h})") + # Get content bounds bounds = self._gl_widget.get_content_bounds() viewport_width = self._gl_widget.width() @@ -200,18 +212,24 @@ class MainWindow( content_height = bounds['height'] content_width = bounds['width'] + print(f"UPDATE_SCROLLBARS: content={content_width:.0f}x{content_height:.0f}, viewport={viewport_width}x{viewport_height}") + # Vertical scrollbar # Scrollbar value 0 = top of content # Scrollbar value max = bottom of content # Pan offset is inverted: positive pan = content moved down = view at top # negative pan = content moved up = view at bottom v_range = int(max(0, content_height - viewport_height)) + v_value = int(max(0, min(v_range, -self._gl_widget.pan_offset[1]))) + print(f"UPDATE_SCROLLBARS: Setting v_scrollbar range=0-{v_range}, value={v_value} (from pan_y={self._gl_widget.pan_offset[1]:.1f})") self._v_scrollbar.setRange(0, v_range) self._v_scrollbar.setPageStep(int(viewport_height)) # Invert pan_offset for scrollbar position - self._v_scrollbar.setValue(int(max(0, min(v_range, -self._gl_widget.pan_offset[1])))) + self._v_scrollbar.setValue(v_value) # Show/hide vertical scrollbar based on whether scrolling is needed + # Set flag to prevent resizeGL from recentering when scrollbar visibility changes + self._updating_scrollbar_visibility = True self._v_scrollbar.setVisible(v_range > 0) # Horizontal scrollbar @@ -223,8 +241,15 @@ class MainWindow( # Show/hide horizontal scrollbar based on whether scrolling is needed self._h_scrollbar.setVisible(h_range > 0) + self._updating_scrollbar_visibility = False + + # Unblock signals + self._v_scrollbar.blockSignals(False) + self._h_scrollbar.blockSignals(False) + print(f"UPDATE_SCROLLBARS: Unblocked signals") self._updating_scrollbars = False + print(f"UPDATE_SCROLLBARS: END - pan_offset = [{self._gl_widget.pan_offset[0]:.1f}, {self._gl_widget.pan_offset[1]:.1f}]") def _register_shortcuts(self): """Register keyboard shortcuts from decorated methods""" diff --git a/pyPhotoAlbum/mixins/mouse_interaction.py b/pyPhotoAlbum/mixins/mouse_interaction.py index 41a147a..0ce75a3 100644 --- a/pyPhotoAlbum/mixins/mouse_interaction.py +++ b/pyPhotoAlbum/mixins/mouse_interaction.py @@ -280,6 +280,7 @@ class MouseInteractionMixin: new_zoom = self.zoom_level * zoom_factor if 0.1 <= new_zoom <= 5.0: + old_zoom = self.zoom_level old_pan_x = self.pan_offset[0] old_pan_y = self.pan_offset[1] diff --git a/pyPhotoAlbum/mixins/viewport.py b/pyPhotoAlbum/mixins/viewport.py index 2e215cc..9d16520 100644 --- a/pyPhotoAlbum/mixins/viewport.py +++ b/pyPhotoAlbum/mixins/viewport.py @@ -21,6 +21,9 @@ class ViewportMixin: self.pan_offset = [0, 0] self.initial_zoom_set = False # Track if we've set initial fit-to-screen zoom + # Track previous viewport size to detect scrollbar-induced resizes + self._last_viewport_size = (0, 0) + def initializeGL(self): """Initialize OpenGL resources""" glClearColor(1.0, 1.0, 1.0, 1.0) @@ -34,11 +37,26 @@ class ViewportMixin: glOrtho(0, w, h, 0, -1, 1) glMatrixMode(GL_MODELVIEW) + # Detect if this is a small resize (likely scrollbar visibility change) + # Scrollbars are typically 14-20 pixels wide + last_w, last_h = self._last_viewport_size + width_change = abs(w - last_w) + height_change = abs(h - last_h) + is_small_resize = width_change <= 20 and height_change <= 20 + is_first_resize = last_w == 0 and last_h == 0 + # Recalculate centering if we have a project loaded - if self.initial_zoom_set: + # Recenter on: + # 1. First resize (initial setup) + # 2. Large resizes (window resize, NOT scrollbar changes) + # Don't recenter on small resizes (scrollbar visibility changes during zoom) + if self.initial_zoom_set and (is_first_resize or not is_small_resize): # Maintain current zoom level, just recenter self.pan_offset = self._calculate_center_pan_offset(self.zoom_level) + # Update tracked viewport size + self._last_viewport_size = (w, h) + self.update() # Update scrollbars when viewport size changes @@ -164,27 +182,130 @@ class ViewportMixin: Pan offset semantics: - Positive pan_offset = content moved right/down (viewing top-left) - Negative pan_offset = content moved left/up (viewing bottom-right) + + For horizontal clamping, we use a centerline-based approach that interpolates + between page centers based on vertical scroll position. This prevents jumps + when zooming on pages of different widths. """ - bounds = self.get_content_bounds() + main_window = self.window() + if not hasattr(main_window, 'project') or not main_window.project or not main_window.project.pages: + return + viewport_width = self.width() viewport_height = self.height() - content_width = bounds['width'] - content_height = bounds['height'] - - # Only clamp if content is larger than viewport - # If content is smaller, allow any pan offset (for centering) - - # Horizontal clamping - if content_width > viewport_width: - # Content is wider than viewport - restrict panning - max_pan_left = 0 # Can't pan beyond left edge - min_pan_left = -(content_width - viewport_width) # Can't pan beyond right edge - self.pan_offset[0] = max(min_pan_left, min(max_pan_left, self.pan_offset[0])) + 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: + 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 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])) diff --git a/pyPhotoAlbum/models.py b/pyPhotoAlbum/models.py index 568bf28..b26e461 100644 --- a/pyPhotoAlbum/models.py +++ b/pyPhotoAlbum/models.py @@ -204,7 +204,7 @@ class ImageData(BaseLayoutElement): for path in possible_paths: if os.path.exists(path): image_full_path = path - print(f"ImageData: Resolved {self.image_path} → {path}") + # print(f"ImageData: Resolved {self.image_path} → {path}") break else: print(f"ImageData: Could not resolve path: {self.image_path}") @@ -399,7 +399,7 @@ class ImageData(BaseLayoutElement): # Update metadata for future renders - always update to reflect rotated dimensions self.image_dimensions = (pil_image.width, pil_image.height) - print(f"ImageData: Async loaded texture for {self.image_path}") + # print(f"ImageData: Async loaded texture for {self.image_path}") except Exception as e: print(f"ImageData: Error creating texture from async loaded image: {e}")