Files
pyWebLayout/docs/LAYOUT_REMEDIATION_SPEC.md
T
dtourolle 1262be6a38 fix(text): constant word space for ragged alignment, exact justification (S13)
LeftAlignmentHandler spread each line's residual space across its word gaps,
clamped to max_spacing. A line whose residual divided to under max_spacing was
stretched flush, one that exceeded it was not, so left-aligned text was
justified sometimes, by a different amount per line - which reads as a wobbling
right edge rather than as ragged-right. Centre/right did the same, and computed
their start position from a different spacing than the one they returned, so
centred lines were not centred.

Ragged alignments now use a constant word space - the font's own space advance,
clamped to the style's bounds - and report overflow instead of tightening, so
line breaking decides what fits rather than rendering squeezing it.

Justification kept two further defects:

  - the final line of a paragraph was stretched across the measure, so a
    three-word tail was spread edge to edge. Line now carries is_paragraph_end,
    set on the line holding the last word, and renders flush left. A paragraph
    continued on the next page is not marked, so it stays justified.

  - gaps were floored per gap with a truncated remainder, discarding the
    fractional part of both. Lines stopped one or two pixels short, differently
    each time. Distributing by cumulative rounding makes the gaps sum to the
    residual exactly; advance ends now land identically on every line.

Alignment is configurable rather than hardcoded: PageStyle.default_alignment,
defaulting to JUSTIFY for body text. text_align on abstract and concrete styles
defaults to None meaning "unspecified", so HTML without text-align inherits the
page default while explicit CSS still wins. Headings are never justified.
2026-08-06 22:18:04 +02:00

1184 lines
50 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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
```
<p>hello <b>world</b> again</p> -> Paragraph ['hello', 'world', 'again'] ok
<div>hello <b>world</b> again</div> -> (no blocks at all) lost
<li>hello <b>world</b> again</li> -> HList with no words lost
<td>hello <b>world</b> again</td> -> Table with no words lost
<td><a href="u">link</a> text</td> -> Paragraph ['text'] link lost
```
Bare text nodes that *do* survive (in `td`) each become their own `Paragraph`,
so `a <b>b</b> c` would fragment onto three lines even once `<b>` 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 `<a>``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.
`<br>` 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 `<img>` 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 `<p>`
control, in document order, with `LinkedWord` preserved for `<a href>`.
- `<div>a <b>b</b> c</div>` yields **one** `Paragraph` of three words, not three
paragraphs.
- `<td>text<p>para</p>more</td>` yields three blocks in order: Paragraph, Paragraph,
Paragraph — inline runs on either side of a block child are not merged.
- `<td>a<br>b</td>` yields two paragraphs.
- `<p>text<img src=x>more</p>` 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.250.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 + (n1) × 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 `<tr><td colspan=2>` followed by
`<tr><td><td><td>` 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+k1* only if the accumulated height of
rows *r…r+k1* 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
- `<tr><td colspan=2></tr><tr><td><td><td></tr>``grid.n_cols == 3`, and all
four cells render.
- `<td rowspan=2>` 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 14 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 12px 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.