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 <noreply@anthropic.com>
This commit is contained in:
@@ -793,60 +793,6 @@ class BidirectionalLayouter:
|
|||||||
# Keep same position so it will be attempted on the next page
|
# Keep same position so it will be attempted on the next page
|
||||||
return False, position
|
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,
|
def _position_compare(self, pos1: RenderingPosition,
|
||||||
pos2: RenderingPosition) -> int:
|
pos2: RenderingPosition) -> int:
|
||||||
"""Compare two positions (-1: pos1 < pos2, 0: equal, 1: pos1 > pos2)"""
|
"""Compare two positions (-1: pos1 < pos2, 0: equal, 1: pos1 > pos2)"""
|
||||||
|
|||||||
@@ -570,30 +570,6 @@ class TestBidirectionalLayouter:
|
|||||||
# Should return same block
|
# Should return same block
|
||||||
assert scaled == paragraph
|
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):
|
def test_scale_block_fonts_paragraph(self, sample_font):
|
||||||
"""Test scaling fonts in a paragraph block."""
|
"""Test scaling fonts in a paragraph block."""
|
||||||
layouter = BidirectionalLayouter([], PageStyle())
|
layouter = BidirectionalLayouter([], PageStyle())
|
||||||
@@ -784,50 +760,6 @@ class TestBidirectionalLayouter:
|
|||||||
# Start position should be before or at end position
|
# Start position should be before or at end position
|
||||||
assert start_pos.block_index <= end_position.block_index
|
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(
|
def test_layout_paragraph_on_page_with_pretext(
|
||||||
self, sample_font, sample_page_style):
|
self, sample_font, sample_page_style):
|
||||||
"""Test paragraph layout with pretext (hyphenated word continuation)."""
|
"""Test paragraph layout with pretext (hyphenated word continuation)."""
|
||||||
|
|||||||
Reference in New Issue
Block a user