Skip to content

fix(engine): lay out composite children inside a composed table cell - #600

Merged
DemchaAV merged 2 commits into
developfrom
fix/composed-cell-composite-recursion
Aug 25, 2026
Merged

DemchaAV merged 2 commits into
developfrom
fix/composed-cell-composite-recursion

Conversation

@DemchaAV

Copy link
Copy Markdown
Owner

Why

DocumentTableCell.node(...) holding a SectionNode, ContainerNode, RowNode, LayerStackNode or AlignNode measured the child, reserved its full height, and then drew nothing inside it — a correctly-sized blank hole in the table. Only leaf children (paragraph, list) and nested tables ever rendered.

A composite leaves its children to the compiler and emits only its own decoration from emitFragments, so dispatching a composed cell straight at the child's emitFragments picked up the section background and dropped every paragraph under it. ADR 0013 promises the opposite — "a layer-stack, even a sub-table renders correctly" — and the sub-table half was the only half that held.

The cell reserves the child's measured height either way, so nothing threw and nothing looked misaligned: the page just had a hole where the content belonged.

What changed

FragmentContext.emitChildFragments lays a composite child's whole sub-tree out inside the placement, through LayoutCompiler.compileFixedBoxSubtree → the existing compileNodeInFixedSlot walk, so the sub-tree gets the same column / row / stack layout it gets at document level. Absolute coordinates come back rebased onto the placement, so callers translate them exactly as they do for a leaf. Leaf children and nested tables keep their single-dispatch path and are unchanged.

Reusing the compiler's walk rather than recursing inside NodeDefinitionSupport avoids reimplementing RowSlots, LayerStackGeometry, CompositeDecoration and the stack z-order in the fragment layer.

That walk turned out to have no horizontal branch at all. A RowNode nested in a fixed rectangle — a LayerStackNode layer, allowed since 1.6.2, and now a table cell — stacked its children downwards, and because the band is measured one row tall, every child after the first spilled out of the rectangle. placeRowBandInFixedSlot now resolves slot widths through the same RowSlots.resolveLayout the page-flow row band uses, so weights, fixed columns, flex arrangement and vertical alignment behave identically, and a nested horizontal row is still rejected with the page-level diagnostic.

Docs: the advanced-tables recipe demonstrated a multi-line cell by putting \n inside text(...), which renders as a single line. The recipe and the DocumentTableCell Javadoc now name the three cell shapes — text(...) for one line, lines(...) for several, node(...) for a node — and note that ParagraphNode does honour \n as a hard break, inside a cell as anywhere else. The two examples that showed the misleading form use lines(...).

Verification

./mvnw -B -ntp clean verify -pl :graph-compose-core,:graph-compose-render-pdf,:graph-compose-render-docx,:graph-compose-render-pptx,:graph-compose-templates,:graph-compose-testing,:graph-compose-qa,:graph-compose-coverage -amBUILD SUCCESS, qa 767 tests, 0 failures; javadoc:javadoc -pl :graph-compose-coreBUILD SUCCESS. No layout snapshot and no pixel baseline moved, so no existing document's geometry changed.

Four new qa classes, 23 cases:

Class Covers
TableCellComposedNodeTypeTest 8-case matrix: paragraph, list, nested table (controls) + section, container, row, layer stack, align — fragment count and rendered PDF text
RowInFixedSlotLayoutTest row geometry in a stack layer vs page level, row in a cell on one band, row child inside the table bounds
DocumentTableCellCopyTest withStyle / colSpan / rowSpan preserve content, style and spans; a styled composed cell still renders its child
FooterPageNumberingOverflowTest Page {page} of {pages} per page across 1 / 2 / 5 pages driven by table overflow, with and without repeatHeader()

The three geometry cases in RowInFixedSlotLayoutTest were confirmed red with the horizontal branch disabled; the text-only matrix stayed green, which is why both exist.

ComposedTableCellExample gained a section-in-a-cell and a row-in-a-cell panel; assets/readme/examples/composed-table-cell-showcase.pdf and table-advanced.pdf are regenerated.

Known limits

Composed-cell children still produce no PlacedNode, so layout snapshots cannot see inside a composed cell — unchanged by this PR, and the reason the new coverage reads fragments and rendered text instead. Publishing them would churn every composed-cell baseline; better suited to a minor.

DocumentTableCell.node(...) holding a SectionNode, ContainerNode, RowNode,
LayerStackNode or AlignNode measured the child, reserved its full height and
then drew nothing inside it — a correctly-sized blank hole in the table. A
composite leaves its children to the compiler and emits only its own
decoration from emitFragments, so dispatching a composed cell straight at the
child's emitFragments picked up the section background and dropped every
paragraph under it.

FragmentContext.emitChildFragments now lays a composite child's whole sub-tree
out inside the placement, through the compiler's fixed-slot walk, so the
sub-tree gets the same column / row / stack layout it gets at document level.
Leaf children and nested tables keep their existing single-dispatch path and
are unchanged.

That walk turned out to have no horizontal branch at all: a RowNode nested in
a fixed rectangle — a LayerStack layer since 1.6.2, and now a table cell —
stacked its children downwards and, because the band is measured one row tall,
spilled every child after the first out of the rectangle. It now resolves slot
widths through the same RowSlots path the page-level row band uses, and still
rejects a nested horizontal row with the page-level diagnostic.

Docs: the advanced-tables recipe demonstrated a multi-line cell with a "\n"
inside text(...), which renders as one line. The recipe and the
DocumentTableCell Javadoc now name the three cell shapes — text(...) for one
line, lines(...) for several, node(...) for a node — and the two examples that
showed the misleading form use lines(...).

Tests: ./mvnw clean verify on the eight-module reactor — BUILD SUCCESS, qa 767
tests, no baselines moved. Four new qa classes, 23 cases: the composed-content
matrix over every composite kind plus leaf and nested-table controls; row
geometry in a stack layer and in a cell, each red without the horizontal
branch; DocumentTableCell copy-factory preservation of content, style and
spans; and Page {page} of {pages} across 1 / 2 / 5 pages, with and without a
repeated header row.
…posed cell

Two behaviours the first pass changed or newly relied on, neither of which
anything guarded.

A row nested directly inside another row in a fixed rectangle now raises the
page-level "cannot contain a nested horizontal row" diagnostic instead of
laying out. What it laid out before was not usable — with a two-child inner
row inside a two-child outer row in a layer, two of the three leaves landed on
the same point, 17pt below the layer's own bottom edge — but the change from a
render to an exception is worth stating, so the CHANGELOG entry says it and a
negative case pins it.

A clipping composite (ShapeContainerNode with CLIP_PATH) writes graphics state
that has to be handed back. Inside a cell it emits Shape / ClipBegin / child /
ClipEnd, the same ordered sequence it emits at page level, and the new case
asserts the cell sequence against the page-level one rather than a hardcoded
list. A dropped ClipEnd would leak the clip onto later fragments with nothing
missing from the page and no exception raised.

Tests: ./mvnw clean verify on the eight-module reactor — BUILD SUCCESS, qa 769
tests (+2).
@DemchaAV

Copy link
Copy Markdown
Owner Author

Follow-up commit d3ce7239 after a regression-focused pass over the diff.

Two behaviour changes were found; one needed recording.

RowNode nested directly inside another RowNode in a fixed rectangle now raises the page-level "cannot contain a nested horizontal row" diagnostic where it previously laid out. Measured what it laid out before: for a two-child inner row inside a two-child outer row in a layer, the layer's bottom sits at y=267.0 while two of the three leaves land at y=250.1 — 17pt below the layer, overlapping each other at the same x. Not a usable layout, so the exception is the better outcome and its message names the fix (wrap the inner row in its own layer). The CHANGELOG entry now states it and a negative case pins it.

Three regression candidates came back clean, each checked against a measurement rather than reasoning:

  • Decoration loss. A SectionNode with fill + stroke in a composed cell emits ShapeFragmentPayload and ParagraphFragmentPayload — same payload set as the identical section at page level. The background is not traded for the content.
  • Unbalanced clip. A ShapeContainerNode with CLIP_PATH in a cell emits Shape / ClipBegin / Paragraph / ClipEnd, byte-for-byte the page-level sequence. The only two definitions overriding emitOverlayFragments (LayerStackDefinition, ShapeContainerDefinition) are both STACK-axis, and the fixed-slot STACK branch emits fill and overlay; no vertical or horizontal definition overrides it, so nothing is dropped. Now asserted against the page-level sequence rather than a hardcoded list.
  • Chart dispatch. ChartDefinition is the other emitChildFragments caller. Its primitives are leaf nodes only — no composite constructor anywhere under document.chart — so charts take the unchanged branch.

Blast radius of the row fix is empirically zero for existing content: no template preset or example puts a row in a stack layer (every .layer(...) call in templates/ and examples/ holds a paragraph, shape, icon or badge), no layout snapshot or pixel baseline moved, and CI's Examples Generation Smoke Test passes — which also rules out the new exception firing on any shipped example.

Gate: BUILD SUCCESS, qa 769 tests (+2).

@DemchaAV
DemchaAV merged commit aedd455 into develop Aug 25, 2026
12 checks passed
@DemchaAV
DemchaAV deleted the fix/composed-cell-composite-recursion branch August 25, 2026 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant