fix backwards rendering

pyWebLayout/layout/ereader_layout.py

@@ -286,6 +286,9 @@ class BidirectionalLayouter:
         Render a page that ends at the given position, filling backward.
         Critical for "previous page" navigation.
 
+        Uses iterative refinement to find the correct start position that
+        results in a page ending at (or very close to) the target position.
+
         Args:
             end_position: Position where page should end
             font_scale: Font scaling factor
@@ -293,24 +296,48 @@ class BidirectionalLayouter:
         Returns:
             Tuple of (rendered_page, start_position)
         """
-        # This is a complex operation that requires iterative refinement
+        # Handle edge case: already at beginning
-        # We'll start with an estimated start position and refine it
+        if end_position.block_index == 0 and end_position.word_index == 0:
+            return self.render_page_forward(end_position, font_scale)
 
+        # Start with initial estimate
         estimated_start = self._estimate_page_start(end_position, font_scale)
 
-        # Render forward from estimated start and see if we reach the target
+        # Iterative refinement: keep adjusting until we converge or hit max iterations
+        max_iterations = 10
+        best_page = None
+        best_start = estimated_start
+        best_distance = float('inf')
+
+        for iteration in range(max_iterations):
+            # Render forward from current estimate
             page, actual_end = self.render_page_forward(estimated_start, font_scale)
 
-        # If we overshot or undershot, adjust and try again
+            # Calculate how far we are from target
-        # This is a simplified implementation - a full version would be more
+            comparison = self._position_compare(actual_end, end_position)
-        # sophisticated
+
-        if self._position_compare(actual_end, end_position) != 0:
+            # Perfect match or close enough (within same block)
-            # Adjust estimate and try again (simplified)
+            if comparison == 0:
+                return page, estimated_start
+
+            # Track best result so far
+            distance = abs(actual_end.block_index - end_position.block_index)
+            if distance < best_distance:
+                best_distance = distance
+                best_page = page
+                best_start = estimated_start.copy()
+
+            # Adjust estimate for next iteration
             estimated_start = self._adjust_start_estimate(
                 estimated_start, end_position, actual_end)
-            page, actual_end = self.render_page_forward(estimated_start, font_scale)
 
-        return page, estimated_start
+            # Safety: don't go before document start
+            if estimated_start.block_index < 0:
+                estimated_start.block_index = 0
+                estimated_start.word_index = 0
+
+        # If we exhausted iterations, return best result found
+        return best_page if best_page else page, best_start
 
     def _scale_block_fonts(self, block: Block, font_scale: float) -> Block:
         """Apply font scaling to all fonts in a block"""
@@ -491,15 +518,35 @@ class BidirectionalLayouter:
             current_start: RenderingPosition,
             target_end: RenderingPosition,
             actual_end: RenderingPosition) -> RenderingPosition:
-        """Adjust start position estimate based on overshoot/undershoot"""
+        """
-        # Simplified adjustment logic
+        Adjust start position estimate based on overshoot/undershoot.
+        Uses proportional adjustment to converge faster.
+        """
         adjusted = current_start.copy()
 
+        # Calculate the difference between actual and target end positions
+        block_diff = actual_end.block_index - target_end.block_index
+
         comparison = self._position_compare(actual_end, target_end)
-        if comparison > 0:  # Overshot
+
-            adjusted.block_index = max(0, adjusted.block_index + 1)
+        if comparison < 0:  # Undershot - rendered to block X but need to reach block Y where X < Y
-        elif comparison < 0:  # Undershot
+            # We didn't render far enough forward
-            adjusted.block_index = max(0, adjusted.block_index - 1)
+            # Need to start at a LATER block (higher index) so the page includes more content
+            adjustment = max(1, abs(block_diff) // 2)
+            new_index = adjusted.block_index + adjustment
+            # Clamp to valid range
+            if len(self.blocks) > 0:
+                adjusted.block_index = min(len(self.blocks) - 1, max(0, new_index))
+            else:
+                adjusted.block_index = max(0, new_index)
+        elif comparison > 0:  # Overshot - rendered past the target
+            # We rendered too far forward
+            # Need to start at an EARLIER block (lower index) so the page doesn't go as far
+            adjustment = max(1, abs(block_diff) // 2)
+            adjusted.block_index = max(0, adjusted.block_index - adjustment)
+
+        # Reset word index when adjusting blocks
+        adjusted.word_index = 0
 
         return adjusted
 

pyWebLayout/layout/ereader_manager.py

@@ -194,6 +194,11 @@ class EreaderLayoutManager:
         self.current_position = RenderingPosition()
         self.font_scale = 1.0
 
+        # Page position history for fast backward navigation
+        # List of (position, font_scale) tuples representing the start of each page visited
+        self._page_history: List[Tuple[RenderingPosition, float]] = []
+        self._max_history_size = 50  # Keep last 50 page positions
+
         # Load last reading position if available
         saved_position = self.bookmark_manager.load_reading_position()
         if saved_position:
@@ -246,6 +251,9 @@ class EreaderLayoutManager:
         Returns:
             Next page or None if at end of document
         """
+        # Save current position to history before moving forward
+        self._add_to_history(self.current_position, self.font_scale)
+
         page, next_position = self.renderer.render_page(
             self.current_position, self.font_scale)
 
@@ -261,17 +269,33 @@ class EreaderLayoutManager:
         """
         Go to the previous page.
 
+        Uses cached page history for instant navigation when available,
+        falls back to iterative refinement algorithm when needed.
+
         Returns:
             Previous page or None if at beginning of document
         """
         if self._is_at_beginning():
             return None
 
-        # Use backward rendering to find the previous page
+        # Fast path: Check if we have this position in history
+        previous_position = self._get_from_history(self.current_position, self.font_scale)
+
+        if previous_position is not None:
+            # Cache hit! Use the cached position for instant navigation
+            self.current_position = previous_position
+            self._notify_position_changed()
+            return self.get_current_page()
+
+        # Slow path: Use backward rendering to find the previous page
+        # This uses the iterative refinement algorithm we just fixed
         page, start_position = self.renderer.render_page_backward(
             self.current_position, self.font_scale)
 
         if start_position != self.current_position:
+            # Save this calculated position to history for future use
+            self._add_to_history(start_position, self.font_scale)
+
             self.current_position = start_position
             self._notify_position_changed()
             return page
@@ -328,10 +352,75 @@ class EreaderLayoutManager:
             return self.jump_to_position(chapters[chapter_index].position)
         return None
 
+    def _add_to_history(self, position: RenderingPosition, font_scale: float):
+        """
+        Add a page position to the navigation history.
+
+        Args:
+            position: The page start position to remember
+            font_scale: The font scale at this position
+        """
+        # Only add if it's different from the last entry
+        if not self._page_history or \
+           self._page_history[-1][0] != position or \
+           self._page_history[-1][1] != font_scale:
+
+            self._page_history.append((position.copy(), font_scale))
+
+            # Trim history if it exceeds max size
+            if len(self._page_history) > self._max_history_size:
+                self._page_history.pop(0)
+
+    def _get_from_history(
+            self,
+            current_position: RenderingPosition,
+            current_font_scale: float) -> Optional[RenderingPosition]:
+        """
+        Get the previous page position from history.
+
+        Searches backward through history to find the last position that
+        comes before the current position at the same font scale.
+
+        Args:
+            current_position: Current page position
+            current_font_scale: Current font scale
+
+        Returns:
+            Previous page position or None if not found in history
+        """
+        # Search backward through history
+        for i in range(len(self._page_history) - 1, -1, -1):
+            hist_position, hist_font_scale = self._page_history[i]
+
+            # Must match font scale
+            if hist_font_scale != current_font_scale:
+                continue
+
+            # Must be before current position
+            if (hist_position.chapter_index < current_position.chapter_index or
+                (hist_position.chapter_index == current_position.chapter_index and
+                 hist_position.block_index < current_position.block_index) or
+                (hist_position.chapter_index == current_position.chapter_index and
+                 hist_position.block_index == current_position.block_index and
+                 hist_position.word_index < current_position.word_index)):
+
+                # Found a previous position - remove it and everything after from history
+                # since we're navigating backward
+                self._page_history = self._page_history[:i]
+                return hist_position.copy()
+
+        return None
+
+    def _clear_history(self):
+        """Clear the page navigation history."""
+        self._page_history.clear()
+
     def set_font_scale(self, scale: float) -> Page:
         """
         Change the font scale and re-render current page.
 
+        Clears page history since font changes invalidate all cached positions.
+
         Args:
             scale: Font scaling factor (1.0 = normal, 2.0 = double size, etc.)
 
@@ -340,6 +429,8 @@ class EreaderLayoutManager:
         """
         if scale != self.font_scale:
             self.font_scale = scale
+            # Clear history since font scale changes invalidate all cached positions
+            self._clear_history()
             # The renderer will handle cache invalidation
 
         return self.get_current_page()
@@ -352,6 +443,8 @@ class EreaderLayoutManager:
         """
         Increase line spacing and re-render current page.
 
+        Clears page history since spacing changes invalidate all cached positions.
+
         Args:
             amount: Pixels to add to line spacing (default: 2)
 
@@ -361,12 +454,15 @@ class EreaderLayoutManager:
         self.page_style.line_spacing += amount
         self.renderer.page_style = self.page_style  # Update renderer's reference
         self.renderer.buffer.invalidate_all()  # Clear cache to force re-render
+        self._clear_history()  # Clear position history
         return self.get_current_page()
 
     def decrease_line_spacing(self, amount: int = 2) -> Page:
         """
         Decrease line spacing and re-render current page.
 
+        Clears page history since spacing changes invalidate all cached positions.
+
         Args:
             amount: Pixels to remove from line spacing (default: 2)
 
@@ -376,12 +472,15 @@ class EreaderLayoutManager:
         self.page_style.line_spacing = max(0, self.page_style.line_spacing - amount)
         self.renderer.page_style = self.page_style  # Update renderer's reference
         self.renderer.buffer.invalidate_all()  # Clear cache to force re-render
+        self._clear_history()  # Clear position history
         return self.get_current_page()
 
     def increase_inter_block_spacing(self, amount: int = 5) -> Page:
         """
         Increase spacing between blocks and re-render current page.
 
+        Clears page history since spacing changes invalidate all cached positions.
+
         Args:
             amount: Pixels to add to inter-block spacing (default: 5)
 
@@ -391,12 +490,15 @@ class EreaderLayoutManager:
         self.page_style.inter_block_spacing += amount
         self.renderer.page_style = self.page_style  # Update renderer's reference
         self.renderer.buffer.invalidate_all()  # Clear cache to force re-render
+        self._clear_history()  # Clear position history
         return self.get_current_page()
 
     def decrease_inter_block_spacing(self, amount: int = 5) -> Page:
         """
         Decrease spacing between blocks and re-render current page.
 
+        Clears page history since spacing changes invalidate all cached positions.
+
         Args:
             amount: Pixels to remove from inter-block spacing (default: 5)
 
@@ -407,12 +509,15 @@ class EreaderLayoutManager:
             0, self.page_style.inter_block_spacing - amount)
         self.renderer.page_style = self.page_style  # Update renderer's reference
         self.renderer.buffer.invalidate_all()  # Clear cache to force re-render
+        self._clear_history()  # Clear position history
         return self.get_current_page()
 
     def increase_word_spacing(self, amount: int = 2) -> Page:
         """
         Increase spacing between words and re-render current page.
 
+        Clears page history since spacing changes invalidate all cached positions.
+
         Args:
             amount: Pixels to add to word spacing (default: 2)
 
@@ -422,12 +527,15 @@ class EreaderLayoutManager:
         self.page_style.word_spacing += amount
         self.renderer.page_style = self.page_style  # Update renderer's reference
         self.renderer.buffer.invalidate_all()  # Clear cache to force re-render
+        self._clear_history()  # Clear position history
         return self.get_current_page()
 
     def decrease_word_spacing(self, amount: int = 2) -> Page:
         """
         Decrease spacing between words and re-render current page.
 
+        Clears page history since spacing changes invalidate all cached positions.
+
         Args:
             amount: Pixels to remove from word spacing (default: 2)
 
@@ -437,6 +545,7 @@ class EreaderLayoutManager:
         self.page_style.word_spacing = max(0, self.page_style.word_spacing - amount)
         self.renderer.page_style = self.page_style  # Update renderer's reference
         self.renderer.buffer.invalidate_all()  # Clear cache to force re-render
+        self._clear_history()  # Clear position history
         return self.get_current_page()
 
     def get_table_of_contents(

tests/layout/test_ereader_layout.py

@@ -790,13 +790,14 @@ class TestBidirectionalLayouter:
 
         current_start = RenderingPosition(block_index=5)
         target_end = RenderingPosition(block_index=10)
-        actual_end = RenderingPosition(block_index=12)  # Overshot
+        actual_end = RenderingPosition(block_index=12)  # Overshot (went too far)
 
         adjusted = layouter._adjust_start_estimate(
             current_start, target_end, actual_end)
 
-        # Should move start forward (increase block_index)
+        # Overshot means we rendered too far forward
-        assert adjusted.block_index > current_start.block_index
+        # So we need to start EARLIER (decrease block_index) to not go as far
+        assert adjusted.block_index < current_start.block_index
 
     def test_adjust_start_estimate_undershot(self):
         """Test adjustment when forward render undershoots target."""
@@ -804,13 +805,14 @@ class TestBidirectionalLayouter:
 
         current_start = RenderingPosition(block_index=5)
         target_end = RenderingPosition(block_index=10)
-        actual_end = RenderingPosition(block_index=8)  # Undershot
+        actual_end = RenderingPosition(block_index=8)  # Undershot (didn't go far enough)
 
         adjusted = layouter._adjust_start_estimate(
             current_start, target_end, actual_end)
 
-        # Should move start backward (decrease block_index)
+        # Undershot means we didn't render far enough forward
-        assert adjusted.block_index <= current_start.block_index
+        # So we need to start LATER (increase block_index) to include more content
+        assert adjusted.block_index > current_start.block_index
 
     def test_adjust_start_estimate_exact(self):
         """Test adjustment when forward render hits target exactly."""