Files
ai-app/docs/IRIS_TODO.md
T
iris ae0af8f5e3 iris: ScrollArea measures its content instead of drawing it twice
A container that probes a child's size was still doing it with a real
draw, so `Painter::measure` existed and almost nothing used it. Tracing
every draw of one streamed frame: 1,083 `Widget::draw` calls over 113
distinct widgets, the worst drawn 11 times at nesting depth 7-8, every
one of them mode `Draw` and none of them dirty. The cache was working --
each of the 11 was offered a genuinely different region, alternating
between an oversized probe box and a real one.

ScrollArea::draw was one of the two sources. It drew its whole content
at a box built from a stale hint, read the length back, corrected the
scroll position, and drew the content again where it belonged. The first
of those is now a measurement.

The other half is the measure fast path. A widget's reported size is a
function of its own state and the size it was offered, not of where it
was offered -- so an undirtied widget already drawn at a region of this
size has already answered, and `active.size` is the answer. This is the
same assumption `mov` makes one branch further down (same offered size,
therefore identical output, therefore a translation); it is only stated
as a size here rather than acted on as a move. Without it a measurement
costs a full recursive walk, which is what made the nesting compound.

A measurement now also peeks at the redraw mark instead of consuming it
-- it is not the redraw the mark asked for, and swallowing it would
leave the widget stale until something marked it again.

453 draws from 1,083, and the streamed frame is p50 1.18ms (from 1.22ms,
and 2.20ms before this run of work). The headless phone render is
byte-identical to the previous commit's on the real GPU.

What is deliberately NOT here: the same change to `Span::draw`'s phase 1,
which is the remaining 2x and which moves the layout by a few pixels.
The layout stays intact -- it is a position difference, not a broken
frame -- but which of the two is correct was not established, and the
suspicion (that phase 2 now takes `mov`, and `mov` accumulates deltas
where a redraw recomputes) points at a bug in `mov` rather than in
`Span`. Written up in docs/IRIS_TODO.md with Iris's target shape for
`Span`: no probe phase at all for `abs` children, and a `rest` child
forcing a reposition pass rather than a redraw.
2026-09-09 12:08:20 -04:00

307 lines
17 KiB
Markdown

# iris: known problems and things still to build
Iris's own list for the library, recorded 2026-09-04 in her words where it
matters, so the work in RUST.md picks these up in a sensible order rather
than rediscovering them. Each item says where it sits in the order and
what "done" looks like.
**Only open items live here.** An item is deleted when it lands, not
ticked: a list of finished work is context every future session pays for,
and what a change did belongs at the code it changed. Fifty closed items
and six phone-report sections went on 2026-09-08 for that reason.
## Fix
- [ ] **`Span` draws every child twice, and making the first one a
measurement moves the layout.** Found 2026-09-09; the safe half landed
and this is the part that needs a decision.
`Span::draw`'s phase 1 draws each child at `UiRegion::FULL` purely to
learn its length along the axis, then phase 2 draws it at its real
share. The provisional slot is the whole span, so it is wrong by
construction for every child, and the doubling compounds through nested
spans: one streamed frame of the bench fixture made **1,083
`Widget::draw` calls over 113 distinct widgets** before `ScrollArea`'s
probe became a `Painter::measure`, and **453** after, with 102 of the
113 still at 4 draws (2 real, 2 measurements).
Changing phase 1 to `painter.measure(child, UiRegion::FULL)` takes it
to 2, and **it renders differently**: 28,771 pixels, and the headless
phone shot shows the transcript shifted a few pixels vertically. The
layout is intact -- panels, code fences and text all draw correctly --
so this is a position difference, not a broken frame, but which of the
two is *right* was not established and it must be before this lands.
The mechanism to check first. With phase 1 drawing, the child's
`active.region` is the full span when phase 2 asks, so phase 2 always
finds a different size and does a real redraw. With phase 1 measuring,
`active.region` is still *last frame's* share, which usually matches,
so phase 2 takes `draw_inner`'s `mov` branch -- one `move_offsets`
write instead of a redraw, which is the intended win. But `mov`
accumulates (`entry.delta += delta`) where a redraw recomputes from
scratch, so the suspicion is float drift that phase 1's unconditional
redraw was hiding. If that is it, the fix is in `mov`, not in `Span`.
Iris's framing, which is the target shape (2026-09-09): *"if you draw
one child in a list of fixed sized children, then you know immediately
where the second one must be and shouldn't need to probe its size
again. The only time redraws should actually be needed are if you're
using a `rest` length, where you don't know how long it's gonna be
until you draw everything else first, and so you have to move things.
Even then it should just be moving, not redrawing."* So the endpoint is
no probe phase at all for `abs` children -- draw each in turn at
`[cursor, end]`, read its length, advance the cursor -- with a `rest`
child forcing a reposition pass over what follows it rather than a
redraw. `Aligned` already has exactly this shape (one draw, then
`Painter::reposition`) and is the example to copy.
- [ ] **A row moving because the list grew should be one `move_offsets`
write, and today it is a redraw.** Found 2026-09-09 by
`scripts/rigs/ui-profile`'s `arena_churn` and left for whoever picks
this up next; the upload half of it is done and this is the layout
half.
The measurement. Over the bench fixture's 401 streamed deltas, the
instance arena uploads **72.7%** of itself per frame, and that number
*is* the floor -- those entries genuinely differ, so no amount of
better dirty-tracking touches it. The control that says it is wrong is
the fling phase on the same screen and the same content: it moves the
same primitives every frame and uploads **3.3%**, because a scroll
reaches `UiRenderState::mov` and writes one `move_offsets` delta for
the subtree (LAYOUT.md section 2) instead of rewriting every
primitive's absolute region.
What is different about the streaming path. The list is pinned to the
newest end, so a growing reply pushes every row above it up by the
amount the last row grew. That is a translation of an already-drawn
subtree -- exactly what `move_offsets` is for -- but it arrives as a
new offered region per row, and `draw_inner`'s fast path only takes
`mov` when `active.region.size() == region.size()`. Worth checking
first: whether `LazySpan::place` is offering each row a region whose
*size* differs (it computes `edges(height)` fresh each frame, so an
identical height should compare equal -- unless a float differs in the
last bit, which the `// TODO: epsilon?` beside that comparison already
suspects), or whether something upstream marks the rows dirty so the
fast path is skipped entirely.
Done looks like: `arena_churn`'s `what_a_streamed_reply_uploads` shows
stream instances in the same range as the fling's, and its `whole`
column stops being the interesting one. The rig prints floor,
uploaded and whole per array precisely so this is checkable rather
than argued.
- [ ] **Where the scroll *pin* lives.** The rest of "scrolling moves out
of the list" landed on 2026-09-08 -- `List` is `LazySpan`, the physics
and the gesture live in one `ScrollController`, `.scrollable()` is the
only way anything scrolls, and **`docs/SCROLL.md` is the standing
reference**; read that rather than reconstructing it here.
What is left is one design question. The pin ("stay at the end as rows
are appended") is still each widget's own: `Scroll` has `snap_end` for
an ordinary child, `LazySpan` has one for itself, and the constructor
argument sets each. Iris asked for `amt` and "other controls (iirc only
at end for now)" to live in `Scroll` so a caller always edits the
`Scroll`; that is done for `amt` and not for the pin, because a pin has
to be *applied* when a row is appended -- between frames, with no
painter in hand -- so moving it needs either a fourth `Widget` method or
a parameter on `apply_scroll`. Nothing external edits a pin today (the
transcript sets it once at construction and calls `jump_to_end` on the
span for the rest), so this is a design question rather than a missing
capability.
- [ ] **A read-only text display has no widget of its own — P0's bench
report area is a `TextEdit` standing in for one (2026-09-05).** The only
way to get selectable text on screen today is `.editable(...)` plus
`.attr::<Selectable>(())` (`Selectable` is only implemented for
`TextEdit`, `iris/src/attr.rs`), which also makes the field focusable —
tapping the bench report opens the soft keyboard over text nothing lets
you type into. Harmless for a bench-only debug screen (not fixed this
pass), but a real "selectable, not editable" text primitive would
remove the keyboard side effect and is worth having before another
screen wants the same thing (P1's own transcript rows already read
their content from a `TextEdit` for the same reason).
## Build
- [ ] **Positions as a single float per scroll.** Iris raised, and half
rejected, letting a scroll update one float rather than positions:
input handling cares about most elements in a list, so absolute
positions must be computed on the CPU anyway. LAYOUT.md's design
already lands here (GPU walks the chain, CPU resolves on demand for
hit tests). Keep the CPU resolution lazy and per query; do not
materialise every row's absolute position per frame.
- [ ] **Animations, last.** Cosmetic, so after everything above. Must be
**modular — a piece of the library rather than a core part forced into
everything, the same way input is**. Whatever the mechanism, a widget
that does not animate must pay nothing and import nothing for it.
## Found by P1a (2026-09-06)
- [ ] **Desktop colours are washed out: the winit surface is sRGB and
the shader writes the palette's bytes as linear.** Mocha Crust
(17,17,27) is drawn as (73,73,91), measured off
`run-headless.sh --shot`. Android is correct, so this is the surface
format rather than the palette -- but it makes the desktop build
useless as a colour reference, which is exactly what P1a needed it for
when the emulator could not draw glyphs.
- [ ] **The bench report pane draws over the transcript rows instead of
replacing them.** Visible on the emulator for the first time now that
glyphs render there (`/tmp/emu-final.png`, 2026-09-06): after a bench
run the report's lines and the transcript's occupy the same rows in the
top third of the screen, both legible, neither on top. Pre-existing --
the same overlap is in a screenshot taken before the move-slot fix -- so
it is its own item, most likely the report pane not masking or not
claiming its region.
## Found by P1b (2026-09-06), all with a headless repro
Each was found by looking at `iris/run-headless.sh transcript -- -p
transcript-ui` rather than at a diff. docs/RUST.md's P1b box has the
fuller account.
**No entry here is worked around any more** (Iris, 2026-09-08: "All of
those should be fixed. There should never be workaround code. Do the
same for those; fix them if they're trivial, diagnose and report if
not."). Two are fixed and ticked; the two that are left are missing
*capabilities* rather than defects being dodged, and each carries its
diagnosis and what building it actually costs.
- [ ] **No overflow ellipsis.** `TextAttrs` can wrap or not wrap; there is
no "one line, ellipsised" the way `maxLines = 1` + `TextOverflow.
Ellipsis` gives Compose. A tool card's summary is clipped instead, so
nothing on screen says it was cut. Whichever end is cut has to be a
choice when this lands: a path is identified by its tail, a command by
its head.
**Diagnosed 2026-09-08, and it is not trivial.** parley has no
ellipsis of its own (checked: nothing in the vendored crates), so iris
would build it, and the shape that looks easy is the one that breaks
something. The easy half really is easy: shape at
`max_advance = width - ellipsis_advance` with wrapping on, take line
0's `text_range()`, and re-shape `text[..end].trim_end() + "…"` with
wrapping off -- parley's own line breaker finds the cut, so nothing
here counts glyph advances by hand. The hard half is that
`TextBuffer` has exactly one string and everything addresses it by
byte offset: the inline spans that carry a fence's colours and a
link's range, `TextEditCtx::byte_at` (which turns a tap into a byte to
match a link against), `Selection`'s `select`/`selected_text`, and
`RowBlocks::apply_delta`. Truncating the buffer moves every one of
those. So the real work is giving `TextBuffer` a **displayed** string
distinct from its source, with one mapping from display byte to source
byte that all of those go through -- worth doing, and not a
by-the-way. Doing it only for text that is neither editable nor
selectable would avoid all of that and is exactly the kind of
exemption that comes back later.
It also wants an API change while it is open: `TextAttrs::wrap: bool`
cannot say three states. Something like `Overflow::{Wrap, Clip,
Ellipsis(End)}` replaces it, with `End::{Head, Tail}` making
UI_RULES's "choose which end to truncate" a thing a caller must
answer rather than a default nobody reads.
- [ ] **A tool card's text is not selectable.** `Selection` is keyed
`(RowKey, block index)` and a card has no markdown blocks, so nothing in
a card registers. Compose's `SelectionContainer` covers tool output,
which is the text people most want to copy.
**Diagnosed 2026-09-08: mechanical, but more than a sitting.** There
is no key collision to design around, which was the open question:
a `TranscriptRow::Tools` has *only* cards and no markdown blocks at
all, so a card is free to number its own texts from 0 in reading
order. What it costs is the registration lifecycle rather than the
key. Each card's `TextEdit`s have to `Selection::register` as they are
built and `unregister` when they are not -- and a card is rebuilt from
several directions (`redraw_card` when a result arrives,
`Shared::set_content` when the group is toggled or a call joins the
run, and the per-card `WidgetPtr` swap), each of which frees widgets
the map would otherwise still point at. That is the exact shape of the
crash `Selection::clear`'s doc records from
review, 2026-09-06: a handle in that map outliving the widget
panics on the *next* long press, somewhere else entirely. So the work
is a per-card base index with a stride (and a `debug_assert` that a
card stays inside it), one register/unregister path that every rebuild
route goes through, and a test per route that a rebuilt card leaves no
stale handle behind.
## Warnings standing in the bench build (2026-09-08)
Seen while checking `cargo ndk -t arm64-v8a check --lib
--no-default-features --features "transcript-screen bench"` from
`app-rust/`, and left rather than silenced because it is a decision:
- [ ] **`PlatformHandle::show_diagnostics_overlay` has no caller.** It
and the ~60 lines of `IrisView.showDiagnosticsOverlay` behind it are a
plain-`TextView` overlay with Copy and Close, drawn over whatever iris
is doing -- built so a report can be read *even if iris itself has
stopped drawing*, which is the one case the in-iris diagnostics pane
that replaced it cannot cover. So this is a live escape hatch nobody
calls, not dead code: deleting both halves clears the warning and
removes the fallback, and wiring it back to something is a product
decision (Iris has no `logcat` on her phone). Ask before doing either.
## Build (for the port)
Widgets `RUST.md`'s "The port, in order (decided 2026-09-05)" needs and
iris does not have yet, one entry per gap, named against the P-step that
first needs it. Move an entry up to "Fix" or tick it in place once built;
do not duplicate it there.
- [ ] **A history-paging cushion measured in on-screen viewports, not a
row count.** (**P1**.) `iris::widget::List` has no equivalent of the
Compose app's `HISTORY_SCREENS` — AGENTS.md's "Things that have
bitten" is explicit that a fixed row count under-fills a screen on a
tool-heavy transcript and over-fills one on a text-heavy one, so
whatever loads the next page has to ask the list how many viewports
are actually on screen, not assume a constant.
- [ ] **A scaled thumbnail/image widget for an in-transcript image.**
(**P1**.) `SessionImage.kt`'s bitmap decode-and-downscale has no iris
counterpart; iris's own image widget (used by `bench_images.rs`) draws
a loaded texture but does nothing about sourcing or scaling one from a
server-produced attachment.
- [ ] **A modal/dialog primitive.** (**P1**, reused by **P3** and
**P5**.) Needed for the session settings dialog, `UsageDialog`'s
equivalent, and the delete-with-`deleteForeign` confirmation with its
toggle switch. Build once, wherever it is first needed, rather than
once per screen that wants one.
- [ ] **A horizontal gauge/bar widget.** (**P1**.) For
`SessionUsageBar`'s equivalent — a bounded fill reflecting a fraction,
nothing fancier.
- [ ] **A `BusyItem` equivalent: a dimmed row carrying an operation
label that does not block its list's own scroll/drag.** (**P3**.) The
Compose version tried an overlay first and it swallowed the drag along
with the tap (AGENTS.md's "Shared appearance") — worth not repeating
that attempt in iris before building the row-level version directly.
- [ ] **A toggle switch.** (**P3**.) For the delete dialog's
`deleteForeign` control; iris has no switch/checkbox widget yet as far
as this pass found.
## Reconsider
- [ ] **`WidgetView`.** Iris is unsure of it: what she wants is an easy way
to compose a widget from others (a button is the main case). With
sizing folded into `draw`, composing may be easy enough that `View` is
redundant. Decide after the layout change lands, by writing a button
both ways and keeping the one that is shorter to explain; delete the
other rather than keeping two ways.
- [ ] **A `Stack` that chooses its mask the way it chooses its size
(Iris, 2026-09-08).** She asked whether `masked_by` deserves to exist:
"a method that just does 2 separate things you can already easily do
does not deserve to exist." For a square-cornered surface it is indeed
redundant -- `.background(rect(BAR_FILL)).masked()` was measured
against `.masked_by(rect(BAR_FILL))` on the composer at the phone's own
size and density and the two are identical to the pixel. What the pair
cannot express is a clip that is not a box: `Painter::set_mask` writes
a `RectPrimitive::color(Color::NONE)` at the widget's own region, with
no radius, so `.background(rect(fill).radius(r)).masked()` draws a
rounded panel and then cuts its content square. Both other call sites
(`row.rs`'s fence, `tool.rs`'s raw output) are rounded, which is why
the method stands for now.
Her suggestion for removing it properly: **`Stack` already names where
its size comes from (`StackSize::Child(n)`); let it name where its
*mask* comes from the same way.** Then `.background(x)` is the one way
to put a surface behind something, and clipping to that surface is a
property of the stack rather than a second wrapper -- `masked_by` goes,
and `Masked::shape` with it. Worth checking while designing it: what a
stack with no mask child means (today's behaviour), whether the mask
child must also have been *drawn* first (`set_mask_to_widget` requires
it, and `Stack` draws in order, so naming child 0 is safe and naming a
later one is not), and what happens when the named child is the same
one the size comes from.