From bcae45a023814a53eb843cc9b76d0fc61dd5bb00 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 8 Aug 2026 13:07:53 +0200 Subject: [PATCH] refactor(ereader): drop the estimator helpers S16 superseded (R8) S16 replaced backward pagination's estimate-render-compare-adjust loop with anchor replay, but left _estimate_page_start and _adjust_start_estimate behind. Nothing in the library calls them; only their own tests did. _estimate_page_start guessed max(1, int(10 / font_scale)) blocks per page - a constant with no relationship to page size, block length or font metrics - and _adjust_start_estimate halved the error each round to converge on it. Anchor replay makes both meaningless: it walks forward from a known page boundary instead of guessing at one. Removes their five tests with them rather than leaving tests pinning behaviour nothing depends on. 889 passed. Co-Authored-By: Claude Opus 5 --- pyWebLayout/layout/ereader_layout.py | 54 ---------------------- tests/layout/test_ereader_layout.py | 68 ---------------------------- 2 files changed, 122 deletions(-) diff --git a/pyWebLayout/layout/ereader_layout.py b/pyWebLayout/layout/ereader_layout.py index 1888a01..8e50738 100644 --- a/pyWebLayout/layout/ereader_layout.py +++ b/pyWebLayout/layout/ereader_layout.py @@ -793,60 +793,6 @@ class BidirectionalLayouter: # Keep same position so it will be attempted on the next page return False, position - def _estimate_page_start( - self, - end_position: RenderingPosition, - font_scale: float) -> RenderingPosition: - """Estimate where a page should start to end at the given position""" - # This is a simplified heuristic - a full implementation would be more - # sophisticated - estimated_start = end_position.copy() - - # Move back by an estimated number of blocks that would fit on a page - estimated_blocks_per_page = max(1, int(10 / font_scale)) # Rough estimate - estimated_start.block_index = max( - 0, end_position.block_index - estimated_blocks_per_page) - estimated_start.word_index = 0 - - return estimated_start - - def _adjust_start_estimate( - self, - current_start: RenderingPosition, - target_end: RenderingPosition, - actual_end: RenderingPosition) -> RenderingPosition: - """ - 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: # Undershot - rendered to block X but need to reach block Y where X < Y - # We didn't render far enough forward - # 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 - def _position_compare(self, pos1: RenderingPosition, pos2: RenderingPosition) -> int: """Compare two positions (-1: pos1 < pos2, 0: equal, 1: pos1 > pos2)""" diff --git a/tests/layout/test_ereader_layout.py b/tests/layout/test_ereader_layout.py index 5436514..62122f2 100644 --- a/tests/layout/test_ereader_layout.py +++ b/tests/layout/test_ereader_layout.py @@ -570,30 +570,6 @@ class TestBidirectionalLayouter: # Should return same block assert scaled == paragraph - def test_estimate_page_start(self): - """Test estimation of page start position.""" - layouter = BidirectionalLayouter([], PageStyle()) - - end_pos = RenderingPosition(chapter_index=0, block_index=20, word_index=0) - - estimated = layouter._estimate_page_start(end_pos, 1.0) - - # Should estimate some blocks before the end position - assert estimated.block_index < end_pos.block_index - assert estimated.block_index >= 0 - - def test_estimate_page_start_with_font_scale(self): - """Test that font scale affects page start estimation.""" - layouter = BidirectionalLayouter([], PageStyle()) - - end_pos = RenderingPosition(chapter_index=0, block_index=20, word_index=0) - - est_normal = layouter._estimate_page_start(end_pos, 1.0) - est_large = layouter._estimate_page_start(end_pos, 2.0) - - # Larger font should estimate fewer blocks - assert est_large.block_index >= est_normal.block_index - def test_scale_block_fonts_paragraph(self, sample_font): """Test scaling fonts in a paragraph block.""" layouter = BidirectionalLayouter([], PageStyle()) @@ -784,50 +760,6 @@ class TestBidirectionalLayouter: # Start position should be before or at end position assert start_pos.block_index <= end_position.block_index - def test_adjust_start_estimate_overshot(self): - """Test adjustment when forward render overshoots target.""" - layouter = BidirectionalLayouter([], PageStyle()) - - current_start = RenderingPosition(block_index=5) - target_end = RenderingPosition(block_index=10) - actual_end = RenderingPosition(block_index=12) # Overshot (went too far) - - adjusted = layouter._adjust_start_estimate( - current_start, target_end, actual_end) - - # Overshot means we rendered too far forward - # 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.""" - layouter = BidirectionalLayouter([], PageStyle()) - - current_start = RenderingPosition(block_index=5) - target_end = RenderingPosition(block_index=10) - actual_end = RenderingPosition(block_index=8) # Undershot (didn't go far enough) - - adjusted = layouter._adjust_start_estimate( - current_start, target_end, actual_end) - - # Undershot means we didn't render far enough forward - # 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.""" - layouter = BidirectionalLayouter([], PageStyle()) - - current_start = RenderingPosition(block_index=5) - target_end = RenderingPosition(block_index=10) - actual_end = RenderingPosition(block_index=10) # Exact - - adjusted = layouter._adjust_start_estimate( - current_start, target_end, actual_end) - - # Should return same or similar position - assert adjusted.block_index >= 0 - def test_layout_paragraph_on_page_with_pretext( self, sample_font, sample_page_style): """Test paragraph layout with pretext (hyphenated word continuation)."""