diff --git a/pyWebLayout/layout/ereader_layout.py b/pyWebLayout/layout/ereader_layout.py index 3699e3b..2a1878b 100644 --- a/pyWebLayout/layout/ereader_layout.py +++ b/pyWebLayout/layout/ereader_layout.py @@ -344,7 +344,14 @@ class BidirectionalLayouter: scaled_block, page, current_pos, font_scale) if not success: - # Block doesn't fit, we're done with this page + # The block did not fit in its entirety. It may still have been + # laid out partially - a paragraph larger than one page places as + # many lines as fit and reports the word it stopped at. Keeping + # that resume point is what allows the next page to continue; + # discarding it tells the caller no progress was made, which + # dead-ends navigation on the block forever. + if self._position_compare(new_pos, current_pos) > 0: + current_pos = new_pos break # Add inter-block spacing after successfully laying out a block diff --git a/pyWebLayout/layout/ereader_manager.py b/pyWebLayout/layout/ereader_manager.py index bbde43f..7faadb1 100644 --- a/pyWebLayout/layout/ereader_manager.py +++ b/pyWebLayout/layout/ereader_manager.py @@ -9,6 +9,7 @@ into a unified, easy-to-use API. from __future__ import annotations from typing import List, Dict, Optional, Tuple, Any, Callable import json +import logging from pathlib import Path from .ereader_layout import RenderingPosition, ChapterNavigator, ChapterInfo @@ -20,6 +21,8 @@ from pyWebLayout.style.page_style import PageStyle from pyWebLayout.style.fonts import BundledFont from pyWebLayout.layout.document_layouter import image_layouter +logger = logging.getLogger(__name__) + class BookmarkManager: """ @@ -417,6 +420,21 @@ class EreaderLayoutManager: self._notify_position_changed() return self.get_current_page() + # No progress. That is the correct answer only at the end of the + # document; anywhere else a block has failed to lay out and would trap + # the reader on this page. Skipping the block costs one block, not the + # rest of the book. + if self.current_position.block_index < len(self.blocks): + logger.error( + "Block %d made no layout progress; skipping it. This is a layout " + "bug - the block placed nothing and reported no resume point.", + self.current_position.block_index) + self.current_position = RenderingPosition( + chapter_index=self.current_position.chapter_index, + block_index=self.current_position.block_index + 1) + self._notify_position_changed() + return self.get_current_page() + return None # At end of document def previous_page(self) -> Optional[Page]: diff --git a/tests/layout/test_page_spanning_blocks.py b/tests/layout/test_page_spanning_blocks.py new file mode 100644 index 0000000..384da6f --- /dev/null +++ b/tests/layout/test_page_spanning_blocks.py @@ -0,0 +1,123 @@ +""" +Regression tests for blocks that span more than one page. + +A block larger than a single page is laid out partially, and the layouter reports +where it stopped. If that resume point is discarded, the reader is told it made no +progress and navigation dead-ends on that block (spec S11). +""" + +import pytest + +from pyWebLayout.abstract.block import Paragraph +from pyWebLayout.abstract.inline import Word +from pyWebLayout.concrete.page import Page +from pyWebLayout.layout.ereader_layout import BidirectionalLayouter, RenderingPosition +from pyWebLayout.style import Font +from pyWebLayout.style.page_style import PageStyle + + +PAGE_SIZE = (800, 600) + + +def make_paragraph(word_count, font): + """A paragraph of distinct words, so we can verify none are lost or repeated.""" + paragraph = Paragraph(font) + for i in range(word_count): + paragraph.add_word(Word(f"word{i}", font)) + return paragraph + + +@pytest.fixture +def font(): + return Font(font_size=16) + + +@pytest.fixture +def huge_paragraph(font): + """A paragraph far larger than one page - the shape that dead-ended.""" + return make_paragraph(2877, font) + + +class TestPageSpanningParagraph: + """A single paragraph larger than one page must paginate, not dead-end.""" + + def test_layouter_reports_where_it_stopped(self, huge_paragraph): + layouter = BidirectionalLayouter([huge_paragraph], PageStyle(), PAGE_SIZE) + page = Page(size=PAGE_SIZE, style=PageStyle()) + + success, new_pos = layouter._layout_block_on_page( + huge_paragraph, page, RenderingPosition(), 1.0) + + assert not success, "a 2877-word paragraph cannot fit on one page" + assert new_pos.word_index > 0, "the resume point must be reported" + + def test_first_page_advances(self, huge_paragraph): + """The regression: next position equalled the start position.""" + layouter = BidirectionalLayouter([huge_paragraph], PageStyle(), PAGE_SIZE) + start = RenderingPosition() + + page, next_pos = layouter.render_page_forward(start, 1.0) + + assert len(page.children) > 0, "content was placed on the page" + assert (next_pos.block_index, next_pos.word_index) > \ + (start.block_index, start.word_index), \ + "a page with content on it must advance the position" + + def test_paginates_to_completion(self, huge_paragraph): + """Every page advances, and the document terminates.""" + layouter = BidirectionalLayouter([huge_paragraph], PageStyle(), PAGE_SIZE) + pos = RenderingPosition() + positions = [(pos.block_index, pos.word_index)] + + for _ in range(100): + page, next_pos = layouter.render_page_forward(pos, 1.0) + key = (next_pos.block_index, next_pos.word_index) + + if next_pos.block_index >= 1: + break # ran off the end of the (single-block) document + + assert key > positions[-1], f"no progress at page {len(positions)}" + positions.append(key) + pos = next_pos + else: + pytest.fail("pagination did not terminate") + + assert len(positions) > 5, "a 2877-word paragraph spans several pages" + + def test_no_words_lost_or_repeated(self, huge_paragraph): + """Word coverage across pages is exactly the paragraph, in order.""" + layouter = BidirectionalLayouter([huge_paragraph], PageStyle(), PAGE_SIZE) + pos = RenderingPosition() + boundaries = [0] + + for _ in range(100): + _, next_pos = layouter.render_page_forward(pos, 1.0) + if next_pos.block_index >= 1: + break + boundaries.append(next_pos.word_index) + pos = next_pos + + assert boundaries == sorted(boundaries), "word indices must not go backward" + assert len(boundaries) == len(set(boundaries)), "a page must not be re-rendered" + + +class TestNonSpanningBlocksUnaffected: + """The fix must not change behaviour for blocks that fit.""" + + def test_small_paragraphs_still_advance_by_block(self, font): + blocks = [make_paragraph(20, font) for _ in range(3)] + layouter = BidirectionalLayouter(blocks, PageStyle(), PAGE_SIZE) + + _, next_pos = layouter.render_page_forward(RenderingPosition(), 1.0) + + assert next_pos.block_index == 3, "all three short paragraphs fit on one page" + assert next_pos.word_index == 0 + + def test_empty_document_terminates(self): + layouter = BidirectionalLayouter([], PageStyle(), PAGE_SIZE) + start = RenderingPosition() + + page, next_pos = layouter.render_page_forward(start, 1.0) + + assert next_pos.block_index == start.block_index + assert len(page.children) == 0