Files

T

Gitea Action 735face593 Update coverage badges [skip ci]

2026-08-08 20:35:15 +00:00

60 KiB

Raw Permalink Blame History

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.

ID	Spec	Phase
S1	Inline content in non-paragraph containers	0
S2	Page geometry: origin and content rect	1
S3	Draw/canvas lifecycle	1
S4	One block dispatch, one measurement	2
S5	Cells as sub-layouts	2
S6	Table grid model	3
S7	Retained-mode table rendering	3
S8	Table and list pagination	4
S9	Interactivity inside tables	4
S10	Contracts and hygiene	5
S11	Partial-block progress is discarded	0
S12	Background rendering	4
S13	Word spacing and alignment	0
S14	Vertical centring in buttons and fields	0
S15	Form field label geometry	0
S16	Backward page navigation	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.

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.
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.
Measure and render agree by construction. Reported height comes from the same objects that get drawn, never from a parallel estimator.
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.
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 because they are meant to be consumed by extract_text_content(). 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 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:

# 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.

  inside a run terminates the current paragraph and starts a new one, replacing the current no-op line_break_handler.

Callers become one line each:

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 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  control, in document order, with LinkedWord preserved for <a href>.
<div>a b c</div> yields one Paragraph of three words, not three paragraphs.
<td>textparamore</td> yields three blocks in order: Paragraph, Paragraph, Paragraph — inline runs on either side of a block child are not merged.
<td>a b</td> yields two paragraphs.
text<img src=x>more 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.

Horizontal padding is ignored. paragraph_layouter sets x_cursor = page.border_size (document_layouter.py:154) 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), so the asymmetry is horizontal only.
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.

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() 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 sets self._canvas = None but leaves self._draw bound to the discarded canvas, and the draw property 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).

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.

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), 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), 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`	`0.25–0.5 × font_size`	real, via `Line.add_word`
Height	`_estimate_wrapped_lines`	`0.25 × font_size`	"assume one line"
Width	`layout_cell_content`	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, EreaderLayout._layout_block_on_page, 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:

@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:

@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 hand-rolls line breaking and handles only Paragraph, Heading and Image (table.py:141-146); 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), 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).

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) 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:

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:

size is wrong. table.py:445-446 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.
Uniform row heights. _calculate_row_height_for_section takes the max across all rows in a section, so one verbose cell inflates every row in the table.
colspan is not counted. get_column_count returns first_row.cell_count. For <tr><td colspan=2> followed by <tr><td><td><td> it returns 1, and TableRowRenderer.render silently drops every cell past len(column_widths) — two of three cells never render.
rowspan is parsed and stored but never read by any renderer or measurer; spanned rows just shift left.
Row height ignores the cell padding it must contain. The 40px minimum in _calculate_row_height_for_section is a constant, so a larger cell_padding eats into the content box rather than growing the row, and _render_cell_content then clips the text against available_height. Rendering the same header at two paddings:
```
padding=(8,10,8,10)   border=1: header h=40, ink=593
padding=(10,12,10,12) border=2: header h=40, ink=288
```
Both rows are 40px tall; the second silently loses half its text. This is visible in docs/images/example_05_html_table_with_images.png, whose second table renders an empty header row. It is the same measure/render disagreement as defect 1, and S5 removes it by construction: the cell page's content box is the box its padding leaves.

Design

A resolved grid, built once, that every other stage consumes.

# 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

<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 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) 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:

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 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), 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.

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) 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), 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).

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, which currently promises a PIL.Image, and annotate accordingly.

10.2 Abstract/concrete leak

concrete/__init__.py:18 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: 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: 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, 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 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:

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

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). 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.

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), so ordering semantics stay in one place.

Two supporting changes, because a silent dead-end should not be possible again:

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.
The same audit for the backward path. render_page_backward's refinement loop (ereader_layout.py:400-465) 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) and submits page renders to it. Every job fails. _render_page_worker returns pickle.dumps(page) (page_buffer.py:50), 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). 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.

It is also not inert. The pool is started from PageBuffer.initialize inside a process that already has threads, and CPython warns about exactly this:

DeprecationWarning: This process (pid=...) is multi-threaded,
use of fork() may lead to deadlocks in the child.

tests/layout/test_ereader_image_rendering.py intermittently hangs at interpreter exit as a result — every test reports PASSED, then the process never returns. Observed roughly one run in four. A reader that hangs on shutdown once in four launches would be a shipped bug; the test suite is just where it shows up first. This raises S12 from "wasted work" to "actively harmful".

Four further defects in the same file, which matter only if the decision is to keep it:

_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). Fixing the pickling alone would poison the cache with wrong-size pages.
check_completed_renders caches every result with is_backward=False (page_buffer.py:268), so backward renders land in the forward buffer.
_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.
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) 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.

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.

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.
The last line of a justified paragraph was justified. A three-word tail was spread across the full measure.
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

S14 — Vertical centring in buttons and fields

Problem

ButtonText.render and FormFieldText.render both placed the text baseline at box_top + box_height / 2 + descent / 2. Centring glyphs whose visual height is ascent + descent inside a box of height H puts the baseline at box_top + H/2 + (ascent - descent)/2. The two agree only when ascent == 2 * descent; DejaVu is nearer 4:1, so labels rode high against the top edge of the control.

ButtonText also sized itself as font_size + padding, but the text's visual height exceeds the nominal size — DejaVu at 14px measures 17 — so the button was too short to centre its own label in.

Evidence

A 14px "Save Document" button with 6px vertical padding, measuring the label's ink against the button rectangle:

gap above text:  5px
gap below text: 11px

Design

baseline = area_top + (area_height - (ascent + descent)) / 2 + ascent in both renderers.
ButtonText._padded_height derives from ascent + descent, guarded so a mock or unusual font object falls back to the nominal size.

Acceptance criteria

Label ink is centred within ±2px at font sizes 10, 14 and 20.
Label ink stays inside the button rectangle.
Button height is at least ascent + descent + vertical padding.
A form field's value is centred within its input box (±3px).

Files

pyWebLayout/concrete/functional.py

Note

docs/images/example_07_pressed_state.png was stale — no example regenerates it; 07_pressed_state_demo.py writes demo_07_pressed.png at the repository root and the docs copy had been placed by hand. It has been refreshed. Worth wiring the demo to write straight to docs/images/ so it cannot drift again.

S15 — Form field label geometry

Problem

FormFieldText treats its origin as the control's top-left: size and in_object both measure down from it. But it drew the label by calling Text.render at that origin, and Text anchors on the baseline, so the label's glyphs landed above the origin — outside the box the control claims, on top of whatever was there. In a stacked form that is the previous field's input box, which is what docs/images/example_10_forms.png showed: every label but the first crowding and touching the box above it.

The height was also computed as font_size + 5 + field_height, understating the label by the difference between nominal size and ink height, which left the gap between label and box smaller than the intended 5px.

Design

The origin is documented as the top-left of the whole control.
Rendering offsets the label down by its ascent, so the glyphs occupy [origin.y, origin.y + ascent + descent].
LABEL_GAP names the 5px gap, and field_area_offset gives the distance from the origin to the top of the input box. render, handle_click and the height calculation all derive from it, instead of each recomputing font_size + 5.

Acceptance criteria

No label ink is drawn above the control's origin.
All ink lies within [origin.y, origin.y + size[1]].
Consecutive fields laid out by form_layouter do not overlap.
A click in the input area focuses the field; a click on the label does not.

Files

pyWebLayout/concrete/functional.py

Problem

render_page_backward searched for the previous page's start: estimate a block index, lay out forward, compare the end against the target, bisect on the block difference, repeat up to ten times. Both the estimator and the adjuster pinned word_index to 0 and moved only block_index.

Pages routinely start mid-block. Any such start was therefore not in the search space, the loop could never match, and it fell through to a fallback that jumped several blocks back or to the document start.

Evidence

A document of short paragraphs around one 1200-word paragraph. Forward pagination gives page starts at (0,0), (2,208), (2,494), (2,780), (2,1057). Asking for the page that ends where each of those begins:

from page 1 -> got (0,0)   expected (0,0)     ok     (1 forward layout)
from page 2 -> got (0,0)   expected (2,208)   WRONG  (10 forward layouts)
from page 3 -> got (0,0)   expected (2,494)   WRONG  (10 forward layouts)
from page 4 -> got (0,0)   expected (2,780)   WRONG  (10 forward layouts)

Every mid-paragraph case threw the reader to the start of the document after ten full page layouts. The bisection was also unsound within its own space: a document of 40 small paragraphs, where every page does start on a block boundary, failed too.

This is complementary to S11 rather than caused by it. Before S11 forward pagination dead-ended at the first page-spanning block, so mid-block starts were never produced and the block-granular search looked adequate.

Design

Pagination is a pure function: laying out from q yields a page and the position it stopped at, next(q). The page before P is the q with next(q) == P. That is found by replaying the chain forward from an anchor, not by guessing q. Three sources, in order:

The recorded chain. render_page_forward now records (font_scale, next(q)) -> q. Stepping back to anywhere the reader has been is exact and costs one layout. Keyed by font scale, since changing it repaginates.
Replay from an anchor. Anchors are block starts, nearest first: the block containing P, then up to MAX_BACKWARD_ANCHORS earlier ones, then the document start. Lay out forward from the anchor until a page ends exactly on P; that page's start is the answer. MAX_REPLAY_PAGES caps the walk so one page turn cannot traverse a whole chapter.
Nearest start before P. If no chain passes exactly through P — which happens when P was reached by a jump or a restored bookmark rather than by reading forward, so it lies on no natural chain — return the last page start before it. That overlaps P's page slightly rather than skipping content, which is the safe direction to be wrong in.

The estimator and the bisecting adjuster are deleted.

What "correct" means here. Each backward step returns a page ending exactly where the reader currently is, so paging back never skips or repeats content. That chain can differ from the one you would have seen reading forward from page one, if you entered the document by a jump — pagination from a different starting point is genuinely a different chain, and no algorithm can recover the original without replaying from the start.

Measurements

Same document, after the change:

warm (chain recorded by the forward pass):  4/4 exact, 1 layout each
cold, fresh layouter per call:             12/13 exact, worst 17 layouts
cold, one layouter, repeated back presses:  4 layouts per turn typical

The single inexact case is a target that lies on the canonical chain but not on any chain reachable from a nearby anchor; it returns a start 15 words early, i.e. a slightly overlapping page.

Acceptance criteria

For every page of a document, render_page_backward(start[i]) returns start[i-1] — verified for both a mid-paragraph-paginating document and one where every page starts on a block boundary.
Laying out forward from the returned position ends exactly on the requested position.
Forward-then-back returns to the original position.
At the document start, backward stays there; an empty document is safe.
Cost stays within a small bounded number of forward layouts.

Files

pyWebLayout/layout/ereader_layout.py

Test plan

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) — 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.

60 KiB Raw Permalink Blame History Unescape Escape

Layout Remediation Spec

Contents

Design invariants

Phasing

S1 — Inline content in non-paragraph containers

Problem

Evidence

Design

Acceptance criteria

Files

Risk

S2 — Page geometry: origin and content rect

Problem

Design

Acceptance criteria

Files

Risk

S3 — Draw/canvas lifecycle

Problem

Evidence

Design

Acceptance criteria

Files

S4 — One block dispatch, one measurement

Problem

Design

Acceptance criteria

Files

Risk

S5 — Cells as sub-layouts

Problem

Design

Acceptance criteria

Files

S6 — Table grid model

Problem

Design

Acceptance criteria

Files

S7 — Retained-mode table rendering

Problem

Evidence

Design

Acceptance criteria

Files

S8 — Table and list pagination

Problem

Design

Acceptance criteria

Files

Risk

S9 — Interactivity inside tables

Problem

Design

Acceptance criteria

Files

S10 — Contracts and hygiene

10.1 Render contract

10.2 Abstract/concrete leak

10.3 Dead code

10.4 Exception handling

10.5 Image source resolution

Acceptance criteria

S11 — Partial-block progress is discarded

Problem

Evidence

Design

Acceptance criteria

Files

Risk

S12 — Background rendering

Problem

Evidence

Design

Acceptance criteria

Files

Risk

S13 — Word spacing and alignment

Problem

60 KiB

Raw Permalink Blame History