dtourolle/pyWebLayout
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ereader): replay the page chain instead of searching for it (S16)

Browse Source 
render_page_backward searched for the previous page's start: estimate a block
index, lay out forward, bisect on the block difference, up to ten times. Both the
estimator and the adjuster pinned word_index to 0 and moved only block_index -
but pages routinely start mid-block, so the answer was not in the search space.
The loop could never match, exhausted its iterations and fell through to a
fallback that jumped to the start of the document.

Measured on a document with one 1200-word paragraph, whose page starts are
(0,0), (2,208), (2,494), (2,780): three of four backward calls returned (0,0),
each after ten full page layouts. The bisection was unsound in its own space too
- 40 small paragraphs, every page starting on a block boundary, also failed.

Pagination is a pure function, so the page before P is the q with next(q) == P,
and it is found by replaying the chain forward rather than guessing q. Three
sources: the chain recorded as pages are laid out forward (exact, one layout,
covers paging back and forth); replay from the block containing P and then from
earlier blocks (exact when P lies on that chain); and failing that, the last
start before P, which overlaps slightly rather than skipping content.

  warm:                          4/4 exact, 1 layout each
  cold, fresh layouter per call: 12/13 exact, worst 17 layouts
  cold, repeated back presses:   ~4 layouts per turn

Each step returns a page ending exactly where the reader is, so paging back never
skips or repeats content. That chain can differ from the one seen reading forward
from page one if the reader arrived by a jump - pagination from a different start
is a different chain, and nothing can recover the original without replaying the
whole document.

Complementary to S11 rather than caused by it: before S11 forward pagination
dead-ended at the first page-spanning block, so mid-block starts never arose and
the block-granular search looked adequate.
This commit is contained in:
dtourolle
2026-08-08 12:43:30 +02:00
parent 767e4c135c
commit 456824d6d6
3 changed files with 398 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/LAYOUT_REMEDIATION_SPEC.md
+98
View File
@@ -30,6 +30,7 @@ It is independent of every other spec here.
| [S13](#s13--word-spacing-and-alignment) | Word spacing and alignment | 0 | | [S13](#s13--word-spacing-and-alignment) | Word spacing and alignment | 0 |
| [S14](#s14--vertical-centring-in-buttons-and-fields) | Vertical centring in buttons and fields | 0 | | [S14](#s14--vertical-centring-in-buttons-and-fields) | Vertical centring in buttons and fields | 0 |
| [S15](#s15--form-field-label-geometry) | Form field label geometry | 0 | | [S15](#s15--form-field-label-geometry) | Form field label geometry | 0 |
| [S16](#s16--backward-page-navigation) | Backward page navigation | 0 |
## Design invariants ## Design invariants
@@ -1265,6 +1266,103 @@ between label and box smaller than the intended 5px.
--- ---
## S16 — Backward page navigation
### Problem
`render_page_backward` *searched* for the previous page's start: estimate a block
index, lay out forward, compare the end against the target, bisect on the block
difference, repeat up to ten times. Both the estimator and the adjuster pinned
`word_index` to 0 and moved only `block_index`.
Pages routinely start mid-block. Any such start was therefore **not in the search
space**, the loop could never match, and it fell through to a fallback that
jumped several blocks back or to the document start.
### Evidence
A document of short paragraphs around one 1200-word paragraph. Forward pagination
gives page starts at `(0,0), (2,208), (2,494), (2,780), (2,1057)`. Asking for the
page that ends where each of those begins:
```
from page 1 -> got (0,0) expected (0,0) ok (1 forward layout)
from page 2 -> got (0,0) expected (2,208) WRONG (10 forward layouts)
from page 3 -> got (0,0) expected (2,494) WRONG (10 forward layouts)
from page 4 -> got (0,0) expected (2,780) WRONG (10 forward layouts)
```
Every mid-paragraph case threw the reader to the start of the document after ten
full page layouts. The bisection was also unsound within its own space: a
document of 40 small paragraphs, where every page *does* start on a block
boundary, failed too.
This is complementary to S11 rather than caused by it. Before S11 forward
pagination dead-ended at the first page-spanning block, so mid-block starts were
never produced and the block-granular search looked adequate.
### Design
Pagination is a pure function: laying out from `q` yields a page and the position
it stopped at, `next(q)`. The page before `P` is the `q` with `next(q) == P`.
That is found by **replaying the chain forward from an anchor**, not by guessing
`q`. Three sources, in order:
1. **The recorded chain.** `render_page_forward` now records
`(font_scale, next(q)) -> q`. Stepping back to anywhere the reader has been is
exact and costs one layout. Keyed by font scale, since changing it
repaginates.
2. **Replay from an anchor.** Anchors are block starts, nearest first: the block
containing `P`, then up to `MAX_BACKWARD_ANCHORS` earlier ones, then the
document start. Lay out forward from the anchor until a page ends exactly on
`P`; that page's start is the answer. `MAX_REPLAY_PAGES` caps the walk so one
page turn cannot traverse a whole chapter.
3. **Nearest start before `P`.** If no chain passes exactly through `P` — which
happens when `P` was reached by a jump or a restored bookmark rather than by
reading forward, so it lies on no natural chain — return the last page start
before it. That overlaps `P`'s page slightly rather than skipping content,
which is the safe direction to be wrong in.
The estimator and the bisecting adjuster are deleted.
**What "correct" means here.** Each backward step returns a page ending exactly
where the reader currently is, so paging back never skips or repeats content.
That chain can differ from the one you would have seen reading forward from page
one, if you entered the document by a jump — pagination from a different starting
point is genuinely a different chain, and no algorithm can recover the original
without replaying from the start.
### Measurements
Same document, after the change:
```
warm (chain recorded by the forward pass): 4/4 exact, 1 layout each
cold, fresh layouter per call: 12/13 exact, worst 17 layouts
cold, one layouter, repeated back presses: 4 layouts per turn typical
```
The single inexact case is a target that lies on the canonical chain but not on
any chain reachable from a nearby anchor; it returns a start 15 words early,
i.e. a slightly overlapping page.
### Acceptance criteria
- For every page of a document, `render_page_backward(start[i])` returns
`start[i-1]` — verified for both a mid-paragraph-paginating document and one
where every page starts on a block boundary.
- Laying out forward from the returned position ends exactly on the requested
position.
- Forward-then-back returns to the original position.
- At the document start, backward stays there; an empty document is safe.
- Cost stays within a small bounded number of forward layouts.
### Files
`pyWebLayout/layout/ereader_layout.py`
---
## Test plan ## Test plan
Findings were reproduced with four probe scripts; each becomes a regression test Findings were reproduced with four probe scripts; each becomes a regression test
pyWebLayout/layout/ereader_layout.py
+131 -77
View File
@@ -313,6 +313,13 @@ class BidirectionalLayouter:
self.alignment_override = alignment_override self.alignment_override = alignment_override
self.font_family_override = font_family_override self.font_family_override = font_family_override
# Maps (font_scale, end position) -> the position the page started at.
# Filled in as pages are laid out forward, which makes "previous page"
# exact and free for anywhere the reader has already been. Keyed by font
# scale because changing it repaginates the document.
self._page_chain: Dict[Tuple[float, Tuple[int, int, int]],
RenderingPosition] = {}
def render_page_forward(self, position: RenderingPosition, def render_page_forward(self, position: RenderingPosition,
font_scale: float = 1.0) -> Tuple[Page, RenderingPosition]: font_scale: float = 1.0) -> Tuple[Page, RenderingPosition]:
""" """
@@ -367,109 +374,156 @@ class BidirectionalLayouter:
current_pos = new_pos current_pos = new_pos
# Remember this link in the chain so stepping back to it later is exact.
if self._position_compare(current_pos, position) > 0:
self._page_chain[(font_scale, self._position_key(current_pos))] = \
position.copy()
return page, current_pos return page, current_pos
# How many block starts before the target to try as replay anchors before
# settling for the best inexact answer.
MAX_BACKWARD_ANCHORS = 4
# Ceiling on pages replayed from a single anchor, so a pathologically long
# block cannot make one page turn walk an entire chapter.
MAX_REPLAY_PAGES = 8
def render_page_backward(self, def render_page_backward(self,
end_position: RenderingPosition, end_position: RenderingPosition,
font_scale: float = 1.0) -> Tuple[Page, font_scale: float = 1.0) -> Tuple[Page,
RenderingPosition]: RenderingPosition]:
""" """
Render a page that ends at the given position, filling backward. Render the page that ends at the given position - "previous page".
Critical for "previous page" navigation.
Uses iterative refinement to find the correct start position that Pagination is a pure function: laying out from a position q yields a page
results in a page ending at (or very close to) the target position. and the position where it stopped, next(q). The page before P is therefore
the q for which next(q) == P, and it is found by *replaying* the chain
forward from an anchor, not by guessing q.
The previous implementation searched instead: it estimated a block index
and bisected on it, pinning word_index to 0. Pages routinely start
mid-block, so the answer was frequently not in the search space at all -
the search then exhausted its iterations and fell back to a position that
was not the previous page, usually the start of the document.
Three sources are tried in order:
1. The recorded chain, from pages already laid out going forward. Exact,
and the common case when the reader is paging back and forth.
2. Replay from the start of the block containing P, then from
progressively earlier blocks. Exact when P lies on the resulting chain.
3. Failing an exact hit - which happens when P was reached by a jump or a
restored bookmark rather than by reading forward, so it is on no
natural chain - the latest page start before P. That overlaps P's page
slightly rather than skipping content, which is the safe direction to
be wrong in.
Args: Args:
end_position: Position where page should end end_position: Position where the page should end
font_scale: Font scaling factor font_scale: Font scaling factor
Returns: Returns:
Tuple of (rendered_page, start_position) Tuple of (rendered_page, start_position)
""" """
# Handle edge case: already at beginning document_start = RenderingPosition()
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 # Nothing precedes the start of the document.
estimated_start = self._estimate_page_start(end_position, font_scale) if self._position_compare(end_position, document_start) <= 0:
page, _ = self.render_page_forward(document_start, font_scale)
return page, document_start
# Iterative refinement: keep adjusting until we converge or hit max iterations # 1. The chain we have already walked.
max_iterations = 10 remembered = self._page_chain.get((font_scale, self._position_key(end_position)))
best_page = None if remembered is not None:
best_start = estimated_start page, actual_end = self.render_page_forward(remembered, font_scale)
best_distance = float('inf') if self._position_compare(actual_end, end_position) == 0:
return page, remembered
for iteration in range(max_iterations): # 2/3. Replay from anchors, keeping the best inexact result as a fallback.
# Render forward from current estimate fallback = None
page, actual_end = self.render_page_forward(estimated_start, font_scale) for anchor in self._backward_anchors(end_position):
page, start, exact = self._replay_to(anchor, end_position, font_scale)
if page is None:
continue
if exact:
return page, start
if fallback is None:
fallback = (page, start)
# Calculate how far we are from target if fallback is not None:
comparison = self._position_compare(actual_end, end_position) return fallback
# Perfect match or close enough (within same block) page, _ = self.render_page_forward(document_start, font_scale)
# BUT: ensure we actually moved backward (estimated_start < end_position) return page, document_start
if comparison == 0:
# Check if we actually found a valid previous page def _backward_anchors(self, target: RenderingPosition):
if self._position_compare(estimated_start, end_position) < 0: """
return page, estimated_start Yield positions to replay from, nearest first.
# If estimated_start >= end_position, we haven't moved backward
# Continue iterating to find a better position Block starts are used as anchors because they are the coarsest positions
elif iteration == 0: that are certainly valid to lay out from. The block containing the target
# On first iteration, if we can't find a previous position, comes first: when the target is mid-block, the page before it usually
# we're likely at or near the beginning starts in that same block or the one before.
"""
first_block = target.block_index if target.word_index > 0 \
else target.block_index - 1
for offset in range(self.MAX_BACKWARD_ANCHORS):
block_index = first_block - offset
if block_index < 0:
break break
yield RenderingPosition(
# Track best result so far chapter_index=target.chapter_index,
distance = abs(actual_end.block_index - end_position.block_index) block_index=block_index,
if distance < best_distance: word_index=0,
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)
# 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
# BUT: ensure we didn't return the same position (no backward progress)
final_page = best_page if best_page else page
final_start = best_start
# Safety check: if final_start >= end_position, we failed to move backward
# This can happen at the beginning of the document or when estimation failed
if self._position_compare(final_start, end_position) >= 0:
# Can't go further back - check if we're at the absolute beginning
if end_position.block_index == 0 and end_position.word_index == 0:
# Already at beginning, return as-is
return final_page, final_start
# Fallback strategy: try a more aggressive backward jump
# Start from several blocks before the current position
blocks_to_jump = max(1, min(5, end_position.block_index))
fallback_pos = RenderingPosition(
chapter_index=end_position.chapter_index,
block_index=max(0, end_position.block_index - blocks_to_jump),
word_index=0
) )
# Render forward from the fallback position if first_block - self.MAX_BACKWARD_ANCHORS >= 0:
fallback_page, fallback_end = self.render_page_forward(fallback_pos, font_scale) yield RenderingPosition()
# Verify the fallback actually moved us backward def _replay_to(self,
if self._position_compare(fallback_pos, end_position) < 0: anchor: RenderingPosition,
return fallback_page, fallback_pos target: RenderingPosition,
font_scale: float):
"""
Lay out pages forward from `anchor`, looking for the one ending at `target`.
# If even the fallback didn't work, we're likely at the beginning Returns:
# Return a page starting from the beginning (page, start, exact). `exact` is True when a page ended precisely on
return self.render_page_forward(RenderingPosition(), font_scale) the target. When the chain steps over the target instead, the last
page starting before it is returned with exact=False. (None, None,
False) means the anchor yielded nothing usable.
"""
position = anchor
last = (None, None)
return final_page, final_start for _ in range(self.MAX_REPLAY_PAGES):
if self._position_compare(position, target) >= 0:
break
page, next_position = self.render_page_forward(position, font_scale)
comparison = self._position_compare(next_position, target)
if comparison == 0:
return page, position, True
if comparison > 0:
# Stepped over the target: this chain does not pass through it.
return last[0], last[1], False
if self._position_compare(next_position, position) <= 0:
break # no progress; give up on this anchor
last = (page, position)
position = next_position
return last[0], last[1], False
@staticmethod
def _position_key(position: RenderingPosition) -> Tuple[int, int, int]:
"""Hashable identity of a position, for the page chain map."""
return (position.chapter_index, position.block_index, position.word_index)
def _scale_block_fonts(self, block: Block, font_scale: float) -> Block: def _scale_block_fonts(self, block: Block, font_scale: float) -> Block:
"""Apply font scaling and font family override to all fonts in a block""" """Apply font scaling and font family override to all fonts in a block"""
tests/layout/test_backward_navigation.py
+171
View File
@@ -0,0 +1,171 @@
"""
Regression tests for backward page navigation (spec S16).
The previous page of P is the position q for which laying out forward from q ends
exactly at P. The old implementation searched for q by guessing a block index and
bisecting, with word_index pinned to 0 - so a page starting mid-paragraph was not
in the search space at all. It exhausted its ten iterations and fell back to a
position that was not the previous page, typically the start of the document.
"""
import pytest
from pyWebLayout.abstract.block import Paragraph
from pyWebLayout.abstract.inline import Word
from pyWebLayout.layout.ereader_layout import BidirectionalLayouter, RenderingPosition
from pyWebLayout.style import Font
from pyWebLayout.style.page_style import PageStyle
PAGE_SIZE = (800, 600)
@pytest.fixture
def font():
return Font(font_size=16)
def paragraph(font, count, tag):
block = Paragraph(font)
for i in range(count):
block.add_word(Word(f"{tag}{i}", font))
return block
@pytest.fixture
def long_document(font):
"""Short paragraphs around one that spans several pages."""
return [
paragraph(font, 60, "a"),
paragraph(font, 80, "b"),
paragraph(font, 1200, "long"),
paragraph(font, 70, "c"),
paragraph(font, 90, "d"),
]
@pytest.fixture
def block_document(font):
"""Many small blocks, so every page starts on a block boundary."""
return [paragraph(font, 40, f"p{i}") for i in range(40)]
def forward_chain(layouter, limit=30):
"""The page start positions a reader would visit going forward."""
starts = []
pos = RenderingPosition()
for _ in range(limit):
starts.append(pos)
_, nxt = layouter.render_page_forward(pos, 1.0)
if nxt.block_index >= len(layouter.blocks):
break
if (nxt.block_index, nxt.word_index) == (pos.block_index, pos.word_index):
pytest.fail("forward pagination made no progress")
pos = nxt
return starts
def key(position):
return (position.chapter_index, position.block_index, position.word_index)
class TestBackwardMatchesForward:
"""The defining invariant: forward from the answer lands exactly on P."""
@pytest.mark.parametrize("document", ["long_document", "block_document"])
def test_previous_page_is_the_forward_predecessor(self, document, request):
blocks = request.getfixturevalue(document)
layouter = BidirectionalLayouter(blocks, PageStyle(), PAGE_SIZE)
starts = forward_chain(layouter)
assert len(starts) > 2, "need a few pages to test against"
for i in range(1, len(starts)):
_, got = layouter.render_page_backward(starts[i], 1.0)
assert key(got) == key(starts[i - 1]), (
f"page {i}: expected to land on page {i - 1} "
f"{key(starts[i - 1])}, got {key(got)}")
def test_result_lays_out_to_the_target(self, long_document):
"""Independent of the recorded chain: replaying the answer must reach P."""
layouter = BidirectionalLayouter(long_document, PageStyle(), PAGE_SIZE)
starts = forward_chain(layouter)
for target in starts[1:]:
_, start = layouter.render_page_backward(target, 1.0)
_, end = layouter.render_page_forward(start, 1.0)
assert key(end) == key(target), (
f"a page starting at {key(start)} ends at {key(end)}, "
f"not at the requested {key(target)}")
def test_mid_paragraph_targets_are_reachable(self, long_document):
"""The specific regression: starts inside a block, not on its boundary."""
layouter = BidirectionalLayouter(long_document, PageStyle(), PAGE_SIZE)
starts = forward_chain(layouter)
mid = [s for s in starts if s.word_index > 0]
assert mid, "this document should paginate mid-paragraph"
for target in mid:
_, got = layouter.render_page_backward(target, 1.0)
assert key(got) != (0, 0, 0) or key(target) == key(starts[1]), \
"backward navigation fell back to the document start"
class TestRoundTrip:
def test_forward_then_back_returns_to_the_same_place(self, long_document):
layouter = BidirectionalLayouter(long_document, PageStyle(), PAGE_SIZE)
pos = RenderingPosition()
for _ in range(4):
_, nxt = layouter.render_page_forward(pos, 1.0)
_, back = layouter.render_page_backward(nxt, 1.0)
assert key(back) == key(pos), \
f"round trip drifted: {key(pos)} -> {key(nxt)} -> {key(back)}"
pos = nxt
class TestEdges:
def test_at_document_start_stays_there(self, long_document):
layouter = BidirectionalLayouter(long_document, PageStyle(), PAGE_SIZE)
_, got = layouter.render_page_backward(RenderingPosition(), 1.0)
assert key(got) == (0, 0, 0)
def test_second_page_goes_back_to_the_first(self, long_document):
layouter = BidirectionalLayouter(long_document, PageStyle(), PAGE_SIZE)
_, second = layouter.render_page_forward(RenderingPosition(), 1.0)
_, got = layouter.render_page_backward(second, 1.0)
assert key(got) == (0, 0, 0)
def test_empty_document_is_safe(self):
layouter = BidirectionalLayouter([], PageStyle(), PAGE_SIZE)
page, got = layouter.render_page_backward(RenderingPosition(), 1.0)
assert page is not None
assert key(got) == (0, 0, 0)
class TestCost:
def test_backward_is_not_wildly_more_expensive_than_forward(self, long_document):
"""The old path burned ten full layouts per call and still got it wrong."""
layouter = BidirectionalLayouter(long_document, PageStyle(), PAGE_SIZE)
starts = forward_chain(layouter)
calls = {"n": 0}
original = BidirectionalLayouter.render_page_forward
def counting(self, position, font_scale=1.0):
calls["n"] += 1
return original(self, position, font_scale)
BidirectionalLayouter.render_page_forward = counting
try:
worst = 0
for target in starts[1:]:
calls["n"] = 0
layouter.render_page_backward(target, 1.0)
worst = max(worst, calls["n"])
finally:
BidirectionalLayouter.render_page_forward = original
assert worst <= 10, f"backward navigation cost {worst} forward layouts"