feat(layout,render): converge inline pipeline — vertical writing modes (slice 2)#271
Merged
Conversation
…s (slice 2) Extend the InlineFlow persist+consume convergence (#270, slice 1) to vertical writing modes, dropping the `!is_vertical` gate. Layout (elidex-layout-block): - Drop `!is_vertical` from the InlineFlow persist gate. - Apply the is_vertical projection swap when folding the IFC origin (mirroring static_positions / assign_inline_layout_boxes): inline-axis → physical x (horizontal) or y (vertical), block-axis → y or x. Each stored scalar then holds the absolute physical coordinate for its axis (no vertical-rl block reversal, matching the box convention). FlowAlign's resolution is already inline-axis- agnostic, so vertical text-align needs no new logic — only enabling. Render (elidex-render): - Hoist the parent_style read above the InlineFlow dispatch so the converged path can branch on writing_mode (inherited → IFC parent authoritative). - emit_inline_flow dispatches horizontal vs new emit_inline_flow_vertical, which consumes per-line/per-run positions (center_x = block_start + block_size/2, cursor_y = inline_start). Fixes silent single-column mis-render of multi-line / multi-run vertical text. - Extract shared emit_vertical_text_segment / vertical_text_orientation (mirrors the horizontal emit_text_segment sharing). - Normalize Writing-Modes citations L3→L4; fix the §4.1 (= Introduction to Baselines) → §3.1 (Introduction to Vertical Writing) content mismatch. Tests: +6 layout (vertical persist, axis-swap, multi-column, vertical+justify still gated) + 1 render (multi-column vertical consume); removed the obsolete slice-1 gate_excludes_vertical_writing_mode. Cap-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onsume - emit_inline_run: read only writing_mode + text_orientation (Copy) for the converged dispatch instead of cloning the whole ComputedStyle above the fast-path return (the clone is restored to the legacy path only). Fixes a per-run hot-path regression introduced by the slice-2 dispatch hoist. - A styleless parent now falls back to the horizontal default (mirroring layout's get_style unwrap_or_default), so a persisted flow is still painted rather than dropped — closes a latent layout/render divergence the hoist introduced. - Fold emit_inline_flow_vertical into emit_inline_flow as a single per-line/run loop branching only at the glyph emit (One-issue-one-way; removes the duplicated loop skeleton). - Use the canonical WritingMode::is_horizontal() predicate (shared with layout) instead of open-coded matches!(.., HorizontalTb). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the layout↔render inline-pipeline convergence (InlineFlow persist+consume) to vertical writing modes, fixing previously incorrect rendering of wrapped/multi-line vertical text by having render consume layout’s per-line/per-run geometry.
Changes:
- Layout: removes the vertical-writing-mode persistence exclusion and applies an is_vertical-aware origin fold so persisted InlineFlow coordinates are absolute physical coordinates.
- Render: unifies InlineFlow consumption for horizontal/vertical (branching at glyph emission), extracts shared vertical segment emission/orientation helpers, and avoids cloning
ComputedStyleon the converged hot path. - Tests/docs: adds vertical persistence/axis-swap/multi-column coverage and a render consume test for multi-column vertical flow; updates Writing-Modes citations and InlineFlow coordinate documentation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/layout/elidex-layout-block/src/inline/tests/inline_flow.rs | Adds vertical-writing-mode persistence tests; removes the slice-1 vertical gate test. |
| crates/layout/elidex-layout-block/src/inline/pack.rs | Updates FlowAlign docs to be writing-mode/inline-axis agnostic. |
| crates/layout/elidex-layout-block/src/inline/mod.rs | Drops !is_vertical from the persistence gate; applies axis-swap when folding IFC-local to absolute physical coordinates. |
| crates/core/elidex-render/src/builder/tests/inline_flow.rs | Adds a vertical InlineFlow consume test for multi-column output. |
| crates/core/elidex-render/src/builder/inline.rs | Threads writing-mode/orientation into InlineFlow consume; adds vertical branch consumption and extracts shared vertical segment emission/orientation. |
| crates/core/elidex-ecs/src/components.rs | Documents InlineFlow coordinate semantics after writing-mode projection at persist (horizontal vs vertical). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ecision) - Vertical multi-column render test: use the same leading glyph in both columns so the x-delta isolates the column-center difference instead of depending on per-glyph metrics (which vary by the CI runner's first available font). Tighten the tolerance to 0.5 now that the offsets cancel exactly. - emit_inline_run / emit_inline_flow docs: the dispatch reads the IFC parent's writing_mode because render must interpret the persisted coordinates with the SAME writing mode layout projected them with — not because inherited properties can't be overridden (they can, on a descendant that establishes its own FC). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vertical text-decoration comment claimed under/overline placement differs by vertical-rl vs vertical-lr, but the code places underline +x / overline −x of the column center regardless of writing mode. Corrected the comment to describe the actual writing-mode-agnostic placement, and tracked the spec-correct vertical-rl/lr-dependent sidedness as deferred alongside the other vertical-rl physical-correctness work (plan §10). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
crates/core/elidex-render/src/builder/inline.rs:419
emit_inline_flowcurrently skips a run entirely whenrun.entityhas noComputedStyle. That contradicts the nearby intent comment about a styleless parent still being painted (layout persists withunwrap_or_defaultsemantics), and can make persisted flows silently render nothing if style data is missing. Consider falling back toComputedStyle::default()for missing styles (and keep the visibility check).
for run in &line.runs {
let Ok(style) = dom.world().get::<&ComputedStyle>(run.entity) else {
continue;
};
// visibility: hidden text occupies space (laid out) but is not painted.
if style.visibility != Visibility::Visible {
continue;
The dispatch comment said the consume path reads writing_mode/text_orientation "without cloning the whole ComputedStyle" — accurate for the dispatch, but it could read as claiming the consume path is allocation-free. Clarified that the hoist only avoids cloning the PARENT ComputedStyle per call; emit_inline_flow still reads each run's own paint style (StyledTextSegment::from_style) from its entity, exactly as the horizontal path always has. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Slice 2 of the render↔layout inline-pipeline convergence (#270 = slice 1). Extends the
InlineFlowpersist+consume convergence to vertical writing modes, dropping the!is_verticalgate.Render's vertical legacy path (
emit_styled_segments_vertical) is a single-column linear pass with no line-breaking → multi-line / multi-run vertical text renders wrong today (all lines collapse into one column). This slice fixes it by having render consume layout's per-lineInlineFlow— a correctness fix, not just dedup.Changes
Layout (
elidex-layout-block)!is_verticalfrom theInlineFlowpersist gate.is_verticalprojection swap when folding the IFC origin — the same rulestatic_positions/assign_inline_layout_boxesuse: inline-axis → physical x (horizontal) or y (vertical), block-axis → y or x. Each stored scalar then holds the absolute physical coordinate for its axis (no vertical-rl block reversal — matching the box convention).FlowAlign's resolution was already inline-axis-agnostic, so vertical text-align needed only enabling.Render (
elidex-render)emit_inline_flowbranches horizontal vs vertical at the glyph emit (shared per-line/run loop); vertical derives the columncenter_xfromblock_start/block_sizeand the pen y frominline_start.emit_vertical_text_segment/vertical_text_orientation(mirrors the horizontalemit_text_segmentsharing; the legacy driver now delegates to it).writing_mode+text_orientation(Copy) — noComputedStyleclone on the hot path; a styleless parent → horizontal default (mirrors layout'sget_styleunwrap_or_default, so a persisted flow is still painted).Tests
+6 layout (vertical-rl/lr persist, the axis-swap proof, multi-column, vertical + justify-still-gated) + 1 render (multi-column vertical consume — x-separation between columns + downward y-advance within a column). Removed the now-obsolete slice-1
gate_excludes_vertical_writing_mode.Edge matrix (per the inline white-space density lesson)
Covered: vertical-rl/lr persist · axis-swap (block↔x / inline↔y) · multi-column (wrap) · text-align baked into
inline_start· all other gates (justify/pseudo/atomic/relpos/bidi/text-transform/fragmentation) still excluded · multi-column render. The shared orientation (mixed/upright/sideways) + vertical-decoration logic is a byte-identical extraction from the verified legacy path (covered by extraction fidelity); forced-break columns use the same packer line-break mechanism as wrap.Deferred (program-internal, plan §10, cap-neutral)
static_positions/box convention (consistent with clientRects — not a new divergence).WritingModeContext::is_block_reversed()exists but is unwired across boxes + static-pos +InlineFlow; wiring it is a standalone WM-correctness project.font_size) · sharedinline_logical_to_physicalhelper (unify the 3 projection copies) · bidi reorder in the converged vertical path → slice 4.Cap-neutral (One-issue-one-way + conformance/correctness, no new web-platform feature).
Pre-push gate:
cargo fmt+mise run ci(10634 tests) +/code-review+/simplify+/review+/elidex-review(0 CRIT / 0 IMP) all green.🤖 Generated with Claude Code