Skip to content

Engine track E5: sprite parity in iced (shared geometry crate) - #310

Merged
charliek merged 8 commits into
mainfrom
feature/plan-020-iced-sprites
Aug 7, 2026
Merged

Engine track E5: sprite parity in iced (shared geometry crate)#310
charliek merged 8 commits into
mainfrom
feature/plan-020-iced-sprites

Conversation

@charliek

@charliek charliek commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Engine-track slice E5: both shipped UIs draw U+2500–U+259F (box drawing + block elements) geometrically because font glyphs don't tile pixel-perfectly across cells — roost-iced had no sprite path at all, so TUI chrome showed hairline seams there and in neither shipped UI. This PR moves the geometry to a shared crate and gives iced the missing sprite path, at parity with both shipped UIs. Plan 020; closes the parity-inventory P1 box-drawing row and the E5 half of the M6 entry gate.

What changed, per workstream

  • C0 warm-uptest_osc_pipeline.py wired into the iced e2e lists (Makefile ICED_E2E_TESTS + the three ci.yml lanes); carried unwired since Engine track E1-E3: render-state dirty tracking + the measurement that proves it #306.
  • C1 shared geometry — new roost_ui_model::sprite: sprite_geometry(cp, w, h) emits cell-relative primitives (rects+alpha, corner arcs, diagonals), arc_path is the single source for the rounded-corner three-segment path, tessellate flattens stroked primitives into stamped rects for quad-only renderers. All f64 math moved verbatim; verified byte-identical against the cairo renderer for all 160 codepoints × 10 cell sizes via a throwaway cross-check before the old code was touched. Plus a committed golden-hash fixture (153 codepoints × 3 cell sizes, FNV-1a over the ARGB32 surface, generated from the pre-refactor renderer; the 7 stroked glyphs excluded as cairo-version-sensitive).
  • C2 GTK adaptercrates/roost-linux/src/sprite.rs 2,715 → 569 lines; renders shared primitives with the identical cairo call patterns (AA-off block bracket, exact arc/diagonal stroke sequences). The pre-refactor pixel-assertion suite and golden fixture pass unchanged.
  • C3 iced sprite draw — intercept in the glyph pass (single-codepoint cells, inside the non-blank guard) drawing tessellated primitives as fill_quads with integer-snapped edges (iced has no per-quad AA switch — snapping is the seam mechanism; it also makes stamp overlap composite-safe). Sprite draws count into fill_text_calls, making the counter apples-to-apples across UIs; stale perf-caveat comments and cli.md/ipc.md "GTK reports all zeros" claims fixed.
  • C4 e2e guardtest_sprite_pixels.py (iced-only): full-block seam scan, partial-block internal hard-edge assertions (▀▄▌), █→▀ tiling, unbroken ─/═ runs, and a fill_text_calls > 0 counter assertion read before any screenshot. Wired into all four iced lists.
  • C5 docs — roadmap E5 marked complete with the shipped shape; M6 entry gate now leaves release-profile CI + the inventory audit; parity-inventory box-drawing row closed.

Verification

  • Per-commit gates green throughout; make test-rust green (20 suites); make check-iced green.
  • GTK byte-identical oracle (shed, real Linux): hermetic Xvfb instance, deterministic sprite scene (DECTCEM off; all glyph families incl. inverse/bold rows and ╲-over-explicit-bg) fed via tab.feed_pty_bytes, roostctl screenshotmain vs this branch: byte-identical, md5 9d88a549d3455bb8fd85fcdb55dcf3d3 both.
  • Full GTK e2e suite in the shed (CI parity, shed Linux binaries): 171 passed, 11 skipped.
  • iced visual on macOS: hermetic run, screenshots at 12 pt + 20 pt with 3× zooms — line runs scan as single contiguous exact-fg pixel runs, blocks tile seamlessly, arcs render as clean rounded corners; live counter check 196 → 1003 fill_text_calls on a sprite-only scene.
  • test_sprite_pixels.py + test_osc_pipeline.py verified green against iced locally before wiring.

Accepted/recorded divergences (decided, not discovered)

  • wgpu blends shade (░▒▓) alpha in linear space → iced shades render lighter than cairo's sRGB blend (measured 128/174/207 vs ~80/132/174). Cross-UI pixel parity has no CI gate by standing decision (No CI gate for GTK↔Iced visual parity (capture tooling is human-reviewed only) #284).
  • Arcs/diagonals are stamped-quad staircases on iced vs cairo AA strokes (v1; ~28–33 quads/arc-cell, ╳ worst case 92; rect glyphs 1–4 — no perceptible frame impact).
  • iced's interleaved bg+glyph draw order can shave diagonal overshoot (≤ ~t/2 + 0.5 px) at explicit-bg cell boundaries where GTK's two-pass order preserves it — not observable at current stroke widths; two-pass restructure named as future work.
  • Pre-existing, untouched: GTK's block cursor redraws the covered sprite in canvas_bg, iced dims through a translucent overlay; GTK's cursor-sprite draw is uncounted in fill_text_calls on both UIs.

Dependency impact: none (no new crates; the golden-test hash is an inline FNV-1a). No privacy/secret impact.

Panel: Codex usage-limited; GLM 5.2 + CodeRabbit reviewed the plan — their corrections (edge snapping as the iced seam mechanism, arc-leg API respec, golden-hash fast-fail, cursor-pass resolution, counter e2e assertion, overshoot draw-order divergence) are incorporated and noted above. Per-commit external reviewers were unavailable (Codex limited, Cursor CLI empty output twice) — self-review fallback per flow, noted in each commit message.

Plan 020 (full text)

Plan 020 — Engine track E5: sprite parity in iced

Status: draft for panel review
Branch: feature/plan-020-iced-sprites (from main @ 48087d4), PR into main, manual merge after ci-success.
Brief: BRIEF-020-iced-sprites.md — its pinned decisions are honored, not re-litigated.
Panel roster: Codex attempted first per the brief but still usage-limited (reset 2026-08-08 9:35 AM; session ran before it) — GLM 5.2 + CodeRabbit reviewed the plan (same roster as plan 019). Per-commit reviews: Cursor CLI (retry once, then self-review); CodeRabbit local CLI (coderabbit review --committed --base main) at PR time if cloud is still rate-limited.
Artifacts folder: ~/.claude/plans/roost/020-iced-sprites/
Panel corrections applied (2026-08-06): GLM + CodeRabbit, converging on:
(1) iced has no per-quad AA-off — the seam-killer doesn't port as a flag;
the iced adapter now snaps all sprite rect edges to integers, and the
C4 e2e asserts partial-block internal edges, not just full-block
boundaries. (2) The CornerArc/tessellate API was under-specified
(missing the arc's straight legs) — respecified. (3) The byte-identical
GTK bar gets an automated fast-fail: a full-surface golden-hash fixture
generated from the pre-refactor renderer, asserted in cargo test -p roost-linux from C1 on. (4) The cursor-pass audit is resolved in-plan
(iced block cursor = translucent quad overlay, no glyph redraw — no
intercept needed). (5) AC4 counter change gets a real e2e assertion via
app.render_stats. (6) iced's interleaved bg+glyph draw order can shave
diagonal overshoot at explicit-bg cell boundaries — accepted for v1,
documented. Plus: f64→f32 cast named, intercept placed inside the
non-blank guard, ipc.md "all counters zero" staleness fixed by grep.

1. Problem / motivation

mac/Sources/Roost/Sprite.swift and crates/roost-linux/src/sprite.rs draw
U+2500–U+259F (box drawing + block elements) geometrically because font
glyphs don't tile pixel-perfectly across adjacent cells. roost-iced has
no sprite path at all
— TUI chrome shows hairline seams there and in
neither shipped UI. This is roadmap slice E5
(docs/development/iced-migration-roadmap.md:435-443), a P1 parity gap
(docs/development/iced-parity-inventory.md:151), and one of the two
remaining M6 entry gates (iced-migration-roadmap.md:603).

Additionally (Phase-0 warm-up, ships even if E5 doesn't):
tools/roosttest/test_osc_pipeline.py was verified green against iced on
all three platform/renderer combos in the 017/018 session but has sat
unwired from the iced e2e lists since PR #306.

2. Current state — verified this session

The GTK sprite renderer (source of the port)

  • crates/roost-linux/src/sprite.rs (2,612 lines). Sole public item:
    draw_cell_sprite(cr: &cairo::Context, x, y, w, h: f64, fg: ColorRgb, cp: u32) -> bool
    (sprite.rs:24-38); dispatches 0x2580..=0x259Fdraw_block_element,
    0x2500..=0x257Fdraw_box_glyph. Declared port of Ghostty's
    font/sprite/draw/{block,box}.zig (sprite.rs:1-16).
  • Primitive census — not all rects:
    • Block elements (sprite.rs:71-327): axis-aligned rect fills with
      cairo::Antialias::None for the whole layer (sprite.rs:88; the
      seam-killer). Shades ░▒▓ are alpha fills 0.25/0.5/0.75
      (sprite.rs:265-272).
    • Box drawing: rect fills via box_rect (sprite.rs:2045-2052),
      junction precedence in pick_junction (sprite.rs:2018-2042),
      dashes as rect segments (sprite.rs:2152-2251, with a
      too-narrow fallback to draw_box_lines); thickness
      box_thickness(h) = (h/14).round().max(1), heavy = 2× light
      (sprite.rs:1857-1871).
    • Non-rect families: rounded corners U+256D–2570 = cubic-Bézier
      strokes with S=0.25 control points, butt cap, width t
      (sprite.rs:2055-2124); diagonals U+2571–2573 = stroked lines with
      corner overshoot (sprite.rs:2129-2148).
  • In-module test suite (sprite.rs:2253-2611): renders each glyph to a
    Cairo ARGB32 surface and asserts raw bytes — 20 tests including the
    opencode-wordmark seam regression block_tiling_no_gap
    (sprite.rs:2548). No integration tests reference sprite.

GTK call sites and cache interaction

  • Glyph-pass intercept: terminal_view.rs:2130-2141 — single-codepoint
    check (chars.next() × 2) then sprite::draw_cell_sprite(...);
    handled → fill_text_calls += 1; continue, else Pango fallback.
  • Second call site in the block-cursor painter, drawn with canvas_bg
    as fg: terminal_view.rs:2560-2578 (this draw is not counted —
    pre-existing, both call sites keep their behavior).
  • E3b cache holds logical content only (RenderedRow { bg, glyphs },
    terminal_view.rs:1707-1716; build at :1724-1749 has no
    codepoint-range test) — the sprite decision is recomputed from cached
    Strings at paint time with live cell_metrics
    (terminal_view.rs:2077-2078). Stateless draw-from-text confirmed.
  • Cell metrics: CellMetrics { cell_width, cell_height, baseline },
    width/height .floor().max(1.0) (cell_metrics.rs:64-65); no stroke
    metric crosses into sprite — thickness derives from h internally.

iced draw path (target of the addition)

  • TerminalWidget::draw (crates/roost-iced/src/terminal_widget.rs:638-786):
    iterates snapshot.grid: Vec<Arc<RenderedRow>> (:660), per cell
    computes cell_position (:665, TERMINAL_PADDING = 0.0 at :22),
    fills background quad when explicit_background (:666-678), then
    glyph via renderer.fill_text (:679-699) and fill_text_calls += 1
    (:698). The sprite intercept slots between :678 and :679.
  • DrawCell.text is an owned per-cell String (:132-142) — the GTK
    single-codepoint pattern works verbatim.
  • fill_quad helper builds renderer::Quad { snap: false, .. }
    (:791-800). Cell metrics floored (measure_with_font, :70-71) —
    integer cell strides are why sprite tiling can work; the E4 no-go
    pinned that un-flooring would break sprites
    (iced-migration-roadmap.md:414-417).
  • Cursor pass :736-779; selection :703-718; hover underline
    :720-734; perf record :781. No canvas/geometry API anywhere in
    roost-iced — fill_quad is the only shape primitive in play.

Counters

  • Shared types: roost-ui-model/src/render_stats.rs:18-26
    (fill_text_calls at :25); record_draw at :121-127.
  • GTK increments: sprite draw terminal_view.rs:2138, Pango draw
    :2146, recorded at :2189. Cross-UI caveat — "iced has no sprite
    path (roadmap E5), so this field is not apples-to-apples across UIs" —
    at crates/roost-linux/src/perf.rs:17-20, duplicated at
    terminal_view.rs:2060-2062; iced-side counterpart caveat at
    crates/roost-iced/src/perf.rs:22-25.
  • Stale doc: docs/reference/cli.md:210 still claims "The GTK UI
    reports all zeros (no instrumentation yet)".
  • iced draw counters only exist in a running app (no unit-test
    iced::Renderer) — crates/roost-iced/src/perf.rs:26-28.

roost-ui-model (destination)

  • Pure-data: deps are roost-ipc, roost-vt, bitflags, serde,
    serde_json, tracing, unicode-segmentation
    (crates/roost-ui-model/Cargo.toml:11-19) — no toolkit deps. 18
    modules today; render_stats is the precedent for exactly this move
    (documented at roost-iced/src/perf.rs:31-34, "moved to
    roost-ui-model::render_stats once GTK needed the identical shape").
  • Both UIs already depend on it (roost-iced/Cargo.toml:21;
    roost-linux likewise via perf/render_stats).

Warm-up wiring (Phase 0)

  • tools/roosttest/test_osc_pipeline.py: class-gated on
    ROOST_TEST_MODE=1 only (:44-51) — no target-specific skips; all
    three iced CI lanes set ROOST_TEST_MODE: "1". Docstring line 26
    says "Both targets run these in CI (e2e-gtk + e2e-mac)" — goes stale
    with this change; update it.
  • Exactly 4 wiring sites, all full repo-relative paths with .py:
    1. Makefile:72 ICED_E2E_TESTS (single line, space-separated)
    2. .github/workflows/ci.yml:539-551 (Iced e2e, Linux X11)
    3. .github/workflows/ci.yml:584-594 (Iced e2e, Linux Wayland — base
      list only, clipboard pair deliberately excluded)
    4. .github/workflows/ci.yml:608-620 (Iced e2e, macOS)
      Convention: append after test_sidebar_resize.py, before the
      clipboard pair where present, keeping Makefile and CI base lists in
      the same order. e2e-gtk/e2e-mac run the whole dir — no edits
      there.

Verification tooling

  • tools/screenshot/ treats iced as a first-class target
    (lib.sh:46-65,92-116; launch.sh, quit.sh, smoke.sh).
    roostctl screenshot works against iced (in-process render;
    crates/roost-cli/src/main.rs:23,1057-1064).
  • pngtool.py is stdlib-only and importable from pytest
    (test_tab_strip_pixels.py:54-55 precedent); subcommands: info,
    pixel, hscan, vscan, crop, textscan, findcolor. Capture helper
    _capture at test_sidebar_pixels.py:177-190 (tolerates transient
    empty snapshots).
  • The 018 GTK pixel-oracle recipe (plan 018 :344-350, :505-509):
    launch under ROOST_TEST_MODE=1, feed one deterministic scene via
    tab.feed_pty_bytes, DECTCEM off (ESC[?25l — blink-phase
    determinism, load-bearing), roostctl screenshot pre/post binaries on
    the same host/session, byte/MD5 compare. Counters must be read
    before any screenshot (screenshot re-renders and inflates them —
    plan 017 :687-691, roost-iced/src/perf.rs:21-25).
  • Parity-inventory row already exists:
    docs/development/iced-parity-inventory.md:151 (P1, added
    2026-08-06, tracks E5, acceptance evidence: "Seam-free capture of a
    box-drawing/block fixture under both renderers … codepoint-dispatch
    unit coverage mirroring the existing Swift/Rust sprite tests"). The
    brief's "add the missing row" is already satisfied; this plan
    updates the row to closed instead.

3. Design decisions — PINNED

D1. Shared geometry module: roost_ui_model::sprite

A new module in roost-ui-model (not a new crate — roost-ui-model is
the shared crate both UIs already depend on, and render_stats is the
precedent). Pure data in, pure data out; no cairo, no iced.

Why quads-only on the iced side (restating the brief's constraint so the
design is auditable standalone): roost-iced deliberately has no
Canvas/mesh layer in the terminal widget path — fill_quad is the only
shape primitive in play (terminal_widget.rs:791-800), and introducing
a canvas dependency for sprites was ruled out by the brief. All curved/
diagonal strokes therefore reduce to axis-aligned rects on iced.

pub struct SpriteRect { pub x: f64, pub y: f64, pub w: f64, pub h: f64 }

pub enum SpritePrimitive {
    /// Axis-aligned fill in cell-relative px. `alpha` 1.0 except shades.
    Rect { rect: SpriteRect, alpha: f64 },
    /// Rounded corner ╭╮╯╰ = today's full three-segment cairo path:
    /// straight leg from a cell edge, cubic Bézier (S=0.25 control
    /// points), straight leg to the other edge. Carries the cell dims
    /// so BOTH consumers derive the identical path from shared code.
    CornerArc { corner: Corner, w: f64, h: f64, thickness: f64 },
    /// Diagonal stroke ╱╲ endpoint pair (overshoot baked in) + thickness.
    Diagonal { x0: f64, y0: f64, x1: f64, y1: f64, thickness: f64 },
}

pub struct SpriteGeometry {
    pub primitives: Vec<SpritePrimitive>,
    /// GTK-ONLY semantics: block-element layer brackets its fills in
    /// cairo Antialias::None exactly as today. iced has no per-quad AA
    /// switch and ignores this — its seam story is edge snapping (D3).
    pub antialias: bool,
}

/// None when `cp` is not a sprite codepoint (caller falls back to font).
pub fn sprite_geometry(cp: u32, w: f64, h: f64) -> Option<SpriteGeometry>;

/// The CornerArc's exact path in cell-relative coordinates (leg start,
/// leg end, Bézier control points, end leg) — the single source both
/// the GTK adapter's cairo calls and `tessellate` flatten from.
pub fn arc_path(corner: Corner, w: f64, h: f64) -> ArcPath; // concrete struct

/// Flatten CornerArc/Diagonal into overlapping stamped rects
/// (geometry only — alpha/antialias ride on the primitive/geometry,
/// never on the tessellation output). Rect passes through unchanged.
pub fn tessellate(prim: &SpritePrimitive) -> Vec<SpriteRect>;

Pinned properties:

  • All coordinate math moves verbatimbox_thickness,
    pick_junction, aligned_block, draw_quads, dash layout, the
    Lines4 dispatch table — preserving the exact f64 expressions and
    operation order so emitted rect coordinates are bit-identical to what
    the current cairo calls receive. Cell-relative coordinates; the
    adapter adds the cell origin with the same x + l / y + t
    arithmetic as today.
  • The Corner/LineStyle/Lines4 types move as-is.
  • Color does not cross the boundary: sprites are monochrome fg;
    each adapter applies its own color type (GTK ColorRgb→rgb/rgba,
    iced Color with alpha).
  • tessellate lives in the shared module (geometry stays in one
    place): flatten the same three-segment arc path / diagonal line at
    step spacing ≤ t/2 and stamp t×t rects centered on each sample.
    Overlap is safe because the iced adapter snaps stamp edges to
    integers
    (D3) — snapped hard-edged quads have no partial-coverage
    AA edges, so overlapping stamps cannot composite darker (the AA
    overlap-darkening artifact the panel flagged); the cost is a pure
    staircase on arcs/diagonals, accepted for v1 (§9). Unit tests pin:
    stamp-count bounds, consecutive-stamp overlap/abutment, and
    bbox containment within the cell — with the documented exception
    that Diagonal stamps may exceed the cell by the deliberate
    overshoot (±0.5·slope + t/2), which exists for cross-cell slope
    continuity. Alternatives considered: (a) per-scanline strips —
    more code for no visual gain at terminal stroke widths (1–2 px);
    (b) iced Canvas/mesh — no canvas exists in the widget path (pinned by
    brief); (c) font-glyph fallback for arcs/diagonals in iced — rejected
    because font corner strokes wouldn't align with geometric line
    strokes in adjacent cells, recreating the seam class this slice
    exists to kill.

D2. GTK adapter: behavior-identical, tests unmoved

crates/roost-linux/src/sprite.rs shrinks to an adapter that keeps the
exact public signature draw_cell_sprite(cr, x, y, w, h, fg, cp) -> bool
and renders SpritePrimitives:

  • Rectcr.rectangle + fill per rect (same per-rect fill
    granularity as today), with the same save/Antialias::None/
    restore bracket when antialias == false, and set_source_rgba
    for alpha ≠ 1.0.
  • CornerArc → the identical move_to/line_to/curve_to/stroke
    sequence rebuilt from the shared arc_path description (S=0.25
    control points, butt cap, width t) — cairo calls unchanged.
  • Diagonal → identical move_to/line_to/stroke.

The entire in-module pixel test suite stays in roost-linux
unchanged
— it renders through the adapter into Cairo surfaces, so
it is the cheap local oracle that the move didn't change pixels.
Additionally (panel finding): a full-surface golden-hash fixture
generated from the pre-refactor renderer (i.e., before C2 rewrites
sprite.rs) — hashes the rendered ARGB32 surface for every
codepoint 0x2500–0x259F at 2–3 cell sizes, committed as a fixture with
an #[ignore]d regenerator, asserted by a new
crates/roost-linux/tests/sprite_golden_test.rs. This puts the
byte-identical bar inside cargo test -p roost-linux where C2 fails
fast, instead of only at the ship-time shed oracle. The shed
screenshot oracle (D6) remains the end-to-end bar. Neither
terminal_view.rs call site changes.

D3. iced sprite draw: quads in the cell loop

In TerminalWidget::draw, inside the non-blank guard
(!cell.text.is_empty() && cell.text != " ", terminal_widget.rs:679)
and before fill_text: single-codepoint check (GTK's chars.next()
pattern), then sprite_geometry(cp, cell_w, cell_h); if Some, emit
each primitive via tessellaterenderer.fill_quad, color =
color(cell.foreground) with per-primitive alpha; then
fill_text_calls += 1; continue to skip fill_text. (Placement inside
the guard matches GTK — blank cells never reach the codepoint test.)

  • Edge snapping (the iced seam story — panel finding): iced has no
    per-quad AA-off; fill_quad antialiases fractional edges, and a
    fractional shared edge between two quads composites to a hairline
    (partial coverage twice ≠ full coverage). The adapter therefore
    computes each rect's absolute edges (cell origin + cell-relative
    coords) in f64 and rounds each edge to an integer before building
    the Rectangle. Cell strides are integer (floored metrics), so
    adjacent cells' shared edges round identically — tiling is exact
    regardless of a fractional widget origin, and snapped hard edges
    make stamp overlap safe (D1). Quads keep snap: false (the
    existing helper's setting); our own rounding is the mechanism.
  • f64→f32: the cast to iced's Rectangle<f32> happens after edge
    rounding; integer-valued edges at terminal magnitudes are exactly
    representable in f32, so the shared-edge property survives the cast.
  • Cells drawn: same predicate as GTK — exactly one char, in range;
    multi-codepoint graphemes fall through to fill_text.
  • Cursor pass — resolved during panel review (no C3 audit needed):
    iced's cursor is a plain quad overlay — Block is a translucent
    a: 0.55 quad, BlockHollow a border quad, Bar/Underline small quads
    (terminal_widget.rs:736-779, verified) — it never redraws the
    covered glyph, so no sprite intercept is needed there. Pre-existing
    divergence, recorded not fixed: GTK's block cursor redraws the
    covered glyph/sprite in canvas_bg over an opaque fill
    (terminal_view.rs:2560-2578); iced dims the cell through a
    translucent overlay instead. Not introduced by E5.
  • Draw-order divergence, accepted for v1 (panel finding): GTK
    paints all backgrounds, then all glyphs — a diagonal's deliberate
    overshoot into the neighbor cell survives. iced interleaves each
    cell's bg quad and glyph, so when the next cell has
    explicit_background, its bg quad can shave the previous cell's
    diagonal overshoot (≤ ~t/2 + 0.5 px) at the boundary. Accepted as a
    documented iced divergence — the common case (default background →
    no bg quad) is unaffected; the §8 iced scene includes a ╲╲╲ run over
    a colored background so the effect is observable and reviewable.
    Restructuring iced's draw into two passes is named future work.

D4. Counter semantics: iced counts sprites into fill_text_calls

Pinned per the brief's option 1: a sprite draw replaces a glyph draw,
so iced increments fill_text_calls by 1 per sprite-handled cell —
same as GTK (terminal_view.rs:2138). The field becomes
apples-to-apples across UIs; consequently the caveats at
roost-linux/src/perf.rs:17-20, terminal_view.rs:2060-2062, and
roost-iced/src/perf.rs:22-25 are rewritten to say sprite draws are
counted as glyph draws on both UIs. The stale claims at
docs/reference/cli.md:210 ("GTK reports all zeros") and in
docs/reference/ipc.md ("The GTK UI answers with the same shape, all
counters zero", ~line 645 — locate by grepping the phrase, not by line
number) are both fixed in the same docs pass. GTK's uncounted
cursor-sprite draw is pre-existing and equally uncounted on both UIs —
left alone, noted in §9. The counter change is e2e-asserted (panel
finding):
C4's test, after feeding the sprite scene and before any
screenshot (screenshot re-renders inflate counters), resets and reads
app.render_stats and asserts fill_text_calls > 0 on a redraw of the
sprite-only scene — pinning that sprite cells register as glyph draws.

D5. Warm-up commit (C0): the 4-place wiring

tools/roosttest/test_osc_pipeline.py appended to Makefile:72
ICED_E2E_TESTS and the three ci.yml lists (X11/Wayland/macOS),
following the append-after-test_sidebar_resize.py convention; its
docstring line 26 updated to name all three targets. Pre-verified by
running the module against iced locally before commit. First commit on
the branch — ships even if E5 stalls.

D6. Verification bars (what proves what)

  • GTK: byte-identical screenshot pre/post on the same shed
    host/session — the 018 oracle extended with a sprite-heavy scene
    (see §8). This is the no-behavior-change gate for the D2 refactor.
  • iced: seam-free is asserted two ways — (a) a new committed pixel
    e2e tools/roosttest/test_sprite_pixels.py, iced-target-only
    (skip otherwise, walking-skeleton precedent), feeding a block/box
    scene via tab.feed_pty_bytes and asserting via pngtool:
    (i) full-block runs have no background-colored seam column/row at
    cell boundaries, AND (ii) partial-block internal edges (panel
    finding — where AA softening would actually appear): a ▀ row and a
    ▄ row transition from fg to bg in a hard 1-px step at the half-cell
    boundary, ▌/▐ likewise on the vertical axis — no intermediate-color
    band; (iii) the D4 fill_text_calls > 0 counter assertion (read
    before any screenshot). (b) human-reviewable screenshot artifacts of
    TUI-style chrome at two font sizes into the plan's artifacts folder.
    Cross-UI pixel parity is NOT a gate (No CI gate for GTK↔Iced visual parity (capture tooling is human-reviewed only) #284, standing decision).
  • iced unit tests: geometry-level (dispatch ranges, rect coordinates
    for known glyphs, tessellation stamp-count/overlap/bbox invariants)
    live in roost-ui-model where no renderer is needed.

4. Deviations from the brief / non-goals

  • Parity-inventory row: brief said "add the missing row"; the row
    already exists (iced-parity-inventory.md:151, added 2026-08-06).
    Deviation: update it to closed with evidence instead of adding.
  • Out of scope (per brief): E6 IME, E3c GTK draw phase, E8/E9, any
    Swift change, release-profile CI for iced, cross-UI pixel-parity CI
    gate (No CI gate for GTK↔Iced visual parity (capture tooling is human-reviewed only) #284), GTK's uncounted cursor-sprite draw, un-flooring cell
    metrics.
  • Swift Sprite.swift stays the macOS reference implementation —
    untouched; parity is behavioral (same codepoint set, same visual
    intent), enforced by the shared-geometry port matching the same Zig
    original.

5. Work breakdown — gated commits

One repo, one PR. Gates run per commit via /flows:gated-commit.

  • C0 (sonnet) — warm-up: wire test_osc_pipeline.py into iced e2e.
    4 list edits + docstring touch. Gate: pytest tools/roosttest/test_osc_pipeline.py --roost-target iced green
    locally (mac) before commit; make check-iced untouched-code sanity.
  • C1 (opus) — roost_ui_model::sprite: the geometry port + the
    golden fixture.

    New module: types + sprite_geometry + arc_path + tessellate,
    all pixel math moved verbatim from roost-linux/src/sprite.rs, plus
    pure unit tests (dispatch ranges incl. the non-sprite rejection set;
    full-range smoke — every in-range cp emits ≥1 primitive; rect
    coordinates for representative glyphs at known cell sizes mirroring
    the existing Cairo assertions; tessellation: stamp-count bounds,
    consecutive-stamp overlap/abutment, bbox containment with the
    diagonal-overshoot exception). Also in C1: the GTK golden-hash
    fixture
    crates/roost-linux/tests/sprite_golden_test.rs +
    fixture file hashing the ARGB32 render of every cp 0x2500–0x259F at
    2–3 cell sizes through the unchanged renderer, with an
    #[ignore]d regenerator (the fixture must be generated before C2
    rewrites sprite.rs). No consumer change yet. Gate:
    cargo test -p roost-ui-model, cargo test -p roost-linux
    (golden test green against unchanged code),
    cargo clippy -p roost-ui-model -- -D warnings, workspace build.
  • C2 (opus) — GTK adapter over shared geometry.
    roost-linux/src/sprite.rs becomes the D2 adapter; public API and
    the entire in-module pixel test suite unchanged; terminal_view.rs
    untouched. Gate: cargo test -p roost-linux — the unchanged pixel
    suite plus the C1 golden-hash fixture are the byte-identical
    fast-fail; cargo clippy -p roost-linux -- -D warnings.
  • C3 (opus) — iced sprite draw + counters + caveat docs.
    D3 intercept in terminal_widget.rs (snapped edges, inside the
    non-blank guard), D4 counter + the three perf-caveat rewrites +
    cli.md:210 fix + ipc.md "all counters zero" fix. Gate:
    make check-iced, cargo test -p roost-iced, manual visual: run
    iced on mac, feed a box-drawing scene including an early
    arc-glyph eyeball check
    (the tessellation prototype gate — if
    stamped arcs look wrong, fix before C4 builds assertions on them)
    and a ╲╲╲-over-colored-bg run; screenshot artifact; record the
    observed frame feel / any quad-count note for the PR (arcs go from
    1 stroke to ~10–30 quads per cell — no budget, but recorded).
  • C4 (sonnet) — iced sprite pixel e2e.
    tools/roosttest/test_sprite_pixels.py (iced-only skip guard):
    full-block seam scan + partial-block internal-edge assertions + the
    app.render_stats counter assertion (D6), wired into
    ICED_E2E_TESTS + the three ci.yml lists (same 4-place edit as
    C0). Gate: module green locally against iced (mac); non-iced skip
    behavior verified.
  • C5 (sonnet) — docs closeout.
    Roadmap: E5 entry marked complete with shipped shape; M6 entry-gate
    line updated (release-profile CI remains the last gate);
    parity-inventory row :151 closed with evidence. Gate: docs-only;
    link/reference sanity.

Order: C0 → C1 → C2 → C3 → C4 → C5. C2 and C3 both depend only on C1
but land sequentially (C2 first — the byte-identical bar is the
riskiest, fail fast).

6. File map (indicative)

crates/roost-ui-model/src/sprite.rs          NEW  geometry + tessellation + unit tests
crates/roost-ui-model/src/lib.rs             +1 module line
crates/roost-linux/src/sprite.rs             REWRITE to adapter (tests stay)
crates/roost-linux/tests/sprite_golden_test.rs  NEW  full-range golden-hash oracle (+ fixture file)
crates/roost-linux/src/perf.rs               caveat rewrite
crates/roost-linux/src/terminal_view.rs      caveat comment rewrite only (:2060-2062)
crates/roost-iced/src/terminal_widget.rs     sprite intercept in draw (+cursor audit)
crates/roost-iced/src/perf.rs                caveat rewrite
docs/reference/cli.md                        :210 stale fix
docs/reference/ipc.md                        staleness check
docs/development/iced-migration-roadmap.md   E5 complete; M6 gate line
docs/development/iced-parity-inventory.md    row :151 → closed
tools/roosttest/test_osc_pipeline.py         docstring line 26
tools/roosttest/test_sprite_pixels.py        NEW  iced-only pixel e2e
Makefile                                     ICED_E2E_TESTS × 2 entries (C0, C4)
.github/workflows/ci.yml                     3 lists × 2 entries (C0, C4)

7. Acceptance criteria

  1. Shared geometry: U+2500–U+259F geometry lives in
    roost_ui_model::sprite; GTK and iced both consume it; no toolkit
    type crosses the module boundary. (D1/C1)
  2. GTK unchanged: existing sprite pixel tests green unmodified;
    shed screenshot of the sprite oracle scene byte-identical pre/post.
    (D2/C2, §8)
  3. iced draws sprites: box-drawing/block cells render as quads, not
    font glyphs; test_sprite_pixels.py green in all three iced CI
    lanes; seam-free screenshot artifacts at 2 font sizes archived.
    (D3/C3/C4)
  4. Counters consistent: iced counts sprite cells into
    fill_text_calls; all three caveat docs rewritten; cli.md:210
    fixed. (D4/C3)
  5. Docs/inventory: parity row :151 closed with evidence; roadmap
    E5 complete; M6 entry-gate line leaves release-profile CI as the
    only remaining gate. (C5)
  6. Warm-up shipped: test_osc_pipeline.py in the 4 lists, green in
    iced lanes. (C0)

8. Verification plan (beyond automated tests)

  • GTK byte-identical oracle (shed, before ship): df -h / first
    (disk-fill memory). Build pre (main) and post (branch) roost
    binaries; for each: launch under ROOST_TEST_MODE=1 + Xvfb, feed one
    deterministic sprite scene via tab.feed_pty_bytes
    ESC[?25l + ESC[2J ESC[H + rows exercising: adjacent light/heavy/
    double lines + corners/tees/crosses (tiling adjacency), the eighth
    blocks, shades, quadrants, rounded corners, diagonals, one ESC[7m
    inverse row, one bold row, and a ╲╲╲ run over a colored (explicit
    bg) region; no trailing scroll — then roostctl screenshot,
    byte/MD5-compare. Also run make e2e-gtk in the shed. Artifacts:
    both PNGs + md5s + the feed script.
  • iced visual: same scene on mac via tools/screenshot/launch.sh iced + roostctl screenshot at default and one larger font size;
    before/after shots archived; eyeball for seams + pngtool seam scan
    (the committed e2e automates the block-run seam check; the manual
    pass judges arcs/diagonals/shade quality, which have no pixel gate).
  • Mac primary loop per commit: gates in §5; make test-rust before
    ship.
  • Automated coverage statement: C1's geometry tests + C2's unchanged
    Cairo pixel tests + C4's e2e cover dispatch, coordinates, GTK pixels,
    and iced seams; the only things left to manual verification are the
    GTK end-to-end byte-oracle and the subjective quality of tessellated
    arcs/diagonals in iced.

9. Risks / open items / future work

  • f64 drift breaking byte-identical GTK — mitigated by verbatim
    expression preservation (D1), the unmoved Cairo pixel suite + the
    full-range golden-hash fixture (C1/C2 gates — the automated
    fast-fail), and the shed oracle (§8) end-to-end. If any fails, diff
    crops via pngtool / rerun the golden test per-codepoint to
    localize the glyph and fix the expression — do not relax the bar.
  • iced AA has no off-switch — the seam mechanism on iced is edge
    snapping (D3), not an AA flag; SpriteGeometry.antialias is
    GTK-only by documented contract. Residual risk: partial-block
    internal edges — covered by C4's dedicated internal-edge
    assertions.
  • Tessellated arc/diagonal quality in iced (staircase vs cairo AA)
    — accepted for v1; snapping eliminates overlap darkening (D1);
    C3's early arc eyeball check catches gross problems before C4;
    screenshot artifacts make the rest reviewable; a future AA approach
    (edge-alpha stamps or an image path) is future work, not this
    slice. Note in the roadmap E5 closing entry.
  • iced draw-order overshoot shaving (D3, panel finding) — a
    neighbor cell's explicit-bg quad can shave diagonal overshoot at
    the boundary (≤ ~t/2 + 0.5 px); accepted v1 divergence, observable
    in the §8 scene; two-pass draw restructure named as future work.
  • Quad-count growth on arc/diagonal cells — 1 cairo stroke
    becomes stamped quads on iced. C3 measured: arcs 33 quads/cell at
    9×19 (28 at 15×32 — count falls as cells grow, thickness outpaces
    path length), ╳ worst case 92, rect-family glyphs 1–4. No
    perceptible frame-feel change; no budget set; fill_text_calls
    stays per-cell by pinned semantics, so it does not measure this.
  • Shade blending differs cross-UI (C3 observation, accepted):
    wgpu composites ░▒▓ alpha in linear space, cairo in sRGB — iced's
    shades render lighter (measured 128/174/207 vs cairo ~80/132/174 on
    the same scene). Uniform per cell, alphas exact; not a defect and
    not gated (No CI gate for GTK↔Iced visual parity (capture tooling is human-reviewed only) #284), named in the PR.
  • wgpu vs tiny-skia rendering variance — both Linux iced lanes run
    in CI; the e2e seam assertion runs in both, catching
    renderer-specific quad behavior.
  • Pre-existing, untouched: GTK cursor-sprite draw uncounted in
    fill_text_calls; iced per-tab draw counters don't exist (unit-test
    limitation, roost-iced/src/perf.rs:26-28); cursor-over-sprite
    divergence
    — GTK redraws the covered glyph in canvas_bg, iced
    dims through a translucent overlay (D3, recorded not fixed).
  • Future work: macOS Swift adoption of the shared geometry is an M6
    question, not this slice; iced release-profile CI remains the last
    M6 entry gate; iced two-pass draw restructure (bg pass, then glyph
    pass) if the overshoot shaving ever matters in practice.

§ Verified (beyond the automated tests)

All automated gates ran green per commit (documented in each commit
message); make test-rust green on the Mac after C5 (20 suites). What
was verified beyond them, with artifacts in
~/.claude/plans/roost/020-iced-sprites/:

  • GTK byte-identical oracle (shed, real Linux): PASS. Same shed
    host/session, same deterministic sprite scene (DECTCEM off, clear +
    home, all glyph families incl. ╲-over-explicit-bg, inverse + bold
    rows) fed via tab.feed_pty_bytes into a hermetic Xvfb instance,
    captured via roostctl screenshot --scale 1:
    main (48087d4) and the branch (76cbd57) produce byte-identical
    PNGs
    — md5 9d88a549d3455bb8fd85fcdb55dcf3d3 both.
    Artifacts: gtk-oracle-pre-main.png, gtk-oracle-post-branch.png,
    run-oracle.sh, setup-and-feed.py.
    Method note for future oracles: the offscreen GTK screenshot does
    NOT reflect a background tab.open+tab.focus switch under Xvfb
    (three captures byte-identically showed the previously active tab
    while IPC state said otherwise) — feed the already-active tab,
    exactly as the 018 oracle did.
  • iced visual (mac, C3): hermetic run, same scene, screenshots at
    12 pt and 20 pt plus 3× zooms (c3-iced-sprites-*.png,
    c3-iced-zoom-*.png, feed script c3-sprite-scene-feed.py).
    Pixel-scan evidence: 60-cell line runs are single contiguous
    exact-fg runs; blocks tile seamlessly incl. the █→▀ boundary; arcs
    read as clean rounded corners; no overlap darkening. Live counter
    check over IPC: fill_text_calls 196 → 1003 on the sprite scene.
  • e2e suites: test_osc_pipeline.py (C0) and
    test_sprite_pixels.py (C4) each verified green against iced
    locally before their commits; the full GTK e2e suite (CI-parity:
    --roost-fresh, ROOST_TEST_MODE=1, Xvfb, shed Linux binaries)
    ran post-C5: 171 passed, 11 skippedtest_sprite_pixels.py
    correctly skips on the gtk target.
  • Known-unexercised paths: the three iced CI lanes
    (X11/Wayland × wgpu/tiny-skia matrix) run only in CI — locally only
    macOS was exercised; cairo-version variance on the 7 stroked glyphs
    is deliberately outside the golden fixture (covered by property
    tests + this oracle); the D3 overshoot-shave divergence was not
    observable at current stroke widths (recorded, unasserted); Swift
    Mac rendering untouched and untested beyond make test-rust
    workspace neighbors.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d

Summary by CodeRabbit

  • Improvements

    • Improved rendering of Unicode block and box-drawing characters across GTK and Iced interfaces.
    • Added pixel-aligned, seamless sprite rendering for cleaner edges, joins, curves, and diagonals.
    • Preserved font-based rendering for unsupported or multi-character cells.
    • Render statistics now consistently count sprite-rendered cells across both interfaces.
  • Documentation

    • Updated rendering metrics, parity details, and migration roadmap documentation.
  • Tests

    • Added automated pixel-regression coverage for sprite rendering across supported platforms.

charliek and others added 6 commits August 6, 2026 21:02
Carried unwired since PR #306: the module was verified green against
iced on mac + linux/wgpu + linux/tiny-skia but never added to the
enumerated iced lists. Adds it to Makefile ICED_E2E_TESTS and the
three ci.yml iced lanes (X11, Wayland, macOS), after
test_sidebar_resize.py per list convention, and updates the module
docstring that claimed only two targets run it in CI.

Pre-verified: 10 passed, 1 documented skip (#145) against iced
locally; ci.yml parses. Review: codex usage-limited, cursor CLI
returned empty output twice — self-review (4-site grep, lane
ROOST_TEST_MODE check, ordering parity).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
Plan 020 C1 (engine track E5). Ports the U+2500-U+259F geometric sprite
math from roost-linux's cairo renderer to a pure-data
roost_ui_model::sprite module: sprite_geometry() emits cell-relative
primitives (Rect+alpha, CornerArc, Diagonal), arc_path() is the single
source for the rounded-corner three-segment path, and tessellate()
flattens stroked primitives into stamped rects for quad-only renderers.
All f64 expressions and operation order preserved verbatim; verified
byte-identical against the cairo renderer for all 160 codepoints at 10
cell sizes via a throwaway cross-check harness (removed after use).

Also adds the pre-refactor golden-hash oracle: an in-module test in
roost-linux hashes the ARGB32 render of 153 codepoints x 3 cell sizes
(FNV-1a, no new deps) against a committed fixture generated from the
unchanged renderer, so the upcoming adapter rewrite fails fast in cargo
test if any pixel drifts. The 7 stroked glyphs (arcs/diagonals) are
excluded: cairo AA stroke rasterization varies across cairo versions
and would fail CI spuriously; they stay covered by the property pixel
tests and the shed screenshot oracle.

Review: codex usage-limited, cursor CLI empty output — self-review
(D1 API conformance, no toolkit deps, machine-verified byte identity);
simplify pass applied (render-helper reuse, test dedup, pre-sizing).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
Plan 020 C2 (engine track E5). crates/roost-linux/src/sprite.rs drops
from 2,715 to 569 lines: the pixel math now comes from
roost_ui_model::sprite::sprite_geometry and the module only turns
primitives into cairo calls (rect fills with the block-layer AA-off
bracket, arc_path rebuilt with the identical move/line/curve/stroke
sequence, diagonals likewise). Public signature unchanged; both
terminal_view.rs call sites untouched.

Behavior bar: the entire pre-refactor pixel-assertion suite and the
golden-hash fixture pass unchanged (the diff contains zero edits inside
either test module, fixture not regenerated). The fixture-excluded
stroked glyphs (U+256D-2573) were probe-verified byte-identical against
HEAD's renderer at 5 cell sizes before the probe was removed.

Review: codex usage-limited, cursor CLI empty output — self-review
(adapter read end-to-end against the old cairo call patterns; simplify
skipped, net-deletion diff).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
Plan 020 C3 (engine track E5). The terminal widget's draw pass now
intercepts single-codepoint U+2500-U+259F cells inside the non-blank
guard and renders roost_ui_model::sprite geometry as quads instead of
font glyphs — closing the sprite-parity gap against both shipped UIs
(TUI chrome no longer shows hairline seams).

iced has no per-quad AA switch, so the seam mechanism is integer edge
snapping: each tessellated rect's absolute edges round to integers
before the f32 cast (adjacent cells share integer strides, so shared
edges round identically). Stroked glyphs (arcs/diagonals) come from the
shared tessellator as stamped quads; snapped hard edges make stamp
overlap composite-safe.

Sprite draws count into fill_text_calls (a sprite replaces a glyph
draw), making the field apples-to-apples across UIs — the cross-UI
caveats in both perf modules and the terminal_view comment are updated
accordingly, and stale "GTK reports all zeros" claims in cli.md/ipc.md
are fixed.

Verified live on macOS: hermetic roost-iced run, deterministic sprite
scene via tab.feed_pty_bytes, screenshots at 12pt+20pt (plan-020
artifacts) — line runs scan as single contiguous exact-fg pixel runs,
blocks tile seamlessly, arcs render as clean rounded corners;
fill_text_calls confirmed counting over app.render_stats. Known
cross-UI difference recorded: wgpu blends shade alpha in linear space
(lighter than cairo's sRGB blend); no CI parity gate per #284.

Review: codex usage-limited, cursor CLI empty output — self-review
(intercept + draw_sprite read against plan D3; independent screenshot
eyeball). Simplify skipped (small focused diff).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
Plan 020 C4 (engine track E5). New iced-only module
test_sprite_pixels.py pins the sprite path end-to-end: (1) counter
semantics — app.render_stats reports fill_text_calls > 0 for a redraw
of a sprite-only scene, asserted before any screenshot since
screenshots re-render and inflate the counters; (2) pixel guards —
full-block runs show no background seam column across 40 cells,
partial blocks (upper/lower/left half) step fg->bg in a hard edge at
the half-cell boundary with no intermediate AA band, a full-block row
tiles seamlessly into the half-block row below, and light/double box
lines run unbroken. Colors come from tab.dump_resolved so the
classification tracks the live theme.

Wired into the four iced e2e lists (Makefile ICED_E2E_TESTS + the
three ci.yml lanes) after test_osc_pipeline.py. Verified green twice
against iced on macOS (fresh harness-owned instance); non-iced targets
skip via the walking-skeleton fixture pattern.

Review: codex usage-limited, cursor CLI empty output — self-review;
simplify pass applied (dead pngtool import + unused constant).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
…y row

Plan 020 C5. Roadmap E5 entry rewritten as complete (shared
roost_ui_model::sprite geometry, GTK thin adapter with three-way
behavior pinning, iced integer-edge-snapped quads, counter now
apples-to-apples, recorded cross-UI divergences); scoreboard's
not-perf line extended; M4 audit expected-set paragraph updated (the
sprite row closed within the same track); M6 entry gate now leaves
release-profile CI + the inventory audit as the remaining gates.
Parity-inventory box-drawing row P1 -> closed with the committed e2e
guards and golden fixture as acceptance evidence.

Review: docs-only commit; table pipe counts and reference links
verified.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8485c063-94ef-4a9d-9e52-03f76da81cd1

📥 Commits

Reviewing files that changed from the base of the PR and between 76cbd57 and c433576.

📒 Files selected for processing (4)
  • crates/roost-linux/src/sprite.rs
  • crates/roost-ui-model/src/sprite.rs
  • tools/roosttest/test_osc_pipeline.py
  • tools/roosttest/test_sprite_pixels.py
📝 Walkthrough

Walkthrough

The change adds shared Unicode sprite geometry, integrates sprite rendering into GTK and Iced, updates render-counter semantics, adds golden and pixel regression tests, and enables the tests in Iced CI lanes.

Changes

Sprite geometry and renderer integration

Layer / File(s) Summary
Shared sprite geometry
crates/roost-ui-model/src/lib.rs, crates/roost-ui-model/src/sprite.rs
The UI model exposes geometry for block and box-drawing glyphs. It supports rectangles, arcs, diagonals, antialiasing metadata, tessellation, and geometry tests.
GTK sprite adapter
crates/roost-linux/src/sprite.rs, crates/roost-linux/tests/fixtures/sprite_golden.txt
GTK uses the shared geometry and applies primitive-specific antialiasing. Golden raster tests compare supported glyphs at three cell sizes.
Iced sprite rendering
crates/roost-iced/src/terminal_widget.rs
Iced replaces eligible single-codepoint glyph draws with integer-snapped filled quads. Unsupported and multi-codepoint cells retain glyph rendering.
Sprite pixel validation and CI coverage
tools/roosttest/test_sprite_pixels.py, Makefile, .github/workflows/ci.yml, tools/roosttest/test_osc_pipeline.py
The Iced E2E suite checks counters, seams, edges, half-blocks, and box lines. The test runs in Linux X11, Linux Wayland, and macOS lanes.
Metrics and parity documentation
crates/roost-iced/src/perf.rs, crates/roost-linux/src/perf.rs, crates/roost-linux/src/terminal_view.rs, docs/development/*, docs/reference/*
Documentation defines sprite-inclusive fill_text_calls, real GTK render counters, shared geometry, completed parity work, and known renderer differences.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TerminalWidget
  participant SpriteGeometry
  participant Tessellator
  participant Renderer
  participant SpritePixelTest
  TerminalWidget->>SpriteGeometry: Request cell geometry
  SpriteGeometry-->>TerminalWidget: Return sprite primitives
  TerminalWidget->>Tessellator: Tessellate primitives
  Tessellator-->>TerminalWidget: Return rectangle stamps
  TerminalWidget->>Renderer: Submit snapped quads and increment fill_text_calls
  SpritePixelTest->>Renderer: Capture render stats and pixels
  Renderer-->>SpritePixelTest: Return counters and screenshot
Loading

Possibly related PRs

  • charliek/roost#306: Adds the Iced rendering and fill_text_calls instrumentation extended by this change.
  • charliek/roost#307: Shares the render-stat instrumentation updated by this change.
  • charliek/roost#296: Updates OSC pipeline synchronization and CI coverage referenced by the test changes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the E5 sprite parity work in Iced and the shared geometry crate, which matches the main changes.
Docstring Coverage ✅ Passed Docstring coverage is 92.54% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/plan-020-iced-sprites

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
crates/roost-ui-model/src/sprite.rs (2)

628-651: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a named-edge struct for pick_junction's four f64 arguments.

pick_junction takes four bare f64 edges in a row (heavy_edge, double_edge, light_edge_far, light_edge_near). The four call sites in crates/roost-ui-model/src/sprite.rs:495-534 each pass a different permutation. A swapped pair compiles silently and only shows up as a shifted stroke in the golden fixture.

A small JunctionEdges { heavy, double, light_far, light_near } struct would make each call site self-documenting and drop the #[allow(clippy::too_many_arguments)]. The current tests do pin the behaviour, so this is a readability call, not a correctness one.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/roost-ui-model/src/sprite.rs` around lines 628 - 651, Introduce a
named JunctionEdges struct with heavy, double, light_far, and light_near fields,
update pick_junction to accept it instead of the four positional f64 arguments,
and access the fields within the function. Update all four pick_junction call
sites to construct JunctionEdges with named fields, then remove the
no-longer-needed too_many_arguments allowance.

710-805: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

h_dash and v_dash are the same algorithm with the axes swapped.

crates/roost-ui-model/src/sprite.rs:710-756 and crates/roost-ui-model/src/sprite.rs:759-805 duplicate the thickness derivation, the narrow-cell fallback, the gap clamp, and the remainder distribution. Only the axis assignment and the Lines4 fallback field differ. A bug fix to one will have to be mirrored by hand into the other.

One option: extract the segment layout into a helper that returns Vec<(f64, f64)> offset/length pairs along the major axis, then let each wrapper place the rects.

♻️ Sketch of the shared helper
/// `count` segment (offset, length) pairs along an axis of length
/// `extent`, plus the cross-axis band offset.
fn dash_runs(extent: f64, thick: f64, count: i32, style: LineStyle) -> Option<Vec<(f64, f64)>> {
    let mut desired_gap = thick;
    if matches!(style, LineStyle::Light) && desired_gap < 4.0 {
        desired_gap = 4.0;
    }
    let ei = extent as i32;
    if ei < count * 2 {
        return None; // caller draws the solid-line fallback
    }
    let gap = (desired_gap as i32).min(ei / (2 * count));
    let total_dash = ei - gap * count;
    let (dash, mut extra) = (total_dash / count, total_dash % count);
    let mut pos = (gap / 2) as f64;
    let mut runs = Vec::with_capacity(count as usize);
    for _ in 0..count {
        let mut d = dash;
        if extra > 0 {
            d += 1;
            extra -= 1;
        }
        runs.push((pos, d as f64));
        pos += (d + gap) as f64;
    }
    Some(runs)
}

The current code mirrors the Zig original arm for arm, so keeping the two functions is a defensible choice as well. Your call.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/roost-ui-model/src/sprite.rs` around lines 710 - 805, Deduplicate the
shared dash-layout algorithm used by h_dash and v_dash by extracting a helper
that computes major-axis offset/length runs, including gap sizing, narrow-cell
fallback signaling, and remainder distribution. Update both wrappers to derive
thickness, invoke the helper, preserve their respective horizontal/vertical
Lines4 fallback fields, and place rectangles using the returned runs.
crates/roost-iced/src/terminal_widget.rs (1)

686-699: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Sprite geometry is rebuilt and re-tessellated for every sprite cell, every frame.

sprite_geometry allocates a Vec<SpritePrimitive> per call. draw_sprite then calls tessellate per primitive, and tessellate allocates a Vec<SpritePoint> plus a Vec<SpriteRect> for arcs and diagonals (crates/roost-ui-model/src/sprite.rs:885-919). A screen full of TUI box-drawing chrome runs that whole chain once per cell per redraw.

The result is fully determined by (codepoint, cell_width, cell_height). Both dimensions are constant for the frame, and the codepoint domain is 160 values. A cache keyed on the codepoint and invalidated when metrics changes would remove the allocation churn:

// Rebuilt only when cell metrics change.
struct SpriteCache {
    cell: (u32, u32),                       // cell_width/height bits
    geometry: Vec<Option<SpriteGeometry>>,  // indexed by cp - 0x2500
}

I have no profile data from the provided context, so measure before you commit to this. record_draw at crates/roost-iced/src/terminal_widget.rs:801 already gives you the draw-phase timing to compare against.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/roost-iced/src/terminal_widget.rs` around lines 686 - 699, Profile the
sprite rendering path before changing it, using record_draw and comparing
draw-phase timing with and without caching. If the allocation and tessellation
cost is significant, update the sprite handling around sprite_geometry and
draw_sprite to cache geometry for the 160 codepoints in the supported range,
keyed by cell_width and cell_height, rebuilding only when those metrics change
while preserving existing rendering behavior.
crates/roost-linux/src/sprite.rs (1)

43-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cairo context state is mutated without consistent scoping in crates/roost-linux/src/sprite.rs. Both sites handle the shared cr state loosely: one branch saves and restores without checking that the save succeeded, and the other branch mutates stroke state with no save at all. A single save()/restore() wrapper around the draw_primitives call in draw_cell_sprite fixes both and covers the source color as well.

  • crates/roost-linux/src/sprite.rs#L43-L46: move the save()/restore() pair to wrap both branches of draw_cell_sprite, and only call restore() when save() returned Ok. Set Antialias::None inside the saved scope for the !geometry.antialias case.
  • crates/roost-linux/src/sprite.rs#L64-L102: with the wrapper in place at the call site, drop the concern here — set_line_cap and set_line_width in the CornerArc and Diagonal arms no longer escape onto the caller's context.
🛡️ Proposed fix in `draw_cell_sprite`
     let Some(geometry) = sprite_geometry(cp, w, h) else {
         return false;
     };
-    if geometry.antialias {
-        draw_primitives(cr, x, y, fg, &geometry);
-    } else {
+    // One saved scope for every sprite: `draw_primitives` sets the
+    // source color, and the stroked arms set line cap and width. None
+    // of it may leak onto the caller's context.
+    let saved = cr.save().is_ok();
+    if !geometry.antialias {
         // Cairo's default antialiasing softens edges by a fraction of
         // a pixel even on integer-aligned coordinates under some
         // surface transforms, which reopens the seam between adjacent
         // block cells. Curves and diagonals keep the default AA so
         // they don't go jaggy.
-        cr.save().ok();
         cr.set_antialias(cairo::Antialias::None);
-        draw_primitives(cr, x, y, fg, &geometry);
-        cr.restore().ok();
+    }
+    draw_primitives(cr, x, y, fg, &geometry);
+    if saved {
+        cr.restore().ok();
     }
     true

This adds a save/restore pair to the box-drawing path that did not have one. If the golden hashes shift as a result, that would itself be worth knowing — they should not, since save/restore does not change rasterization.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/roost-linux/src/sprite.rs` around lines 43 - 46, Update
draw_cell_sprite in crates/roost-linux/src/sprite.rs at lines 43-46 to wrap both
draw_primitives branches in one save/restore scope, calling restore only when
save succeeds and setting Antialias::None inside that scope for the
!geometry.antialias case. The CornerArc and Diagonal arms at lines 64-102
require no direct change; the wrapper prevents their stroke-state mutations from
escaping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/roost-linux/src/sprite.rs`:
- Around line 507-511: Update surface_hash to hash only the w * 4 pixel bytes
from each row of render’s returned buffer, excluding stride padding while
preserving row order and height. Regenerate the affected 9x19 fixture hashes
after changing the fingerprinting logic.

---

Nitpick comments:
In `@crates/roost-iced/src/terminal_widget.rs`:
- Around line 686-699: Profile the sprite rendering path before changing it,
using record_draw and comparing draw-phase timing with and without caching. If
the allocation and tessellation cost is significant, update the sprite handling
around sprite_geometry and draw_sprite to cache geometry for the 160 codepoints
in the supported range, keyed by cell_width and cell_height, rebuilding only
when those metrics change while preserving existing rendering behavior.

In `@crates/roost-linux/src/sprite.rs`:
- Around line 43-46: Update draw_cell_sprite in crates/roost-linux/src/sprite.rs
at lines 43-46 to wrap both draw_primitives branches in one save/restore scope,
calling restore only when save succeeds and setting Antialias::None inside that
scope for the !geometry.antialias case. The CornerArc and Diagonal arms at lines
64-102 require no direct change; the wrapper prevents their stroke-state
mutations from escaping.

In `@crates/roost-ui-model/src/sprite.rs`:
- Around line 628-651: Introduce a named JunctionEdges struct with heavy,
double, light_far, and light_near fields, update pick_junction to accept it
instead of the four positional f64 arguments, and access the fields within the
function. Update all four pick_junction call sites to construct JunctionEdges
with named fields, then remove the no-longer-needed too_many_arguments
allowance.
- Around line 710-805: Deduplicate the shared dash-layout algorithm used by
h_dash and v_dash by extracting a helper that computes major-axis offset/length
runs, including gap sizing, narrow-cell fallback signaling, and remainder
distribution. Update both wrappers to derive thickness, invoke the helper,
preserve their respective horizontal/vertical Lines4 fallback fields, and place
rectangles using the returned runs.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bfc47ebd-d4cd-46e1-916d-229440aa9a32

📥 Commits

Reviewing files that changed from the base of the PR and between 48087d4 and 76cbd57.

📒 Files selected for processing (16)
  • .github/workflows/ci.yml
  • Makefile
  • crates/roost-iced/src/perf.rs
  • crates/roost-iced/src/terminal_widget.rs
  • crates/roost-linux/src/perf.rs
  • crates/roost-linux/src/sprite.rs
  • crates/roost-linux/src/terminal_view.rs
  • crates/roost-linux/tests/fixtures/sprite_golden.txt
  • crates/roost-ui-model/src/lib.rs
  • crates/roost-ui-model/src/sprite.rs
  • docs/development/iced-migration-roadmap.md
  • docs/development/iced-parity-inventory.md
  • docs/reference/cli.md
  • docs/reference/ipc.md
  • tools/roosttest/test_osc_pipeline.py
  • tools/roosttest/test_sprite_pixels.py

Comment thread crates/roost-linux/src/sprite.rs
charliek and others added 2 commits August 6, 2026 22:13
Four of six findings applied: mislabeled dash-count comment in the
geometry tests; osc_pipeline docstring lane-count wording; the sprite
e2e now measures cell size from the marker pixel the settle wait
already validated (both directions, minus the shared start) so an
edge-snap shift hard-fails less and retries more; zip(strict=True).

Two skipped with rationale: clipping arc strokes to the cell would
change GTK pixels — the unclipped stroke is faithfully-preserved
shipped behavior under the plan's byte-identical bar (possible future
refinement, not this slice); adding ROOST_TEST_MODE=1 to local
e2e-iced would diverge from the repo-wide local-vs-CI convention that
e2e-gtk/e2e-mac share (the *-ci targets and all CI lanes set it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
CodeRabbit PR finding: surface_hash fingerprinted the full
stride*height buffer, coupling the fixture to cairo's stride alignment
(cairo_format_stride_for_width) — the same cross-version drift class
the stroked-glyph exclusion avoids. Now hashes only the w*4 pixel
bytes per row. On current cairo ARGB32 stride == w*4 for all fixture
sizes, so every hash (and the committed fixture) is unchanged —
verified by the regenerator producing a byte-identical file; the
change is purely defensive.

Declined nitpicks (recorded): pick_junction named-edge struct and
h_dash/v_dash dedup keep line-for-line correspondence with the Zig
original under the byte-identical bar; per-cell geometry rebuild is
plan-pinned (no caching, matches GTK's stateless paint); cairo state
scoping mirrors the pre-refactor call patterns exactly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgxiEBQTqgNjPswKqcs12d
@charliek

charliek commented Aug 7, 2026

Copy link
Copy Markdown
Owner Author

CodeRabbit dispositions (cloud review + a local CLI pass, findings de-duplicated):

Fixed

  • surface_hash stride-padding coupling → now hashes only the w*4 pixel bytes per row (3caf1c0…head). On current cairo ARGB32 stride == w*4, so the committed fixture is byte-identical — purely defensive against future stride-alignment drift.
  • Sprite e2e cell measurement hardened to measure from the settle-validated marker pixel (both directions); zip(strict=True); dash-count comment and docstring wording fixes (3caf1c0).

Declined, with rationale

  • Clip arc strokes to the cell — would change GTK pixels; the unclipped stroke is the faithfully-preserved shipped behavior under this PR's byte-identical bar (shed oracle: md5-equal pre/post). Possible future refinement, not this slice.
  • pick_junction named-edge struct / h_dash+v_dash dedup — the geometry module deliberately keeps line-for-line correspondence with Ghostty's box.zig original; restructuring trades review-against-source auditability for cosmetics under a byte-identical bar.
  • Per-cell sprite geometry rebuild each frame — plan-pinned (no caching in v1; GTK's paint is stateless-from-text by design, and rect-family glyphs cost 1–4 quads).
  • ROOST_TEST_MODE=1 on local make e2e-iced — would diverge from the repo-wide local-vs-CI convention shared by e2e-gtk/e2e-mac; the *-ci targets and all CI lanes set it.
  • Cairo state scoping in the adapter — mirrors the pre-refactor call patterns exactly, same bar.

@charliek
charliek merged commit 166d2d6 into main Aug 7, 2026
18 checks passed
@charliek
charliek deleted the feature/plan-020-iced-sprites branch August 7, 2026 03:37
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