diff --git a/docs/LAYOUT_REMEDIATION_SPEC.md b/docs/LAYOUT_REMEDIATION_SPEC.md new file mode 100644 index 0000000..c963739 --- /dev/null +++ b/docs/LAYOUT_REMEDIATION_SPEC.md @@ -0,0 +1,1183 @@ +# Layout Remediation Spec + +Remediation plan for the defects found in the block/table rendering path +(audit of 2026-08-06), plus two outstanding defects in the pagination and +background-rendering paths (S11, S12). Twelve specs, sequenced into five phases. + +Every problem statement below was reproduced against the code at `2a543d0`; the +reproductions are quoted verbatim so each spec has a falsifiable "before" state. + +**S11 is the most user-visible defect in this document** and the cheapest to +fix: a single paragraph larger than one page dead-ends the reader permanently. +It is independent of every other spec here. + +## Contents + +| ID | Spec | Phase | +|----|------|-------| +| [S1](#s1--inline-content-in-non-paragraph-containers) | Inline content in non-paragraph containers | 0 | +| [S2](#s2--page-geometry-origin-and-content-rect) | Page geometry: origin and content rect | 1 | +| [S3](#s3--drawcanvas-lifecycle) | Draw/canvas lifecycle | 1 | +| [S4](#s4--one-block-dispatch-one-measurement) | One block dispatch, one measurement | 2 | +| [S5](#s5--cells-as-sub-layouts) | Cells as sub-layouts | 2 | +| [S6](#s6--table-grid-model) | Table grid model | 3 | +| [S7](#s7--retained-mode-table-rendering) | Retained-mode table rendering | 3 | +| [S8](#s8--table-and-list-pagination) | Table and list pagination | 4 | +| [S9](#s9--interactivity-inside-tables) | Interactivity inside tables | 4 | +| [S10](#s10--contracts-and-hygiene) | Contracts and hygiene | 5 | +| [S11](#s11--partial-block-progress-is-discarded) | Partial-block progress is discarded | 0 | +| [S12](#s12--background-rendering) | Background rendering | 4 | +| [S13](#s13--word-spacing-and-alignment) | Word spacing and alignment | 0 | + +## Design invariants + +These are the rules the specs exist to establish. After remediation, a reviewer +should be able to reject a change by pointing at one of these. + +1. **One dispatch.** There is exactly one function that maps an abstract block to + concrete objects on a page. Tables, the document layouter and the ereader all + call it. No component re-implements paragraph layout. +2. **Retained mode everywhere.** Layout produces children; `Page.render()` draws + them. Nothing paints during layout. A page can be rendered any number of times + and produce identical output. +3. **Measure and render agree by construction.** Reported height comes from the + same objects that get drawn, never from a parallel estimator. +4. **A cell is a page.** Any block the layout engine can place on a page can be + placed in a table cell, a list item or a blockquote, with no per-container + type switch. +5. **Style flows from the document.** No layout component invents a font, a size + or a path. Absolute font paths never appear outside `style/fonts.py`. + +## Phasing + +``` +S11 (independent, ship today) +S1 (independent, ship first) +S12 (independent; decide delete-vs-fix before touching S8) +S2 ─┬─ S4 ── S5 ─┬─ S6 ── S7 ─┬─ S8 +S3 ─┘ │ └─ S9 + └──────────────── S10 (opportunistic, any time after S7) +``` + +S11 is a few lines and unblocks reading the affected books; it ships on its own, +immediately. S1 is independent of everything else and fixes silent content loss, +so it ships next regardless of appetite for the rest. S12 is independent but +should be decided before S8, since table pagination changes the cost model that +justifies background rendering at all. S2+S3 are small and unlock S4/S5, which +are the bulk of the work. S6/S7 are where the table actually becomes correct. +S8/S9 are integration. S10 is cleanup. + +--- + +## S1 — Inline content in non-paragraph containers + +### Problem + +Inline tags (`a`, `b`, `strong`, `em`, `span`, `code`, …) are registered to +[`ignore_handler`](../pyWebLayout/io/readers/html_extraction.py#L825-L828) +because they are meant to be consumed by +[`extract_text_content()`](../pyWebLayout/io/readers/html_extraction.py#L364-L466). +But only `paragraph_handler` and `heading_handler` ever call that function. +`div_handler`, `list_item_handler`, `table_cell_handler` and +`table_header_cell_handler` iterate children and call `process_element` per +child, so inline tags return `None` and their text is discarded. `div_handler` +additionally ignores `NavigableString` children outright. + +### Evidence + +``` +

hello world again

-> Paragraph ['hello', 'world', 'again'] ok +
hello world again
-> (no blocks at all) lost +
  • hello world again
  • -> HList with no words lost +hello world again -> Table with no words lost +link text -> Paragraph ['text'] link lost +``` + +Bare text nodes that *do* survive (in `td`) each become their own `Paragraph`, +so `a b c` would fragment onto three lines even once `` is handled. + +### Design + +Introduce a single helper that every block container uses to process its +children, replacing the four hand-rolled loops. It walks children in order, +accumulating consecutive *inline* children (tags and text) into a run, and +flushing that run into one `Paragraph` whenever a *block* child interrupts it or +the children are exhausted. + +Inline-ness is decided by one predicate, not by each caller: + +```python +# html_extraction.py + +INLINE_TAGS: FrozenSet[str] = frozenset({ + "a", "span", "strong", "b", "em", "i", "u", "s", "del", "ins", "mark", + "small", "sub", "sup", "code", "q", "cite", "abbr", "time", "br", +}) + +def is_inline(node) -> bool: + """True for text nodes and inline tags; False for block tags.""" + +def process_block_children(element: Tag, context: StyleContext) -> List[Block]: + """ + Process an element's children into a block list, coalescing runs of inline + content into paragraphs. + + This is the single entry point for any container that may hold a mix of + inline and block content: div, li, td, th, blockquote, figure, section... + """ +``` + +`process_block_children` delegates each inline run to the existing +`extract_text_content`, which already handles `` → `LinkedWord`, style +nesting and background inheritance — it is called on a synthetic run rather +than on the whole element. The simplest correct implementation wraps the run's +nodes in a detached `Tag` and calls `extract_text_content` on it; that keeps +one code path for inline styling. + +`
    ` inside a run terminates the current paragraph and starts a new one, +replacing the current no-op [`line_break_handler`](../pyWebLayout/io/readers/html_extraction.py#L791-L794). + +Callers become one line each: + +```python +def div_handler(element, context): return process_block_children(element, context) +def list_item_handler(element, context): item = ListItem(...); item._blocks = process_block_children(...) +def table_cell_handler(element, context): cell = TableCell(...); for b in process_block_children(...): cell.add_block(b) +``` + +`paragraph_handler`/`heading_handler` keep their current behaviour (they are +already correct); their image-splitting logic in +[html_extraction.py:492-552](../pyWebLayout/io/readers/html_extraction.py#L492-L552) +is subsumed by `process_block_children` and should be folded in — an `` is +a block child, so the "text before, image after" split falls out of the general +algorithm for free. + +### Acceptance criteria + +- All four markup samples in *Evidence* produce the same words as the `

    ` + control, in document order, with `LinkedWord` preserved for ``. +- `

    a b c
    ` yields **one** `Paragraph` of three words, not three + paragraphs. +- `text

    para

    more` yields three blocks in order: Paragraph, Paragraph, + Paragraph — inline runs on either side of a block child are not merged. +- `a
    b` yields two paragraphs. +- `

    textmore

    ` behaves as it does today (regression). +- Round-trip test over `tests/io_tests/` fixtures shows no block-count or + word-count regressions. + +### Files + +`pyWebLayout/io/readers/html_extraction.py`, `tests/io_tests/test_html_extraction.py` + +### Risk + +Low, and contained to the reader. The change *adds* content where there was +none, so existing assertions on parsed structure may need their expected counts +raised. Grep for tests asserting `len(blocks) == N` before starting. + +--- + +## S2 — Page geometry: origin and content rect + +### Problem + +Two issues, one cause: `Page` assumes it is rooted at (0,0) and has no notion of +a content rectangle. + +1. **Horizontal padding is ignored.** `paragraph_layouter` sets + `x_cursor = page.border_size` ([document_layouter.py:154](../pyWebLayout/layout/document_layouter.py#L154)) + while line width is `page.available_width` (which *does* subtract both + paddings). With `border_width=2, padding=(40,40,40,40)` on a 400px page, the + line lands at `origin.x = 2, width = 316`: text hugs the border and the whole + 80px padding budget accumulates on the right. Vertical padding is honoured + ([page.py:34](../pyWebLayout/concrete/page.py#L34)), so the asymmetry is + horizontal only. +2. **A page cannot be placed inside another page**, which is the prerequisite for + S5 (cells as sub-layouts). + +### Design + +Give `Page` an origin and derive a content rect from it. Default `(0, 0)` keeps +every existing caller behaviourally identical apart from the padding fix. + +```python +class Page(Renderable, Queriable): + def __init__(self, size, style=None, origin: Tuple[int, int] = (0, 0)): ... + + @property + def origin(self) -> np.ndarray: + """Absolute top-left of the page box.""" + + @property + def content_origin(self) -> Tuple[int, int]: + """Absolute top-left of the content box (origin + border + padding).""" + + @property + def content_rect(self) -> Tuple[int, int, int, int]: + """(x, y, w, h) of the content box in absolute coordinates.""" + + @property + def remaining_height(self) -> int: + """Content-box height still available below _current_y_offset.""" +``` + +`remaining_height` replaces the ad-hoc +`page.size[1] - page._current_y_offset - page.border_size` computed inline by +`image_layouter`, `table_layouter`, `button_layouter` and `form_layouter` — all +four of which subtract the border but not the bottom padding, so every block +type may currently be placed up to `padding_bottom` past its boundary. + +The existing [`free_space()`](../pyWebLayout/concrete/page.py#L41-L43) has the +same defect (it returns full page width, and height ignoring bottom border and +padding) and has no callers; delete it in favour of `content_rect` / +`remaining_height`. + +All layouters switch from `page.border_size` to `page.content_origin[0]` for the +x cursor, and `_current_y_offset` is initialised to `content_origin[1]`. +`can_fit_line`'s `max_y` becomes `content_rect.y + content_rect.h`. + +`_current_y_offset` stays absolute, so no arithmetic elsewhere changes sign. + +### Acceptance criteria + +- With `size=(400,300), border_width=2, padding=(40,40,40,40)`: the first line + has `origin == (42, 42)` and `size[0] == 316`; rendered ink starts at + x ≥ 42 and ends at x ≤ 358. +- No block is placed within `padding_bottom` of the page bottom, for every block + type (paragraph, image, table, button, form). +- A `Page(size=(100,50), origin=(200,300))` places its first line at + `(200 + border + padding_left, 300 + border + padding_top)`. +- Existing rendering tests with default `padding=(20,20,20,20)` shift right by + 20px — golden images in `docs/images/` and `test_output/` must be regenerated + and eyeballed once, deliberately, as part of this spec. + +### Files + +`pyWebLayout/concrete/page.py`, `pyWebLayout/concrete/dynamic_page.py`, +`pyWebLayout/layout/document_layouter.py` + +### Risk + +Medium — it moves every existing rendering by `padding_left`. That is the point, +but it invalidates every golden image at once. Do it in its own commit, separate +from anything else, so the diff of regenerated images is reviewable. + +--- + +## S3 — Draw/canvas lifecycle + +### Problem + +[`Page.add_child`](../pyWebLayout/concrete/page.py#L140-L154) sets +`self._canvas = None` but leaves `self._draw` bound to the discarded canvas, and +the [`draw` property](../pyWebLayout/concrete/page.py#L131-L138) only rebuilds +when `_draw is None`. So after the first `add_child`, `page.draw` hands out a +draw context pointing at an orphaned image while `page._canvas` stays `None`. + +### Evidence + +``` +after first .draw: _canvas set? True +after add_child: _canvas set? False _draw is stale? True +page.draw returns same stale object? True page._canvas still None? True +=> a table laid out now receives canvas = None +``` + +Downstream: `table_layouter` passes `canvas=None` into `TableRenderer`, so every +image inside a cell silently degrades to a grey `[Image: WxH]` placeholder +([table.py:297-320](../pyWebLayout/concrete/table.py#L297-L320)). + +Note the trap: naively fixing the property so it rebuilds whenever +`_canvas is None` makes layout allocate a full-page RGBA canvas on *every* +`add_child`, because layout calls `page.draw` to measure text. The fix has to +separate the two uses. + +### Design + +Split measurement from rendering. + +```python +class Page: + @property + def measurement_draw(self) -> ImageDraw.ImageDraw: + """ + Persistent 1x1 scratch draw context used for text metrics during layout. + Never invalidated; matches the render canvas's mode so that + Text width caching keys stay consistent. + """ + + @property + def draw(self) -> ImageDraw.ImageDraw: + """Draw context bound to the live render canvas, rebuilt if invalidated.""" +``` + +- Layouters construct `Line`/`Text` with `page.measurement_draw`. +- `Page.render_children` already rebinds `child._draw = self._draw` and + `child._canvas = self._canvas` before rendering + ([page.py:256-269](../pyWebLayout/concrete/page.py#L256-L269)), so children + built against the scratch context draw onto the real canvas at render time. + That rebinding becomes load-bearing rather than incidental — document it as + such, and extend it to recurse into child pages (S5). +- `draw` rebuilds when `self._draw is None or self._canvas is None`. + +Because `Text._calculate_dimensions` keys its width cache on `self._draw.mode` +([text.py](../pyWebLayout/concrete/text.py)), the scratch image must be created +in the same mode as the render canvas (`RGBA`) or the cache will hold two +entries per word. + +### Acceptance criteria + +- Laying out 100 paragraphs allocates **zero** full-page canvases (assert via a + counter patched onto `_create_canvas`). +- After any sequence of `add_child` calls, `page.draw.im` is the same image + object as `page._canvas`. +- A table containing an image, laid out after a paragraph, renders the real + image and not the placeholder. +- `page.render()` called twice returns pixel-identical images. + +### Files + +`pyWebLayout/concrete/page.py`, `pyWebLayout/layout/document_layouter.py` + +--- + +## S4 — One block dispatch, one measurement + +### Problem + +There are three implementations of "lay out content in a box", which must agree +and do not: + +| Purpose | Location | Word spacing | Hyphenation | +|---|---|---|---| +| Rendering | [`_render_cell_content`](../pyWebLayout/concrete/table.py#L110-L242) | `0.25–0.5 × font_size` | real, via `Line.add_word` | +| Height | [`_estimate_wrapped_lines`](../pyWebLayout/concrete/table.py#L578-L639) | `0.25 × font_size` | "assume one line" | +| Width | [`layout_cell_content`](../pyWebLayout/layout/table_optimizer.py#L117-L186) | hardcoded `(3, 6)` | none | + +`layout_cell_content` additionally appends to `line._text_objects` directly, +bypassing the fitting logic it is trying to predict. + +Separately, block *dispatch* is duplicated three times: +[`DocumentLayouter.layout_document`](../pyWebLayout/layout/document_layouter.py#L683-L723), +[`EreaderLayout._layout_block_on_page`](../pyWebLayout/layout/ereader_layout.py#L493-L519), +and the `isinstance` chain in `_render_cell_content`. They support different +block types, which is why tables render paragraphs but not lists, and the +ereader renders neither. + +### Design + +Two new public functions, one module each. + +**Dispatch** — `pyWebLayout/layout/block_layouter.py`: + +```python +@dataclass +class LayoutResult: + complete: bool # block fully placed + next_word: Optional[int] = None # resume index if not complete + pretext: Optional[Text] = None # hyphenated remainder + next_item: Optional[int] = None # resume index for lists + next_row: Optional[int] = None # resume index for tables + +def layout_block(block: Block, + page: Page, + start_word: int = 0, + pretext: Optional[Text] = None, + **resume) -> LayoutResult: + """ + Place one abstract block onto a page, starting from a resume point. + The single dispatch point for Paragraph, Heading, Image, Table, HList, + Quote, CodeBlock, HorizontalRule, PageBreak, Button, Form. + """ +``` + +The existing `paragraph_layouter`, `image_layouter`, `table_layouter`, +`button_layouter`, `form_layouter` stay as the per-type implementations; +`layout_block` is the registry in front of them. `DocumentLayouter`, +`EreaderLayout` and the cell layout of S5 all call it, and gaining a block type +means adding one entry, once. + +**Measurement** — `pyWebLayout/layout/measure.py`: + +```python +@dataclass +class BlockMeasure: + min_width: int # narrowest the block can be without overflow + max_width: int # width at which the block never wraps + +def measure_block(block: Block, style_ctx: RenderingContext) -> BlockMeasure: + """Intrinsic width demands of a block, per the CSS min-content/max-content model.""" +``` + +Rules per type: +- `Paragraph`/`Heading`: `min_width` = widest unbreakable run — a word wider + than its font's `min_hyphenation_width` contributes its longest hyphenation + fragment, not its full width; `max_width` = Σ word widths + (n−1) × min spacing. +- `Image`: intrinsic width for both. +- `Table`: recurse (nested tables measure through the S6 grid). +- `HList`: item measure + marker indent. + +Measurement uses each word's *own* style, so it is correct for mixed formatting +— unlike all three current implementations, which assume one font for the box. + +Heights are **not** part of measurement. Height comes from running +`layout_block` at the final assigned width and reading `_current_y_offset`. +`_estimate_wrapped_lines` and `layout_cell_content` are deleted, and +`DynamicPage.get_min_width`/`get_preferred_width` are reimplemented in terms of +`measure_block` (they currently walk `Line._text_objects`, which only works if +someone has pre-poked the lines). + +### Acceptance criteria + +- `_estimate_wrapped_lines` and `layout_cell_content` no longer exist. +- Property test: for a corpus of paragraphs and widths, laying out at + `measure_block(b).max_width` produces exactly one line, and laying out at + `min_width` produces no line that overflows its box. +- `layout_block` handles every type in the union above; adding a type to the + registry makes it work in documents, cells and the ereader simultaneously + (assert with one test that exercises all three call sites). +- Mixed-font paragraph: a paragraph whose words have different sizes measures + wider than the same word count at the smallest size. + +### Files + +new `pyWebLayout/layout/block_layouter.py`, new `pyWebLayout/layout/measure.py`, +`pyWebLayout/layout/document_layouter.py`, `pyWebLayout/layout/table_optimizer.py`, +`pyWebLayout/concrete/dynamic_page.py`, `pyWebLayout/concrete/table.py` + +### Risk + +Largest spec by volume, but mostly deletion. Land `measure.py` first with tests +against the current optimizer's outputs to establish a baseline, then swap the +optimizer over, then delete. + +--- + +## S5 — Cells as sub-layouts + +### Problem + +[`TableCellRenderer._render_cell_content`](../pyWebLayout/concrete/table.py#L110-L242) +hand-rolls line breaking and handles only `Paragraph`, `Heading` and `Image` +([table.py:141-146](../pyWebLayout/concrete/table.py#L141-L146)); lists, nested +tables, quotes, code blocks, buttons and forms are silently dropped. It also +discards styling: it hardcodes +`/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf` (a Debian path — **it does not +exist on the dev machine**; 6 such literals across the package) and +`font_size = 12`, then rebuilds every word as a plain `Word(text, font)` +([table.py:156-171](../pyWebLayout/concrete/table.py#L156-L171)), throwing away +bold/italic/colour/size, the bundled-font system, the font-family override, and +`LinkedWord` link targets. + +`DynamicPage` — the abstraction that exists precisely for this — is used only +for measurement, despite `3bcd1bf` claiming cells can hold anything. + +### Design + +A cell owns a `DynamicPage` positioned at the cell's content origin, sharing the +parent's canvas, filled by `layout_block` (S4). + +```python +class TableCellRenderer(Box): + def __init__(self, cell, origin, size, style, is_header_section=False): + self._page = DynamicPage( + style=PageStyle(padding=style.cell_padding, border_width=0, + background_color=...), + origin=origin, # S2 + ) + self._page.layout(size) + for block in cell.blocks(): + layout_block(block, self._page) # S4 + + def render(self): + self._draw_background_and_border() + self._page.render_children() # children already positioned absolutely +``` + +Consequences that fall out rather than being coded: + +- **Arbitrary content works.** Nested tables, lists, buttons — anything in the + S4 registry. Invariant 4 is satisfied structurally, not by a type switch. +- **Styling is preserved**, because `layout_block` uses each word's own style. + Header cells get bold via `get_or_create_font(weight=FontWeight.BOLD)` + ([core/base.py:186](../pyWebLayout/core/base.py#L186)) rather than by + substituting a font *file path*. No absolute paths outside `style/fonts.py`. +- **Cell height is the laid-out height** (`_page._current_y_offset - origin.y`), + so S6's row heights are exact by construction (invariant 3). +- **Overflow is bounded**: `can_fit_line` against the cell page's content rect + stops content at the cell edge instead of painting over neighbours. + +`TableCellRenderer` keeps its constructor shape so `TableRowRenderer` is +unaffected, but loses the `draw`/`canvas` parameters — the canvas arrives at +render time via the S3 rebinding, which must now recurse into child pages: + +```python +def render_children(self): + for child in self._children: + if isinstance(child, Page): + child.attach_surface(self._canvas, self._draw) # recurse + ... +``` + +### Acceptance criteria + +- A cell containing an `HList`, a nested `Table`, a `Quote` and a `Button` + renders all four (today: a bare list and a nested table draw nothing but the + cell border). +- A cell whose words carry bold/italic/24px/red styling renders with those + attributes; assert on the `Text` objects' `_style`, not on pixels. +- `grep -rn "/usr/share/fonts" pyWebLayout/` returns 0 hits. +- A cell containing a `LinkedWord` produces a `LinkText` concrete object + (prerequisite for S9). +- Cell content that exceeds the cell's height is clipped at the cell boundary + and does not overpaint the next row. + +### Files + +`pyWebLayout/concrete/table.py`, `pyWebLayout/concrete/dynamic_page.py`, +`pyWebLayout/concrete/page.py` + +--- + +## S6 — Table grid model + +### Problem + +Four separate geometry defects: + +1. **`size` is wrong.** [table.py:445-446](../pyWebLayout/concrete/table.py#L445-L446) + computes total height as `sum(self._row_heights.values())`, but `_row_heights` + is a **three-entry dict** (`header`/`body`/`footer`), not one entry per row. + A 10-row table reports height **44px**; its bottom-most drawn pixel is at + **y=409**. The caption is drawn but not counted either. +2. **Uniform row heights.** [`_calculate_row_height_for_section`](../pyWebLayout/concrete/table.py#L499-L576) + takes the max across all rows in a section, so one verbose cell inflates every + row in the table. +3. **colspan is not counted.** [`get_column_count`](../pyWebLayout/layout/table_optimizer.py#L189-L205) + returns `first_row.cell_count`. For `` followed by + `` it returns **1**, and + [`TableRowRenderer.render`](../pyWebLayout/concrete/table.py#L383) silently + drops every cell past `len(column_widths)` — two of three cells never render. +4. **rowspan is parsed and stored but never read** by any renderer or measurer; + spanned rows just shift left. + +### Design + +A resolved grid, built once, that every other stage consumes. + +```python +# pyWebLayout/layout/table_grid.py + +@dataclass(frozen=True) +class GridCell: + cell: TableCell + row: int # resolved row index across all sections + col: int # resolved column index + colspan: int + rowspan: int + section: str # "header" | "body" | "footer" + +class TableGrid: + """ + Occupancy-resolved view of an abstract Table. + + Built by the standard HTML table algorithm: walk rows in order, maintaining a + set of slots occupied by open rowspans, placing each cell at the first free + column and marking its span. + """ + n_cols: int # max over rows of sum(colspan), not first row's cell count + n_rows: int + def cells(self) -> Iterator[GridCell]: ... + def row(self, index: int) -> List[GridCell]: ... + def cell_at(self, row: int, col: int) -> Optional[GridCell]: ... +``` + +**Column widths.** `optimize_table_layout` takes a `TableGrid` and per-cell +`measure_block` results (S4). Span distribution follows CSS: a cell spanning *k* +columns imposes its demand on the *sum* of those columns, and only widens them +(proportionally to their current demand) if the sum falls short. Single-column +demands are applied first so spanning cells cannot dominate. + +**Row heights.** Per row, not per section: lay out every cell in the row at its +assigned width (S5) and take the max of the resulting content heights. A cell +with `rowspan = k` contributes to row *r+k−1* only if the accumulated height of +rows *r…r+k−1* is less than the cell needs, and the shortfall is distributed +across those rows. + +**Size.** `TableRenderer._size` = caption height + Σ per-row heights + borders, +computed from the same per-row heights that `render()` walks. The two must come +from one list; a test asserts they cannot diverge. + +### Acceptance criteria + +- `` → `grid.n_cols == 3`, and all + four cells render. +- `` spans two rows vertically; the cell below it in the next row + is not shifted left. +- 10-row table: `renderer.size[1]` equals `last_drawn_pixel_y - origin_y + border` + (±1 for border rounding). This is the direct regression test for defect 1 — + today it is 44 vs 409. +- A table with one tall row and nine short ones is shorter than 10 × tall row. +- A captioned table's `size[1]` includes the caption. +- Fuzz: for randomly generated colspan/rowspan tables, no two rendered cells' + rectangles overlap, and every cell in the abstract table appears exactly once + in the grid. + +### Files + +new `pyWebLayout/layout/table_grid.py`, `pyWebLayout/layout/table_optimizer.py`, +`pyWebLayout/concrete/table.py` + +--- + +## S7 — Retained-mode table rendering + +### Problem + +Every other block type is retained-mode: layouters build objects, `add_child` +them, and `Page.render()` draws them onto a fresh canvas. Tables are +immediate-mode: [`table_layouter`](../pyWebLayout/layout/document_layouter.py#L351-L402) +grabs `page.draw`, paints directly, bumps `_current_y_offset`, and never adds +anything to `page._children`. + +### Evidence + +``` +layout_table -> True +pixels on canvas right after layout: 28328 +pixels after page.render(): 0 +children on page: 0 +``` + +`Page.render()` rebuilds the canvas ([page.py:279](../pyWebLayout/concrete/page.py#L279)) +and the table is not a child, so it is erased. Lay out a table *and* a +paragraph, render, and only the paragraph survives. The examples appear to work +only because they read `page._canvas` directly instead of calling `render()`. + +Because `size` lies (S6), the fit check +`if table_height > available_height: return False` is also meaningless: a ~950px +table "fits" a 300px page, draws past the border, and leaves +`_current_y_offset = 64` so the next paragraph overlaps it. + +### Design + +- `TableRenderer.__init__` measures only — it builds the grid, resolves widths, + lays out cells (S5) and computes `size`. It draws nothing and takes no + `draw`/`canvas` parameters. +- `TableRenderer.render()` draws, and is called by `Page.render_children()`. +- `table_layouter` becomes structurally identical to `image_layouter`: + +```python +def table_layouter(table, page, style=None) -> bool: + renderer = TableRenderer(table, origin=(page.content_origin[0], page._current_y_offset), + available_width=page.available_width, style=style) + if renderer.size[1] > page.remaining_height: + return False # honest check, honest size (S6) + page.add_child(renderer) # retained mode + return True +``` + +- `_row_renderers` / `_cell_renderers` are rebuilt per `render()` call, not + appended to (today they grow without bound on re-render). + +### Acceptance criteria + +- `layout_table(...)` then `page.render()` yields a canvas containing the table. +- `page.render()` twice → pixel-identical images. +- Table + following paragraph: both present, non-overlapping, paragraph starts + below `table.size[1]`. +- A table taller than the remaining space returns `False` and adds no child. +- `len(page.children) == 1` after laying out one table. +- Rendering the same `TableRenderer` five times leaves + `len(renderer._row_renderers)` equal to the row count. + +### Files + +`pyWebLayout/concrete/table.py`, `pyWebLayout/layout/document_layouter.py` + +--- + +## S8 — Table and list pagination + +### Problem + +[`EreaderLayout._layout_table_on_page`](../pyWebLayout/layout/ereader_layout.py#L598-L611) +skips tables outright ("For now, skip tables"), and `_layout_list_on_page` does +the same for lists. The ereader — the library's main consumer — renders neither. +`RenderingPosition` already carries `table_row`, `table_col` and +`list_item_index` ([ereader_layout.py:38-40](../pyWebLayout/layout/ereader_layout.py#L33-L45)), +so the state model anticipated this; only the layout half is missing. +`examples/13_table_pagination_demo.py` documents pagination behaviour that the +library does not implement. + +### Design + +With S4 (`layout_block` returning a resume point) and S6/S7 (honest per-row +geometry), pagination is row-splitting rather than new machinery. + +```python +def table_layouter(table, page, style=None, start_row: int = 0) -> LayoutResult: + """ + Place as many rows as fit from start_row. + Returns complete=False with next_row set when the table is split. + """ +``` + +Rules: +- Split only on row boundaries. A single row taller than a full page is placed + anyway and clipped — with a `log.warning`, not silently. +- Header rows repeat at the top of each continuation fragment. This is a + `TableStyle` flag (`repeat_header: bool = True`), because for a two-row table + it is noise. +- Column widths are resolved once for the whole table and reused across + fragments, so columns line up across pages. + +`EreaderLayout._layout_table_on_page` / `_layout_list_on_page` delegate to +`layout_block` and map `LayoutResult.next_row` / `next_item` onto +`RenderingPosition`. The existing bidirectional navigation +(`render_page_backward`) must round-trip through table positions — that is the +part most likely to bite, so it gets its own test. + +### Acceptance criteria + +- A 60-row table across a 3-page document renders every row exactly once, + no duplicates, no gaps. +- Forward then backward navigation across a table returns to the identical + starting position (extends `tests/layout/test_navigation_consistency.py`). +- With `repeat_header=True`, every fragment starts with the header row. +- A serialized `RenderingPosition` inside a table restores to the same page after + reload. +- Lists paginate mid-list and resume at the right item, with markers continuing + their numbering. + +### Files + +`pyWebLayout/layout/ereader_layout.py`, `pyWebLayout/layout/document_layouter.py`, +`pyWebLayout/concrete/table.py` + +### Risk + +The ereader's backward-navigation estimator +([ereader_layout.py:400-465](../pyWebLayout/layout/ereader_layout.py#L430-L465)) +assumes blocks are cheap to re-lay-out repeatedly. Tables are not. Expect to +need a per-table layout cache keyed on `(table_id, available_width, font_scale)` +before this is usable at speed. + +--- + +## S9 — Interactivity inside tables + +### Problem + +Nothing inside a table is hit-testable: `query_point` into a rendered table +returns `object_type="empty"`. Links, images and buttons in cells are invisible +to the query/selection/callback system the rest of the library is built on. +`examples/14_interactive_table.py` works around this by hand-painting buttons +*on top of* the table and doing its own coordinate maths — the demo fakes the +feature. Meanwhile `TableCellRenderer` sets bounds on `InteractiveImage` +([table.py:322-327](../pyWebLayout/concrete/table.py#L322-L327)), a third, +parallel interaction mechanism. + +### Design + +S5 and S7 make this mostly free: cells are pages, tables are children, and +`Page.query_point` already recurses into children that implement `query_point` +([page.py:345-358](../pyWebLayout/concrete/page.py#L345-L358)). + +Required: +- `TableRenderer` and `TableCellRenderer` implement `Queriable.in_object` (they + are `Box` subclasses, so bounds already exist) and `query_point`, delegating to + the cell's `DynamicPage`. +- Because child pages are positioned in absolute coordinates (S2), **no + coordinate translation is needed** — a translating implementation would be a + sign S2 was not applied. +- Callback registration cascades: a cell page's `CallbackRegistry` merges into + the owning page's registry at `add_child` time, so `page.callbacks` remains the + single lookup point. +- `InteractiveImage.set_rendered_bounds` becomes redundant for the table path; + keep it for direct users but stop calling it from the cell renderer. + +### Acceptance criteria + +- `page.query_point(p)` for a point over a link in a cell returns + `object_type == "link"` with the correct `link_target`. +- A `Button` in a cell fires its callback through the normal + `page.callbacks` path. +- `examples/14_interactive_table.py` is rewritten to put real `Button` blocks in + cells and delete its manual overlay maths — the example shrinks substantially, + which is the acceptance signal. +- `query_range` selection spanning a table returns the cell text in document + order. + +### Files + +`pyWebLayout/concrete/table.py`, `pyWebLayout/concrete/page.py`, +`pyWebLayout/core/callback_registry.py`, `examples/14_interactive_table.py` + +--- + +## S10 — Contracts and hygiene + +Small independent items. Each is a one-commit change; none blocks the others. + +### 10.1 Render contract + +`TableRenderer.render()`, `TableRowRenderer.render()` and +`TableCellRenderer.render()` are annotated `-> Image.Image` and all +`return None`. After S7, decide and enforce one contract: `render()` draws onto +the bound canvas and returns `None`; only `Page.render()` returns an image. +Update `Renderable.render`'s docstring in +[core/base.py:16-22](../pyWebLayout/core/base.py#L16-L22), which currently +promises a `PIL.Image`, and annotate accordingly. + +### 10.2 Abstract/concrete leak + +[concrete/__init__.py:18](../pyWebLayout/concrete/__init__.py) re-exports the +**abstract** `Table, TableRow as Row, TableCell as Cell` from the concrete +package, aliased to look concrete, while `TableRenderer` is not exported at all. +This contradicts `ARCHITECTURE.md`. Remove the re-export, export the renderers, +and add the `Cell`/`Row` names to a deprecation shim for one release if anything +external depends on them. + +### 10.3 Dead code + +- [table.py:229-242](../pyWebLayout/concrete/table.py#L229-L242): fallback + reading `self._cell._text_content`, an attribute that exists nowhere. +- `TableCellRenderer._children`: written never, read never. +- `dynamic_page.py`: `import numpy as np` unused; + `render_partial`/`has_more_content`/`reset_pagination` are an unused + pagination API superseded by S8 — delete or wire up, do not leave both. +- [document_layouter.py:157-161](../pyWebLayout/layout/document_layouter.py#L157-L161): + `temp_text.width` computed and discarded; `else: pass`. + +### 10.4 Exception handling + +13 bare `except Exception:` / `except BaseException:` in the package. The worst +is [table.py:331-333](../pyWebLayout/concrete/table.py#L331-L333), which swallows +every image failure into a 20px gap with no diagnostic. Policy: catch the +specific exception, log at `warning` with `exc_info=True`, and render a visible +placeholder for content failures. `except BaseException` (which catches +`KeyboardInterrupt`) is never correct here. + +### 10.5 Image source resolution + +[`_render_image_in_cell`](../pyWebLayout/concrete/table.py#L252-L266) probes six +attribute names (`source`, `_source`, `path`, `src`, `_path`, `_src`) to find an +image path. Give `abstract.block.Image` one documented accessor and use it. + +### Acceptance criteria + +- `grep -rn "except BaseException" pyWebLayout/` → 0 hits. +- No `hasattr` chain longer than one alternative anywhere in `concrete/table.py`. +- `python -W error -c "import pyWebLayout"` clean; flake8 reports no unused + imports in touched files. + +--- + +## S11 — Partial-block progress is discarded + +### Problem + +`BidirectionalLayouter.render_page_forward` discards the resume position of a +block that only partially fitted: + +```python +success, new_pos = self._layout_block_on_page(scaled_block, page, current_pos, font_scale) +if not success: + # Block doesn't fit, we're done with this page + break # <-- new_pos dropped on the floor +... +current_pos = new_pos +return page, current_pos +``` +[ereader_layout.py:341-347](../pyWebLayout/layout/ereader_layout.py#L330-L362) + +`paragraph_layouter` correctly returns the index of the first word that did not +fit, and `_layout_paragraph_on_page` correctly packs it into `new_pos.word_index` +([ereader_layout.py:570-582](../pyWebLayout/layout/ereader_layout.py#L570-L582)). +The information exists and is thrown away one frame up the stack. The returned +"next position" is therefore the *same* position the page started at. + +The bug is invisible for a document whose paragraphs each fit on a page — the +`not success` path is only taken when nothing more fits, which for normal prose +means the block boundary. It bites exactly when one block spans a page boundary, +i.e. any paragraph longer than a page. + +### Evidence + +A 2877-word paragraph at 800×600: + +``` +_layout_block_on_page -> success=False, new_pos.word_index=271 (start was 0) +render_page_forward -> next position (b0, w0) (start was b0, w0) + +page 0: start(b0,w0) -> next(b0,w0) lines=26 DEAD END +``` + +The page renders 26 lines of real content, then reports that the reader has made +no progress. Navigation is stuck on that page permanently; the book is +unreadable from that block onward. + +### Design + +Distinguish *nothing placed* from *something placed*. Only the former should +leave the position untouched. + +```python +if not success: + if self._position_compare(new_pos, current_pos) > 0: + # Partially placed: keep the progress, end the page here. + current_pos = new_pos + break +``` + +`_position_compare` already exists and is used by the backward navigation +([ereader_layout.py:439](../pyWebLayout/layout/ereader_layout.py#L439)), so +ordering semantics stay in one place. + +Two supporting changes, because a silent dead-end should not be possible again: + +1. **A no-progress guard in the navigation loop.** `EreaderManager.next_page` + asserts that the returned position is strictly greater than the requested + one; if not, it logs an error naming the block index and force-advances + `block_index += 1`. A malformed block should cost the reader one block, not + the rest of the book. +2. **The same audit for the backward path.** `render_page_backward`'s refinement + loop ([ereader_layout.py:400-465](../pyWebLayout/layout/ereader_layout.py#L430-L465)) + already has fallbacks for failing to move backward, which is the symmetric + symptom — check whether those fallbacks were compensating for this bug and + simplify them if so. + +### Acceptance criteria + +- A 2877-word single-paragraph document paginates to completion: 12 pages at + 800×600, every word appearing exactly once, positions strictly increasing. + (Verified against the patched implementation: pages advance + w0 → w271 → w531 → … → w2684 → end.) +- Forward-then-backward across a page-spanning paragraph round-trips to the + original position. +- A block that genuinely places nothing (e.g. an image taller than the page on + an empty page) still returns the unchanged position, and the no-progress guard + force-advances it with a logged error rather than looping. +- Regression test asserts `next_position > position` for **every** page of a + full-book pagination run over the EPUB fixtures. + +### Files + +`pyWebLayout/layout/ereader_layout.py`, `pyWebLayout/layout/ereader_manager.py` + +### Risk + +Low, and it makes pagination strictly more correct. Watch for page-count changes +in `tests/layout/test_navigation_consistency.py` — any fixture with a +page-spanning paragraph will now produce more pages, which is the fix working. + +--- + +## S12 — Background rendering + +### Problem + +`PageBuffer` starts a `ProcessPoolExecutor(max_workers=4)` +([page_buffer.py:117](../pyWebLayout/layout/page_buffer.py#L117)) and submits +page renders to it. **Every job fails.** `_render_page_worker` returns +`pickle.dumps(page)` ([page_buffer.py:50](../pyWebLayout/layout/page_buffer.py#L50)), +and a `Page` holds a live PIL canvas, which is not picklable. + +### Evidence + +``` +args pickle OK: 877.5 KB in 21 ms # submit succeeds +real pool round trip -> job FAILED: TypeError cannot pickle 'ImagingCore' object +``` + +So the cost is paid in full and the benefit is zero: 4 forked processes, the +entire block list pickled and shipped per job (~880KB for a 200-block document), +a full page laid out in the worker — then the result is thrown away by +`check_completed_renders`, which swallows the exception into a bare `print` +([page_buffer.py:274-276](../pyWebLayout/layout/page_buffer.py#L274-L276)). On a +4-core Pi with 512MB this is actively harmful: four interpreter copies plus four +copies of the book, to populate a cache that never populates. + +Four further defects in the same file, which matter only if the decision is to +keep it: + +1. `_render_page_worker` builds `BidirectionalLayouter(blocks, page_style, font_family_override=...)` + **without `page_size`**, so it silently uses the default `(800, 600)` + ([page_buffer.py:44](../pyWebLayout/layout/page_buffer.py#L44)). Fixing the + pickling alone would poison the cache with wrong-size pages. +2. `check_completed_renders` caches every result with `is_backward=False` + ([page_buffer.py:268](../pyWebLayout/layout/page_buffer.py#L268)), so backward + renders land in the forward buffer. +3. `_queue_forward_renders` / `_queue_backward_renders` `break` at the end of + their first loop body, so despite `for i in range(self.buffer_size)` they + queue at most one page each. +4. Failures are reported with `print`, not the logger. + +### Design + +**Recommendation: delete the process pool.** Replace it with synchronous +readahead, gated on a measurement. + +The justification for multiprocessing was sub-second navigation. That premise +predates the text caches now landing in `concrete/text.py`, whose own +measurements put a page at ~30ms on desktop once warm, with +`EreaderManager.prewarm_caches` ([ereader_manager.py:222](../pyWebLayout/layout/ereader_manager.py#L222)) +priming the working set at open time. If a page costs tens of milliseconds, a +process pool cannot pay for its own IPC, let alone its memory on the target +device. + +```python +class PageBuffer: + def __init__(self, buffer_size: int = 5): + """LRU page cache with synchronous readahead. No worker processes.""" + + def readahead(self, position: RenderingPosition, n: int = 1) -> None: + """ + Render and cache the next n pages on the calling thread. + Called after a navigation completes, when the reader is idle. + """ +``` + +This keeps the LRU buffers, the position maps and the invalidation logic — all +of which are fine — and removes the executor, the worker function, the pickling +and the `threading.Lock` that only guarded the pending-render dict. + +**Gate:** measure first, on the Pi, with the caches warm. Record page render time +at the target page size in the spec's PR description. If p95 is under ~150ms, +delete the pool. If it is materially worse, the fallback is a **single worker +thread**, not processes: layout is PIL-bound, PIL releases the GIL for +rasterisation, and a thread shares the block list instead of copying it. Fixing +the process pool properly would require making the entire concrete tree +picklable (Page → Line → Text → Font → `FreeTypeFont`), which is a large amount +of surface area to maintain for a cache. + +Whichever way the gate goes, defects 1–4 above are fixed or deleted with the code +that contains them, and failures are logged with `exc_info=True` rather than +printed. + +### Acceptance criteria + +- No `ProcessPoolExecutor` in the package, or — if the gate says keep it — a test + that submits a real job through a real pool and asserts it **succeeds**. The + absence of such a test is why this shipped broken. +- Measured page-render p95 on the target device recorded in the PR, before and + after. +- Peak RSS while paginating a full book drops by roughly the pool's share + (expect ~4 interpreter copies' worth on a 4-core device). +- Readahead of *n* pages populates the cache with *n* pages (today: one queued, + zero cached). +- Backward-rendered pages land in the backward buffer. +- Cache invalidation on font-scale and font-family change still clears both + buffers. + +### Files + +`pyWebLayout/layout/page_buffer.py`, `pyWebLayout/layout/ereader_manager.py` + +### Risk + +Low. The feature currently contributes nothing but overhead, so removing it +cannot regress rendering; the only risk is navigation latency, which is what the +gate measures. + +--- + +## S13 — Word spacing and alignment + +### Problem + +Three defects, all visible as a right edge that wobbles from line to line. + +1. **Ragged alignments stretched their gaps.** `LeftAlignmentHandler` distributed + the line's residual space across its word gaps, clamped to `max_spacing`. A + line whose residual divided to less than `max_spacing` was stretched flush; + one that exceeded it was not. So left-aligned text was justified *sometimes*, + by a different amount on each line. `CenterRightAlignmentHandler` did the same, + and additionally returned `ideal_space` while computing its start position from + a different value (`actual_spacing`), so centred lines were not centred. +2. **The last line of a justified paragraph was justified.** A three-word tail was + spread across the full measure. +3. **Justified lines fell 1–2px short.** `base_spacing = int(residual // gaps)` + with `remainder = int(residual % gaps)` discards the fractional part of both + terms, and word widths are fractional. + +### Design + +- Ragged alignments (left, centre, right) use a **constant** word space: the + font's own space advance, clamped to `[min_spacing, max_spacing]`, passed to + the handler as `natural_spacing`. They never absorb residual space — that + belongs in the margin. When a line cannot fit at natural spacing they report + overflow rather than tightening, so line breaking moves the word instead of + rendering deciding to squeeze it. +- `Line` carries `is_paragraph_end`, set by `paragraph_layouter` on the line + holding a paragraph's final word. `render_alignment_handler` substitutes flush + left for justify on that line only. A paragraph continued onto the next page + never reaches the marking code, so its lines stay justified — correct. +- Justification distributes the residual by **cumulative rounding** + (`round(total * i / gaps)` differenced), so the gaps sum to the residual + exactly and every line ends at the same x. +- Alignment becomes configurable: `PageStyle.default_alignment`, defaulting to + `JUSTIFY`, replaces the hardcoded `Alignment.LEFT` in `paragraph_layouter`. + `AbstractStyle.text_align` / `ConcreteStyle.text_align` now default to `None` + meaning "not specified", so HTML that sets no `text-align` inherits the page + default while explicit CSS still wins. Headings are never justified. + +### Acceptance criteria + +- Left-aligned word gaps are constant within a line and across lines (±1px). +- Left-aligned text does not end flush on every line — a flush edge means it was + justified. +- Justified body lines end within 2px of the margin; measured advance ends are + identical across lines, with ≤1px of ink variation from side bearings. +- The final line of a completed justified paragraph is not stretched. +- Centred lines have equal margins either side (±2px). +- Headings are flush left even when the page default is justify. + +### Files + +`pyWebLayout/concrete/text.py`, `pyWebLayout/layout/document_layouter.py`, +`pyWebLayout/style/page_style.py`, `pyWebLayout/style/abstract_style.py`, +`pyWebLayout/style/concrete_style.py` + +--- + +## Test plan + +Findings were reproduced with four probe scripts; each becomes a regression test +rather than being thrown away. + +| Regression test | Guards | Currently | +|---|---|---| +| `test_table_survives_page_render` | S7 | fails (0 px after render) | +| `test_table_size_matches_drawn_extent` | S6 | fails (44 vs 409) | +| `test_table_fit_check_rejects_oversized` | S6/S7 | fails (returns True) | +| `test_colspan_column_count` | S6 | fails (1 vs 3) | +| `test_rowspan_occupancy` | S6 | fails (ignored) | +| `test_inline_content_in_div_li_td` | S1 | fails (content lost) | +| `test_page_draw_not_stale_after_add_child` | S3 | fails (stale) | +| `test_horizontal_padding_honoured` | S2 | fails (x=2 vs 42) | +| `test_arbitrary_blocks_in_cell` | S5 | fails (dropped) | +| `test_query_point_into_table_cell` | S9 | fails ("empty") | +| `test_page_spanning_paragraph_advances` | S11 | fails (dead-ends at page 0) | +| `test_background_render_job_succeeds` | S12 | fails (unpicklable Page) | + +Note that `tests/concrete/test_table_rendering.py:541-553` currently asserts only +`height > 0`, which is why the size defect survived. Assertions of that shape +should be replaced wherever the tests touch geometry. + +Golden images in `docs/images/` are regenerated **once** under S2 and reviewed +deliberately; after that they are treated as fixtures and any diff is a +regression. + +## Out of scope + +Called out so their absence is a decision rather than an oversight: + +- CSS percentage widths in `parse_html_width` + ([table_optimizer.py:283-284](../pyWebLayout/layout/table_optimizer.py#L283-L284)) — + needs a containing-block model. +- `border-collapse: separate`, per-cell borders, per-cell background colour. +- Vertical alignment within cells (`valign`); everything is top-aligned. +- RTL and vertical writing modes. +- Floats and absolute positioning. diff --git a/docs/images/example_04_table_rendering.png b/docs/images/example_04_table_rendering.png index 1c59b29..39d56e9 100644 Binary files a/docs/images/example_04_table_rendering.png and b/docs/images/example_04_table_rendering.png differ diff --git a/docs/images/example_05_html_table_with_images.png b/docs/images/example_05_html_table_with_images.png index a11f45d..80cf98c 100644 Binary files a/docs/images/example_05_html_table_with_images.png and b/docs/images/example_05_html_table_with_images.png differ diff --git a/docs/images/example_07_button_animation.gif b/docs/images/example_07_button_animation.gif index fb3deca..b0f87a3 100644 Binary files a/docs/images/example_07_button_animation.gif and b/docs/images/example_07_button_animation.gif differ diff --git a/docs/images/example_08_pagination_auto.png b/docs/images/example_08_pagination_auto.png index 4875be1..3b855d6 100644 Binary files a/docs/images/example_08_pagination_auto.png and b/docs/images/example_08_pagination_auto.png differ diff --git a/docs/images/example_08_pagination_explicit.png b/docs/images/example_08_pagination_explicit.png index 8ad4219..a109328 100644 Binary files a/docs/images/example_08_pagination_explicit.png and b/docs/images/example_08_pagination_explicit.png differ diff --git a/docs/images/example_09_link_navigation.png b/docs/images/example_09_link_navigation.png index 260b4af..5339820 100644 Binary files a/docs/images/example_09_link_navigation.png and b/docs/images/example_09_link_navigation.png differ diff --git a/docs/images/example_10_forms.png b/docs/images/example_10_forms.png index 2c27290..f3fae75 100644 Binary files a/docs/images/example_10_forms.png and b/docs/images/example_10_forms.png differ diff --git a/docs/images/example_11_table_text_wrapping.png b/docs/images/example_11_table_text_wrapping.png index 2706324..8e3a954 100644 Binary files a/docs/images/example_11_table_text_wrapping.png and b/docs/images/example_11_table_text_wrapping.png differ diff --git a/docs/images/example_11b_simple_wrapping.png b/docs/images/example_11b_simple_wrapping.png index 4c4f655..cfcbf0c 100644 Binary files a/docs/images/example_11b_simple_wrapping.png and b/docs/images/example_11b_simple_wrapping.png differ diff --git a/docs/images/example_12_optimized_table_layout.png b/docs/images/example_12_optimized_table_layout.png index 609b0a9..eeee2b7 100644 Binary files a/docs/images/example_12_optimized_table_layout.png and b/docs/images/example_12_optimized_table_layout.png differ diff --git a/docs/images/font_family_switching.png b/docs/images/font_family_switching.png index 4cd23e4..92b88b9 100644 Binary files a/docs/images/font_family_switching.png and b/docs/images/font_family_switching.png differ diff --git a/docs/images/font_family_switching_vertical.png b/docs/images/font_family_switching_vertical.png index 6b271b4..91007bc 100644 Binary files a/docs/images/font_family_switching_vertical.png and b/docs/images/font_family_switching_vertical.png differ diff --git a/pyWebLayout/concrete/page.py b/pyWebLayout/concrete/page.py index c0b7973..3128214 100644 --- a/pyWebLayout/concrete/page.py +++ b/pyWebLayout/concrete/page.py @@ -15,15 +15,19 @@ class Page(Renderable, Queriable): contains a given point. """ - def __init__(self, size: Tuple[int, int], style: Optional[PageStyle] = None): + def __init__(self, size: Tuple[int, int], style: Optional[PageStyle] = None, + origin: Tuple[int, int] = (0, 0)): """ Initialize a new page. Args: size: The total size of the page (width, height) including borders style: The PageStyle defining borders, spacing, and appearance + origin: Absolute position of the page's top-left corner. Non-zero for + a page nested inside another surface, such as a table cell. """ self._size = size + self._origin = origin self._style = style if style is not None else PageStyle() self._children: List[Renderable] = [] self._canvas: Optional[Image.Image] = None @@ -31,7 +35,8 @@ class Page(Renderable, Queriable): # Initialize y_offset to start of content area # Position the first line so its baseline is close to the top boundary # For subsequent lines, baseline-to-baseline spacing is used - self._current_y_offset = self._style.border_width + self._style.padding_top + self._current_y_offset = (self._origin[1] + self._style.border_width + + self._style.padding_top) self._is_first_line = True # Track if we're placing the first line # Callback registry for managing interactable elements self._callbacks = CallbackRegistry() @@ -39,8 +44,12 @@ class Page(Renderable, Queriable): self._dirty = True def free_space(self) -> Tuple[int, int]: - """Get the remaining space on the page""" - return (self._size[0], self._size[1] - self._current_y_offset) + """ + Get the remaining space in the content area. + + Deprecated: use content_rect and remaining_height, which this delegates to. + """ + return (self.content_rect[2], self.remaining_height) def can_fit_line( self, @@ -59,7 +68,8 @@ class Page(Renderable, Queriable): True if the line fits within page boundaries """ # Calculate the maximum Y position allowed (bottom boundary) - max_y = self._size[1] - self._style.border_width - self._style.padding_bottom + content_y, content_h = self.content_rect[1], self.content_rect[3] + max_y = content_y + content_h # If ascent/descent not provided, use simple check (backward compatibility) if ascent == 0 and descent == 0: @@ -77,6 +87,34 @@ class Page(Renderable, Queriable): """Get the total page size including borders""" return self._size + @property + def origin(self) -> Tuple[int, int]: + """Absolute position of the page's top-left corner""" + return self._origin + + @property + def content_origin(self) -> Tuple[int, int]: + """ + Absolute top-left of the content box: the page origin plus its border and + top/left padding. Layout starts here. + """ + return ( + self._origin[0] + self._style.border_width + self._style.padding_left, + self._origin[1] + self._style.border_width + self._style.padding_top, + ) + + @property + def content_rect(self) -> Tuple[int, int, int, int]: + """(x, y, width, height) of the content box, in absolute coordinates""" + x, y = self.content_origin + return (x, y, self.content_size[0], self.content_size[1]) + + @property + def remaining_height(self) -> int: + """Content-box height still available below the current layout cursor""" + _, y, _, h = self.content_rect + return max(0, y + h - self._current_y_offset) + @property def canvas_size(self) -> Tuple[int, int]: """Get the canvas size (page size minus borders)""" @@ -182,7 +220,7 @@ class Page(Renderable, Queriable): # Clear callback registry when clearing children self._callbacks.clear() # Reset y_offset to start of content area (after border and padding) - self._current_y_offset = self._style.border_width + self._style.padding_top + self._current_y_offset = self.content_origin[1] return self @property @@ -532,6 +570,6 @@ class Page(Renderable, Queriable): True if the point is within the page bounds """ return ( - 0 <= point[0] < self._size[0] and - 0 <= point[1] < self._size[1] + self._origin[0] <= point[0] < self._origin[0] + self._size[0] and + self._origin[1] <= point[1] < self._origin[1] + self._size[1] ) diff --git a/pyWebLayout/concrete/text.py b/pyWebLayout/concrete/text.py index 97f8f64..8e00782 100644 --- a/pyWebLayout/concrete/text.py +++ b/pyWebLayout/concrete/text.py @@ -214,7 +214,9 @@ class AlignmentHandler(ABC): @abstractmethod def calculate_spacing_and_position(self, text_objects: List['Text'], available_width: int, min_spacing: int, - max_spacing: int) -> Tuple[int, int, bool]: + max_spacing: int, + natural_spacing: Optional[int] = None + ) -> Tuple[int, int, bool]: """ Calculate the spacing between words and starting position for the line. @@ -223,9 +225,12 @@ class AlignmentHandler(ABC): available_width: Total width available for the line min_spacing: Minimum spacing between words max_spacing: Maximum spacing between words + natural_spacing: The font's own space width. Ragged alignments use it + as a constant gap; justification ignores it. Defaults to + min_spacing when not supplied. Returns: - Tuple of (spacing_between_words, starting_x_position) + Tuple of (spacing_between_words, starting_x_position, overflow) """ @@ -236,16 +241,23 @@ class LeftAlignmentHandler(AlignmentHandler): text_objects: List['Text'], available_width: int, min_spacing: int, - max_spacing: int) -> Tuple[int, int, bool]: + max_spacing: int, + natural_spacing: Optional[int] = None + ) -> Tuple[int, int, bool]: """ Calculate spacing and position for left-aligned text objects. - CREngine-inspired: never allow negative spacing, always use minimum spacing for overflow. + + Left-aligned text uses a constant word space and leaves whatever is left + over as a ragged right edge. It must not spread the residual space across + the gaps: that stretches each line by a different amount, which reads as + badly-set justified text rather than as ragged-right. Args: text_objects (List[Text]): A list of text objects to be laid out. available_width (int): The total width available for layout. min_spacing (int): Minimum spacing between text objects. max_spacing (int): Maximum spacing between text objects. + natural_spacing (Optional[int]): The font's own space width. Returns: Tuple[int, int, bool]: Spacing, start position, and overflow flag. @@ -254,33 +266,19 @@ class LeftAlignmentHandler(AlignmentHandler): if len(text_objects) <= 1: return 0, 0, False - # Calculate the total length of all text objects - text_length = sum([text.width for text in text_objects]) + spacing = min_spacing if natural_spacing is None else natural_spacing + spacing = max(min_spacing, min(max_spacing, int(spacing))) - # Calculate number of gaps between texts + text_length = sum([text.width for text in text_objects]) num_gaps = len(text_objects) - 1 - # Calculate minimum space needed (text + minimum gaps) - min_total_width = text_length + (min_spacing * num_gaps) + # The spacing is constant whether or not the content fits: tightening a + # full line here would make it differ from its neighbours, which is the + # variation this alignment is supposed to avoid. Report the overflow and + # let line breaking move the offending word instead. + overflow = text_length + (spacing * num_gaps) > available_width - # Check if we have overflow (CREngine pattern: always use min_spacing for - # overflow) - if min_total_width > available_width: - return min_spacing, 0, True # Overflow - but use safe minimum spacing - - # Calculate residual space left after accounting for text lengths - residual_space = available_width - text_length - - # Calculate ideal spacing - actual_spacing = residual_space // num_gaps - # Clamp within bounds (CREngine pattern: respect max_spacing) - if actual_spacing > max_spacing: - return max_spacing, 0, False - elif actual_spacing < min_spacing: - # Ensure we never return spacing less than min_spacing - return min_spacing, 0, False - else: - return actual_spacing, 0, False # Use calculated spacing + return spacing, 0, overflow class CenterRightAlignmentHandler(AlignmentHandler): @@ -291,10 +289,18 @@ class CenterRightAlignmentHandler(AlignmentHandler): def calculate_spacing_and_position(self, text_objects: List['Text'], available_width: int, min_spacing: int, - max_spacing: int) -> Tuple[int, int, bool]: - """Center/right alignment uses minimum spacing with calculated start position.""" + max_spacing: int, + natural_spacing: Optional[int] = None + ) -> Tuple[int, int, bool]: + """ + Centre/right alignment: constant word space, line shifted as a block. + + Like left alignment, the residual space must not be spread across the + gaps - it belongs in the margin. The start position is then derived from + the same spacing that will actually be used, so the line lands where it + was measured to land. + """ word_length = sum([word.width for word in text_objects]) - residual_space = available_width - word_length # Handle single word case if len(text_objects) <= 1: @@ -302,23 +308,21 @@ class CenterRightAlignmentHandler(AlignmentHandler): start_position = (available_width - word_length) // 2 else: # RIGHT start_position = available_width - word_length - return 0, max(0, start_position), False + return 0, max(0, int(start_position)), False - actual_spacing = residual_space // (len(text_objects) - 1) - ideal_space = (min_spacing + max_spacing) / 2 - if actual_spacing > 0.5 * (min_spacing + max_spacing): - actual_spacing = 0.5 * (min_spacing + max_spacing) + spacing = min_spacing if natural_spacing is None else natural_spacing + spacing = max(min_spacing, min(max_spacing, int(spacing))) - content_length = word_length + (len(text_objects) - 1) * actual_spacing + num_gaps = len(text_objects) - 1 + overflow = word_length + (spacing * num_gaps) > available_width + + content_length = word_length + num_gaps * spacing if self._alignment == Alignment.CENTER: start_position = (available_width - content_length) // 2 else: start_position = available_width - content_length - if actual_spacing < min_spacing: - return actual_spacing, max(0, start_position), True - - return ideal_space, max(0, start_position), False + return spacing, max(0, int(start_position)), overflow class JustifyAlignmentHandler(AlignmentHandler): @@ -330,10 +334,14 @@ class JustifyAlignmentHandler(AlignmentHandler): def calculate_spacing_and_position(self, text_objects: List['Text'], available_width: int, min_spacing: int, - max_spacing: int) -> Tuple[int, int, bool]: + max_spacing: int, + natural_spacing: Optional[int] = None + ) -> Tuple[int, int, bool]: """ Justified alignment distributes space to fill the entire line width. + natural_spacing is ignored: filling the measure is the whole point. + For justified text, we ALWAYS try to fill the entire width by distributing space between words, regardless of max_spacing constraints. The only limit is min_spacing to ensure readability. @@ -343,26 +351,28 @@ class JustifyAlignmentHandler(AlignmentHandler): residual_space = available_width - word_length num_gaps = max(1, len(text_objects) - 1) - # For justified text, calculate the actual spacing needed to fill the line - base_spacing = int(residual_space // num_gaps) - remainder = int(residual_space % num_gaps) # The extra pixels to distribute - # Check if we have enough space for minimum spacing - if base_spacing < min_spacing: + if residual_space // num_gaps < min_spacing: # Not enough space - this is overflow self._gap_spacings = [min_spacing] * num_gaps return min_spacing, 0, True - # Distribute remainder pixels across the first 'remainder' gaps - # This ensures the line fills the entire width exactly + # Distribute the residual by cumulative rounding rather than by taking a + # floor per gap and scattering the remainder. Word widths are fractional, + # so flooring each gap loses part of a pixel and truncating the remainder + # loses up to another - the line then stops one or two pixels short of the + # margin, and by a different amount on each line, which is visible as a + # ragged right edge on otherwise justified text. Rounding the running + # total makes the gaps sum to the residual exactly. + total = int(round(residual_space)) self._gap_spacings = [] - for i in range(num_gaps): - if i < remainder: - self._gap_spacings.append(base_spacing + 1) - else: - self._gap_spacings.append(base_spacing) + placed = 0 + for i in range(1, num_gaps + 1): + cumulative = int(round(total * i / num_gaps)) + self._gap_spacings.append(cumulative - placed) + placed = cumulative - return base_spacing, 0, False + return self._gap_spacings[0], 0, False class Text(Renderable, Queriable): @@ -692,6 +702,13 @@ class Line(Box): self._spacing_render = (spacing[0] + spacing[1]) // 2 self._position_render = 0 + # The font's own space advance. Ragged alignments use this as their + # constant word gap rather than stretching to fill the measure. + try: + self._natural_spacing = int(round(self._font.font.getlength(" "))) + except (AttributeError, TypeError, ValueError): + self._natural_spacing = None + # Hyphenation configuration parameters self._min_word_length_for_brute_force = min_word_length_for_brute_force self._min_chars_before_hyphen = min_chars_before_hyphen @@ -700,6 +717,34 @@ class Line(Box): # Create the appropriate alignment handler self._alignment_handler = self._create_alignment_handler(halign) + # Set on the final line of a paragraph. Justification stretches a line to + # fill the column, which is wrong for the last line - a three-word tail + # would be spread across the full measure. The last line takes its + # natural width instead, as in every other typesetting system. + self._is_paragraph_end = False + + @property + def is_paragraph_end(self) -> bool: + """Whether this is the final line of its paragraph""" + return self._is_paragraph_end + + @is_paragraph_end.setter + def is_paragraph_end(self, value: bool): + self._is_paragraph_end = value + + @property + def render_alignment_handler(self) -> AlignmentHandler: + """ + The handler used to position text when rendering. + + This differs from the fitting handler only for the last line of a + justified paragraph, which is rendered flush left. + """ + if self._is_paragraph_end and isinstance( + self._alignment_handler, JustifyAlignmentHandler): + return LeftAlignmentHandler() + return self._alignment_handler + def _create_alignment_handler(self, alignment: Alignment) -> AlignmentHandler: """ Create the appropriate alignment handler based on the alignment type. @@ -775,7 +820,8 @@ class Line(Box): text = Text.from_word(word, self._draw) self._text_objects.append(text) spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1]) + self._text_objects, self._size[0], self._spacing[0], self._spacing[1], + self._natural_spacing) if not overflow: # Word fits! Add it completely @@ -822,7 +868,8 @@ class Line(Box): # Check if first part fits self._text_objects.append(first_text) spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1]) + self._text_objects, self._size[0], self._spacing[0], self._spacing[1], + self._natural_spacing) _ = self._text_objects.pop() if not overflow: @@ -893,7 +940,8 @@ class Line(Box): # Verify the first part actually fits self._text_objects.append(first_text) spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1]) + self._text_objects, self._size[0], self._spacing[0], self._spacing[1], + self._natural_spacing) if not overflow: # Brute force split works! @@ -918,10 +966,15 @@ class Line(Box): Returns: A PIL Image containing the rendered line """ - # Recalculate spacing and position for current text objects to ensure accuracy + # Recalculate spacing and position for current text objects to ensure + # accuracy. Word fitting used the paragraph's alignment; rendering uses + # render_alignment_handler, which differs only for the last line of a + # justified paragraph. + handler = self.render_alignment_handler if len(self._text_objects) > 0: - spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1]) + spacing, position, overflow = handler.calculate_spacing_and_position( + self._text_objects, self._size[0], self._spacing[0], self._spacing[1], + self._natural_spacing) self._spacing_render = spacing self._position_render = position @@ -939,10 +992,10 @@ class Line(Box): 1 < len(self._text_objects) else None # Get the spacing for this specific gap (variable for justified text) - if isinstance(self._alignment_handler, JustifyAlignmentHandler) and \ - hasattr(self._alignment_handler, '_gap_spacings') and \ - i < len(self._alignment_handler._gap_spacings): - current_spacing = self._alignment_handler._gap_spacings[i] + if isinstance(handler, JustifyAlignmentHandler) and \ + hasattr(handler, '_gap_spacings') and \ + i < len(handler._gap_spacings): + current_spacing = handler._gap_spacings[i] else: current_spacing = self._spacing_render diff --git a/pyWebLayout/layout/document_layouter.py b/pyWebLayout/layout/document_layouter.py index 1a4c9d8..95d071e 100644 --- a/pyWebLayout/layout/document_layouter.py +++ b/pyWebLayout/layout/document_layouter.py @@ -8,7 +8,7 @@ from pyWebLayout.concrete.image import RenderableImage from pyWebLayout.concrete.functional import ButtonText, FormFieldText from pyWebLayout.concrete.table import TableRenderer, TableStyle from pyWebLayout.abstract import Paragraph, Word -from pyWebLayout.abstract.block import Image as AbstractImage, PageBreak, Table +from pyWebLayout.abstract.block import Image as AbstractImage, Heading, PageBreak, Table from pyWebLayout.abstract.functional import Button, Form, FormField from pyWebLayout.style.concrete_style import ConcreteStyleRegistry, RenderingContext, StyleResolver from pyWebLayout.style import Font, Alignment @@ -51,6 +51,15 @@ def paragraph_layouter(paragraph: Paragraph, # We need to get word spacing constraints from the Font's abstract style if available # For now, use reasonable defaults based on font size + # Alignment for text that does not specify its own. Headings are never + # justified - stretching a two-word title across the measure is always wrong - + # so they fall back to flush left. + default_alignment = getattr(page.style, 'default_alignment', None) + if not isinstance(default_alignment, Alignment): + default_alignment = Alignment.JUSTIFY + if isinstance(paragraph, Heading): + default_alignment = Alignment.LEFT + if isinstance(paragraph.style, Font): # paragraph.style is already a Font (concrete style) font = paragraph.style @@ -59,7 +68,7 @@ def paragraph_layouter(paragraph: Paragraph, min_spacing = float(font.font_size) * 0.25 # 25% of font size max_spacing = float(font.font_size) * 0.5 # 50% of font size word_spacing_constraints = (int(min_spacing), int(max_spacing)) - text_align = Alignment.LEFT # Default alignment + text_align = default_alignment else: # paragraph.style is an AbstractStyle, resolve it # Ensure font_size is an int (it could be a FontSize enum) @@ -79,7 +88,8 @@ def paragraph_layouter(paragraph: Paragraph, int(concrete_style.word_spacing_min), int(concrete_style.word_spacing_max) ) - text_align = concrete_style.text_align + # text_align is None when the source did not specify one. + text_align = concrete_style.text_align or default_alignment # Apply page-level word spacing override if specified if hasattr( @@ -151,7 +161,7 @@ def paragraph_layouter(paragraph: Paragraph, y_cursor = page._current_y_offset else: y_cursor = page._current_y_offset - x_cursor = page.border_size + x_cursor = page.content_origin[0] # Create a temporary Text object to calculate word width if word: @@ -260,7 +270,13 @@ def paragraph_layouter(paragraph: Paragraph, else: current_pretext = overflow_text # May be None or hyphenated remainder - # All words processed successfully + # All words processed successfully. The line holding the final word is the + # end of the paragraph, so it is rendered at its natural width rather than + # justified to the full column. A paragraph continued on the next page does + # not reach here, so its lines stay justified - which is correct. + if current_line is not None: + current_line.is_paragraph_end = True + return True, None, None @@ -305,7 +321,7 @@ def image_layouter(image: AbstractImage, page: Page, max_width: Optional[int] = max_width = page.available_width # Calculate available height on page - available_height = page.size[1] - page._current_y_offset - page.border_size + available_height = page.remaining_height # If no space available, image doesn't fit if available_height <= 0: @@ -325,7 +341,7 @@ def image_layouter(image: AbstractImage, page: Page, max_width: Optional[int] = return False # Create renderable image - x_offset = page.border_size + x_offset = page.content_origin[0] y_offset = page._current_y_offset # Access page.draw to ensure canvas is initialized @@ -368,7 +384,7 @@ def table_layouter( """ # Calculate available space available_width = page.available_width - x_offset = page.border_size + x_offset = page.content_origin[0] y_offset = page._current_y_offset # Access page.draw to ensure canvas is initialized @@ -388,7 +404,7 @@ def table_layouter( # Check if table fits on current page table_height = renderer.size[1] - available_height = page.size[1] - y_offset - page.border_size + available_height = page.remaining_height if table_height > available_height: return False @@ -436,7 +452,7 @@ def button_layouter(button: Button, font = Font(font_size=14, colour=(255, 255, 255)) # Calculate available space - available_height = page.size[1] - page._current_y_offset - page.border_size + available_height = page.remaining_height # Create ButtonText renderable button_text = ButtonText(button, font, page.draw, padding=padding) @@ -447,7 +463,7 @@ def button_layouter(button: Button, return False, "" # Position the button - x_offset = page.border_size + x_offset = page.content_origin[0] y_offset = page._current_y_offset button_text.set_origin(np.array([x_offset, y_offset])) @@ -486,7 +502,7 @@ def form_field_layouter(field: FormField, page: Page, font: Optional[Font] = Non font = Font(font_size=12, colour=(0, 0, 0)) # Calculate available space - available_height = page.size[1] - page._current_y_offset - page.border_size + available_height = page.remaining_height # Create FormFieldText renderable field_text = FormFieldText(field, font, page.draw, field_height=field_height) @@ -497,7 +513,7 @@ def form_field_layouter(field: FormField, page: Page, font: Optional[Font] = Non return False, "" # Position the field - x_offset = page.border_size + x_offset = page.content_origin[0] y_offset = page._current_y_offset field_text.set_origin(np.array([x_offset, y_offset])) 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/pyWebLayout/style/abstract_style.py b/pyWebLayout/style/abstract_style.py index bc0dac2..30a9be2 100644 --- a/pyWebLayout/style/abstract_style.py +++ b/pyWebLayout/style/abstract_style.py @@ -81,7 +81,8 @@ class AbstractStyle: background_color: Optional[Union[str, Tuple[int, int, int, int]]] = None # Text properties - text_align: TextAlign = TextAlign.LEFT + # None means "not specified": the page's default_alignment applies. + text_align: Optional[TextAlign] = None line_height: Optional[Union[str, float]] = None # "normal", "1.2", 1.5, etc. letter_spacing: Optional[Union[str, float]] = None # "normal", "0.1em", etc. word_spacing: Optional[Union[str, float]] = None diff --git a/pyWebLayout/style/concrete_style.py b/pyWebLayout/style/concrete_style.py index 44796ba..d9a7a9e 100644 --- a/pyWebLayout/style/concrete_style.py +++ b/pyWebLayout/style/concrete_style.py @@ -61,7 +61,8 @@ class ConcreteStyle: decoration: TextDecoration = TextDecoration.NONE # Layout properties - text_align: TextAlign = TextAlign.LEFT + # None means "not specified": the page's default_alignment applies. + text_align: Optional[TextAlign] = None line_height: float = 1.0 # Multiplier letter_spacing: float = 0.0 # In pixels word_spacing: float = 0.0 # In pixels diff --git a/pyWebLayout/style/page_style.py b/pyWebLayout/style/page_style.py index 47d8938..c2ca0c9 100644 --- a/pyWebLayout/style/page_style.py +++ b/pyWebLayout/style/page_style.py @@ -1,5 +1,7 @@ from typing import Tuple -from dataclasses import dataclass +from dataclasses import dataclass, field + +from pyWebLayout.style.alignment import Alignment @dataclass @@ -8,6 +10,10 @@ class PageStyle: Defines the styling properties for a page including borders, spacing, and layout. """ + # Alignment applied to body text that does not specify its own. Headings are + # never justified regardless of this setting. + default_alignment: Alignment = Alignment.JUSTIFY + # Border properties border_width: int = 0 border_color: Tuple[int, int, int] = (0, 0, 0) diff --git a/tests/concrete/test_alignment_spacing.py b/tests/concrete/test_alignment_spacing.py new file mode 100644 index 0000000..6146216 --- /dev/null +++ b/tests/concrete/test_alignment_spacing.py @@ -0,0 +1,176 @@ +""" +Regression tests for word spacing under each alignment (spec S13). + +Only justified text stretches word gaps to fill the measure. Left, centre and +right aligned text use a natural, constant word space and leave a ragged edge; +previously they distributed the residual space across the gaps, which produced +text that looked justified but did not reach the margin, with a right edge that +wobbled by several pixels from line to line. + +The final line of a justified paragraph is also not stretched. +""" + +import pytest + +from pyWebLayout.abstract.block import Paragraph +from pyWebLayout.abstract.inline import Word +from pyWebLayout.concrete.page import Page +from pyWebLayout.concrete.text import ( + CenterRightAlignmentHandler, + JustifyAlignmentHandler, + LeftAlignmentHandler, + Line, +) +from pyWebLayout.layout.document_layouter import paragraph_layouter +from pyWebLayout.style import Alignment, Font +from pyWebLayout.style.page_style import PageStyle + + +PAGE = (500, 400) +PADDING = (20, 20, 20, 20) + + +@pytest.fixture +def font(): + return Font(font_size=14) + + +def lay_out(font, alignment, text, size=PAGE): + page = Page(size=size, style=PageStyle(border_width=0, padding=PADDING)) + paragraph = Paragraph(font) + for word in text.split(): + paragraph.add_word(Word(word, font)) + paragraph_layouter(paragraph, page, alignment_override=alignment) + return page + + +def rendered_lines(page): + lines = [c for c in page.children if isinstance(c, Line) and c._text_objects] + for line in lines: + line.render() + return lines + + +def gaps_of(line): + """Observed pixel gaps between consecutive words on a rendered line.""" + tos = line._text_objects + return [int(tos[i + 1]._origin[0]) - (int(tos[i]._origin[0]) + int(tos[i].width)) + for i in range(len(tos) - 1)] + + +BODY = ("Paragraph text that is automatically laid out when this paragraph does " + "not fit on the current page the layouter will create a new page for it " + "which differs from using an explicit page break marker in the source ") * 2 + + +class TestLeftAlignmentUsesConstantSpacing: + + def test_gaps_are_uniform_within_a_line(self, font): + page = lay_out(font, Alignment.LEFT, BODY) + for line in rendered_lines(page): + gaps = gaps_of(line) + if len(gaps) > 1: + assert max(gaps) - min(gaps) <= 1, \ + f"left-aligned gaps should be constant, got {gaps}" + + def test_gaps_are_uniform_across_lines(self, font): + """The regression: each line got its own stretch factor.""" + page = lay_out(font, Alignment.LEFT, BODY) + all_gaps = [g for line in rendered_lines(page) for g in gaps_of(line)] + assert max(all_gaps) - min(all_gaps) <= 1, \ + f"spacing must not vary line to line, got {sorted(set(all_gaps))}" + + def test_lines_do_not_reach_the_right_margin(self, font): + """Left-aligned text is ragged; a flush right edge means it was stretched.""" + page = lay_out(font, Alignment.LEFT, BODY) + right = page.content_rect[0] + page.content_rect[2] + ends = [max(int(t._origin[0]) + int(t.width) for t in line._text_objects) + for line in rendered_lines(page)] + assert not all(right - e <= 1 for e in ends), \ + "every line reached the margin exactly - text was justified, not left aligned" + + def test_handler_returns_natural_spacing(self, font): + handler = LeftAlignmentHandler() + from pyWebLayout.concrete.text import Text + from PIL import Image, ImageDraw + draw = ImageDraw.Draw(Image.new("RGB", (10, 10))) + texts = [Text(w, font, draw) for w in ["Hello", "World"]] + + spacing, position, overflow = handler.calculate_spacing_and_position( + texts, 400, 3, 7, natural_spacing=5) + + assert spacing == 5, "natural spacing should be used verbatim when it fits" + assert position == 0 + assert not overflow + + def test_handler_clamps_natural_spacing_to_bounds(self, font): + handler = LeftAlignmentHandler() + from pyWebLayout.concrete.text import Text + from PIL import Image, ImageDraw + draw = ImageDraw.Draw(Image.new("RGB", (10, 10))) + texts = [Text(w, font, draw) for w in ["Hello", "World"]] + + assert handler.calculate_spacing_and_position( + texts, 400, 3, 7, natural_spacing=99)[0] == 7 + assert handler.calculate_spacing_and_position( + texts, 400, 3, 7, natural_spacing=1)[0] == 3 + + +class TestJustifyStillFills: + + def test_body_lines_reach_the_margin(self, font): + page = lay_out(font, Alignment.JUSTIFY, BODY) + lines = rendered_lines(page) + right = page.content_rect[0] + page.content_rect[2] + + for line in lines: + if line.is_paragraph_end: + continue + end = max(int(t._origin[0]) + int(t.width) for t in line._text_objects) + assert right - end <= 2, f"justified line fell {right - end}px short" + + def test_last_line_is_not_stretched(self, font): + page = lay_out(font, Alignment.JUSTIFY, + BODY + " and then a deliberately short tail.") + lines = rendered_lines(page) + last = [line for line in lines if line.is_paragraph_end] + assert last, "the final line of a completed paragraph must be marked" + + gaps = gaps_of(last[-1]) + if gaps: + assert max(gaps) <= 8, \ + f"final line was justified across the measure, gaps={gaps}" + + def test_continued_paragraph_keeps_justification(self, font): + """A paragraph split across pages: its lines are not paragraph ends.""" + page = lay_out(font, Alignment.JUSTIFY, BODY * 6, size=(500, 200)) + lines = rendered_lines(page) + assert lines, "the page should hold some lines" + assert not any(line.is_paragraph_end for line in lines), \ + "an unfinished paragraph has no final line on this page" + + +class TestCentreAndRight: + + def test_centre_uses_constant_spacing_and_is_centred(self, font): + page = lay_out(font, Alignment.CENTER, BODY) + right = page.content_rect[0] + page.content_rect[2] + left = page.content_rect[0] + + for line in rendered_lines(page): + tos = line._text_objects + # Float extents: integer truncation of each end would itself skew the + # comparison by a pixel. + start = float(tos[0]._origin[0]) + end = float(tos[-1]._origin[0]) + tos[-1].width + # Equal margins either side, within rounding of the half-space. + assert abs((start - left) - (right - end)) <= 2, \ + f"line not centred: left margin {start - left}, right {right - end}" + + def test_right_aligned_lines_end_at_the_margin(self, font): + page = lay_out(font, Alignment.RIGHT, BODY) + right = page.content_rect[0] + page.content_rect[2] + + for line in rendered_lines(page): + end = max(int(t._origin[0]) + int(t.width) for t in line._text_objects) + assert right - end <= 2, f"right-aligned line fell {right - end}px short" diff --git a/tests/concrete/test_page_geometry.py b/tests/concrete/test_page_geometry.py new file mode 100644 index 0000000..734b55e --- /dev/null +++ b/tests/concrete/test_page_geometry.py @@ -0,0 +1,133 @@ +""" +Regression tests for page content geometry (spec S2). + +Content must be laid out inside the content box - the page box less its border +and padding - on all four sides. Horizontal padding was previously ignored on the +left, shifting every line left by padding_left and leaving a gutter of +padding_left + padding_right on the right, so lines appeared to break early. +""" + +import pytest + +from pyWebLayout.abstract.block import Paragraph +from pyWebLayout.abstract.inline import Word +from pyWebLayout.concrete.page import Page +from pyWebLayout.layout.document_layouter import DocumentLayouter +from pyWebLayout.style import Font +from pyWebLayout.style.page_style import PageStyle + + +PADDING = (40, 30, 40, 20) # top, right, bottom, left - deliberately asymmetric + + +@pytest.fixture +def font(): + return Font(font_size=12) + + +def filled_page(size, style, font, word_count=120): + page = Page(size=size, style=style) + paragraph = Paragraph(font) + for i in range(word_count): + paragraph.add_word(Word(f"word{i}", font)) + DocumentLayouter(page).layout_paragraph(paragraph) + return page + + +class TestContentBox: + """content_origin / content_rect describe the box content lives in.""" + + def test_content_origin_includes_border_and_padding(self): + page = Page(size=(400, 300), style=PageStyle(border_width=2, padding=PADDING)) + assert page.content_origin == (2 + 20, 2 + 40) + + def test_content_rect_subtracts_both_paddings(self): + page = Page(size=(400, 300), style=PageStyle(border_width=2, padding=PADDING)) + x, y, w, h = page.content_rect + assert (x, y) == (22, 42) + assert w == 400 - 2 * 2 - 20 - 30 + assert h == 300 - 2 * 2 - 40 - 40 + + def test_page_origin_offsets_the_content_box(self): + """A page placed inside another surface reports absolute coordinates.""" + page = Page(size=(100, 50), style=PageStyle(border_width=1, padding=(5, 5, 5, 5)), + origin=(200, 300)) + assert page.content_origin == (206, 306) + + def test_remaining_height_respects_bottom_padding(self, font): + style = PageStyle(border_width=2, padding=PADDING) + page = Page(size=(400, 300), style=style) + # Nothing laid out yet: the whole content box is available. + assert page.remaining_height == page.content_rect[3] + + +class TestLinePlacement: + """Lines must start after the left padding and end before the right padding.""" + + def test_first_line_starts_at_content_origin(self, font): + style = PageStyle(border_width=2, padding=PADDING) + page = filled_page((400, 300), style, font) + + line = page.children[0] + assert int(line.origin[0]) == page.content_origin[0] + assert int(line.origin[1]) == page.content_origin[1] + + def test_line_width_matches_content_width(self, font): + style = PageStyle(border_width=2, padding=PADDING) + page = filled_page((400, 300), style, font) + + line = page.children[0] + assert int(line.size[0]) == page.content_rect[2] + + def test_no_line_extends_past_the_right_content_edge(self, font): + style = PageStyle(border_width=2, padding=PADDING) + page = filled_page((400, 300), style, font) + right_edge = page.content_rect[0] + page.content_rect[2] + + for line in page.children: + assert int(line.origin[0]) + int(line.size[0]) <= right_edge + + def test_ink_stays_inside_the_content_box(self, font): + """The rendered pixels, not just the boxes, respect the padding.""" + style = PageStyle(border_width=0, padding=PADDING, + background_color=(255, 255, 255)) + page = filled_page((400, 300), style, font) + image = page.render().convert("L") + pixels = image.load() + + inked_x = [x for x in range(400) for y in range(300) if pixels[x, y] < 128] + assert inked_x, "the page should have text on it" + + x0, _, w, _ = page.content_rect + assert min(inked_x) >= x0 + assert max(inked_x) <= x0 + w + + def test_right_gutter_is_not_double_width(self, font): + """ + The regression: text was shifted left by padding_left, so the right gutter + was padding_left + padding_right wide while the left gutter was zero. + """ + style = PageStyle(border_width=0, padding=(10, 30, 10, 30)) + page = filled_page((400, 300), style, font, word_count=200) + image = page.render().convert("L") + pixels = image.load() + inked_x = [x for x in range(400) for y in range(300) if pixels[x, y] < 128] + + left_gutter = min(inked_x) + right_gutter = 400 - max(inked_x) + # Justification means the right edge is not always exactly flush, so allow + # slack - but the two gutters must be comparable, not 0 vs 60. + assert abs(left_gutter - right_gutter) < 25, \ + f"asymmetric gutters: left={left_gutter} right={right_gutter}" + + +class TestBlockBottomBoundary: + """Blocks must not be placed into the bottom padding.""" + + def test_lines_stop_before_bottom_padding(self, font): + style = PageStyle(border_width=2, padding=PADDING) + page = filled_page((400, 300), style, font, word_count=500) + bottom_edge = page.content_rect[1] + page.content_rect[3] + + for line in page.children: + assert int(line.origin[1]) <= bottom_edge 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 diff --git a/tests/layouter/test_document_layouter.py b/tests/layouter/test_document_layouter.py index 73bde30..ed2eb06 100644 --- a/tests/layouter/test_document_layouter.py +++ b/tests/layouter/test_document_layouter.py @@ -24,6 +24,12 @@ class TestDocumentLayouter: self.mock_page.border_size = 20 self.mock_page._current_y_offset = 50 self.mock_page.available_width = 400 + # Content geometry: a 440x600 page with a 20px border and no padding, so + # the content box starts at (20, 20) and is 400 wide. + self.mock_page.size = (440, 600) + self.mock_page.content_origin = (20, 20) + self.mock_page.content_rect = (20, 20, 400, 560) + self.mock_page.remaining_height = 530 # 20 + 560 - 50 self.mock_page.draw = Mock() self.mock_page.can_fit_line = Mock(return_value=True) self.mock_page.add_child = Mock() @@ -603,6 +609,10 @@ class TestTableLayouter: self.mock_page._current_y_offset = 50 self.mock_page.available_width = 600 self.mock_page.size = (800, 1000) + # Content geometry: 800x1000 page, 20px border, no padding. + self.mock_page.content_origin = (20, 20) + self.mock_page.content_rect = (20, 20, 600, 960) + self.mock_page.remaining_height = 930 # 20 + 960 - 50 # Create mock draw and canvas self.mock_draw = Mock()