Author SHA1 Message Date
irisandClaude Fable 5.1 20303e0b4c IRIS.md: take_counters gained a fourth number, text shapes
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:40:38 -04:00
irisandClaude Fable 5.1 6973a89815 docs: the verification pass over Tasks A and B, and the composer background withdrawn
RUST.md gains the pass's findings with their commits and the numbers:
the block model held under a per-character prefix property, the
size-independent hit-box defect and its fix, the tail-rebuild selection
gap, why the three new debug_asserts are whole-set, the text-shape
counter that turns "a delta costs one block" into a measurement, and the
verification bench run.

IRIS_TODO.md's "the bar's own grey background is not drawn" is
withdrawn: decoding the screencap puts it at rgb(41,40,49), full width,
y2245..y2365 -- drawn, and dark on black, which is most likely what the
earlier reading was.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:40:25 -04:00
irisandClaude Fable 5.1 c3cfc67bb3 iris: count text layouts, so "a delta shapes one block" is measured rather than argued
take_counters gains a fourth counter, text shapes, bumped in
Painter::render_text -- which TextView::render only reaches on a cache
miss, so it counts shapes and not requests. A draw counter cannot stand
in for it in either direction: a widget can be redrawn without
re-shaping (the layout is memoized by width) and re-shaped without any
extra draw, and re-shaping is the whole thing the per-block transcript
row exists to avoid.

With it, a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one
asserts the number docs/DECISIONS.md's 2026-09-06 entry actually claims:
one delta into a 100-paragraph reply shapes exactly one text layout, the
same as into a one-paragraph one. Before the split that was necessarily
O(message), since the reply was one buffer.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:32:31 -04:00
irisandClaude Fable 5.1 155d899e55 transcript-ui: pin the tail rebuild's unregister with the case that broke it
e1030d6 made Selection's key (RowKey, u32) and changed apply's
ReplaceLast arm to unregister unconditionally rather than only when the
key changed -- correctly, but with nothing exercising it. The case is a
tail row rebuilt under the *same* key with fewer blocks than it had: the
blocks that no longer exist keep pointing at widgets replace_back's drop
frees, and Selection::begin resolves every registered handle on an
ordinary press, so the next tap anywhere in the transcript panics. The
old `if new_key != old_key` guard could not see it, because nothing
about the key changed.

Selection::registered_blocks (test-only) is what lets the test assert the
contract unregister states -- every block of the row, not the first --
instead of only that nothing panicked.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:32:05 -04:00
irisandClaude Fable 5.1 e63e923d44 iris: a size-independent widget's hit box lands where it is drawn
draw_inner's third fast path -- offered region changed shape, widget's
output does not depend on it -- rewrites the widget's own primitives in
place and writes no move-slot delta at all. 167862c added a
move_applied increment there, copied from mov, where region and the slot
delta really do move together. Here only region moves, so resolved_region
subtracted a distance the chain never held and every such widget's hit
box sat short of its drawing by exactly the last step it took.

Span reaches this on the first frame of any tree it is in: it measures
each child at the full region and then places it, which for a Rect (the
.background(rect(..)) idiom, list row tints) is a size change through this
branch. So the hit box was wrong from the start, with the drawing correct
-- nothing on screen to say so.

a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at
is the sibling of a_panned_widgets_own_hit_box_moves_exactly_once on the
branch that fix had no reason to touch; it fails on both frames without
this.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:31:58 -04:00
irisandClaude Fable 5.1 a56a928b0c client-core: the transcript's own markdown shapes, and the streaming property as a property
split_blocks was tested on the shapes it was written against. These are
the ones a real reply contains -- a fence with blank lines in it, a `---`
inside a fence, a nested list, a fence directly under a heading, a table,
a quote -- plus the property RowBlocks::apply_delta actually depends on,
checked at every character boundary of a message that has all of them:
growing a message may rewrite its last block and never an earlier one, or
common_prefix must say so. No defect found; the split already held.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 18:28:57 -04:00
iris 0449a324ef docs/RUST.md: P1 started on Iris's word, sub-order P1a-P1e by what makes the bench fair 2026-09-06 18:28:48 -04:00
irisandClaude Fable 5.1 e1030d69f6 iris: a transcript row is a column of markdown blocks, so a streamed delta costs one block
A row was one TextEdit holding the whole message, so every delta
re-shaped every paragraph of a long reply through parley -- the one phase
where iris trails Compose on the phone (p50 18.2ms vs 13.4ms, bench v2).

- client-core/src/markdown_blocks.rs: split a message into its top-level
  blocks with their source, through the same pulldown-cmark the renderer
  parses with so the two cannot disagree about where a block starts, plus
  common_prefix. Appending markdown can rewrite an earlier block (a
  trailing --- turns the paragraph above into a heading), so the fast
  path compares the prefix it keeps rather than assuming it -- with the
  test that says so.
- transcript-ui: a row is a Span of one TextEdit per block;
  RowBlocks::apply_delta replaces the block a delta lands in;
  TranscriptScreen keeps the tail row's blocks, seeded in build_tree as
  well as push_row (a screen opened onto a streaming reply took the
  rebuild path for its first delta otherwise, with nothing to say so).
- A block is the selection unit: Selection is keyed by (RowKey, u32),
  which is reading order at both levels, and the pointer-captured half of
  a drag resolves the block under the finger from its drawn box
  (Selection::locate) instead of from the row's extent.

Pass condition: a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one
drives a real UiRenderState and asserts the draw count for a delta into a
100-paragraph (3,000+ char) reply equals the count for a one-paragraph
one. 30 either way; it read 630 against 30 twice on the way there.

Emulator stream phase, same AVD before and after: p50 61.5 -> 54.5ms,
p90 211.7 -> 113.1ms, p99 342.6 -> 137.4ms, worst 403.6 -> 143.0ms, 202
-> 293 frames in the same 21 seconds. Selection across blocks verified
with a real long-press drag.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 17:33:37 -04:00
irisandClaude Fable 5.1 167862ca1b iris: the composer scrolls on a finger -- a dp cap worth zero, a stale mask slot, a hit box moved twice
Wrapping the composer's field in .scrollable().masked() needed three
layout defects fixed first, each with a headless regression test that was
confirmed to fail without its fix:

- MaxSize/Sized reported a caller's declared dp length unresolved, and
  Span places a child from the abs/rel of what it reported, so dp(168)
  was worth zero: the bar got a slot of nothing the moment its content
  passed six lines and the Scroll inside measured its container at -63px
  (container=-63 content=415.8 amt=478.8 on the emulator). Len::fold_dp,
  used on the way out, plus a debug_assert in draw_inner that a reported
  Size carries no dp -- the rule is about every widget, not those two.
- Masked allocated a fresh mask slot per draw, and draw_inner's
  unchanged-region fast path does not revisit descendants, so they kept
  clipping against a box the bar had moved away from: four live mask
  entries, none of them current, and the field drew nothing.
  ActiveData::own_mask, allocated once and rewritten in place.
- mov updates active.region and accumulates the same delta on the move
  slot, and resolved_region added both, so a panned widget's own hit box
  sat at twice the pan -- the composer's field was untappable after a
  drag. ActiveData::move_applied.

Scroll itself measured the right number by a misleading route; it is
written against painter.px_size() now and still reports its content's
size, since reporting the container makes the answer a function of
itself.

Verified on this checkout's emulator: swipe 540 1200 -> 540 1460 moved
the field's Message box 31,1041..1048,1509 -> 31,1131..1048,1651 with its
height unchanged at 468px.

run-bench.sh polled logcat for a prefix copy_report also logs at startup,
so it printed a report that had never been run.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 17:17:42 -04:00
iris d73db97629 iris/android-app/build-apk.sh: clear jniLibs before building, so only the requested ABI is packaged 2026-09-06 16:47:54 -04:00
irisandClaude Fable 5.1 fb6b459c2c iris: Scroll pans on a finger drag; a vertical drag in a focused field scrolls rather than selects
IRIS_TODO.md's "the composer has no touch-drag scroll". `Scroll::drag`
takes its pan from the same `sense::DragGesture` `List` is driven by --
arbitration, DRAG_SLOP, velocity and pointer capture all stay in sense.rs
and only what a committed pan *means* is decided per caller -- and
`WidgetLike::scrollable()` registers it beside the wheel handler it already
registered, so every scroll area pans on a finger with nothing added at the
call site. No fling: `Scroll` has no per-frame tick to animate one and the
areas it wraps are at most a screenful. `Scroll::amt()` exposes the pan
position.

`attr.rs`'s `on_press` treated an already-focused field as the plain
click_or_drag case, so every Pressing frame extended a selection. It now
applies the same DRAG_SLOP rule its unfocused branch already did: a press
past the slop vertically abandons its pending selection for the rest of the
gesture, so the scroll area around the field wins it. That is Android
EditText's own behaviour and it is what lets a swipe up over the composer
scroll instead of dragging a highlight through what you typed.

Also fixed, found doing it: `ActiveData::mask` stored the mask a widget
*set* rather than the one it was drawn *under*, and `redraw` feeds that
field back in as the inherited mask -- so a targeted redraw of any `Masked`
handed it its own mask and aborted on `set_mask`'s nested-mask assert. A
real abort on the emulator, `assertion failed: self.mask == MaskIdx::NONE`.

And the per-frame orphan guard from 76b1f99 is now a count comparison
(O(active widgets)); the O(primitives) walk only runs to build the failure
message, because running it per frame made a debug build on the emulator too
slow to finish a bench run at all.

Tests: four in scroll.rs (pan past the slop, a tap inside it, a horizontal
drag, the end clamp), `a_finger_drag_over_a_scroll_area_pans_it` in
sense_tests.rs driving the whole registration/dispatch/capture path (fails
with "got 0" without the new registration), and
`redrawing_a_masked_widget_does_not_nest_its_own_mask` in layout_tests.rs
(aborts on the pre-fix code).

The composer itself is deliberately still not `.scrollable()`: `Scroll`
measures against the window rather than its own offered box, so inside the
`MaxSize` capping it at six lines it pans the field out of the bar --
measured, reverted and written down in RUST.md and DECISIONS.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 16:45:56 -04:00
irisandClaude Fable 5.1 76b1f99277 iris: a dirty widget redrawn by its ancestor never freed its old primitives
`draw_inner` read `needs_redraw` without consuming it, and used it to skip
the whole `if let Some(active)` block -- including the `remove(id, false)`
that frees a redrawn widget's previous primitives. So a widget that was
both already active and marked dirty, and was reached by an *ancestor's*
draw rather than by `redraw_updates` picking it first, drew a second full
set of primitives and then had `active.insert` overwrite the only handles
that could ever have freed the first set. Those primitives stay in the
layer's instance buffer for the life of the process, with a leaked move
slot and leaked mask refs, drawn every frame at whatever region they last
had -- and `List` sets no mask, so a row measured at `GENEROUS_PADDING`
leaves its ghost outside the list's own box.

That is the doubled `Compacted:` row in docs/bench/iris-phone-v2-2026-09-06.md:
overlapping copies inside the transcript and one more below the composer.

Fixed by consuming the mark (`needs_redraw.remove`) at the top of
`draw_inner` -- this call *is* the redraw it asked for -- and freeing the
old primitives on the dirty path too.

Guarded so it cannot come back silently: `UiRenderState::orphaned_primitives`
walks every layer's live instances and names any whose owner is no longer
active or no longer holds a handle to them, and `update` `debug_assert!`s it
empty every frame (debug builds only). New regression test
`an_ancestor_redrawing_a_dirty_row_leaves_no_stale_copy` in list.rs fails on
the pre-fix code with "1 primitive(s) survived their own widget's redraw".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 13:59:53 -04:00
iris 3e72a4ef19 docs: the defect pass's findings -- RUST.md boxes, IRIS_TODO ticks, DECISIONS and IRIS entries 2026-09-06 13:47:28 -04:00
iris c02152a4f4 iris: a tap on an empty text field left no caret, so typing was silently dropped
TextEditCtx::select compared the tap against the laid-out text's own box
and cleared the selection for anything outside it. An empty field lays
out to a zero-width box, so tapping the composer granted focus and opened
the keyboard with no caret, and insert_str returns early without one --
every keystroke went nowhere and no glyph was ever emitted. Parley clamps
a point outside the layout by itself, and a press reaching select() has
already been hit-tested to the widget, so there was nothing for the
'outside' branch to mean.

insert_str now debug_asserts rather than dropping input silently, and
UiRenderState::draw_started -- a re-entrancy guard whose test was written
after its own remove(), so it could never fire, and which grew by one
entry per widget ever drawn -- is restored to what it was meant to be:
inserted around Widget::draw, removed when it returns, asserted empty at
the top of every update.
2026-09-06 13:43:56 -04:00
iris d9872989fa iris/android: the composer's launch position was the bench report pane, plus surface/insets lifecycle logging
The empty benchmark-report TextEdit held .height(rest(1)) beside
content.height(rest(2)), so it reserved a third of the window at every
launch and pushed the composer two thirds down -- Iris's 11:39 phone
report. It is sized to its content now, capped and scrollable, and sits
above the transcript rather than under the composer.

New log::info! lines for one insets change, one surface_changed, one
renderer build and one surface_destroyed, each with the glyph/atlas
counts, so a phone's adb logcat can answer the app-switch text loss the
emulator cannot reproduce.
2026-09-06 13:26:34 -04:00
iris 2fed8b34b3 Merge branch 'worktree-agent-a6e37a2335f436d08' into rustify 2026-09-06 13:17:22 -04:00
irisandClaude Fable 5.1 1f379e8384 docs/REVIEW-2026-09-06.md: fix all ten review findings; RUST.md/IRIS_TODO.md: DragGesture merge checks
Finding 1 (the real crash): Selection::clear() drops rows and anchor,
called from TranscriptScreen::apply's Rebuild arm right before
List::clear() -- push_row re-registers survivors as it rebuilds each row.
Fixes a WeakWidget outliving the row group_tool_runs regrouped away,
which panicked the next long-press anywhere. New apply_tests test builds
a real TranscriptScreen, forces the regroup, and confirms no panic.

Findings 2-5: debug_assert!s on List::place's slot, List::fling and
FlingCalculator's velocity finiteness, VelocityTracker::add_sample's
chronological order, and FrameReport::mark_phase's non-decreasing
start_index. Finding 7: bench_client.rs's battery_line guard restructured
so the empty check can't be separated from its unwraps by a future edit.
Findings 9/10: new List tests pinning tick_fling's per-tick deceleration
and replace_back's evicted-key cleanup with a different key than the
existing tests use. IRIS.md's replace_back/clear/apply entry gained the
side-table-clearing note the Docs finding asked for.

Also records this pass's DragGesture-merge verification in RUST.md (tap
stays vs swipe doesn't, a real fling keeps moving after release, keyboard
cycles confirmed via on_insets_changed) and annotates the two IRIS_TODO.md
phone-report items it targets.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 13:16:16 -04:00
41 changed files with 3685 additions and 217 deletions

No files matched your search

+47
View File
@@ -50,6 +50,12 @@ version = "0.23.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5"
[[package]]
name = "bitflags"
version = "2.13.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da"
[[package]] [[package]]
name = "bytes" name = "bytes"
version = "1.12.1" version = "1.12.1"
@@ -77,6 +83,7 @@ name = "client-core"
version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"event-model", "event-model",
"pulldown-cmark",
"serde", "serde",
"serde_json", "serde_json",
"ureq", "ureq",
@@ -206,6 +213,15 @@ dependencies = [
"percent-encoding", "percent-encoding",
] ]
[[package]]
name = "getopts"
version = "0.2.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cfe4fbac503b8d1f88e6676011885f34b7174f46e59956bba534ba83abded4df"
dependencies = [
"unicode-width",
]
[[package]] [[package]]
name = "getrandom" name = "getrandom"
version = "0.2.17" version = "0.2.17"
@@ -490,6 +506,25 @@ dependencies = [
"unicode-ident", "unicode-ident",
] ]
[[package]]
name = "pulldown-cmark"
version = "0.13.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e"
dependencies = [
"bitflags",
"getopts",
"memchr",
"pulldown-cmark-escape",
"unicase",
]
[[package]]
name = "pulldown-cmark-escape"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "007d8adb5ddab6f8e3f491ac63566a7d5002cc7ed73901f72057943fa71ae1ae"
[[package]] [[package]]
name = "quote" name = "quote"
version = "1.0.47" version = "1.0.47"
@@ -783,12 +818,24 @@ dependencies = [
"zerovec", "zerovec",
] ]
[[package]]
name = "unicase"
version = "2.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142"
[[package]] [[package]]
name = "unicode-ident" name = "unicode-ident"
version = "1.0.24" version = "1.0.24"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75"
[[package]]
name = "unicode-width"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254"
[[package]] [[package]]
name = "untrusted" name = "untrusted"
version = "0.9.0" version = "0.9.0"
+41
View File
@@ -47,6 +47,7 @@ name = "client-core"
version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"event-model", "event-model",
"pulldown-cmark",
"serde", "serde",
"serde_json", "serde_json",
"tempfile", "tempfile",
@@ -173,6 +174,15 @@ dependencies = [
"percent-encoding", "percent-encoding",
] ]
[[package]]
name = "getopts"
version = "0.2.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cfe4fbac503b8d1f88e6676011885f34b7174f46e59956bba534ba83abded4df"
dependencies = [
"unicode-width",
]
[[package]] [[package]]
name = "getrandom" name = "getrandom"
version = "0.2.17" version = "0.2.17"
@@ -425,6 +435,25 @@ dependencies = [
"unicode-ident", "unicode-ident",
] ]
[[package]]
name = "pulldown-cmark"
version = "0.13.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e"
dependencies = [
"bitflags",
"getopts",
"memchr",
"pulldown-cmark-escape",
"unicase",
]
[[package]]
name = "pulldown-cmark-escape"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "007d8adb5ddab6f8e3f491ac63566a7d5002cc7ed73901f72057943fa71ae1ae"
[[package]] [[package]]
name = "quote" name = "quote"
version = "1.0.47" version = "1.0.47"
@@ -661,12 +690,24 @@ dependencies = [
"zerovec", "zerovec",
] ]
[[package]]
name = "unicase"
version = "2.9.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142"
[[package]] [[package]]
name = "unicode-ident" name = "unicode-ident"
version = "1.0.24" version = "1.0.24"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75"
[[package]]
name = "unicode-width"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254"
[[package]] [[package]]
name = "untrusted" name = "untrusted"
version = "0.9.0" version = "0.9.0"
+6
View File
@@ -32,6 +32,12 @@ serde_json = { version = "1", features = ["float_roundtrip", "raw_value"] }
# no need of an async runtime, and RUST.md's brief for this port is # no need of an async runtime, and RUST.md's brief for this port is
# "lightweight" throughout. # "lightweight" throughout.
ureq = { version = "3", features = ["json"] } ureq = { version = "3", features = ["json"] }
# The markdown block split (`markdown_blocks`), which has to agree with the
# renderer in `iris/transcript-ui` about where a block begins -- so it is
# the same parser at the same version, rather than a hand-written splitter
# that would drift from it.
pulldown-cmark = "0.13.4"
[dev-dependencies] [dev-dependencies]
tempfile = "3" tempfile = "3"
+1
View File
@@ -7,6 +7,7 @@ pub mod api;
pub mod config; pub mod config;
pub mod event_stream; pub mod event_stream;
pub mod highlight; pub mod highlight;
pub mod markdown_blocks;
pub mod notifications; pub mod notifications;
pub mod sse; pub mod sse;
pub mod transcript_cache; pub mod transcript_cache;
+325
View File
@@ -0,0 +1,325 @@
//! Split a markdown message into its top-level **blocks** -- one
//! paragraph, heading, fenced code block, list, table or quote each, as a
//! byte slice of the original source.
//!
//! This exists for streaming. A transcript row used to be one text widget
//! holding the whole message, so a single streamed delta re-shaped every
//! paragraph of it through the text engine again; the phone's bench v2 put
//! the stream phase at p50 18.2ms against Compose's 13.4ms for exactly
//! that reason (docs/IRIS_TODO.md). A row is a column of one widget per
//! block now, and a delta that lands in the last block leaves every
//! earlier block's layout alone. `docs/DECISIONS.md`'s 2026-09-06 entry has
//! what that rejected and why the split lives here rather than in the UI
//! crate: `docs/CLIENT_CORE.md` already wanted a block model for P1, and
//! keeping it here means iris stays a text renderer that knows nothing
//! about markdown.
//!
//! **Blocks only.** Inline styling (bold, links, inline code) is still the
//! renderer's own job, per block -- this deliberately does not build a
//! full AST, because nothing needs one yet.
//!
//! ## Appending is not guaranteed to leave earlier blocks alone
//!
//! It nearly always does, which is what makes the fast path worth having,
//! but markdown has no such rule: appending a "```" line can turn text
//! that was three paragraphs into one fenced block, and appending "---"
//! under a paragraph turns that paragraph into a heading. So a caller
//! taking the O(last block) path **must compare the prefix it is about to
//! keep** rather than assume it. [`common_prefix`] is that comparison, and
//! it is cheap next to laying the text out again.
use pulldown_cmark::{Event, Options, Parser, Tag};
/// What a block is, for a renderer that wants to style or space blocks
/// differently. `Other` is deliberately present rather than a panic or a
/// silent fallback to `Paragraph`: markdown has more block kinds than this
/// list and more get added, and a renderer treating an unknown one as
/// prose is right, but it should be able to *tell* that is what it is
/// doing.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum BlockKind {
Paragraph,
Heading,
/// A fenced or indented code block.
Code,
List,
Table,
Quote,
/// A thematic break, raw HTML, a footnote -- anything with no
/// distinguished treatment here.
Other,
}
/// One top-level block: its kind and the exact source that produced it.
/// `source` is a slice of the input with trailing whitespace removed, so
/// two splits of the same prefix compare equal even when one of them had a
/// delta arriving after it.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Block {
pub kind: BlockKind,
pub source: String,
}
fn kind_of(tag: &Tag) -> BlockKind {
match tag {
Tag::Paragraph => BlockKind::Paragraph,
Tag::Heading { .. } => BlockKind::Heading,
Tag::CodeBlock(_) => BlockKind::Code,
Tag::List(_) => BlockKind::List,
Tag::Table(_) => BlockKind::Table,
Tag::BlockQuote(_) => BlockKind::Quote,
_ => BlockKind::Other,
}
}
fn options() -> Options {
// The same set `transcript-ui`'s renderer parses with, so a block
// boundary here and the styling there cannot disagree about what the
// source means.
Options::ENABLE_STRIKETHROUGH | Options::ENABLE_TABLES | Options::ENABLE_TASKLISTS
}
/// Split `src` into its top-level blocks, in source order. An empty or
/// whitespace-only input gives no blocks; text the parser does not put
/// inside any block (a stray fence marker mid-stream) still comes back,
/// as `Other`, rather than being dropped.
pub fn split_blocks(src: &str) -> Vec<Block> {
let mut out: Vec<Block> = Vec::new();
let mut depth = 0usize;
let mut kind = BlockKind::Other;
for (event, range) in Parser::new_ext(src, options()).into_offset_iter() {
match event {
Event::Start(tag) => {
if depth == 0 {
kind = kind_of(&tag);
}
depth += 1;
}
Event::End(_) => {
depth -= 1;
if depth == 0 {
push(&mut out, kind, &src[range]);
}
}
// A top-level event that is not part of any block -- a
// thematic break, a block of raw HTML. Inside one, it is the
// enclosing block's business and this does nothing.
_ => {
if depth == 0 {
push(&mut out, BlockKind::Other, &src[range]);
}
}
}
}
out
}
fn push(out: &mut Vec<Block>, kind: BlockKind, source: &str) {
let source = source.trim_end();
if source.is_empty() {
return;
}
out.push(Block {
kind,
source: source.to_string(),
});
}
/// How many leading blocks of `old` and `new` are identical -- what a
/// caller may keep the laid-out widgets for. See the module doc for why
/// this is a comparison rather than an assumption.
pub fn common_prefix(old: &[Block], new: &[Block]) -> usize {
old.iter().zip(new).take_while(|(a, b)| a == b).count()
}
#[cfg(test)]
mod tests {
use super::*;
fn kinds(src: &str) -> Vec<BlockKind> {
split_blocks(src).into_iter().map(|b| b.kind).collect()
}
#[test]
fn a_message_splits_into_its_top_level_blocks() {
let src = "# Title\n\nFirst para.\n\n```rust\nfn main() {}\n```\n\n- a\n- b\n";
assert_eq!(
kinds(src),
vec![
BlockKind::Heading,
BlockKind::Paragraph,
BlockKind::Code,
BlockKind::List
]
);
let blocks = split_blocks(src);
assert_eq!(blocks[1].source, "First para.");
assert_eq!(blocks[2].source, "```rust\nfn main() {}\n```");
}
#[test]
fn blank_input_has_no_blocks() {
assert!(split_blocks("").is_empty());
assert!(split_blocks(" \n\n ").is_empty());
}
/// The property the streaming fast path rests on, in its ordinary
/// shape: a delta landing in the last paragraph must leave every
/// earlier block byte-identical.
#[test]
fn a_delta_into_the_last_paragraph_leaves_earlier_blocks_untouched() {
let before = split_blocks("# Title\n\nFirst para.\n\nSecond par");
let after = split_blocks("# Title\n\nFirst para.\n\nSecond paragraph now.");
assert_eq!(common_prefix(&before, &after), 2);
assert_eq!(before.len(), 3);
assert_eq!(after.len(), 3);
assert_ne!(before[2], after[2]);
}
/// A delta that starts a *new* block keeps every old block, including
/// the one that was last -- so the fast path appends rather than
/// replacing.
#[test]
fn a_delta_that_starts_a_new_block_keeps_every_old_one() {
let before = split_blocks("First para.\n\nSecond para.");
let after = split_blocks("First para.\n\nSecond para.\n\nThird");
assert_eq!(common_prefix(&before, &after), 2);
assert_eq!(after.len(), 3);
}
/// A code fence arrives one delta at a time and is unterminated for
/// most of its life. It must still be *one* block the whole way, or
/// every delta would re-split the message into a different number of
/// pieces.
#[test]
fn an_unterminated_fence_is_one_block_while_it_streams() {
for src in [
"Here:\n\n```rust\n",
"Here:\n\n```rust\nfn main() {\n",
"Here:\n\n```rust\nfn main() {\n println!(\"hi\");\n",
] {
assert_eq!(
kinds(src),
vec![BlockKind::Paragraph, BlockKind::Code],
"{src:?}"
);
}
}
/// The half the fast path had no reason to touch, and the reason
/// `common_prefix` is a comparison rather than an assumption:
/// appending can rewrite what came before. `---` under a paragraph
/// turns that paragraph into a setext heading, so the block that was
/// already laid out is not the block it is now.
#[test]
fn appending_can_rewrite_an_earlier_block_and_the_prefix_says_so() {
let before = split_blocks("Not a heading\n\nsecond");
let after = split_blocks("Not a heading\n\nsecond\n---");
assert_eq!(before[1].kind, BlockKind::Paragraph);
assert_eq!(after[1].kind, BlockKind::Heading);
assert_eq!(
common_prefix(&before, &after),
1,
"the rewritten block must not be reported as keepable"
);
}
#[test]
fn a_thematic_break_is_its_own_block() {
assert_eq!(
kinds("one\n\n---\n\ntwo"),
vec![BlockKind::Paragraph, BlockKind::Other, BlockKind::Paragraph]
);
}
/// The shapes a real transcript actually contains, each checked for
/// the one property the streaming fast path needs: the *number* of
/// blocks and every earlier block's source stay put while the message
/// grows. A fence's own blank lines, a `---` inside one, a nested
/// list and a table are all places where a naive line-based split
/// would break the message into more pieces than there are blocks.
#[test]
fn the_transcripts_own_block_shapes_survive_a_split() {
let fence_with_blanks = "Intro.\n\n```rust\nfn a() {}\n\nfn b() {}\n```\n\nAfter.";
assert_eq!(
kinds(fence_with_blanks),
vec![BlockKind::Paragraph, BlockKind::Code, BlockKind::Paragraph],
"a blank line inside a fence is not a block boundary"
);
assert_eq!(
kinds("```\n---\n```"),
vec![BlockKind::Code],
"a thematic break inside a fence is code, not a break"
);
assert_eq!(
kinds("- a\n - a1\n - a2\n- b"),
vec![BlockKind::List],
"a nested list is one top-level block"
);
assert_eq!(
kinds("## Heading\n```sh\nls\n```"),
vec![BlockKind::Heading, BlockKind::Code],
"a fence directly under a heading, with no blank line"
);
assert_eq!(
kinds("| a | b |\n|---|---|\n| 1 | 2 |"),
vec![BlockKind::Table]
);
assert_eq!(
kinds("> quoted\n> more\n\nplain"),
vec![BlockKind::Quote, BlockKind::Paragraph]
);
}
/// `apply_delta`'s precondition, stated as the property rather than
/// the arithmetic: for every prefix of a realistic streamed message,
/// the blocks before the last one must be exactly the blocks the
/// previous prefix had. Where markdown breaks that (the `---` case
/// above), `common_prefix` has to *say* so -- which is what the
/// `>= len - 1` assertion below checks: the split may rewrite the
/// last block, never an earlier one, or `RowBlocks::apply_delta`
/// would keep a widget whose text is no longer what it holds.
#[test]
fn every_prefix_of_a_streamed_message_keeps_all_but_its_last_block() {
let full = "# Report\n\nFirst finding, at some length.\n\n```rust\nfn main() {\n\n println!(\"hi\");\n}\n```\n\n- one\n - nested\n- two\n\n| a | b |\n |---|---|\n| 1 | 2 |\n\n> and a closing quote.";
// Every character boundary, so a delta landing mid-word and one
// landing exactly on a fence's closing backtick are both covered.
let mut prev = Vec::new();
for end in full.char_indices().map(|(i, _)| i).chain([full.len()]) {
let now = split_blocks(&full[..end]);
let common = common_prefix(&prev, &now);
assert!(
prev.is_empty() || common + 1 >= prev.len(),
"at {end} bytes the split rewrote block {common} of {}, not just the last one:\n before={prev:#?}\nafter={now:#?}",
prev.len()
);
prev = now;
}
}
/// The half a growing message cannot show: a fence that never closes.
/// The stream ends there and the block must still be the code block
/// it has been all along, not re-split into paragraphs.
#[test]
fn a_stream_that_ends_inside_a_fence_still_ends_with_one_code_block() {
let src = "Here is the patch:\n\n```diff\n- old line\n+ new line";
let blocks = split_blocks(src);
assert_eq!(
blocks.iter().map(|b| b.kind).collect::<Vec<_>>(),
vec![BlockKind::Paragraph, BlockKind::Code]
);
assert_eq!(blocks[1].source, "```diff\n- old line\n+ new line");
}
/// A delta that closes a fence changes the *last* block only, so the
/// fast path takes it -- the case the module doc says is the reason
/// `common_prefix` is a comparison.
#[test]
fn the_delta_that_closes_a_fence_changes_only_the_last_block() {
let before = split_blocks("Text.\n\n```\ncode\n");
let after = split_blocks("Text.\n\n```\ncode\n```");
assert_eq!(before.len(), after.len());
assert_eq!(common_prefix(&before, &after), 1);
assert_ne!(before[1], after[1]);
}
}
+12 -9
View File
@@ -191,14 +191,17 @@ does not repeat it again by hand.
## What is not started at all ## What is not started at all
- **The markdown *block* model beyond syntax spans** -- `highlight/markdown.rs` - **A full markdown AST.** `markdown_blocks` (2026-09-06) splits a message
colours a `.md` file or fence for the highlighter, but does not build the into its *top-level* blocks -- heading, paragraph, fence, list, table,
block tree (headings, lists, tables, fences as distinct nodes) that a quote -- with each block's own source, which is what a renderer needs to
renderer walks to lay out prose versus code versus a table. lay out prose versus code and what lets a streamed delta re-lay out one
`CodeFence.kt`'s use of `org.intellij.markdown` for that full CommonMark block instead of the message (docs/RUST.md's Task B). What it
AST is Compose rendering plumbing, not something to port as-is; a Rust deliberately does **not** build is the tree below that: nested list
UI layer will want its own block parser or a crate for it, decided items, table cells, inline spans. Inline styling is still the renderer's
alongside the framework choice in RUST.md. own job per block (`iris/transcript-ui/src/markdown.rs`), and nothing
has needed the rest yet. `CodeFence.kt`'s use of `org.intellij.markdown`
for a full CommonMark AST is Compose rendering plumbing, not something
to port as-is.
- **`TranscriptUnits.kt`** (see above) -- deliberately out of scope, since - **`TranscriptUnits.kt`** (see above) -- deliberately out of scope, since
it flattens a row into bounded units for a *specific* lazy-list it flattens a row into bounded units for a *specific* lazy-list
framework's composition cost, which is a fact about that framework framework's composition cost, which is a fact about that framework
@@ -209,5 +212,5 @@ does not repeat it again by hand.
`./run-tests.sh` from the repo root now runs `event-model`, `client-core` `./run-tests.sh` from the repo root now runs `event-model`, `client-core`
and `server` in that order (each `cargo test`, forwarding arguments the and `server` in that order (each `cargo test`, forwarding arguments the
same way it always has). From `client-core/` directly: `cargo test` same way it always has). From `client-core/` directly: `cargo test`
(109 tests), `cargo clippy --all-targets`, `cargo fmt` -- all clean as of (119 tests), `cargo clippy --all-targets`, `cargo fmt` -- all clean as of
this writing (2026-09-06). this writing (2026-09-06).
+79
View File
@@ -5,6 +5,85 @@ they can be judged and reversed later. Detail lives in RUST.md (and IRIS.md
for iris API changes); this file is only the summary. Newest first. Items for iris API changes); this file is only the summary. Newest first. Items
marked **DEFERRED** are ones the agent chose not to decide alone. marked **DEFERRED** are ones the agent chose not to decide alone.
## 2026-09-06 (composer scroll and the streaming block model)
- **A streamed message becomes a column of per-block widgets.** Decided by
the design agent; recorded here because it is the shape of every message
on screen. A transcript row is one `TextEdit` today, so a streamed delta
re-shapes the entire message through parley on every event -- the stream
phase is the one place iris is behind Compose on your phone (p50 18.2ms
vs 13.4ms). A row becomes a column of one widget per markdown block
(paragraph, heading, fence, list, table) and a delta replaces only the
last block, keeping every earlier block's layout. **Rejected:** splitting
parley's layout at block boundaries inside one text widget (couples
iris's text widget to markdown structure, and parley has no incremental
API), and caching shaped runs per paragraph inside `TextEdit` (a second
cache with its own invalidation beside the glyph cache). Chosen because
P1's markdown block model is needed anyway, so the split happens once, in
`client-core`, and iris stays a text renderer. **Status: designed, not
built** -- this pass spent its budget on the composer's three layout
defects; docs/RUST.md has the design and the pass conditions.
- **The composer's overflowing text now scrolls on a finger**, capped at
six lines and clipped to the bar. Reverses the "still does not scroll"
item below.
- **A widget may not report a `dp` length** (see IRIS.md). A rule for
widget authors, enforced by a `debug_assert!`; nothing changes for app
code.
## 2026-09-06 (stale-primitives and touch-scroll pass)
- **A vertical drag inside a focused composer now scrolls rather than
selects.** Android's own `EditText` does this -- a vertical drag scrolls
the field, and only a long press starts a selection -- so the platform
decided it. What it costs: you can no longer drag straight down inside
the composer to select several lines of what you typed; use a long press
and then drag, or drag sideways. Say if that trade is wrong for you.
- **`Scroll` gets a finger pan but no fling.** `List` flings; a scroll area
does not, because it has no per-frame tick to animate one and the areas
it wraps are at most a screenful (Android does not fling a six-line text
box either). Easy to add later if a scroll area ever wraps something long.
- **The composer still does not scroll its overflowed text**, though the
mechanism it needs is now in place. Wrapping the field in `.scrollable()`
was tried and reverted the same day: `Scroll` measures its content and
container against the *window*, so inside the `MaxSize` that caps the
composer at six lines the two are in different spaces and the field pans
itself entirely out of the bar (measured on the emulator with 474
characters in it -- the bar collapsed to its padding). Fixing that means
`Scroll` measuring against its own offered box, which is a change to a
widget the transcript and the bench shell both use, so it is its own
piece of work rather than a rider on this one.
## 2026-09-06 (defect pass)
- **The keyboard-open diagnostics overlay is gone; the capture only
logs now.** It was added when `on_insets_changed` was not firing at all
and there was no way to get a report off the phone. It fires reliably
since the activity went edge-to-edge -- and what that looks like in
use is a full-screen report covering the app **every time the keyboard
opens**, with its own Copy/Close buttons sitting underneath the
keyboard, so it cannot be dismissed (reproduced on the emulator this
pass: two `tap 'CLOSE'` runs left it up). An interruption for something
nobody asked for, over the app you are trying to type into. The named
`Diagnostics` button still shows the same text on demand, and the new
`iris surface:`/`iris insets:` log lines carry the lifecycle a `logcat`
pull needs. Reversible: `capture_keyboard_diagnostics` is still the one
place this is decided, and `PlatformHandle::show_diagnostics_overlay`
is still there.
- **The bench shell's report pane is sized to its report, not to a share
of the window.** It held `.height(rest(1))` beside the transcript's
`rest(2)`, so an *empty* `TextEdit` reserved a third of every screen --
which is what Iris's "the app does not start with keyboard spacing
correct" screenshot was showing, with the composer two thirds down and
black below it. It is `.max_height(dp(260))` now and sits above the
transcript rather than under the composer, where it was eating the
navigation-bar clearance. Cost: a filled report is clipped at 260dp
rather than scrolling (a `Scroll` there drew itself off the top of the
screen, since `Scroll` pins to the end of its content and reports its
content's full length to the parent -- worth fixing in `Scroll`, not
worked around here). "Copy report" and `logcat` still have the whole
thing.
## 2026-09-05 ## 2026-09-05
- **iris no longer asks every device for compute-shader limits it never - **iris no longer asks every device for compute-shader limits it never
+169 -2
View File
@@ -8,7 +8,147 @@ capability that moved. Small and trivial changes do not go here.
An entry gives the date, what changed, why, and a short before/after where An entry gives the date, what changed, why, and a short before/after where
it helps judge the change without the session that made it. Newest first. it helps judge the change without the session that made it. Newest first.
## 2026-09-06: `List::anchor_position_display`, `FrameReport::mark_phase`/`phase_stats`/`late_at_hz` (RUST.md's "Benchmark v2") ## 2026-09-06: a transcript row is a column of blocks, and a block is the selection unit
`transcript-ui`'s row builder used to make **one** `TextEdit` per message.
It makes one per top-level markdown block now -- heading, paragraph,
fenced code, list, table -- in a `Span::down`, because a streamed delta
into a single buffer re-shaped the whole message through parley on every
event. `client_core::markdown_blocks::split_blocks` does the splitting;
`row::RowBlocks::apply_delta` updates the block a delta lands in and
leaves the rest of the message's layout alone.
**The change to judge, since it is what a reader feels**:
`Selection` is keyed by `SelKey = (RowKey, u32)` -- a row and a block --
so **a block, not a row, is the unit a selection steps in**. A drag still
runs from a reply into the tool output beneath it and copies as one
thing; what changed is that the row under the finger is filled in block by
block rather than all at once, which is if anything closer to what the
old shortcut in `Selection`'s module doc was apologising for. `register`
takes a `SelKey`; `unregister` still takes a `RowKey` and now drops every
block of it (dropping only the first is how a freed widget gets left in
the map -- the shape docs/REVIEW-2026-09-06.md's finding 1 called out).
`Selection::locate(ui, render, pos_window)` is new: which block is under a
window position, with that block's own local position and size. The
list-level handler uses it for the pointer-captured half of a drag,
instead of computing a row-local position from `List::extent`.
`row::build_row` returns `(RowKey, StrongWidget, Option<RowBlocks>)` --
the third is the per-block state a caller keeps only for the row a reply
is streaming into, and is `None` for a tool run, which never streams.
## 2026-09-06: a reported `Size` may not carry `dp`; `Len::fold_dp`
**New: `Len::fold_dp(density) -> Len`** -- the same fold `apply_rest` does
(`dp` becomes physical pixels), but staying a `Len` so `rest` survives.
**New rule, and it is a rule about every widget, not about the two that
broke it**: a `Len` a widget *reports* from `draw` must not carry an
unresolved `dp`. `dp` is an input unit -- a number the widget author wrote
-- and the containers that consume a reported length read `abs`, `rel` and
`rest` straight off it (`Span`'s placement arithmetic, `Pad`'s addition),
so a reported `dp` is silently worth **zero**. `MaxSize` and `Sized` both
returned the caller's declared `Len` as written; a `.max_height(dp(168))`
therefore gave its child a slot of nothing the moment the cap actually
applied, which is what made the composer's bar collapse. Both put their
declared lengths through `fold_dp` now, and
`UiRenderState::draw_inner` `debug_assert!`s the invariant after every
`Widget::draw`, so a widget that gets this wrong says so at the mistake
rather than laying out at zero somewhere else.
Nothing changes for a caller: `.max_height(dp(48))` is written the same
way. It is only widget *authors* who now have a rule to follow, and a
debug build that enforces it.
## 2026-09-06: `Painter::set_mask` reuses one slot; `ActiveData` gains two fields
**`Painter::set_mask(region)` allocates its widget's mask slot once and
rewrites it in place** on every later draw, instead of pushing a new one
each time. It has to: `draw_inner`'s unchanged-region fast path does not
revisit a descendant whose own region did not change, so those descendants
go on referencing whichever slot they were first drawn under. Pushing a
fresh slot per draw left the composer's field clipped to a box the bar had
long since moved away from -- four live mask entries, none of them the
`Masked`'s current region -- and it drew nothing at all. Same call, same
signature; only the lifetime changed.
**`ActiveData` gains `own_mask` and `move_applied`** (both public, since
`ActiveData` is). `own_mask` is the slot above, `MaskIdx::NONE` for a
widget that sets no mask. `move_applied` is how much of a widget's own
move-slot delta its `region` already accounts for: `mov` shifts both,
`Painter::reposition` shifts only the slot, and `resolved_region` -- and so
every hit test -- has to subtract it. Without that a widget that had been
panned had its *own* hit box at twice the pan while its descendants were
correct, which made the composer's field untappable after a finger drag.
## 2026-09-06: `Scroll` pans on a finger drag, and a vertical drag in a focused text field no longer selects
Three related public changes, all in aid of IRIS_TODO.md's "the composer
has no touch-drag scroll".
**`Scroll::drag(render, id, sense, pos_window, now)` is new**, and
`WidgetLike::scrollable()` now registers it alongside the wheel handler it
already registered -- so anything built with `.scrollable()` pans on a
finger drag with no extra wiring at the call site. It goes through the same
`sense::DragGesture` that `transcript-ui::Selection::drag` drives `List`
with (arbitration, `DRAG_SLOP`, velocity, pointer capture), rather than a
second copy of that widget's wiring: `DragGesture` owns the mechanics and
each caller decides only what a committed pan *means*. `Scroll::amt()` is
new too, the read-only pan position a test or a scroll indicator needs.
There is deliberately **no fling** on `Scroll`. Unlike `List` it has no
per-frame tick to animate one with (`List::set_redraw_handle`/`tick_fling`),
and the areas it wraps today are at most a screenful, where Android does not
fling either. The released velocity is dropped rather than approximated.
**A vertical drag inside an already-focused `TextEdit` no longer extends a
selection.** `iris::attr`'s `on_press` used to treat a focused field as the
plain `click_or_drag` case -- every `Pressing` frame updated the selection.
It now applies the same `DRAG_SLOP` rule the *unfocused* branch already
applied: a press that moves past the slop vertically abandons its pending
selection for the rest of the gesture, so the scroll area around the field
gets the drag instead. Horizontal drag-to-select is unchanged, and a long
press still starts a selection. This is Android's own `EditText` behaviour
(a vertical drag scrolls; only a long press selects), and it is what makes
"swipe up over the composer to scroll the transcript" work without dragging
a highlight through the message you were typing.
**`UiRenderState::orphaned_primitives()` is new**, and `update` now
`debug_assert!`s (debug builds only) that nothing is orphaned. An orphan is
a primitive still bound for the GPU that no live `ActiveData` names -- a
copy nothing can move, clip or free. That was the doubled `Compacted:` row
on the phone; see the same date's commit `76b1f99` and docs/RUST.md. The
per-frame guard is a count comparison (O(active widgets)); the walk that
names the offenders only runs when the counts disagree, because the walk is
O(primitives) and made a debug build on a phone too slow to finish a
benchmark run.
## 2026-09-06: a tap on a text field always leaves a caret
`TextEditCtx::select` used to compare the tap position against the
*laid-out text's* own box and set `selection = None` for anything outside
it. A press only reaches `select` after being hit-tested to the widget, so
that "outside" meant the field's own padding -- or, for an **empty** field,
everything, since an empty layout is a zero-width box. So tapping an empty
composer focused it and opened the keyboard while leaving no caret, and
`TextEditCtx::insert`/`insert_str` return early with no caret: every
keystroke was dropped in silence, and no glyph ever appeared. Parley's
`from_point`/`extend_to_point` already clamp a point outside the layout to
the nearest cursor position, which is also what a tap in a field's padding
should do.
Behaviour change a caller would notice, in one line: **`select` with a
non-drag position now always produces a selection; it no longer clears
one.** Clearing is `TextEditCtx::deselect`, which is what the backends'
focus handling already calls. A drag is unchanged -- with no previous
selection there is still nothing to extend, so it produces none.
`insert_str` also gained a `debug_assert!` for the no-caret case, so an
insert routed to an unfocused field fails at the mistake in a debug build
instead of silently swallowing input.
## 2026-09-06: `List::anchor_position_display`## 2026-09-06: `List::anchor_position_display`, `FrameReport::mark_phase`/`phase_stats`/`late_at_hz` (RUST.md's "Benchmark v2")
`List` gained `anchor_position_display(&self) -> String`, reporting the `List` gained `anchor_position_display(&self) -> String`, reporting the
anchor's own row index and pixel offset (`idx=N/off=Mpx`, or anchor's own row index and pixel offset (`idx=N/off=Mpx`, or
@@ -552,7 +692,14 @@ streamed event" cost RUST.md's P0 box measured (20 events/second against a
new rows appended after it. A row changing *before* the tail (only new rows appended after it. A row changing *before* the tail (only
`group_tool_runs` retroactively grouping tool calls into a run does `group_tool_runs` retroactively grouping tool calls into a run does
this) falls back to `List::clear` plus a full rebuild, counted in this) falls back to `List::clear` plus a full rebuild, counted in
`TranscriptScreen::take_rebuilds()`. `bench_client.rs`, `transcript_client.rs` `TranscriptScreen::take_rebuilds()`. **A caller that keeps its own
row-keyed side table alongside `List` (`Selection`'s `rows:
BTreeMap<RowKey, WeakWidget<TextEdit>>` is the one this crate has) must
clear it in step with `List::clear()`** — the fallback drops every row
`List` was holding, so any side table not cleared the same way is left
pointing at widgets the clear just freed (docs/REVIEW-2026-09-06.md
finding 1, fixed 2026-09-06 by `Selection::clear()`, called from
`apply`'s `Rebuild` arm right before `List::clear()`). `bench_client.rs`, `transcript_client.rs`
and `desktop-app/app.rs` all call this now instead of rebuilding on every and `desktop-app/app.rs` all call this now instead of rebuilding on every
event; only the opening page (and `apply`'s own fallback) still calls event; only the opening page (and `apply`'s own fallback) still calls
`build_tree`. `build_tree`.
@@ -680,3 +827,23 @@ still not root-caused).
both CPU-side caches otherwise kept pointing at the old, now-destroyed both CPU-side caches otherwise kept pointing at the old, now-destroyed
device's textures, which is why text used to vanish again after leaving device's textures, which is why text used to vanish again after leaving
and returning to the app. and returning to the app.
## 2026-09-06: `take_counters` counts text layouts too
One public API change, from the verification pass over the composer-scroll
and per-block-row work (RUST.md's "Verification pass over Tasks A and B").
- **`UiRenderState::take_counters` returns four numbers, not three**:
`(draws, region rewrites, move writes, **text shapes**)`. The new one is
bumped in `Painter::render_text`, which `TextView::render` only reaches
on a cache miss, so it counts layouts actually computed rather than
layouts asked for. Callers destructuring the tuple need one more `_`.
It exists because a draw counter cannot answer the question the
per-block transcript row was built for. A widget can be redrawn without
re-shaping (the layout is memoized by width) and re-shaped without any
extra draw, and re-shaping is the expensive half — so "a streamed delta
costs one block" was, until now, argued from the code rather than
measured. With the counter it is a test: one delta into a 100-paragraph
reply shapes exactly **1** text layout, the same as into a
one-paragraph one.
+160 -24
View File
@@ -162,8 +162,33 @@ agent takes them without colliding with that pass's `bench_client.rs`/
genuinely new renderer -- see IRIS.md's 2026-09-06 entry and RUST.md's genuinely new renderer -- see IRIS.md's 2026-09-06 entry and RUST.md's
P0 box, item 4. Verified on the emulator (home, reopen, screenshot); P0 box, item 4. Verified on the emulator (home, reopen, screenshot);
not yet on the phone. not yet on the phone.
- [ ] **Composed/typed text never becomes visible at all -- found - [x] **Composed/typed text never becomes visible at all -- root-caused
2026-09-06, not fixed.** The composer bar stays empty even once the and fixed 2026-09-06.** Not the renderer at all: **the composer's buffer
was empty the whole time.** `TextEditCtx::select` (`iris/src/widget/
text/edit.rs`) compared the tap against the *laid-out text's* box and
set `selection = None` for anything outside it -- and an empty field's
layout is a zero-width box, so tapping an empty composer granted focus
and opened the keyboard while leaving no caret; `insert_str` returns
early with no caret, so every keystroke after that was dropped in
silence. Gboard's suggestion strip is its own composing state, not a
read of our buffer, which is what made the earlier pass conclude the
buffer held the text. Fixed by letting parley clamp a tap outside the
layout to the nearest cursor position (a press that reaches `select`
has already been hit-tested to the widget, so there is no "outside"),
plus a `debug_assert!` in `insert_str` so an insert with no caret fails
at the mistake instead of dropping input -- it immediately caught
`layout_tests::composing_text_after_a_keyboard_resize_...` typing into
an unfocused field. Three new tests in `edit.rs`
(`tapping_an_empty_field_places_a_caret_so_typing_lands`,
`tapping_past_the_end_of_the_text_clamps_to_the_end`,
`dragging_without_a_previous_selection_selects_nothing`); the first
fails on the pre-fix code. Emulator evidence: `adb shell input text`
after `tap 'Message'` now shows the text in the bar
(`/tmp/final-typing.png`) and logs `iris text render: chars=5 ...
glyphs=5`, against `glyphs=0` on every keystroke before.
**The old, superseded diagnosis, kept because it was wrong in an
instructive way:** The composer bar stays empty even once the
buffer genuinely holds the typed text (confirmed indirectly: Gboard's buffer genuinely holds the typed text (confirmed indirectly: Gboard's
own suggestion strip reacts correctly to each keystroke). A new unit own suggestion strip reacts correctly to each keystroke). A new unit
test proves the widget tree's own layout math resolves the field's test proves the widget tree's own layout math resolves the field's
@@ -176,12 +201,38 @@ agent takes them without colliding with that pass's `bench_client.rs`/
a capped/scrollable height, bottom padding tied to the IME/nav-bar a capped/scrollable height, bottom padding tied to the IME/nav-bar
inset) -- structurally in place and unit-tested, but its own visual inset) -- structurally in place and unit-tested, but its own visual
correctness cannot be screenshotted until text actually renders. correctness cannot be screenshotted until text actually renders.
- [ ] **The composer has no touch-drag scroll for overflowing text.** The - [x] **The composer has no touch-drag scroll for overflowing text.**
2026-09-06 rebuild caps the field at ~6 lines and wraps it in **Done 2026-09-06.** `field.scrollable().masked()` in
`.scrollable()` for a wheel/trackpad scroll, but a real finger drag over `transcript-ui/src/composer.rs`: a finger drag inside the bar pans the
text that has overflowed the cap does not scroll it -- `Scroll`'s touch message, the bar stays capped at six lines, and a vertical drag in the
handling is a follow-up, the same shape `List`'s own touch-drag pan focused field no longer extends a selection (Android `EditText`'s own
needed before I3/I5. behaviour). Verified on this checkout's emulator with the
`transcript-screen bench force-gles` debug build -- six repetitions of a
13-word sentence typed in, then
`ui-trace record --do "swipe 540 1200 540 1460 300"`: the field's
`Message` box moved `31,1041..1048,1509` -> `31,1131..1048,1651` (the
content panned down with the finger) with its **height unchanged at
468px** (the bar did not grow), and the two screenshots either side show
different text in the same band.
Three real defects had to be fixed first, each with a headless
regression test in `iris/src/layout_tests.rs` and each confirmed to fail
without its fix (docs/RUST.md's plan box has the measurements):
a `MaxSize` reporting its cap as an unresolved `dp` (`Len::fold_dp`), a
`Masked` allocating a fresh mask slot per draw (`ActiveData::own_mask`),
and a panned widget's own hit box moving twice (`move_applied`).
`Scroll` itself turned out to measure the right number by a misleading
route -- it is written against `painter.px_size()` now, and the claim
below that it "measures against the window" was wrong.
**The grey background was not missing** -- that note (written here on
2026-09-06 and repeated as still open) is withdrawn. Re-measured the
same day on the same AVD by decoding the screencap rather than reading
it: the bar is `rgb(41,40,49)`, the declared `UiColor::new(40, 40, 46)`
after sRGB rounding, **full width and y2245..y2365** on 1080x2424, with
the field at `31,2277..1048,2329` and the 63px nav strip below it. It
is dark by design and sits on black, which is very likely what the
earlier reading was: at a glance the band and the background are hard
to tell apart. If it should read as a bar rather than as a slightly
different black, the colour is the thing to change, not the tree.
## From the phone, 2026-09-06, 11:39 (build delivered 02:07, commit 543f6d9) ## From the phone, 2026-09-06, 11:39 (build delivered 02:07, commit 543f6d9)
@@ -189,8 +240,28 @@ Iris's report on the build with the composing-text, tap-vs-swipe and
atlas-reset fixes, with a screenshot, verbatim. Each is open until an atlas-reset fixes, with a screenshot, verbatim. Each is open until an
agent ticks it here with the evidence. agent ticks it here with the evidence.
- [ ] **"The app definitely does not start with keyboard spacing - [x] **"The app definitely does not start with keyboard spacing
correct. This is how it looks without me doing anything initially."** correct. This is how it looks without me doing anything initially."**
**Not an inset bug at all -- fixed 2026-09-06.** The black third is the
bench shell's own empty *benchmark report* pane: `bench_client.rs`'s
root tree gave it `.height(rest(1))` beside `content.height(rest(2))`,
so an empty `TextEdit` reserved a third of the window at every launch
and pushed the composer up by exactly that. Measured on this checkout's
emulator at the phone's own size (1080x2424, density 420, gesture nav),
which reproduced Iris's screenshot exactly: new `iris insets:` log line
reported `bottom=63 ime_bottom=0` at launch (a nav bar, no keyboard --
so the inset the composer was fed was never large), while `ui-trace
show -m Message --field box` put the field at `31,1488..1048,1540` on a
2282px-tall surface, 789px clear of the bottom -- that pane's third.
**Unit mixing checked explicitly and cleared**: `set_bottom_inset` takes
physical px and stores `Len::abs`, `MainActivity.java`'s `1`/`0`
`ime_bottom` only ever reaches `insets.bottom.max(ime_bottom)` and
`> 0.0`, and every `dp` in the composer resolves at layout time. Fix:
the report pane is sized to its content (`.max_height(dp(260))
.scrollable()`), and moved above the transcript so it cannot eat the
composer's nav-bar clearance. After: field box `31,2277..1048,2329`,
grey bar ending at device y2361 with the 63px nav strip below it
(`/tmp/fix1.png` this pass).
The screenshot shows the composer bar (the grey band) sitting about The screenshot shows the composer bar (the grey band) sitting about
two thirds of the way down a 704x1568 screen, with black below it to two thirds of the way down a 704x1568 screen, with black below it to
the bottom, and the transcript ending at "Claude / Results" just above the bottom, and the transcript ending at "Claude / Results" just above
@@ -202,19 +273,69 @@ agent ticks it here with the evidence.
field the composer may still read as pixels or dp; a stale value from field the composer may still read as pixels or dp; a stale value from
before the first `on_insets_changed`. Reproduce with the phone's before the first `on_insets_changed`. Reproduce with the phone's
screen size and density on the emulator before guessing. screen size and density on the emulator before guessing.
- [ ] **"Swiping still gets caught by the grey bar but keeps working - [~] **"Swiping still gets caught by the grey bar but keeps working
after I go past it."** A pan that starts on the composer is held by after I go past it."** Improved 2026-09-06 by the focused-field rule
the composer until the finger leaves its region, then the list takes below, still needs her phone to close. `attr.rs`'s `on_press` treated an
over. The tap-vs-swipe fix in `attr.rs` stops the *focus*, but the already-focused composer as the plain drag-to-select case, so a swipe
press frames are still being handled by the field rather than passed starting inside it dragged a highlight through the typed text for the
to the list from the first slop-crossing frame. The `DragGesture` whole gesture; it now abandons that the moment the press passes
merge (RUST.md's plan box) should make this one mechanism: once a `DRAG_SLOP` vertically (Android `EditText`'s own rule), which removes one
gesture commits to a pan, the list captures it wherever it began. of the two things that made the bar feel like it caught the swipe. The
- [ ] **"Flinging still does not work."** Expected on this build: finger residual `DRAG_SLOP` measured from the boundary crossing, described
flings are dropped by per-widget hit testing, which `DragGesture`'s below, is unchanged. Original note follows.
pointer capture (commit `e12c708`, not yet merged at 02:07) targets. Not closeable from the emulator, annotated
Stays open until verified on her phone, not the emulator. 2026-09-06 after the `DragGesture` merge. `attr.rs`'s `on_press` never
- [ ] **"Text still disappears if I leave and come back to the app."** calls `capture_pointer` and never consumes a `Pressing` frame past
`DRAG_SLOP` (it just stops watching), so once the finger's *current*
position leaves the composer's box and enters the list's, `List`
starts receiving ordinary hit-tested `Pressing` frames there --
`DragArbiter::is_idle()`'s 2026-09-05 recovery (a missed `PressStart`)
picks it up rather than leaving it stuck. What this does **not** do is
what "wherever it began" implies literally: `DragArbiter::press_start`
restarts from the *boundary-crossing* position, not from the original
touch-down inside the composer, so the pan still needs a fresh
`DRAG_SLOP` of travel measured from the boundary rather than from the
start of the gesture -- composer and list are adjacent, non-overlapping
widgets (`lib.rs`'s `(list, composer_bar).span(Dir::DOWN)`), and only
the composer forwarding its own drag to the list would remove that
residual slop entirely, which is more than this pass's merge changes.
RUST.md's merge-pass box has the reasoning in full and an emulator
swipe confirming the composer's own box never moves/resizes during it;
whether the residual slop is still perceptible as "caught" needs Iris's
phone, since the emulator's per-widget boundary is a few dp wide and
easy to cross without noticing on a real screen too.
- [ ] **"Flinging still does not work."** No longer expected to reproduce
after the `DragGesture` merge (`e12c708`, pointer capture +
`CursorSense::Drop`), 2026-09-06. Emulator evidence (RUST.md's
merge-pass box, check (b)): a real `ui-trace` finger swipe followed by
screenshot-hash sampling caught a post-release frame distinct from the
drag's own last frame in one run, and every run showed 28-32
`render()` frames per gesture against an idle baseline of 0 and ~8
expected from the drag alone -- redraw kept being requested well past
the finger lifting, which only happens while a fling is still
animating. Left unticked in spirit until Iris's phone confirms it,
since only she can say whether it *feels* like a fling now; the
emulator's screenshot timing could not always catch the tail of a
fast-settling one visually (same caveat noted in RUST.md).
- [~] **"Text still disappears if I leave and come back to the app."**
**Instrumented 2026-09-06 so the phone can answer it**, since no
emulator here has a Vulkan adapter. `iris/src/android/view.rs` now logs
one `log::info!` line per surface event with the glyph/atlas counts:
`iris surface: surface_destroyed, tearing the renderer down
(glyphs_cached=387 atlas_pages=1)`, `iris surface: surface_changed
1080x2424 already_live=false glyphs_cached=387 atlas_pages=1`, `iris
surface: new renderer built (Gl), clearing glyph atlas: glyphs=387
pages=1`, plus `iris insets: ... window=(1080, 2424)` on every insets
change. That is the emulator's own healthy app-switch cycle, verified
this pass (home, reopen, screenshot: all text intact,
`/tmp/appswitch.png`). **The one line to look for on the phone is
`already_live=`**: `true` on the return from backgrounding would mean
the surface came back *without* a `surface_destroyed`, so
`surface_changed` reconfigured a renderer whose Vulkan swapchain and
atlas textures belong to a window that is gone -- the reuse branch
never clears the atlas, by design. `false` with no `new renderer built`
line after it would mean the renderer failed to rebuild. Either answer
names the fix; guessing between them from here does not.
The `GlyphAtlas::clear`/`Textures::reset` fix was verified on the The `GlyphAtlas::clear`/`Textures::reset` fix was verified on the
emulator under `force-gles` only; the phone runs Vulkan. So either the emulator under `force-gles` only; the phone runs Vulkan. So either the
reset is not reached on the phone's path (a different surface- reset is not reached on the phone's path (a different surface-
@@ -528,8 +649,23 @@ do not duplicate it there.
## From the phone, bench v2 (2026-09-06): streaming re-lays out the whole message ## From the phone, bench v2 (2026-09-06): streaming re-lays out the whole message
- [ ] **Streaming a delta into a long message costs a full text layout of - [x] **Streaming a delta into a long message costs a full text layout of
that message.** Iris's phone report (`docs/bench/iris-phone-v2-2026-09-06.md`): that message.** **Done 2026-09-06** -- a row is a column of one
`TextEdit` per markdown block (`client_core::markdown_blocks`,
`row::RowBlocks::apply_delta`), so a delta re-shapes the last block and
keeps every earlier block's layout. A block is the selection unit now
(`Selection`'s `SelKey`); selection across blocks and rows still works,
checked on the emulator with a real long-press drag. Pass condition met
in `a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one`:
a delta into a 100-paragraph reply redraws the same widget count as one
into a one-paragraph reply (30 either way). Emulator stream phase, same
AVD before and after: **p50 61.5 -> 54.5ms, p90 211.7 -> 113.1ms, p99
342.6 -> 137.4ms, worst 403.6 -> 143.0ms**, 202 -> 293 frames in the same
21 seconds. docs/RUST.md's Task B box has the detail and the two dead
ends. **The phone is the measurement that decides it** -- these are
emulator numbers and only the ratio transfers.
The original entry, for the record: Iris's phone report (`docs/bench/iris-phone-v2-2026-09-06.md`):
the stream phase is the one place iris is behind Compose (p50 18.2 ms vs the stream phase is the one place iris is behind Compose (p50 18.2 ms vs
13.4 ms; p99 level at ~43 ms). `TranscriptScreen::apply` replaces only 13.4 ms; p99 level at ~43 ms). `TranscriptScreen::apply` replaces only
the last row, but that row is the growing message, and replacing it the last row, but that row is the growing message, and replacing it
+214
View File
@@ -0,0 +1,214 @@
# Review: iris changes since 0e46293
Scope: `git diff 0e46293..HEAD -- iris/ client-core/` (58 files, +5224/-226).
Read-only review; no source changed. Ordered likely-bug, then invariant
guards, then rules, then tests/docs.
## Likely bugs
1. **`iris/transcript-ui/src/lib.rs:152-160` (`RowDiff::Rebuild` arm of
`TranscriptScreen::apply`) never unregisters the rows it drops from
`Selection`, so a stale `WeakWidget<TextEdit>` outlives the widget it
points to and the next touch on *any* row panics.**
`Selection::rows: BTreeMap<RowKey, WeakWidget<TextEdit>>` documents its
own contract at `selection.rs:69-71`: "every addition here needs its
removal ... called when `List` evicts the row." The `ReplaceLast` arm
above it honours this (`lib.rs:143-145`, `self.selection.borrow_mut()
.unregister(old_key)` when the key changes). The `Rebuild` arm calls
`(self.list)(rsc).clear()` and rebuilds every row from `new_rows`, but
never touches `self.selection` — any key present in `old_rows` and
*absent* from `new_rows` (exactly what `group_tool_runs` regrouping two
separate tool-call rows into one produces — see `diff_tests::
a_tool_run_closing_and_joining_an_earlier_call_is_a_regroup_fallback`,
which tests the diff decision but not `apply` itself) is left in
`self.rows` pointing at a widget `List::clear()` just freed.
`TextEditable::edit` (`iris/src/widget/text/edit.rs:582-587`) resolves
that handle with `ui.widgets.get_mut(self).unwrap()` — an unconditional
panic on the freed slot. `Selection::begin` (`selection.rs:88-101`)
iterates *every* registered row (`w.edit(ui).deselect()`) on an
ordinary fresh press, so the crash fires on the next tap anywhere in
the transcript after a regroup, not only on a tap targeting the
orphaned row.
Fix: give `Selection` a way to reconcile against the row set that
survived a rebuild (e.g. `Selection::retain(&self, keys: &BTreeSet<RowKey>)`
removing everything else, called from the `Rebuild` arm before
rebuilding), or simplest — call `self.selection.borrow_mut()` cleared
the same way `List::clear()` clears the list, then let the rebuild's
`push_row` calls re-`register` everything as they already do.
## Guarded invariants missing
2. **`iris/src/widget/list.rs:751` (`List::place`) indexes/expects on
`slot` with no assertion that it exists.** `slot_widget` (`:563-575`)
panics via `.expect(...)` for a sentinel with no widget set, and does
an unchecked `&self.items[s as usize]` for a real index — a bare
"index out of bounds" with no context if `place` is ever reached with a
stale slot. Every current caller happens to derive `slot` from
`repair_anchor`/`prev_slot`/`next_slot`, which already check existence,
but that invariant is enforced by convention across three call sites,
not by the function that depends on it. Add
`debug_assert!(self.slot_exists(slot), "place() called with a slot that doesn't exist: {slot:?}");`
at the top of `place`.
3. **`iris/src/widget/list.rs:426` (`List::fling`) and `sense.rs`'s
`FlingCalculator::distance`/`duration`/`position_at` never check that
the incoming velocity is finite.** A `NaN`/`inf` velocity (a
`VelocityTracker::velocity()` divide-by-near-zero span, or a caller
passing a raw device value straight through) propagates through
`deceleration_for`'s `.ln()` silently — the fling either never settles
(`settled_on_schedule` compares against a `NaN` `duration()`, which is
always `false`) or jumps to `NaN` positions with nothing on screen
saying why. Add `debug_assert!(velocity_px_per_s.is_finite())` in
`List::fling` and `FlingCalculator::new`/`distance`.
4. **`iris/src/sense.rs:592-604` (`VelocityTracker::velocity`) has no
assertion that samples are chronological.** `add_sample` trusts its
caller's `Instant` ordering; a caller that samples out of order (a
restored/replayed gesture, a test) would silently produce a negative
`span` handled only by the `span <= 0.0 => 0.0` catch-all, masking the
bug that produced it rather than surfacing it. Add
`debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last))`
in `add_sample`.
5. **`iris/core/src/render/frame_report.rs:247-252` (`mark_phase`) has no
assertion that phases are pushed in non-decreasing `start_index`
order.** `phase_stats`'s slicing (`:274`, `idx >= phase.start_index &&
idx < end_index`) silently produces an empty or nonsensical slice for
an out-of-order phase rather than surfacing the misuse — cheap to add
given `self.phases.last()` is already in scope:
`debug_assert!(self.phases.last().is_none_or(|p| self.total_frames >= p.start_index));`
## Rules
6. **Two mechanisms answer "what row selection points at, still valid?"**
`Selection` relies on callers remembering to `unregister` (finding 1);
`List` relies on callers deriving slots only from already-checked
sources (finding 2). Both are the same class of problem — a derived
handle that silently outlives what it points to — solved ad hoc twice
rather than once. Not asking for a shared abstraction here, but the two
should at minimum cross-reference each other's doc comment so the next
caller who adds a third handle-into-`List`-rows type (the code rules'
"a rule that governs a set belongs to the set") finds both existing
examples.
7. **`iris/android-app/src/bench_client.rs:224-225` (`battery_line`)
calls `.min().unwrap()`/`.max().unwrap()` on `samples` guarded three
lines above by `if samples.is_empty()`, which is fine — but the guard
and the two unwraps are two statements apart with a `let mean = ...`
in between reading the same slice; a future edit reordering those
lines loses the guard's protection silently.** Low severity (this is
the bench tool, not the app), but worth a one-line comment tying the
unwraps back to the guard, or restructuring as
`let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max())`
pattern so the empty case can't be separated from the check by a future
edit.
## Tests
8. **No test exercises `TranscriptScreen::apply`'s `Rebuild` arm through
`Selection`.** `lib.rs`'s `diff_tests` module (`:284-379`) tests only
the pure `diff_rows` decision function, never `apply` itself wired to a
real `Selection`; `selection.rs`'s own tests (`a_missed_press_start_
recovers_on_the_next_pressing_frame`, `unregister_forgets_the_row_and_
clears_a_matching_anchor`) never go through `apply`/`List::clear`
either. This is exactly the gap that let finding 1 through: the two
pieces (`apply`'s fallback, `Selection`'s registration contract) are
each tested in isolation and never together. Add: build a
`TranscriptScreen`, force a `RowDiff::Rebuild` (two adjacent tool-call
rows regrouping, per the existing `diff_tests` case), then call
`selected_text`/simulate a fresh press on a surviving row and assert no
panic.
9. **`iris/src/widget/list.rs`'s fling tests check total distance and the
start/end clamp but not the speed profile in between.**
`fling_moves_the_list_and_then_settles`/`fling_distance_is_positive_
toward_the_end` only assert the fling started, moved in the right
direction, and eventually stopped — none checks that
`tick_fling`'s per-tick delta is *monotonically decreasing* once past
the fling's peak (the property `fling_calculator_tests::position_at_
is_monotonic_and_clamped_past_the_end` already checks one level down,
for `FlingCalculator` alone, but never through `List::tick_fling`'s own
`scroll`/`anchor.offset` accumulation). A regression that made
`tick_fling` apply the *total* distance every tick instead of the
incremental one, for instance, would still pass both existing tests
(final position and direction are unaffected by how the interior ticks
split it up) while being wildly wrong every intermediate frame.
10. **`iris/src/widget/list.rs::replacing_the_last_row_stays_pinned_to_
the_bottom` and its sibling test `replace_back`'s effect on the
displayed row, never that the row it evicted is actually gone from
`heights`/`extents`.** Both tests assert the *new* row's position;
neither asserts `old.key` is absent from `list_ref.heights`/`extents`
after the replace (the "stale primitive" class finding 1 is a
production instance of). A cheap addition: assert
`!list_ref.heights.contains_key(&old.key)` after `replace_back` in the
existing test, since `old.key` is already returned to the test as
`evicted`... (`lib.rs` calls it that way; the `list.rs` test would need
to capture the key from `old` similarly.)
## Docs
No missing `IRIS.md` entry found for a *public* API change in this diff —
`List::fling`/`VelocityTracker`/`FlingCalculator`, `List::
anchor_position_display`, `FrameReport::mark_phase`/`phase_stats`/
`late_at_hz`, `UiRenderNode::new`'s `Result` change, `Len::dp`, and
`List::replace_back`/`clear`/`TranscriptScreen::apply` all have entries.
The `List::replace_back`/`clear`/`TranscriptScreen::apply` entry
(`docs/IRIS.md:526`) predates this review's finding 1 and does not mention
`Selection`'s registration contract at all — once finding 1 is fixed,
that entry should gain a line noting what the fix requires of a caller
that keeps its own row-keyed side table (the same shape `Selection` is),
so the next such table doesn't reproduce the same gap.
## Fixed, 2026-09-06
All ten findings addressed after the `DragGesture` merge (`selection.rs`
was rewritten by that merge, but finding 1's shape and location were
unchanged — `TranscriptScreen::apply`'s `Rebuild` arm, `iris/transcript-ui/
src/lib.rs`).
1. **Fixed.** `Selection::clear()` (`selection.rs`) drops `rows` and
`anchor`, called from `apply`'s `Rebuild` arm right before
`List::clear()` — `push_row` re-`register`s whatever survives as it
rebuilds each row, the "simplest" fix option the finding named.
2. **Fixed.** `debug_assert!(self.slot_exists(slot), ...)` at the top of
`List::place` (`iris/src/widget/list.rs`).
3. **Fixed.** `debug_assert!(velocity_px_per_s.is_finite())` in
`List::fling`, and `debug_assert!(velocity.is_finite())` in
`FlingCalculator::distance`/`duration` (`iris/src/sense.rs`).
`position_at` calls both, so it inherits the guard rather than needing
its own.
4. **Fixed.** `debug_assert!` on chronological sample order in
`VelocityTracker::add_sample` (`iris/src/sense.rs`).
5. **Fixed.** `debug_assert!` on non-decreasing `start_index` in
`FrameReport::mark_phase` (`iris/core/src/render/frame_report.rs`).
6. **Fixed (doc cross-reference only, as asked).** `Selection::register`'s
doc now points at `List::place`'s `slot_exists` assertion and vice
versa isn't needed since finding 2's fix already cites this file in
its own comment; both are grep-able on "docs/REVIEW-2026-09-06.md" and
on each other's type names.
7. **Fixed.** `bench_client.rs::battery_line` restructured to
`let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max())`,
so the empty-guard and the two lookups can no longer be separated by a
future edit.
8. **Fixed.** `transcript-ui`'s new `apply_tests::
a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection`
(`lib.rs`) builds a real `TranscriptScreen`, forces the same regroup
shape `diff_tests` already covers at the pure-diff level, calls `apply`,
and then `Selection::begin` on a surviving row — which panicked before
fix 1, resolving a `WeakWidget` `List::clear()` had just freed.
9. **Fixed.** `list.rs`'s new `tick_fling_applies_shrinking_incremental_
deltas` flings toward the end from `jump_to_start` and asserts each
tick's `extents[&0]` delta is no larger than the previous one — would
fail against a `tick_fling` that applied the total spline distance
every tick instead of the incremental slice, which the two pre-existing
fling tests cannot catch.
10. **Fixed.** `list.rs`'s new `replace_back_forgets_the_evicted_keys_own_
height` replaces row 4 with a row keyed `100` (the two existing
`replace_back` tests always reuse the same key, so neither actually
exercises the removal) and asserts `heights` no longer contains the
evicted key.
Docs: `docs/IRIS.md`'s 2026-09-05 `List::replace_back`/`clear`/
`TranscriptScreen::apply` entry now has a line on what the fix requires of
a caller with its own row-keyed side table, naming `Selection` as the
example and dating the fix.
Verification run alongside the rest of this pass's checks: `cargo fmt
--all`, `cargo clippy --workspace --all-targets`, `cargo test --workspace`
from `iris/` — see docs/RUST.md's plan box for the pass/fail and any
caveats from this same session.
+545 -34
View File
@@ -43,27 +43,340 @@ gated on her verdict**, so this pass works the P0 defects and the pure
prerequisites in this order. Each item is ticked here by the agent that prerequisites in this order. Each item is ticked here by the agent that
closes it. closes it.
- [ ] **Merge the `DragGesture` work** left complete but unmerged in the ### Task A, closed 2026-09-06: the composer scrolls on a finger
worktree branch `worktree-agent-a754368325fa06839` (commit
`e12c708`, 2026-09-06 02:10, two minutes after the last merge to `iris/transcript-ui/src/composer.rs` is `field.scrollable().masked()` now.
`rustify`; already contains `rustify` at `543f6d9`). It targets two Verified on this checkout's emulator -- the evidence and the numbers are
of the four bench-v2 defects: finger flings dropped by per-widget in docs/IRIS_TODO.md's ticked "composer has no touch-drag scroll" item.
hit testing (pointer capture + `CursorSense::Drop`), and IME insets
never redelivered (`MainActivity.java` edge-to-edge). Before **The premise the task was given under was wrong, and that is worth
merging: build, clippy, tests; then on the emulator confirm the recording**: `Scroll` did *not* measure against the window. Its
three things `20b1225` (tap-vs-swipe focus) and the phone asked for `used.within_len(container).to_abs(output_size)` came to exactly
still hold together a swipe over the composer does not summon `abs + rel * container_px` -- the right number by a route that reads as if
the keyboard, a tap does, a finger fling on the list keeps moving the window were the container, which is what cost a session. It is
after the finger lifts, and `on_insets_changed` now fires on an `painter.px_size()` and `to_abs(container_len)` now: same arithmetic,
IME toggle. Then remove the worktree. stated the way the invariant is. `Scroll` also still reports its
- [ ] **Fix `docs/REVIEW-2026-09-06.md`** (after the merge, since the **content's** size upward, deliberately -- reporting the container makes
review's finding 1 is in `selection.rs`, which the merge rewrites). the answer a function of itself (the bar is sized *from* that report, so
Finding 1 is a real crash — `TranscriptScreen::apply`'s `Rebuild` it collapses to nothing and never recovers; measured in the headless
arm leaves `Selection` holding `WeakWidget`s to freed rows, and the harness before the shape was settled).
next tap anywhere panics. Findings 25 are `debug_assert!`s on
invariants, 8 and 10 are the missing tests. Commit the review file What actually broke the composer was three separate defects, each now
with the fixes. carrying a headless regression test in `iris/src/layout_tests.rs` that was
- [ ] **Iris's 11:39 phone report on the 02:07 build** (four items, confirmed to fail without its fix:
1. **A `MaxSize` reported its cap as an unresolved `dp`.**
`Span::draw` places a child from the `abs`/`rel` of the length it
reported, so `dp(168)` was worth **zero** and the bar got a slot of
nothing the instant its content passed six lines; the `Scroll` inside
then measured its container at **-63px** (the padding subtracted from
nothing) and panned the whole message out of view. Emulator log, before
the fix: `container=-63 content=415.8 amt=478.8`. Fixed by
`Len::fold_dp` (new), used by `MaxSize` and `Sized` on the way out, and
guarded for every widget by a `debug_assert!` in
`UiRenderState::draw_inner` that a reported `Size` carries no `dp`.
Test: `a_dp_cap_is_reported_in_pixels_so_a_span_can_place_it`.
2. **A `Masked` allocated a fresh mask slot on every draw.**
`draw_inner`'s unchanged-region fast path means its descendants are
mostly *not* redrawn with it, so they kept clipping against the slot
they were first drawn under -- measured on the composer's tree at
**four live mask entries, none of them the widget's current box**, and
the field drew nothing at all. The slot is allocated once and rewritten
in place now (`ActiveData::own_mask`, `Painter::set_mask`), with its
path out in `remove`'s `undraw` branch. Test:
`a_masked_widget_keeps_one_mask_slot_that_is_always_its_own_region`.
3. **A panned widget's own hit box moved twice.** `mov` updates
`active.region` *and* accumulates the same delta on the widget's move
slot, and `resolved_region` added both -- so after a finger pan the
composer's field was untappable, while its descendants were fine (which
is why `hit_testing_follows_a_scrolled_widget`, which checks a
descendant, never saw it). `ActiveData::move_applied` records the part
of the slot's delta `region` already accounts for. Test:
`a_panned_widgets_own_hit_box_moves_exactly_once` (fails at exactly
2x the pan without it).
Still open, and **pre-existing** (present in the build before this change,
so not the scroll area's doing): the composer bar's grey background is not
drawn on the `transcript-screen bench` build, so the message reads as
white text over the transcript. `Stack{StackSize::Child(1)}` is the thing
to look at.
Rig fix on the way past: `iris/android-app/run-bench.sh` polled logcat for
`"iris bench report:"`, which `copy_report` also logs at startup
("nothing to copy -- run the benchmark first"), so it returned instantly
and printed a report that had never been run. It polls for the report's
own first line now.
### Task B, closed 2026-09-06: a streamed delta costs one markdown block
A transcript row was one `TextEdit` holding the whole message, so every
delta re-shaped every paragraph of a long reply through parley -- the one
phase where iris trailed Compose on Iris's phone. A row is a **column of
one `TextEdit` per top-level markdown block** now, and a delta that lands
in the last block is one `set_with_spans` on that block.
- **`client-core/src/markdown_blocks.rs`** is the split: `split_blocks`
(top-level blocks with their source, via the same `pulldown-cmark` the
renderer parses with, so the two cannot disagree about where a block
starts) and `common_prefix`. Seven tests, including the one that says
the fast path must **compare** rather than assume: appending `---` under
a paragraph turns that paragraph into a heading, so an already
laid-out block is not always still what it was.
- **`iris/transcript-ui/src/row.rs`** builds the column and owns
`RowBlocks::apply_delta`; **`lib.rs`** keeps the *tail* row's blocks
(`TranscriptScreen::tail`) since that is the only row a delta reaches.
- **A block is the selection unit**, not a row: `Selection` is keyed by
`SelKey = (RowKey, u32)`, which compares in reading order at both
levels so every range query in that file is unchanged. The list-level
(pointer-captured) half of a drag resolves the block under the finger
from its drawn box (`Selection::locate`) instead of doing arithmetic
from the row's extent.
**Pass condition, met**: `a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one`
(`transcript-ui/src/lib.rs`) drives a real `UiRenderState` and asserts the
`Widget::draw` count for one delta into a 100-paragraph (3,000+ character)
reply equals the count for the same delta into a one-paragraph reply.
**30 either way.** It is a real test, not a tautology: it read **630
against 30** at three points on the way -- once because `Span`'s measure
pass redrew every child, and once because `build_tree` did not seed
`tail`, so the first delta after opening a screen took the rebuild path
with nothing on screen or in `take_rebuilds()` to say so.
Two things tried and dropped, so the next session does not redo them.
`Painter::measure` (a container asking a clean child for its size instead
of drawing it provisionally) fixed one of the 630s but the test passes
without it once the `tail` seeding is right, so it was removed rather than
kept on speculation. And the emulator's own numbers say the remaining
cost is not in the block split.
**Verified on the emulator** beyond the counter: the transcript draws its
blocks with their own spacing (heading, prose, fence), and
`ui-trace record --do "holddrag 300 700 700 1000 700 600"` logs
`iris selection: begin at row (3187, 0)` then `extend to row (3187, 1)`
with the highlight crossing from the heading into the code block -- a
selection that spans blocks, which is what the re-key had to keep.
### Bench, stream phase, before and after Task B (emulator, 2026-09-06)
`iris/android-app/build-apk.sh debug --abi x86_64 --features
"transcript-screen bench force-gles"` + `run-bench.sh`, this checkout's
AVD. Emulator absolutes transfer nothing; the before/after ratio on the
same emulator does.
Same AVD, same fixture, same build flags, 20 minutes apart. Emulator
absolutes transfer nothing; the ratio does.
before after
stream: 202 frames over 21.0s stream: 293 frames over 21.0s
late: 197 (97.5%) late: 285 (97.3%)
p50 61.5ms p50 54.5ms (-11%)
p90 211.7ms p90 113.1ms (-47%)
p99 342.6ms p99 137.4ms (-60%)
worst 403.6ms worst 143.0ms (-65%)
The tail is where the whole-message re-layout lived, and it is where the
change shows: 91 more frames delivered in the same 21 seconds. The p50
moves least, which is consistent -- a delta into a *short* message never
cost much. **The phone number is Iris's to take**; nothing here is a
statement about her device.
### Verification pass over Tasks A and B, 2026-09-06
Read of `git diff fb6b459..HEAD -- iris/ client-core/` against LAYOUT.md,
TEXTURES.md, IRIS.md/DECISIONS.md's 2026-09-06 entries and CODE_RULES.md,
with the emulator. **Verdict: deliverable to the phone.** One real defect
found and fixed, two missing guards added, one open item closed as stale.
1. **The block model is correct.** `split_blocks` was checked against the
shapes a real transcript has -- a fence with blank lines, a `---`
inside a fence, a nested list, a fence directly under a heading, a
table, a quote -- and against the property `apply_delta` rests on, at
**every character boundary** of a message containing all of them:
growing a message may rewrite its last block and never an earlier one,
or `common_prefix` says so. No defect (`client-core`'s
`every_prefix_of_a_streamed_message_keeps_all_but_its_last_block`,
commit `a56a928`). A delta closing a fence, a delta mid-word and a
stream ending inside an unterminated fence are each their own test.
2. **`iris/core/src/ui/render_state.rs`, `draw_inner`'s size-independent
fast path: fixed** (commit `e63e923`). It rewrites the widget's own
primitives in place and writes **no** move-slot delta, so unlike `mov`
there is nothing for `move_applied` to count; `167862c` counted one
anyway, and `resolved_region` then subtracted a distance the chain
never held. Every such widget's hit box sat short of its drawing by
its last step, with the drawing correct -- nothing on screen to say
so. `Span` reaches this on the **first frame** of any tree containing
a `Rect` (the `.background(rect(..))` idiom, list row tints), because
it measures each child at the full region and then places it. Pinned
by `a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at`,
the sibling of `a_panned_widgets_own_hit_box_moves_exactly_once` on the
branch that fix had no reason to touch.
3. **Selection across blocks is sound; its rebuild path had no test**
(commit `155d899`). `SelKey = (RowKey, u32)` orders lexicographically,
which is reading order at both levels, so `begin`/`extend`/`locate`
and the range queries carry over unchanged; `selected_text` joining
with a blank line is right for blocks as well as rows, since that is
how markdown separates them. The gap was the tail rebuilt under the
**same key with fewer blocks** -- the dropped blocks keep pointing at
widgets `replace_back`'s drop frees, and `Selection::begin` resolves
every registered handle on an ordinary press, so the next tap anywhere
panics. `e1030d6`'s unconditional `unregister` is correct and now has
`a_tail_rebuilt_with_fewer_blocks_leaves_none_of_them_in_selection`,
confirmed to fail (3 blocks still registered, expected 1) without it.
`Selection::registered_blocks` is the test-only accessor that lets it
assert the contract rather than only that nothing panicked.
4. **The three new `debug_assert!`s are whole-set, not one member.**
`Len::fold_dp`'s is in `draw_inner` after *every* `Widget::draw`, so
it governs the set by construction; `Pad` and `Span` were checked and
already fold through `apply_rest`, and `Sized`/`MaxSize` are the two
that reported a caller-written `Len` raw. `own_mask`'s reuse lives
inside `Painter::set_mask` itself, whose only caller is
`widget/mask.rs`. `move_applied` has exactly two writers, `mov` and
`reposition` (now one, after finding 2), and `resolved_region` is the
only reader -- `window_region` goes through it.
5. **The O(last block) claim now holds for parley, by counter**
(commit `c3cfc67`). `take_counters` gained a fourth number, text
shapes, bumped in `Painter::render_text` -- which `TextView::render`
only reaches on a cache miss, so it counts shapes and not requests. A
draw counter cannot stand in for it either way. Measured: **one delta
into a 100-paragraph reply shapes exactly 1 text layout, the same as
into a one-paragraph reply.**
6. **The composer bar's grey background *is* drawn** -- IRIS_TODO.md's
"still open, and pre-existing" note is stale and has been corrected.
Measured by decoding the screencap rather than eyeballing it: the bar
is `rgb(41,40,49)` (the declared `40,40,46` after sRGB rounding),
**full width, y2245..y2365** on the 1080x2424 AVD, with the field at
`31,2277..1048,2329` and the 63px nav strip below. Whatever the note
saw, Task A's `MaxSize`/`own_mask` fixes closed it.
**Checks run**: `cargo fmt --all --check` clean in both workspaces;
`cargo clippy --workspace --all-targets` warning-free (only the
pre-existing future-incompat note about `wgpu`/`naga`/`winit`);
`cargo test` 81 (iris) + 13 (iris-core) + 20 (transcript-ui) + 123
(client-core), all passing. One bench run on this checkout's AVD with the
assertions live, debug x86_64 `force-gles`, no abort and nothing in
logcat: **stream: 298 frames over 21.0s, late 287 (96.3%), p50 52.8ms,
p90 108.1ms, p99 137.3ms, worst 148.9ms** -- reproducing the "after"
column above.
- [x] **Merge the `DragGesture` work** -- done 2026-09-06 (merge commit
`f802de9`, `git merge --no-ff worktree-agent-a754368325fa06839`,
clean, no conflicts across the 8 files `e12c708` touched). Targets
two of the four bench-v2 defects: finger flings dropped by
per-widget hit testing (pointer capture + `CursorSense::Drop`), and
IME insets never redelivered (`MainActivity.java` edge-to-edge).
**Tap-vs-swipe/`DragGesture` overlap, reasoned through**: `attr.rs`'s
`on_press` (composer focus) and `sense.rs`'s `DragArbiter`/
`DragGesture` (list pan-vs-select) do not share a mechanism, but
they don't need to -- `on_press` never calls `capture_pointer`, so
it only ever sees an ordinary per-frame hit-tested `Pressing`/
`PressEnd` (`run_sensors`' `region.contains(cursor.pos)` check,
unaffected by capture unless *this* widget requested it), the same
as before `DragGesture` existed. The two only interact where a
gesture starts on the composer and travels into the list's region;
`run_sensors` already delivers `Pressing` to whichever widget's
*current* position contains the pointer, so `List` starts getting
frames the instant the finger crosses the boundary -- with no
`PressStart` of its own, which is exactly what `DragArbiter::
is_idle()`'s 2026-09-05 recovery branch exists for. No consolidation
needed; `DRAG_SLOP` is already the one shared constant (`attr.rs`
imports it from `sense.rs`, not a second copy).
**Checks, 2026-09-06 merge pass**: `cargo fmt --all` clean;
`cargo clippy -p iris -p iris-core -p transcript-ui --all-targets`
and the same for `-p desktop-app -p tabs-ui`, zero warnings beyond
the pre-existing external-crate future-incompat notice
(naga/wgpu/wgpu-core/wgpu-hal/winit); `cargo test --lib -p iris -p
iris-core -p transcript-ui` and `-p desktop-app -p tabs-ui`, 97
passed/0 failed, including the review-fix tests below.
`cargo test --workspace`/`cargo clippy --workspace --all-targets`
(the full-workspace forms, which also build `iris`'s winit examples)
were abandoned after 40+ minutes each stuck compiling one example
binary with `uptime` reading a load average of 66-78 on this 8-core
VM (3-4 concurrent peer `cargo`/`cargo check` invocations the whole
session) -- `ps -o time` on the stuck `rustc` showed 2 seconds of
accumulated CPU time after 38 minutes of wall time, confirming
scheduler starvation rather than a hang. The per-package `--lib`
form above is what actually exercises the changed code and finished
in under 4 minutes warm. `android-app` (`iris-android-app`) is
excluded from the host workspace (`iris/Cargo.toml`, needs the NDK
target) and is covered instead by the APK build below, which
compiles it for `x86_64-linux-android`.
**Emulator checks, 2026-09-06** (this checkout's `ai-app-2` AVD,
`iris/android-app/build-apk.sh debug --abi x86_64 --features
"transcript-screen bench force-gles"` -- plain Vulkan crashed on
this AVD's boot this pass, `wgpu_core::instance: enabled backend
Vulkan has no adapters`, unrelated to this merge and worked around
with `force-gles` the way I5's own box already documents for this
hardware):
- **(a) tap-vs-swipe still holds.** Fresh app launch, `dumpsys
input_method`'s `mInputShown=false` at rest. `ui-trace record
--do "swipe 540 1510 540 700 200"` (a swipe starting on the
composer's own box, read from `ui-trace show -m Message --field
box` as `31,1488..1048,1540`) leaves `mInputShown=false` and the
box unmoved (no keyboard-driven resize). `ui-trace record --do
"tap 540 1510"` on the same field then reads `mInputShown=true`.
Matches `20b1225`'s original result -- the `DragGesture` merge
did not disturb it, confirming the reasoning above.
- **(b) a real finger fling keeps the list moving after release.**
Screenshot-hash sampling (`adb exec-out screencap`, `md5`, since
transcript rows carry no per-row accessibility label yet -- I5's
own leftover -- so `ui-trace show` cannot track them) at ~40-60ms
intervals through and after a fast `swipe 540 1400 540 400 120`
(with room to scroll confirmed by a preceding slow drag) caught
two *distinct* post-release frames in one run (a settle-position
beyond the raw drag's own last frame), and every run showed
28-32 `iris::android::view: render()` log lines per gesture
against an idle baseline of 0 in 1.5s and roughly 8 expected from
a bare 120ms drag's own `Pressing` frames alone -- i.e. redraw
kept being requested well past the finger lifting, which only
happens while `List::tick_fling` is still returning `true`.
Some runs' screenshots showed only the drag's own jump with nothing
further *visibly different*, which is consistent with a real but
small/fast-settling fling (a modest synthetic-touch velocity's
spline tail moves little per frame) rather than absence of one --
the render-count signal did not vary between those runs and the
one with a visible second frame. Recorded as confirmed, with that
caveat, rather than measured to a number; a phone verification
(Iris's own report closes this properly) is still open per
`IRIS_TODO.md`'s item.
- **(c) `on_insets_changed` fires on an IME toggle, with confirmed
cycles.** `run-bench.sh`'s report: `keyboard: shown 4/5, hidden
5/5 (confirmed via on_insets_changed)` -- the "could not be
shown" unknown-state line (`bench_client.rs::run_keyboard_phase`)
did not fire, unlike the pre-`DragGesture` build this same report
format existed for.
Worktrees removed after the checks above: `agent-a754368325fa06839`
(the source branch, its own emulator stopped first via `cd` into
it + `emu down`), `agent-a27094a7db775552a`, `agent-a1ff0294b6c29127e`,
`agent-a9002910a315fe719` -- each confirmed `git rev-list --count
rustify..<branch>` = 0 and no uncommitted changes first; their
branches deleted too. `agent-a16b22e34539b810e` and
`agent-a6e37a2335f436d08` left alone -- both `git worktree list`
`locked` to a live peer agent.
- [x] **Fix `docs/REVIEW-2026-09-06.md`**, done 2026-09-06, after the
merge (finding 1's shape and location in `selection.rs`/`lib.rs`
were unchanged by the merge, which touched `Selection` but not
`apply`'s `Rebuild` arm). All ten findings fixed -- new
`Selection::clear()` for finding 1 (the simplest option the review
named: clear the same way `List::clear()` clears the list, let
`push_row` re-`register` survivors), five `debug_assert!`s
(2-5, plus 7's restructure), and three new tests (8, 9, 10),
confirmed with the `apply_tests::a_row_dropped_by_a_regroup_does_
not_outlive_itself_in_selection` test passing (it exercises exactly
finding 1's shape: build a real `TranscriptScreen`, force the same
regroup `diff_tests` already covers, `apply`, then a surviving
row's `begin` -- panics pre-fix, per the review's own test-8 ask).
`docs/IRIS.md`'s 2026-09-05 entry gained the line the review's
"Docs" section asked for. See `docs/REVIEW-2026-09-06.md`'s own "Fixed, 2026-09-06"
section for the per-finding account. Committed together with the
review file.
- [~] **Iris's 11:39 phone report on the 02:07 build** (four items,
verbatim in `IRIS_TODO.md`'s "From the phone, 2026-09-06, 11:39"): verbatim in `IRIS_TODO.md`'s "From the phone, 2026-09-06, 11:39"):
composer floating two thirds down the screen at launch with black composer floating two thirds down the screen at launch with black
below it; a swipe starting on the composer held until the finger below it; a swipe starting on the composer held until the finger
@@ -71,18 +384,153 @@ closes it.
lost on app-switch on the phone despite the emulator-verified lost on app-switch on the phone despite the emulator-verified
atlas reset. The first and last are the same class as the next atlas reset. The first and last are the same class as the next
box and go to that agent; the middle two are the merge box's. box and go to that agent; the middle two are the merge box's.
- [ ] **Stale primitives and invisible composer text** — the header drawn - [~] **Stale primitives and invisible composer text**, 2026-09-06:
twice after a keyboard resize, the `Compacted:` row drawn twice on **typed text is fixed and was never a renderer bug at all**; the two
Iris's phone, and typed text never appearing (P0 box item 2). All duplicate-drawing halves are **not reproducible** on this checkout's
three sit on the `redraw_updates` targeted-redraw path and may be emulator any more and are recorded below with what changed.
one bug; the P0 box says the next step is instrumentation inside
`Span::draw`/`draw_inner` showing where each placement's **1. Typed text (P0 box item 2, `IRIS_TODO.md`'s own item) --
primitives actually land on the frame it goes wrong. fixed.** The composer's buffer was empty the whole time.
`list.rs`'s new `replacing_the_last_row_many_times_does_not_leak_ `TextEditCtx::select` (`iris/src/widget/text/edit.rs`) compared the
primitives` test already pins the widget arena as *not* the leak. tap against the *laid-out text's* box and set `selection = None` for
- [ ] **Composer touch-drag scroll** for overflowed text — now that anything outside it; an empty field lays out to a zero-width box, so
dragging is a default-input `DragGesture`, `Scroll` should get its tapping an empty composer granted focus and opened the keyboard with
touch pan from the same mechanism `List` uses, not a copy. no caret, and `insert_str` returns early without one -- every
keystroke was dropped in silence. **Gboard's suggestion strip is
Gboard's own composing state, not a read of our buffer**, which is
what made the earlier pass conclude the buffer held the text and
send the search downstream into the renderer; the `accessibility`
dump saying `text=""` for the `Message` node was the first
contradicting evidence. Parley clamps a point outside the layout by
itself, and a press reaching `select` has already been hit-tested to
the widget, so the "outside" branch had nothing left to mean. Three
new tests in `edit.rs`, one of which fails on the pre-fix code, plus
a `debug_assert!` in `insert_str` so an insert with no caret fails at
the mistake rather than dropping input -- it caught
`layout_tests::composing_text_after_a_keyboard_resize_...` typing
into an unfocused field the moment it was added. **Emulator
evidence**: `ui-trace record --do "tap 'Message'"` then `adb shell
input text` shows the text in the bar with a caret
(`/tmp/final-typing.png`) and logs `iris text render: chars=5 ...
glyphs=5`, against `glyphs=0` per keystroke before.
**2. The header drawn twice after a keyboard resize (this box's own
"(a)") no longer has a path to happen on this emulator, for a
measured reason**: since `MainActivity.java` went edge-to-edge
(`e12c708`), **opening the keyboard no longer resizes the surface at
all**. Measured: `render()` reports `out_size=(1080, 2282)` unchanged
across an IME open, while the new `iris insets:` line reports
`bottom=63 ime_bottom=0` -> `bottom=883 ime_bottom=1`. So the IME is
an inset now, not a `surface_changed`, and the two-phase `Span::draw`
the duplicate was blamed on is not re-entered. Reproduction attempts
this pass, all negative: `tap 'Message'` + `ui-trace elements`
(exactly one "Run benchmark" in every frame of the trace), a
screenshot with the keyboard open, and a real `adb shell wm size
1080x2200` *while the keyboard was open* (a genuine
`surface_changed`) -- one header row, no stray copy
(`/tmp/resize-dup.png`).
**3. The `Compacted:` row drawn twice on Iris's phone is still
open**, and nothing here reproduces it. What was ruled out this
pass: the widget arena (`list.rs`'s
`replacing_the_last_row_many_times_does_not_leak_primitives`), the
`top_bar` rebuild (`last_top_pad`, a previous pass), and now the
keyboard-resize trigger above. One real defect *was* found by
reading the path and is fixed, though it cannot be shown to be her
bug: `UiRenderState::draw_started` -- the guard whose whole job is
"do not redraw a widget an ancestor is drawing right now, or one of
the two copies is orphaned" -- **tested its own set after removing
the id from it**, so the test was constant `false` and the guard
could never fire, while the set grew by one entry per widget ever
drawn and was never emptied. It is now inserted around
`Widget::draw` and removed when it returns, with a `debug_assert!`
at the top of `update` that it is empty between frames. Her build
has both.
- [x] **Stale primitives, the phone's half** — **root-caused and fixed
2026-09-06, commit `76b1f99`.** It was neither `Span::draw` nor
`List`: `UiRenderState::draw_inner` *read* `needs_redraw` without
consuming it, and used it to skip the whole `if let Some(active)`
block — **including the `remove(id, false)` that frees a redrawn
widget's previous primitives**. So a widget that was both already
active and marked dirty, and was reached by an **ancestor's** draw
rather than by `redraw_updates` picking it first (the order a
`HashSet` makes arbitrary, which is why it was intermittent), wrote
a second full set of primitives and then had `active.insert`
overwrite the only handles that could ever have freed the first
set. Those instances stay in the layer's buffer for the life of the
process, with a leaked move slot and leaked mask refs, redrawn every
frame at whatever region they last had — and `List` sets no mask, so
a row measured at `GENEROUS_PADDING` leaves its ghost outside the
list's own box, which is the copy below the composer.
`Painter::draw_twice` (`List::place`'s measurement pass) reaches
`draw_inner` twice for one id in one frame and so hits the same
fault with no ancestor involved.
**Fix**: consume the mark at the top of `draw_inner` — this call *is*
the redraw it asked for — and free the old primitives on the dirty
path too.
**Why the earlier passes could not see it**: `replacing_the_last_row_
many_times_does_not_leak_primitives` counts *widgets*, and the
orphan's owner is very much alive; it is an earlier set of that same
widget's primitives that is stranded.
**Guard**: `UiRenderState::orphaned_primitives()` names every live
instance no `ActiveData` owns, and `update` `debug_assert!`s it empty
every frame in debug builds. The per-frame form is a *count*
comparison (`primitive_counts_agree`, O(active widgets)); the
O(primitives) walk only runs to build the failure message, because
running it per frame made a debug build on the emulator too slow to
finish a bench run at all (260s timeout, no report).
**Test**: `an_ancestor_redrawing_a_dirty_row_leaves_no_stale_copy`
(`iris/src/widget/list.rs`), which fails on the pre-fix code with
`1 primitive(s) survived their own widget's redraw`.
**Emulator evidence, 2026-09-06** (this checkout's `ai-app-2` AVD,
`build-apk.sh debug --abi x86_64 --features "transcript-screen bench
force-gles"`, a **debug** build so the guard is live): a complete
`run-bench.sh` run — 3,142 frames over 147s across the fling, the
400-event stream (which is 400 `apply` calls including the fixture's
compaction event), the typing and the keyboard phases — with the
assert firing zero times and `logcat` showing no abort. That is the
whole of the phone's reported scenario exercised with the invariant
checked on every frame.
- [~] **Composer touch-drag scroll** for overflowed text — **the
mechanism is done, the composer is not.** `Scroll::drag`
(`iris/src/widget/position/scroll.rs`) takes its pan from the same
`sense::DragGesture` `List` is driven by, and
`WidgetLike::scrollable()` registers it beside the wheel handler, so
every `.scrollable()` in the codebase pans on a finger with nothing
added at the call site. No fling (`Scroll` has no per-frame tick and
the areas it wraps are at most a screenful) — see IRIS.md and
DECISIONS.md. A vertical drag inside a *focused* field no longer
extends a selection either (`attr.rs`'s `on_press` now applies the
same `DRAG_SLOP` rule its unfocused branch already did), which is
Android `EditText`'s own behaviour and what lets the scroll area
around a field win the gesture.
**Tests**: four in `scroll.rs` (pan past the slop, a tap inside it,
a horizontal drag, the end clamp) plus
`a_finger_drag_over_a_scroll_area_pans_it` in `sense_tests.rs`, which
drives the whole path — `scrollable()`'s registration, `run_sensors`'
dispatch, `Scroll::drag`, arbitration and pointer capture — and fails
with `got 0` if the registration is removed.
**What is left, with the measurement**: wrapping the composer's field
in `.scrollable().masked()` was tried and reverted the same day.
`Scroll` resolves `content_len`/`container_len` against
`Painter::output_size` — the whole window — so inside the `MaxSize`
that caps the composer at six lines the two are in different spaces
and the field pans itself entirely out of the bar: measured on the
emulator with 474 characters in it (`iris text render: ...
size=(1016.7, 623.7)` against a 441px cap) the bar collapsed to its
padding with no text in it. Making `Scroll` measure against its own
offered box is the next step, and it touches a widget the transcript
and the bench shell both use.
One real bug **was** found and fixed on the way (`ActiveData::mask`
stored the mask a widget *set* rather than the one it was drawn
*under*, and `redraw` feeds that field straight back in as the
inherited mask — so a targeted redraw of any `Masked` handed it its
own mask and aborted on `set_mask`'s nested-mask assert; that is a
real abort on the emulator, `assertion failed: self.mask ==
MaskIdx::NONE`, reproduced as
`redrawing_a_masked_widget_does_not_nest_its_own_mask` in
`layout_tests.rs`).
- [ ] **Streaming re-layout** (IRIS_TODO.md's last section) — after the - [ ] **Streaming re-layout** (IRIS_TODO.md's last section) — after the
above, since they make the stream phase unrepresentative today. above, since they make the stream phase unrepresentative today.
- [x] **client-core prerequisites for P1, in parallel** (pure Rust, - [x] **client-core prerequisites for P1, in parallel** (pure Rust,
@@ -4766,6 +5214,22 @@ device.
extended for the longer run (260s poll cap, `-A 60` instead of extended for the longer run (260s poll cap, `-A 60` instead of
`-A 6`) to fit v2's four phases. `-A 6`) to fit v2's four phases.
**Redelivered, 2026-09-06, the defect pass.** `./build-apk.sh
release --abi arm64-v8a --features "transcript-screen bench"`
(Vulkan, no `force-gles`; the x86_64 `jniLibs` slice from this
pass's emulator work was removed first, confirmed arm64-only by
listing the APK's `lib/` entries), `apksigner verify` showing the
same `CN=ai-app` cert, copied to `~/host/bench/
iris-bench-arm64.apk`; that README gained a dated entry naming what
to look for. What changed: the composer sits at the bottom of the
screen at launch again (the black third was the bench shell's empty
report pane, not an inset -- see the plan box above), typing into it
works at all (the empty-field caret bug), the keyboard no longer
throws up an undismissable diagnostics overlay, and every surface
and insets event is logged so a phone `logcat` can answer the
app-switch text loss. Not fixable from here and still open: the
duplicated `Compacted:` row.
**(a) The header-duplicate bug (found by a concurrent pass on this **(a) The header-duplicate bug (found by a concurrent pass on this
branch): investigated, not fixed.** Reproduced reliably branch): investigated, not fixed.** Reproduced reliably
(`ui-trace record --do "tap 'Message'"` then `adb exec-out (`ui-trace record --do "tap 'Message'"` then `adb exec-out
@@ -4993,7 +5457,54 @@ device.
before/after screenshot pair for item 3 (blocked on item 2); anything before/after screenshot pair for item 3 (blocked on item 2); anything
on Vulkan or the real phone. on Vulkan or the real phone.
- [ ] **P1 — session screen parity.** History paging backward (with the - [ ] **P1 — session screen parity.** **Started 2026-09-06, on Iris's
word**: "just continue with the plan for now; try to move towards
feature parity for the transcript screen so that the test can be
more fair." So P0's "must pass before P1 starts" is lifted — the
phone bench continues alongside, and parity is what makes its
comparison fair. **Sub-order, decided by the design agent, by what
the bench fixture exercises and Compose already draws** (each is
one agent; tick and date in place):
- [ ] **P1a — markdown block rendering parity.** Now that a row is
a column of per-block widgets (`e1030d6`), draw each block
the way Compose does: headings at their sizes, fences in a
mono face with `client-core`'s `highlight` spans and a
distinct background, bullet/numbered lists with indent,
tables as aligned columns, block quotes, links as tappable
spans (opening through the platform — the "tappable link"
primitive in IRIS_TODO's "Build (for the port)"). Against
`app/bench-fixture/`, screenshot beside the Compose bench
build on the same emulator. Pure parts (block → style
mapping) tested in `client-core`/`transcript-ui`.
- [ ] **P1b — tool-call cards and grouping.** `ToolRows.kt`/
`ToolInput.kt`'s cards: a collapsed row per call with name
and a one-line summary, expand to input and output, runs of
calls grouped (`adopt_run` already groups in `client-core`),
the busy/failed states, and the kilobyte outputs the fixture
carries without laying them out while collapsed.
- [ ] **P1c — history paging and jump-to-latest.** Wire
`client-core::transcript_source` into `transcript-ui`:
the opening page, paging back on scroll with the cushion
measured in on-screen viewports (`HISTORY_SCREENS`, IRIS_TODO
"Build (for the port)"), the `NothingLoaded`/empty/error
states drawn distinctly (UI_RULES: design the unknown state
first), `join_pages` at each seam, and a jump-to-latest
control that pins to the newest end. Pass condition: the
P1 pass condition below, against `ui-sandbox.sh` with
`AI_SANDBOX_BIG_MB` and `--delay`.
- [ ] **P1d — images, the session settings dialog, attachments,
usage bar.** `SessionImage` thumbnails (the scaled image
widget), the modal primitive and `SessionSettingsDialog`/
`UsageDialog`, `PendingAttachments` over the attachments
route (`api.rs` gap), `SessionUsageBar` (the gauge widget).
- [ ] **P1e — keyboard and insets behaviours** from AGENTS.md's
"Things that have bitten", re-verified on the phone build:
composer never left floating after the keyboard closes
mid-stream, `adjustResize` + edge-to-edge together, one
recomposition-equivalent per keyboard toggle (the
`iris insets:` log line count).
History paging backward (with the
page-boundary healing `client-core` does not have yet, below), page-boundary healing `client-core` does not have yet, below),
`TranscriptSource`-backed cache/server stitching, jump-to-latest, `TranscriptSource`-backed cache/server stitching, jump-to-latest,
tool-call cards and grouping, the session settings dialog, composer tool-call cards and grouping, the session settings dialog, composer
+8
View File
@@ -20,6 +20,14 @@ overlapping, and once more below the composer bar: primitives of a
replaced/removed row surviving in the GPU buffers, the same shape as the replaced/removed row surviving in the GPU buffers, the same shape as the
header drawn twice after a keyboard resize. header drawn twice after a keyboard resize.
**Root-caused and fixed 2026-09-06** (commit `76b1f99`): the diagnosis in
that sentence was right and the location was not -- `UiRenderState::
draw_inner` read the `needs_redraw` mark without consuming it and skipped
the branch that frees a redrawn widget's old primitives. docs/RUST.md's
"Stale primitives, the phone's half" box has the full account, the guard
(`orphaned_primitives`, `debug_assert`ed every frame) and the emulator run
that exercises it.
``` ```
iris bench report iris bench report
per phase: per phase:
+1
View File
@@ -721,6 +721,7 @@ name = "client-core"
version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"event-model", "event-model",
"pulldown-cmark",
"serde", "serde",
"serde_json", "serde_json",
"ureq", "ureq",
+5 -3
View File
@@ -15,6 +15,11 @@ wgpu = { workspace = true }
image = { workspace = true } image = { workspace = true }
accesskit = { workspace = true } accesskit = { workspace = true }
tokio = { workspace = true, features = ["sync", "rt", "rt-multi-thread"] } tokio = { workspace = true, features = ["sync", "rt", "rt-multi-thread"] }
# For diagnostics visible through android_logger (or whatever logger the
# app crate installs) -- this crate never installs one itself. Not in the
# android-only block below any more: the lines that matter most are in
# shared widget code, which the host backend compiles too.
log = "0.4.28"
# winit everywhere except Android; android-view (below) is what stands in # winit everywhere except Android; android-view (below) is what stands in
# for it there. Both backends live in this crate (see `src/android/mod.rs`'s # for it there. Both backends live in this crate (see `src/android/mod.rs`'s
@@ -53,9 +58,6 @@ accesskit_android = "0.8.0"
# for `android/insets.rs`'s own id -> state map -- the same reason # for `android/insets.rs`'s own id -> state map -- the same reason
# android-view's own `PEER_MAP` carries one. # android-view's own `PEER_MAP` carries one.
send_wrapper = "0.6.0" send_wrapper = "0.6.0"
# For diagnostics visible through android_logger, wherever the app crate
# installs it -- this crate never installs a logger itself.
log = "0.4.28"
[features] [features]
# RUST.md's I5 "Where iris's frame time goes" diagnosis: forces the Android # RUST.md's I5 "Where iris's frame time goes" diagnosis: forces the Android
+1
View File
@@ -745,6 +745,7 @@ name = "client-core"
version = "0.1.0" version = "0.1.0"
dependencies = [ dependencies = [
"event-model", "event-model",
"pulldown-cmark",
"serde", "serde",
"serde_json", "serde_json",
"ureq", "ureq",
+5
View File
@@ -52,6 +52,11 @@ if [ -z "$NDK_DIR" ]; then
fi fi
export ANDROID_NDK_HOME="$NDK_DIR" export ANDROID_NDK_HOME="$NDK_DIR"
# Only the ABI asked for goes into the APK. cargo ndk adds its output beside
# whatever earlier builds left here, and Gradle packages every directory it
# finds -- a debug x86_64 emulator build left behind made an arm64 "release"
# 339 MB on 2026-09-06.
rm -rf app/src/main/jniLibs
echo "build-apk.sh: cargo ndk -t $ABI build ${BUILD_TYPE:+(${BUILD_TYPE})} --features \"$FEATURES\"" echo "build-apk.sh: cargo ndk -t $ABI build ${BUILD_TYPE:+(${BUILD_TYPE})} --features \"$FEATURES\""
if [ "$BUILD_TYPE" = "release" ]; then if [ "$BUILD_TYPE" = "release" ]; then
cargo ndk -t "$ABI" -P 26 -o app/src/main/jniLibs/ build --release --features "$FEATURES" cargo ndk -t "$ABI" -P 26 -o app/src/main/jniLibs/ build --release --features "$FEATURES"
+7 -2
View File
@@ -51,9 +51,14 @@ ui-trace record -s "$SERIAL" -d 3000 --do "tap 'Run benchmark'" -o /tmp/run-benc
# phase, ~61s of typing, 10s of keyboard toggles, roughly 2.5 minutes end # phase, ~61s of typing, 10s of keyboard toggles, roughly 2.5 minutes end
# to end) but device speed varies. 260s cap rather than v1's 90s -- v2 is # to end) but device speed varies. 260s cap rather than v1's 90s -- v2 is
# a longer script than v1's swipe-loop-only run. # a longer script than v1's swipe-loop-only run.
# The report's own first line, not the bare "iris bench report:" prefix:
# `copy_report` logs that prefix too ("nothing to copy -- run the benchmark
# first", which the app emits at startup), so polling for the prefix
# returned instantly and the script printed a report that was never run.
REPORT_LINE="iris bench report: iris bench report"
i=0 i=0
while [ "$i" -lt 260 ]; do while [ "$i" -lt 260 ]; do
LINE=$(adb -s "$SERIAL" logcat -d -s iris-android-app:I 2>/dev/null | grep "iris bench report:" || true) LINE=$(adb -s "$SERIAL" logcat -d -s iris-android-app:I 2>/dev/null | grep "$REPORT_LINE" || true)
if [ -n "$LINE" ]; then if [ -n "$LINE" ]; then
break break
fi fi
@@ -66,4 +71,4 @@ if [ -z "$LINE" ]; then
fi fi
# -A 60 rather than v1's -A 6 -- v2's report has a per-phase block (four # -A 60 rather than v1's -A 6 -- v2's report has a per-phase block (four
# phases, four lines each) on top of the frames/bench sections v1 had. # phases, four lines each) on top of the frames/bench sections v1 had.
adb -s "$SERIAL" logcat -d -s iris-android-app:I | grep -A 60 "iris bench report:" adb -s "$SERIAL" logcat -d -s iris-android-app:I | grep -A 60 "$REPORT_LINE"
+60 -33
View File
@@ -92,6 +92,12 @@ const ANIM_STEP_MS: u64 = 16;
const FIXTURE_JSONL: &str = include_str!("../../../app/bench-fixture/assets/transcript.jsonl"); const FIXTURE_JSONL: &str = include_str!("../../../app/bench-fixture/assets/transcript.jsonl");
/// How much of the screen a *filled* benchmark report may take before it
/// scrolls instead of growing -- roughly a third of a phone screen, the
/// share the pane used to reserve unconditionally. An empty report takes
/// nothing at all; see `new`'s comment at the tree it is used in.
const REPORT_MAX_HEIGHT_DP: f32 = 260.0;
pub struct BenchClient { pub struct BenchClient {
ui_state: AndroidUiState, ui_state: AndroidUiState,
content: WeakWidget<WidgetPtr>, content: WeakWidget<WidgetPtr>,
@@ -221,8 +227,15 @@ fn battery_line(samples: &[i32]) -> String {
return " battery current: unavailable on this device".to_string(); return " battery current: unavailable on this device".to_string();
} }
let mean = samples.iter().map(|&v| v as i64).sum::<i64>() / samples.len() as i64; let mean = samples.iter().map(|&v| v as i64).sum::<i64>() / samples.len() as i64;
let min = samples.iter().min().unwrap(); // `min`/`max` are guarded by the `is_empty` check above, three lines
let max = samples.iter().max().unwrap(); // up -- pairing the `Option` unwraps with the emptiness check right
// here (rather than two statements apart, with `mean` in between
// reading the same slice) is what keeps a future reorder from
// separating the guard from what it protects (docs/
// REVIEW-2026-09-06.md finding 7).
let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max()) else {
unreachable!("samples is non-empty, checked above");
};
format!( format!(
" battery current: mean {mean}\u{b5}A over {} samples (min {min}, max {max})", " battery current: mean {mean}\u{b5}A over {} samples (min {min}, max {max})",
samples.len() samples.len()
@@ -248,10 +261,28 @@ impl AndroidAppState for BenchClient {
let top_bar = WidgetPtr::new().add(rsc); let top_bar = WidgetPtr::new().add(rsc);
let controls = bench_controls(rsc, 0.0); let controls = bench_controls(rsc, 0.0);
top_bar(rsc).set(controls); top_bar(rsc).set(controls);
// The report pane is sized to whatever report it is holding, not
// to a share of the window: `rest(1)` here reserved a third of
// the screen for an *empty* `TextEdit` at every launch, which is
// what Iris's 2026-09-06 11:39 phone report described as "the app
// does not start with keyboard spacing correct" -- the composer
// two thirds down with black below it, nothing to do with the IME
// inset (measured: `iris insets:` reports bottom=63 ime_bottom=0
// at launch, while the `Message` field's own box sat 789px above
// the bottom of a 2282px surface -- exactly this pane's third).
// Capped and scrollable so a long report cannot take the screen
// back over, the same idiom `composer.rs` uses for the field.
// Above the transcript, not below it: the report is what the
// header's own "Run benchmark" button produces (UI_RULES.md --
// results appear where the action was started), and a pane under
// the composer would eat the navigation-bar clearance
// `set_bottom_inset` gives it.
let tree = ( let tree = (
top_bar, top_bar,
content.height(rest(2)), report_display
report_display.height(rest(1)).pad(dp(8)), .pad(dp(8))
.max_height(dp(REPORT_MAX_HEIGHT_DP)),
content.height(rest(1)),
) )
.span(Dir::DOWN) .span(Dir::DOWN)
.add_strong(rsc) .add_strong(rsc)
@@ -523,47 +554,43 @@ impl BenchClient {
/// text is currently shown -- `last_report` is what `copy_report` reads, /// text is currently shown -- `last_report` is what `copy_report` reads,
/// so it's set here too rather than adding a second copy path. /// so it's set here too rather than adding a second copy path.
fn show_diagnostics(&mut self, rsc: &mut Rsc) { fn show_diagnostics(&mut self, rsc: &mut Rsc) {
let report = self.diagnostics_text(rsc);
self.report_display.edit(rsc).set(&report);
self.last_report = Some(report);
}
/// The diagnostics report as text, with no side effect on what is on
/// screen -- shared by the `Diagnostics` button (which shows it) and
/// the keyboard-open capture (which only logs it), so the two can
/// never drift into reporting different things.
fn diagnostics_text(&self, rsc: &mut Rsc) -> String {
let font = rsc.ui.text.font_diagnostics(); let font = rsc.ui.text.font_diagnostics();
let frame_report = match self.android_state().frame_report.report() { let frame_report = match self.android_state().frame_report.report() {
Some(stats) => format!("{stats}"), Some(stats) => format!("{stats}"),
None => "no frames recorded yet".to_string(), None => "no frames recorded yet".to_string(),
}; };
let report = match &self.android_state().renderer { match &self.android_state().renderer {
Some(renderer) => renderer.diagnostics_report(&font, &frame_report), Some(renderer) => renderer.diagnostics_report(&font, &frame_report),
None => "iris diagnostics: no renderer yet (no surface)".to_string(), None => "iris diagnostics: no renderer yet (no surface)".to_string(),
}; }
self.report_display.edit(rsc).set(&report);
self.last_report = Some(report);
} }
/// The keyboard's own diagnostics capture -- see `on_insets_changed`'s /// The keyboard's own diagnostics capture -- see `on_insets_changed`'s
/// doc comment. Reuses `show_diagnostics`'s exact report (so it is the /// doc comment. **Logged only.** It used to also copy the report to
/// same text the on-screen `Diagnostics` button produces, plus the /// the clipboard unprompted and put it in the shell's overlay view,
/// per-frame log `FrameReport` already keeps around the resize -- /// from when the keyboard-inset callback was not firing at all and a
/// `frame_report.report()` above covers "the frames around the /// report could not be got off the phone any other way. Both are gone
/// resize" without a second accounting mechanism), then does three /// as of 2026-09-06: the callback fires reliably now (edge-to-edge,
/// things the button does not: logs it (so a `logcat` pull gets it /// `MainActivity.java`), and the overlay covered the whole screen on
/// even if nothing on screen does), copies it to the clipboard /// *every* keyboard open with its own Copy/Close buttons underneath
/// unprompted, and shows it in the shell's plain overlay view, which /// the keyboard, so it could not be dismissed -- an interruption for
/// draws independently of iris's own renderer -- the whole point, /// something nobody asked for, over an app you are trying to type
/// since the renderer is exactly what might be in the wiped state /// into (UI_RULES.md). The named `Diagnostics` button still shows the
/// this exists to report on. /// same text on demand, and `iris surface:`/`iris insets:` (view.rs)
/// carry the lifecycle a `logcat` pull actually needs.
fn capture_keyboard_diagnostics(&mut self, rsc: &mut Rsc) { fn capture_keyboard_diagnostics(&mut self, rsc: &mut Rsc) {
self.show_diagnostics(rsc); let report = self.diagnostics_text(rsc);
let Some(report) = self.last_report.clone() else {
return;
};
log::info!("iris keyboard diagnostics:\n{report}"); log::info!("iris keyboard diagnostics:\n{report}");
let Some(platform) = &self.platform else {
log::info!("iris keyboard diagnostics: no platform handle, can't reach the shell");
return;
};
if platform.copy_to_clipboard("iris keyboard diagnostics", &report) {
log::info!("iris keyboard diagnostics: copied to clipboard");
} else {
log::info!("iris keyboard diagnostics: clipboard copy failed");
}
platform.show_diagnostics_overlay(&report);
} }
fn copy_report(&mut self) { fn copy_report(&mut self) {
+5 -5
View File
@@ -142,7 +142,7 @@ fn bench_first_frame(n: usize) {
let start = Instant::now(); let start = Instant::now();
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
let elapsed = start.elapsed(); let elapsed = start.elapsed();
let (draws, rewrites, moves) = render.take_counters(); let (draws, rewrites, moves, _shapes) = render.take_counters();
report( report(
&format!("(a) first frame, N={n}"), &format!("(a) first frame, N={n}"),
elapsed, elapsed,
@@ -177,7 +177,7 @@ fn bench_scroll(n: usize, ticks: usize) {
let start = Instant::now(); let start = Instant::now();
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
total += start.elapsed(); total += start.elapsed();
let (draws, rewrites, moves) = render.take_counters(); let (draws, rewrites, moves, _shapes) = render.take_counters();
total_draws += draws; total_draws += draws;
total_rewrites += rewrites; total_rewrites += rewrites;
total_moves += moves; total_moves += moves;
@@ -245,7 +245,7 @@ fn bench_input_grows(n: usize, lines: usize) {
let start = Instant::now(); let start = Instant::now();
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
total += start.elapsed(); total += start.elapsed();
let (draws, rewrites, moves) = render.take_counters(); let (draws, rewrites, moves, _shapes) = render.take_counters();
total_draws += draws; total_draws += draws;
total_rewrites += rewrites; total_rewrites += rewrites;
total_moves += moves; total_moves += moves;
@@ -302,7 +302,7 @@ fn bench_insert_above_anchor(n: usize, inserts: usize) {
let start = Instant::now(); let start = Instant::now();
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
total += start.elapsed(); total += start.elapsed();
let (draws, rewrites, moves) = render.take_counters(); let (draws, rewrites, moves, _shapes) = render.take_counters();
total_draws += draws; total_draws += draws;
total_rewrites += rewrites; total_rewrites += rewrites;
total_moves += moves; total_moves += moves;
@@ -384,7 +384,7 @@ fn bench_expand_holds_edge(n: usize, growths: usize) {
let start = Instant::now(); let start = Instant::now();
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
total += start.elapsed(); total += start.elapsed();
let (draws, rewrites, moves) = render.take_counters(); let (draws, rewrites, moves, _shapes) = render.take_counters();
total_draws += draws; total_draws += draws;
total_rewrites += rewrites; total_rewrites += rewrites;
total_moves += moves; total_moves += moves;
+23
View File
@@ -147,6 +147,29 @@ impl Len {
} }
} }
/// The same fold as [`Self::apply_rest`] but staying a `Len`, so
/// `rest` survives: `dp` becomes physical pixels and every other
/// component is left alone.
///
/// **A `Len` a widget *reports* must have been through this.** `dp` is
/// an input unit -- a number the widget author wrote -- and the
/// containers that consume a reported length read `abs`/`rel`/`rest`
/// directly (`Span::draw`'s placement arithmetic, `Pad`'s addition),
/// so a reported `dp` is silently worth zero. That is what made the
/// composer's bar collapse to nothing the moment its content grew past
/// `MaxSize`'s cap: the cap was `dp(168)` and was returned unresolved,
/// so the bar was given a slot of 0 and the field inside it was panned
/// out of a container measured at -63px. `UiRenderState::draw_inner`
/// debug-asserts the invariant after every `Widget::draw`.
pub fn fold_dp(&self, density: f32) -> Self {
Self {
abs: self.abs + self.dp * density,
dp: 0.0,
rel: self.rel,
rest: self.rest,
}
}
pub fn abs(abs: impl UiNum) -> Self { pub fn abs(abs: impl UiNum) -> Self {
Self { Self {
abs: abs.to_f32(), abs: abs.to_f32(),
+9
View File
@@ -245,6 +245,15 @@ impl FrameReport {
/// this once per phase (fling/stream/type/keyboard) so `phase_stats` /// this once per phase (fling/stream/type/keyboard) so `phase_stats`
/// can slice one whole run's frames by what was happening during each. /// can slice one whole run's frames by what was happening during each.
pub fn mark_phase(&mut self, name: &str) { pub fn mark_phase(&mut self, name: &str) {
// `phase_stats`'s slicing (`idx >= phase.start_index && idx <
// end_index`) silently produces an empty or nonsensical slice for
// a phase pushed out of order rather than surfacing the misuse
// (docs/REVIEW-2026-09-06.md finding 5).
debug_assert!(
self.phases
.last()
.is_none_or(|p| self.total_frames >= p.start_index)
);
self.phases.push(PhaseMark { self.phases.push(PhaseMark {
name: name.to_string(), name: name.to_string(),
start_index: self.total_frames, start_index: self.total_frames,
+26
View File
@@ -6,6 +6,7 @@ use crate::{
ArrBuf, ArrBuf,
data::{MaskIdx, MoveIdx, PrimitiveInstance}, data::{MaskIdx, MoveIdx, PrimitiveInstance},
}, },
util::HashSet,
}; };
use bytemuck::Pod; use bytemuck::Pod;
use wgpu::*; use wgpu::*;
@@ -277,6 +278,31 @@ impl Primitives {
} }
} }
/// How many instances are still bound for the GPU -- the O(1) half of
/// the orphan check, so the O(primitives) walk below only runs on a
/// frame that already looks wrong. See
/// [`crate::UiRenderState::orphaned_primitives`].
pub fn live_count(&self) -> usize {
(self.instances.len() - self.free.len()) + (self.images.len() - self.image_free.len())
}
/// Every instance that is still bound for the GPU, as `(inst_idx,
/// owner, is_image)` -- everything except the slots already handed to
/// [`Self::free`] and waiting for [`Self::apply_free`] to compact them
/// away. Only [`crate::UiRenderState::orphaned_primitives`] uses this,
/// to check that every drawn primitive still belongs to a live widget.
pub fn live_instances(&self) -> impl Iterator<Item = (usize, WidgetId, bool)> + '_ {
let free: HashSet<usize> = self.free.iter().copied().collect();
let image_free: HashSet<usize> = self.image_free.iter().copied().collect();
let rects = (0..self.instances.len())
.filter(move |i| !free.contains(i))
.map(|i| (i, self.assoc[i], false));
let images = (0..self.images.len())
.filter(move |i| !image_free.contains(i))
.map(|i| (i, self.image_assoc[i], true));
rects.chain(images)
}
pub fn data(&self) -> &PrimitiveData { pub fn data(&self) -> &PrimitiveData {
&self.data &self.data
} }
+33 -1
View File
@@ -1,4 +1,6 @@
use crate::{LayerId, MaskIdx, MoveIdx, PrimitiveHandle, Size, TextureHandle, UiRegion, WidgetId}; use crate::{
LayerId, MaskIdx, MoveIdx, PrimitiveHandle, Size, TextureHandle, UiRegion, WidgetId, util::Vec2,
};
/// important non rendering data for retained drawing /// important non rendering data for retained drawing
#[derive(Debug)] #[derive(Debug)]
@@ -9,7 +11,22 @@ pub struct ActiveData {
pub textures: Vec<TextureHandle>, pub textures: Vec<TextureHandle>,
pub primitives: Vec<PrimitiveHandle>, pub primitives: Vec<PrimitiveHandle>,
pub children: Vec<WidgetId>, pub children: Vec<WidgetId>,
/// The mask this widget was drawn **under** (its parent's), not the
/// one it set for itself -- see `own_mask` for that.
pub mask: MaskIdx, pub mask: MaskIdx,
/// The mask slot this widget allocated for *itself* with
/// `Painter::set_mask`, or `MaskIdx::NONE`. Kept across redraws and
/// rewritten in place, the way `move_slot` is: a `Masked` that pushed
/// a fresh slot each draw left every already-drawn descendant --
/// which `draw_inner`'s unchanged-region fast path does not revisit --
/// clipping to the *old* slot's region, so a composer whose bar had
/// since been placed at the bottom of the screen was still being
/// clipped to a box at the top of it and drew nothing (measured
/// 2026-09-06: four mask entries live, none of them the widget's
/// current region). Its path out is the `undraw` branch of
/// `UiRenderState::remove`, which drops the self-ownership ref taken
/// when the slot was allocated.
pub own_mask: MaskIdx,
pub layer: LayerId, pub layer: LayerId,
/// What `Widget::draw` returned the last time this widget was actually /// What `Widget::draw` returned the last time this widget was actually
/// drawn -- read by a parent placing this widget again without /// drawn -- read by a parent placing this widget again without
@@ -21,4 +38,19 @@ pub struct ActiveData {
/// so a retained child's `parent` link never goes stale). See /// so a retained child's `parent` link never goes stale). See
/// LAYOUT.md section 2. /// LAYOUT.md section 2.
pub move_slot: MoveIdx, pub move_slot: MoveIdx,
/// How much of this widget's own `move_slot` delta is already folded
/// into `region` above, in window pixels. The two mechanisms that
/// write that slot disagree about this and cannot be told apart from
/// the slot alone: `UiRenderState::mov` shifts `region` and the delta
/// together (the *offered* region genuinely moved), while
/// `Painter::reposition` writes only the delta (`region` stays the
/// offered box and the delta says where inside it the content was
/// placed). So anything that wants the widget's real position --
/// `resolved_region`, and through it every hit test -- must subtract
/// this from the chain sum. Without it a panned widget's own hit box
/// sits at twice the pan while its descendants' are correct, which is
/// how it went unnoticed: the composer's field became untappable
/// after a finger pan (2026-09-06). Reset to zero whenever the widget
/// is really redrawn, since `draw_inner` zeroes the slot then too.
pub move_applied: Vec2,
} }
+31 -2
View File
@@ -13,6 +13,10 @@ pub struct Painter<'a> {
pub(super) region: UiRegion, pub(super) region: UiRegion,
pub(super) mask: MaskIdx, pub(super) mask: MaskIdx,
pub(super) move_slot: MoveIdx, pub(super) move_slot: MoveIdx,
/// This widget's own mask slot, reused across redraws -- see
/// `ActiveData::own_mask`. `MaskIdx::NONE` until `set_mask` is called
/// for the first time in this widget's life.
pub(super) own_mask: MaskIdx,
pub(super) textures: Vec<TextureHandle>, pub(super) textures: Vec<TextureHandle>,
pub(super) primitives: Vec<PrimitiveHandle>, pub(super) primitives: Vec<PrimitiveHandle>,
pub(super) children: Vec<WidgetId>, pub(super) children: Vec<WidgetId>,
@@ -48,12 +52,32 @@ impl<'a> Painter<'a> {
self.primitive_at(primitive, region.within(&self.region)); self.primitive_at(primitive, region.within(&self.region));
} }
/// Clip everything this widget draws, itself and its descendants, to
/// `region`. One per widget: a second call would need the two to be
/// intersected, which nothing here does.
///
/// The slot is allocated once and **rewritten in place** on every
/// later draw rather than pushed again, because a descendant whose own
/// region did not change is not redrawn (`draw_inner`'s fast path) and
/// so keeps pointing at whichever slot it was drawn under. See
/// `ActiveData::own_mask` for what pushing a fresh one cost.
pub fn set_mask(&mut self, region: UiRegion) { pub fn set_mask(&mut self, region: UiRegion) {
assert!(self.mask == MaskIdx::NONE); assert!(self.mask == MaskIdx::NONE);
self.mask = self.rsc.ui_mut().masks.push(Mask { let mask = Mask {
region, region,
move_idx: self.move_slot, move_idx: self.move_slot,
}); };
if self.own_mask == MaskIdx::NONE {
let slot = self.rsc.ui_mut().masks.push(mask);
// The one ref this widget holds on its own slot, so the slot
// outlives any single frame's primitives; released in
// `UiRenderState::remove`'s `undraw` branch.
self.rsc.ui_mut().masks.push_ref(slot);
self.own_mask = slot;
} else {
*self.rsc.ui_mut().masks.get_mut(self.own_mask) = mask;
}
self.mask = self.own_mask;
} }
/// Draws a widget within this widget's region, returning the size it /// Draws a widget within this widget's region, returning the size it
@@ -86,6 +110,7 @@ impl<'a> Painter<'a> {
self.mask, self.mask,
None, None,
None, None,
crate::render::MaskIdx::NONE,
self.rsc, self.rsc,
); );
self.state self.state
@@ -166,6 +191,10 @@ impl<'a> Painter<'a> {
width: Option<f32>, width: Option<f32>,
) -> RenderedText { ) -> RenderedText {
let density = self.state.density; let density = self.state.density;
// Counted here rather than in `TextView::render`, which returns
// its memoized layout without reaching this -- so this counts
// shapes, not requests. `UiRenderState::take_counters`.
self.state.shape_count += 1;
let ui = self.rsc.ui_mut(); let ui = self.rsc.ui_mut();
ui.text ui.text
.render(buffer, attrs, width, &mut ui.textures, density) .render(buffer, attrs, width, &mut ui.textures, density)
+199 -11
View File
@@ -1,7 +1,7 @@
use crate::{ use crate::{
ActiveData, IdLike, MaskIdx, MoveIdx, Painter, PixelRegion, PrimitiveLayers, RegionAlign, ActiveData, IdLike, MaskIdx, MoveIdx, Painter, PixelRegion, PrimitiveLayers, RegionAlign,
StrongWidget, UiRegion, UiRsc, UiVec2, WidgetId, Widgets, StrongWidget, UiRegion, UiRsc, UiVec2, WidgetId, Widgets,
render::MoveOffset, render::{IMAGE_BINDING, MoveOffset},
util::{HashMap, HashSet, Id, Vec2}, util::{HashMap, HashSet, Id, Vec2},
}; };
@@ -18,6 +18,18 @@ pub struct UiRenderState {
old_root: Option<WidgetId>, old_root: Option<WidgetId>,
resized: bool, resized: bool,
/// The widgets whose `Widget::draw` is on the stack right now -- so
/// [`Self::redraw`] can tell "this widget needs drawing again" from
/// "an ancestor is drawing it at this very moment", where a second
/// draw would leave the first one's primitives behind with nothing
/// owning them. An id is inserted immediately before `draw` is called
/// and removed the moment it returns (both in `draw_inner`), so this
/// is empty between frames -- asserted at the end of `update`.
///
/// It used to only ever be inserted into, and `redraw` removed the id
/// *before* testing for it, which made the test constant `false`: the
/// guard could never fire and the set grew by one entry per widget
/// ever drawn and was never emptied.
draw_started: HashSet<WidgetId>, draw_started: HashSet<WidgetId>,
/// The widget currently holding exclusive pointer input, if any -- /// The widget currently holding exclusive pointer input, if any --
@@ -43,6 +55,9 @@ pub struct UiRenderState {
draw_count: u64, draw_count: u64,
region_mut_count: u64, region_mut_count: u64,
mov_count: u64, mov_count: u64,
/// Text layouts actually computed -- bumped by `Painter::render_text`,
/// which `TextView::render` only reaches on a cache miss.
pub(super) shape_count: u64,
} }
/// A move chain more than this deep would mean something else is wrong /// A move chain more than this deep would mean something else is wrong
@@ -64,17 +79,25 @@ impl UiRenderState {
draw_count: 0, draw_count: 0,
region_mut_count: 0, region_mut_count: 0,
mov_count: 0, mov_count: 0,
shape_count: 0,
} }
} }
/// Reads and zeroes the (draws, region_mut rewrites, move_offsets /// Reads and zeroes the (draws, region_mut rewrites, move_offsets
/// writes) counters -- call once per frame before `update()` to /// writes, text shapes) counters -- call once per frame before
/// measure exactly that frame, per LAYOUT.md section 8. /// `update()` to measure exactly that frame, per LAYOUT.md section 8.
pub fn take_counters(&mut self) -> (u64, u64, u64) { ///
/// The fourth is the one a draw count cannot stand in for: a widget
/// can be redrawn without re-shaping (`TextView::render` memoizes by
/// width) and re-shaped without any extra draw, and it is re-shaping
/// that the per-block transcript row exists to avoid -- see
/// `transcript_ui`'s `a_delta_into_a_long_reply_shapes_one_block`.
pub fn take_counters(&mut self) -> (u64, u64, u64, u64) {
( (
std::mem::take(&mut self.draw_count), std::mem::take(&mut self.draw_count),
std::mem::take(&mut self.region_mut_count), std::mem::take(&mut self.region_mut_count),
std::mem::take(&mut self.mov_count), std::mem::take(&mut self.mov_count),
std::mem::take(&mut self.shape_count),
) )
} }
@@ -115,6 +138,11 @@ impl UiRenderState {
); );
} }
let root = root.into(); let root = root.into();
debug_assert!(
self.draw_started.is_empty(),
"a previous frame left {} widget(s) marked as mid-draw",
self.draw_started.len(),
);
if self.needs_redraw_all(root) { if self.needs_redraw_all(root) {
self.redraw_all(root, rsc); self.redraw_all(root, rsc);
self.old_root = root.map(|r| r.id()); self.old_root = root.map(|r| r.id());
@@ -122,6 +150,8 @@ impl UiRenderState {
} else if rsc.widgets().has_updates() { } else if rsc.widgets().has_updates() {
self.redraw_updates(rsc); self.redraw_updates(rsc);
} }
#[cfg(debug_assertions)]
debug_assert!(self.primitive_counts_agree(), "{}", self.orphan_report(rsc),);
} }
fn redraw_all(&mut self, root: Option<&StrongWidget>, rsc: &mut dyn UiRsc) { fn redraw_all(&mut self, root: Option<&StrongWidget>, rsc: &mut dyn UiRsc) {
@@ -137,6 +167,7 @@ impl UiRenderState {
MaskIdx::NONE, MaskIdx::NONE,
None, None,
None, None,
MaskIdx::NONE,
rsc, rsc,
); );
} }
@@ -171,12 +202,27 @@ impl UiRenderState {
mask: MaskIdx, mask: MaskIdx,
old_children: Option<Vec<WidgetId>>, old_children: Option<Vec<WidgetId>>,
old_move_slot: Option<MoveIdx>, old_move_slot: Option<MoveIdx>,
old_own_mask: MaskIdx,
rsc: &mut dyn UiRsc, rsc: &mut dyn UiRsc,
) { ) {
let mut old_children = old_children.unwrap_or_default(); let mut old_children = old_children.unwrap_or_default();
let mut old_move_slot = old_move_slot; let mut old_move_slot = old_move_slot;
let mut own_mask = old_own_mask;
// Consumed here, not merely read: this call *is* the redraw the mark
// asked for, and leaving the mark set is what stranded a widget's
// primitives. `Painter::draw_twice` calls this twice for the same id
// in one frame (`List::place`'s measurement pass), and on the second
// call the still-set mark took the whole `if let` below -- including
// the `remove` that frees the first draw's primitives -- out of play,
// so `active.insert` at the end overwrote the only handles that could
// ever have freed them. The result is a full second copy of the row,
// drawn every frame from then on at the oversized measurement region
// and, with `List` setting no mask, outside the list's own bounds:
// the doubled `Compacted:` row in docs/bench/iris-phone-v2-2026-09-06.md.
// The same shape reaches any dirty widget an ancestor redraws first.
let dirty = rsc.widgets_mut().needs_redraw.remove(&id);
if let Some(active) = self.active.get_mut(&id) if let Some(active) = self.active.get_mut(&id)
&& !rsc.widgets().needs_redraw.contains(&id) && !dirty
{ {
// check to see if we can skip drawing first // check to see if we can skip drawing first
if active.region == region { if active.region == region {
@@ -203,6 +249,15 @@ impl UiRenderState {
*r = r.outside(&from).within(&region); *r = r.outside(&from).within(&region);
self.region_mut_count += 1; self.region_mut_count += 1;
} }
// `move_applied` is deliberately **not** touched here,
// unlike in `mov`: it counts the part of this widget's own
// move-slot delta that `region` has already absorbed, and
// this branch writes no delta at all -- the primitives were
// moved directly. Counting one would make
// `resolved_region` subtract a distance the chain never
// held, putting the hit box short of the drawing by
// exactly this step. See `ActiveData::move_applied`, and
// `a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at`.
active.region = region; active.region = region;
return; return;
} }
@@ -210,10 +265,25 @@ impl UiRenderState {
let active = self.remove(id, false, rsc).unwrap(); let active = self.remove(id, false, rsc).unwrap();
old_children = active.children; old_children = active.children;
old_move_slot = Some(active.move_slot); old_move_slot = Some(active.move_slot);
own_mask = active.own_mask;
} else if dirty && self.active.contains_key(&id) {
// Dirty and already drawn: none of the fast paths above may be
// taken (the widget's own content changed, so its old primitives
// say nothing about its new ones), but they are also the only
// thing that frees them. Same two lines, reached the other way.
let active = self.remove(id, false, rsc).unwrap();
old_children = active.children;
old_move_slot = Some(active.move_slot);
own_mask = active.own_mask;
} }
// draw widget // draw widget
self.draw_started.insert(id); let reentrant = !self.draw_started.insert(id);
debug_assert!(
!reentrant,
"widget {id:?} is being drawn while its own draw is already on the stack; \
the second draw's primitives would orphan the first's"
);
let move_slot = match old_move_slot { let move_slot = match old_move_slot {
// Reused across a real redraw of the same id: the fresh // Reused across a real redraw of the same id: the fresh
@@ -242,11 +312,22 @@ impl UiRenderState {
} }
}; };
// The mask this widget was drawn *under*, kept aside because
// `Painter::set_mask` overwrites `painter.mask` with the widget's
// own new one -- and `ActiveData::mask`'s only consumer is
// `redraw`, which feeds it back in as the *inherited* mask. Storing
// the set one instead handed a `Masked` its own mask on every
// targeted redraw, tripping `set_mask`'s nested-mask assert:
// `assertion failed: self.mask == MaskIdx::NONE`, an abort the
// first time the composer's scroll area was redrawn on the
// emulator.
let inherited_mask = mask;
let mut painter = Painter { let mut painter = Painter {
state: self, state: self,
region, region,
mask, mask,
move_slot, move_slot,
own_mask,
layer, layer,
id, id,
textures: Vec::new(), textures: Vec::new(),
@@ -258,14 +339,26 @@ impl UiRenderState {
let mut widget = painter.rsc.widgets().get_dyn_dynamic(id); let mut widget = painter.rsc.widgets().get_dyn_dynamic(id);
painter.state.draw_count += 1; painter.state.draw_count += 1;
let size = widget.draw(&mut painter); let size = widget.draw(&mut painter);
// A reported length is consumed by containers that read `abs`,
// `rel` and `rest` straight off it (`Span`'s placement, `Pad`'s
// addition), so an unresolved `dp` in one is silently worth zero
// -- see `Len::fold_dp`, which is what a widget reporting a
// caller-declared size has to put it through.
debug_assert!(
size.x.dp == 0.0 && size.y.dp == 0.0,
"widget {id:?} reported an unresolved `dp` size ({size:?}); \
report `Len::fold_dp(painter.density())` instead"
);
drop(widget); drop(widget);
painter.state.draw_started.remove(&id);
let Painter { let Painter {
state: _, state: _,
rsc: _, rsc: _,
region, region,
mask, mask: _,
move_slot, move_slot,
own_mask,
textures, textures,
primitives, primitives,
children, children,
@@ -281,10 +374,12 @@ impl UiRenderState {
textures, textures,
primitives, primitives,
children, children,
mask, mask: inherited_mask,
layer, layer,
size, size,
move_slot, move_slot,
own_mask,
move_applied: Vec2::ZERO,
}; };
// remove old children that weren't kept // remove old children that weren't kept
@@ -312,6 +407,7 @@ impl UiRenderState {
let from_px = from.top_left().to_abs(self.output_size); let from_px = from.top_left().to_abs(self.output_size);
let to_px = to.top_left().to_abs(self.output_size); let to_px = to.top_left().to_abs(self.output_size);
let delta = to_px - from_px; let delta = to_px - from_px;
active.move_applied += delta;
let entry = rsc.ui_mut().move_offsets.get_mut(slot); let entry = rsc.ui_mut().move_offsets.get_mut(slot);
entry.delta[0] += delta.x; entry.delta[0] += delta.x;
entry.delta[1] += delta.y; entry.delta[1] += delta.y;
@@ -346,6 +442,12 @@ impl UiRenderState {
let Some(active) = self.active.get(&id) else { let Some(active) = self.active.get(&id) else {
return; return;
}; };
debug_assert!(
active.move_applied == Vec2::ZERO,
"widget {id:?} is both moved by its parent's own layout (`mov`) and repositioned \
within it; the two write the same slot with different conventions -- see \
`ActiveData::move_applied`"
);
let from = active let from = active
.size .size
.to_uivec2(self.density) .to_uivec2(self.density)
@@ -387,6 +489,11 @@ impl UiRenderState {
// the parent's own `ActiveData` may already be gone by the // the parent's own `ActiveData` may already be gone by the
// time a deep descendant is retired (see LAYOUT.md // time a deep descendant is retired (see LAYOUT.md
// section 2's lifecycle note). // section 2's lifecycle note).
if active.own_mask != MaskIdx::NONE {
// The self-ownership ref `Painter::set_mask` took when
// it allocated this widget's own mask slot.
rsc.ui_mut().masks.remove(active.own_mask);
}
let parent_slot = rsc.ui_mut().move_offsets[active.move_slot.idx()].parent; let parent_slot = rsc.ui_mut().move_offsets[active.move_slot.idx()].parent;
rsc.ui_mut().move_offsets.remove(active.move_slot); rsc.ui_mut().move_offsets.remove(active.move_slot);
if parent_slot != MoveOffset::NONE_PARENT { if parent_slot != MoveOffset::NONE_PARENT {
@@ -452,6 +559,79 @@ impl UiRenderState {
self.active.len() self.active.len()
} }
/// Primitive instances still bound for the GPU whose owner is no
/// longer in `active`, or whose owner's `ActiveData` no longer names
/// them: a copy nothing can move, clip, resize or free, redrawn every
/// frame at whatever position it last had. `(layer, inst_idx, owner)`
/// each.
///
/// Asserted empty at the end of every [`Self::update`], because this
/// is exactly the shape of the duplicated transcript row on Iris's
/// phone (`docs/bench/iris-phone-v2-2026-09-06.md`): counting
/// `active` alone cannot see it, since the orphan's owner is very
/// much alive -- it is the *earlier* set of primitives that got
/// stranded when the widget was drawn a second time without the first
/// draw being freed. O(primitives), debug builds only.
pub fn orphaned_primitives(&self) -> Vec<(usize, usize, WidgetId)> {
let mut orphans = Vec::new();
for (layer, primitives) in self.layers.iter() {
for (inst_idx, owner, is_image) in primitives.live_instances() {
let owned = self.active.get(&owner).is_some_and(|a| {
a.primitives.iter().any(|h| {
h.layer == layer
&& h.inst_idx == inst_idx
&& (h.binding == IMAGE_BINDING) == is_image
})
});
if !owned {
orphans.push((layer, inst_idx, owner));
}
}
}
orphans
}
/// Whether every primitive still bound for the GPU is owned by a live
/// widget, decided by counting rather than by walking: an orphan is a
/// live instance no `ActiveData` names, so it can only ever make the
/// live count exceed the owned one. O(active widgets) -- a few dozen --
/// against [`Self::orphaned_primitives`]'s O(primitives), which on a
/// transcript is tens of thousands and made a debug build on a phone
/// too slow to finish a benchmark run.
fn primitive_counts_agree(&self) -> bool {
let live: usize = self.layers.iter().map(|(_, p)| p.live_count()).sum();
let owned: usize = self.active.values().map(|a| a.primitives.len()).sum();
live == owned
}
/// The message [`Self::update`]'s orphan assert prints -- built here
/// rather than inline so the (allocating, O(primitives)) work only
/// happens on the failing path.
#[cfg(debug_assertions)]
fn orphan_report(&self, rsc: &dyn UiRsc) -> String {
let orphans = self.orphaned_primitives();
let mut lines: Vec<String> = orphans
.iter()
.take(8)
.map(|(layer, idx, owner)| {
let alive = self.active.contains_key(owner);
format!(
" layer {layer} instance {idx}: owner '{}' ({owner:?}), owner still active: {alive}",
rsc.widgets().label(*owner),
)
})
.collect();
if orphans.len() > lines.len() {
lines.push(format!(" ... and {} more", orphans.len() - lines.len()));
}
format!(
"{} primitive(s) are drawn but owned by nobody -- a stale copy \
nothing will ever move or free:\n{}",
orphans.len(),
lines.join("\n"),
)
}
/// Give `id` exclusive pointer input from the next `run_sensors` call /// Give `id` exclusive pointer input from the next `run_sensors` call
/// on -- see `captured`'s field doc. Overwrites any previous capture /// on -- see `captured`'s field doc. Overwrites any previous capture
/// (a gesture that starts a new one has already decided the old one /// (a gesture that starts a new one has already decided the old one
@@ -499,7 +679,12 @@ impl UiRenderState {
/// section 2b. /// section 2b.
pub fn resolved_region(&self, id: &impl IdLike, rsc: &dyn UiRsc) -> Option<UiRegion> { pub fn resolved_region(&self, id: &impl IdLike, rsc: &dyn UiRsc) -> Option<UiRegion> {
let active = self.active.get(&id.id())?; let active = self.active.get(&id.id())?;
let delta = self.resolve_move_chain(active.move_slot, rsc); // The chain sum is what the shader adds to this widget's
// *primitives*, which were written before any of those moves.
// `region`, unlike them, has already been shifted by whatever
// part of this widget's own slot `mov` put there -- see
// `ActiveData::move_applied`, which is exactly that part.
let delta = self.resolve_move_chain(active.move_slot, rsc) - active.move_applied;
Some(active.region.offset(UiVec2::abs(delta))) Some(active.region.offset(UiVec2::abs(delta)))
} }
@@ -535,7 +720,10 @@ impl UiRenderState {
/// redraws a widget that's currently active (drawn) /// redraws a widget that's currently active (drawn)
pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) { pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) {
rsc.widgets_mut().needs_redraw.remove(&id); rsc.widgets_mut().needs_redraw.remove(&id);
self.draw_started.remove(&id); // An ancestor is drawing this widget right now, and that draw is
// about to write fresh primitives for it. Drawing it a second time
// here would leave one of the two copies on screen with nothing
// owning it -- see `draw_started`'s own doc.
if self.draw_started.contains(&id) { if self.draw_started.contains(&id) {
return; return;
} }
@@ -559,9 +747,9 @@ impl UiRenderState {
active.mask, active.mask,
Some(active.children), Some(active.children),
Some(active.move_slot), Some(active.move_slot),
active.own_mask,
rsc, rsc,
); );
// If this widget's own reported size changed, its parent's layout // If this widget's own reported size changed, its parent's layout
// (which placed it using the old size) is now stale and needs to // (which placed it using the old size) is now stale and needs to
// relay out too. Checked after the real draw, not before it -- // relay out too. Checked after the real draw, not before it --
+34
View File
@@ -368,6 +368,21 @@ impl<State: AndroidAppState> IrisViewPeer<State> {
let current_insets = ui_state.insets(); let current_insets = ui_state.insets();
if current_insets != ui_state.last_insets { if current_insets != ui_state.last_insets {
let physical = WindowInsets::from_physical(current_insets); let physical = WindowInsets::from_physical(current_insets);
// One line per real insets change. Iris's phone is the only
// place several of these bugs reproduce and `adb logcat` is
// the only instrument there (this-machine-android: system
// tracing is broken on that device), so the numbers a layout
// is actually fed have to reach the log -- "the composer
// floats at launch" is unanswerable from a screenshot alone.
log::info!(
"iris insets: left={} top={} right={} bottom={} ime_bottom={} window={:?}",
physical.left,
physical.top,
physical.right,
physical.bottom,
physical.ime_bottom,
self.window_size(),
);
self.state.android_state_mut().last_insets = current_insets; self.state.android_state_mut().last_insets = current_insets;
self.state.on_insets_changed(&mut self.rsc, physical); self.state.on_insets_changed(&mut self.rsc, physical);
} }
@@ -627,6 +642,12 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
// backgrounding) still goes through `AndroidRenderer::new` below, // backgrounding) still goes through `AndroidRenderer::new` below,
// since `renderer` is `None` in that case. // since `renderer` is `None` in that case.
let already_live = self.state.android_state().renderer.is_some(); let already_live = self.state.android_state().renderer.is_some();
log::info!(
"iris surface: surface_changed {width}x{height} already_live={already_live} \
glyphs_cached={} atlas_pages={}",
self.rsc.ui.text.atlas.glyph_count(),
self.rsc.ui.text.atlas.page_count(),
);
if already_live { if already_live {
let ui_state = self.state.android_state_mut(); let ui_state = self.state.android_state_mut();
ui_state ui_state
@@ -671,6 +692,13 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
// builds a new renderer, exactly where invalidation is // builds a new renderer, exactly where invalidation is
// needed, never on the reuse branch, where it would throw // needed, never on the reuse branch, where it would throw
// away perfectly valid GPU state for nothing. // away perfectly valid GPU state for nothing.
log::info!(
"iris surface: new renderer built ({:?}), clearing glyph atlas: \
glyphs={} pages={}",
renderer.adapter_backend,
self.rsc.ui.text.atlas.glyph_count(),
self.rsc.ui.text.atlas.page_count(),
);
self.rsc.ui.text.atlas.clear(); self.rsc.ui.text.atlas.clear();
self.rsc.ui.textures.reset(); self.rsc.ui.textures.reset();
self.state.android_state_mut().renderer = Some(renderer); self.state.android_state_mut().renderer = Some(renderer);
@@ -707,6 +735,12 @@ impl<State: AndroidAppState> ViewPeer for IrisViewPeer<State> {
_ctx: &mut CallbackCtx<'local>, _ctx: &mut CallbackCtx<'local>,
_holder: &android_view::SurfaceHolder<'local>, _holder: &android_view::SurfaceHolder<'local>,
) { ) {
log::info!(
"iris surface: surface_destroyed, tearing the renderer down \
(glyphs_cached={} atlas_pages={})",
self.rsc.ui.text.atlas.glyph_count(),
self.rsc.ui.text.atlas.page_count(),
);
self.state.android_state_mut().renderer = None; self.state.android_state_mut().renderer = None;
} }
+33 -2
View File
@@ -129,8 +129,39 @@ fn on_press(
sense: CursorSense, sense: CursorSense,
) { ) {
if state.is_focused(id) { if state.is_focused(id) {
let recent = matches!(sense, CursorSense::PressStart(_)) && state.recent_click(); // Already focused, so there is no keyboard to withhold -- but a
id.edit(rsc).select(pos, size, sense.is_dragging(), recent); // vertical drag still is not a selection. Android's own `EditText`
// scrolls its overflowed text on a vertical drag and starts a
// selection only from a long press; a scroll area wrapping this
// field (`Scroll::drag`) is what actually pans, and it needs the
// first frames of the gesture not to have selected anything behind
// it before it crosses `DRAG_SLOP` and takes pointer capture.
// `press_origin` carries the same meaning here as in the unfocused
// branch below -- "this gesture is still eligible", cleared the
// moment it becomes a drag -- so there is one flag, not two.
match sense {
CursorSense::PressStart(_) => {
let recent = state.recent_click();
id.edit(rsc).text.press_origin = Some(pos);
id.edit(rsc).select(pos, size, false, recent);
}
CursorSense::Pressing(_) | CursorSense::PressEnd(_) => {
let mut ctx = id.edit(rsc);
let Some(origin) = ctx.text.press_origin else {
return;
};
let (dx, dy) = (pos.x - origin.x, pos.y - origin.y);
if dy.abs() > DRAG_SLOP && dy.abs() >= dx.abs() {
ctx.text.press_origin = None;
return;
}
if matches!(sense, CursorSense::PressEnd(_)) {
ctx.text.press_origin = None;
}
ctx.select(pos, size, true, false);
}
_ => {}
}
return; return;
} }
+304 -2
View File
@@ -68,7 +68,7 @@ fn an_unchanged_frame_draws_and_rewrites_nothing() {
render.take_counters(); // discard the first, real draw render.take_counters(); // discard the first, real draw
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
let (draws, rewrites, moves) = render.take_counters(); let (draws, rewrites, moves, _shapes) = render.take_counters();
assert_eq!((draws, rewrites, moves), (0, 0, 0)); assert_eq!((draws, rewrites, moves), (0, 0, 0));
} }
@@ -101,7 +101,7 @@ fn scrolling_moves_in_o1_without_a_redraw() {
// already clamped) rather than actually moving anything. // already clamped) rather than actually moving anything.
rsc.ui.widgets.get_mut(&scroll).unwrap().scroll(-40.0); rsc.ui.widgets.get_mut(&scroll).unwrap().scroll(-40.0);
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
let (draws, _rewrites, moves) = render.take_counters(); let (draws, _rewrites, moves, _shapes) = render.take_counters();
// The pass condition (LAYOUT.md section 8, condition 3) is 0 draws and // The pass condition (LAYOUT.md section 8, condition 3) is 0 draws and
// 1 move_offsets write, independent of how many rects are in the // 1 move_offsets write, independent of how many rects are in the
@@ -147,6 +147,37 @@ fn hit_testing_follows_a_scrolled_widget() {
); );
} }
/// `ActiveData::mask` is the mask a widget was drawn **under**, not the one
/// it set for itself -- `redraw` feeds it straight back in as the inherited
/// mask, so storing the set one hands a `Masked` its own mask the second
/// time round and trips `Painter::set_mask`'s nested-mask assert. That was
/// an abort (`assertion failed: self.mask == MaskIdx::NONE`) the first time
/// the composer's new scroll area was redrawn on the emulator; a targeted
/// redraw of a `Masked` is what any real screen does whenever anything
/// inside it changes.
#[test]
fn redrawing_a_masked_widget_does_not_nest_its_own_mask() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let (_scroll, inner_root, _rects) = scrolled_rects(&mut rsc, 8);
let masked = rsc.ui.widgets.add_strong(Masked { inner: inner_root });
let masked_id = masked.id();
let root = masked.any();
let mut render = UiRenderState::new();
render.resize((800.0, 600.0));
render.update(&root, &mut rsc);
render.redraw(masked_id, &mut rsc);
render.redraw(masked_id, &mut rsc);
assert_eq!(
render.active.get(&masked_id).unwrap().mask,
MaskIdx::NONE,
"a `Masked` at the root is drawn under no mask of its own"
);
}
#[test] #[test]
fn a_mask_stays_put_while_its_scrolled_content_moves() { fn a_mask_stays_put_while_its_scrolled_content_moves() {
let mut rsc = TestRsc { let mut rsc = TestRsc {
@@ -226,6 +257,14 @@ fn composing_text_after_a_keyboard_resize_lands_in_the_bars_own_region() {
render.resize((1080.0, 2298.0)); render.resize((1080.0, 2298.0));
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
// Focusing a field is what places its caret on a real tap
// (`attr.rs`'s `on_press` -> `TextEditCtx::select`), and an insert
// with no caret is a routing bug rather than a state to simulate --
// `insert_str`'s own `debug_assert!` says so, and caught this test
// typing into an unfocused field when it was added.
field
.edit(&mut rsc)
.select(vec2(40.0, 2250.0), vec2(1080.0, 2298.0), false, false);
field.edit(&mut rsc).insert("a"); field.edit(&mut rsc).insert("a");
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
@@ -260,3 +299,266 @@ fn composing_text_after_a_keyboard_resize_lands_in_the_bars_own_region() {
"expected the bar near the bottom of the shorter window: {after_px:?}" "expected the bar near the bottom of the shorter window: {after_px:?}"
); );
} }
/// `Scroll` used to be documented as resolving its own lengths against
/// `Painter::output_size` -- the window -- which read as if a scroll area
/// smaller than the screen could not work, and cost a session's
/// investigation before the composer was wired up (docs/RUST.md,
/// 2026-09-06). It measures `painter.px_size()` now, so this pins the
/// three numbers that follow from the offered box: what it reports
/// upward, what its capping parent reports, and how far it can pan.
#[test]
fn a_scroll_measures_the_box_it_was_offered_not_the_window() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let rect = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let tall = rsc.ui.widgets.add_strong(Sized {
inner: rect.any(),
x: None,
y: Some(Len::abs(1000.0)),
});
let scroll = rsc.ui.widgets.add_strong(Scroll::new(tall.any(), Axis::Y));
let scroll_w = scroll.weak();
let scroll_id = scroll.id();
let capped = rsc.ui.widgets.add_strong(MaxSize {
inner: scroll.any(),
x: None,
y: Some(Len::abs(100.0)),
});
let capped_id = capped.id();
let root = capped.any();
let mut render = UiRenderState::new();
render.resize((800.0, 600.0));
// Two passes: the first offers the content a zero-length region
// (nothing measured yet) and learns the real content length from what
// comes back -- see `scrolling_moves_in_o1_without_a_redraw` for why
// that warm-up is deliberate rather than a bug.
render.update(&root, &mut rsc);
rsc.ui.widgets.get_mut(&scroll_w).unwrap().scroll(0.0);
render.update(&root, &mut rsc);
// Reports the *content*, so the cap above it has something to cap;
// reporting the container instead would make the answer a function of
// itself, since the container is sized from this very number.
assert_eq!(
render.active.get(&scroll_id).unwrap().size.y,
Len::abs(1000.0)
);
assert_eq!(
render.active.get(&capped_id).unwrap().size.y,
Len::abs(100.0),
"the cap, not the content and not the window"
);
// Panning is bounded by content minus *container*: 900, not the 400
// a 600px window would give.
rsc.ui.widgets.get_mut(&scroll_w).unwrap().scroll(-10_000.0);
assert!(
(rsc.ui.widgets.get_mut(&scroll_w).unwrap().amt() - 900.0).abs() < 0.01,
"amt={}",
rsc.ui.widgets.get_mut(&scroll_w).unwrap().amt()
);
}
/// The half `hit_testing_follows_a_scrolled_widget` could not see: it
/// checks a *descendant* of the widget `Scroll` actually moves, whose own
/// `region` is stale and is corrected entirely by the move chain. The
/// moved widget itself had its `region` updated *and* the chain delta
/// added on top, so its hit box sat at twice the pan -- which is why a
/// finger pan of the composer left its field untappable. See
/// `ActiveData::move_applied`.
#[test]
fn a_panned_widgets_own_hit_box_moves_exactly_once() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let rect = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let tall = rsc.ui.widgets.add_strong(Sized {
inner: rect.any(),
x: None,
y: Some(Len::abs(1000.0)),
});
let tall_w = tall.weak();
let scroll = rsc.ui.widgets.add_strong(Scroll::new(tall.any(), Axis::Y));
let scroll_w = scroll.weak();
let root = scroll.any();
let mut render = UiRenderState::new();
render.resize((800.0, 600.0));
render.update(&root, &mut rsc);
rsc.ui.widgets.get_mut(&scroll_w).unwrap().scroll(0.0);
render.update(&root, &mut rsc);
let before = render.window_region(&tall_w, &rsc).unwrap();
rsc.ui.widgets.get_mut(&scroll_w).unwrap().scroll(-37.0);
render.update(&root, &mut rsc);
let after = render.window_region(&tall_w, &rsc).unwrap();
assert!(
(after.top_left.y - (before.top_left.y - 37.0)).abs() < 0.01,
"the pan was applied twice: before={before:?} after={after:?}"
);
}
/// A `Masked` used to allocate a **new** mask slot on every draw, and
/// `draw_inner`'s unchanged-region fast path means its descendants are
/// mostly *not* redrawn with it -- so they went on referencing the slot
/// they were first drawn under, whose region had since stopped being the
/// widget's. Measured 2026-09-06 on the composer's tree: four live mask
/// entries, none of them the `Masked`'s current box, and the field it was
/// meant to clip drew nothing at all on the emulator. The slot is
/// allocated once and rewritten in place now (`ActiveData::own_mask`), so
/// this pins both halves: one entry, and that entry is the widget's own
/// region.
#[test]
fn a_masked_widget_keeps_one_mask_slot_that_is_always_its_own_region() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let (_scroll, inner_root, _rects) = scrolled_rects(&mut rsc, 8);
let masked = rsc.ui.widgets.add_strong(Masked { inner: inner_root });
let masked_id = masked.id();
// Placed at the bottom of a `Span::DOWN` behind a `rest(1)` sibling,
// which is what moves the bar away from the provisional slot it is
// first drawn at -- the move that left the stale mask behind.
let filler = rsc.ui.widgets.add_strong(Rect::new(UiColor::BLACK));
let filler = rsc.ui.widgets.add_strong(Sized {
inner: filler.any(),
x: None,
y: Some(rest(1)),
});
let capped = rsc.ui.widgets.add_strong(MaxSize {
inner: masked.any(),
x: None,
y: Some(Len::abs(60.0)),
});
let mut span = Span::empty(Dir::DOWN);
span.push(filler.any());
span.push(capped.any());
let root = rsc.ui.widgets.add_strong(span).any();
let mut render = UiRenderState::new();
render.resize((800.0, 600.0));
for _ in 0..3 {
render.update(&root, &mut rsc);
render.redraw(masked_id, &mut rsc);
}
assert_eq!(
rsc.ui.masks.iter().count(),
1,
"one `Masked` must own exactly one mask slot, however often it is redrawn"
);
let mask = *rsc.ui.masks.iter().next().unwrap();
assert_eq!(
mask.region,
render.active.get(&masked_id).unwrap().region,
"the mask a descendant clips against must be this widget's current box"
);
}
/// A `dp` cap that has done its job must be reported in pixels. `Span`
/// places a child using the `abs`/`rel` of the length it reported, so a
/// `MaxSize` handing back the caller's own `dp(168)` gave the composer's
/// bar a slot of **zero** the moment its content grew past six lines --
/// and the `Scroll` inside then measured its container at -63px (the
/// padding, subtracted from nothing) and panned the whole message out of
/// view. Measured on this checkout's emulator, 2026-09-06:
/// `container=-63 content=415.8 amt=478.8`. See `Len::fold_dp`.
#[test]
fn a_dp_cap_is_reported_in_pixels_so_a_span_can_place_it() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let rect = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let tall = rsc.ui.widgets.add_strong(Sized {
inner: rect.any(),
x: None,
y: Some(Len::abs(1000.0)),
});
let capped = rsc.ui.widgets.add_strong(MaxSize {
inner: tall.any(),
x: None,
y: Some(Len::dp(100.0)),
});
let capped_w = capped.weak();
let filler = rsc.ui.widgets.add_strong(Rect::new(UiColor::BLACK));
let filler = rsc.ui.widgets.add_strong(Sized {
inner: filler.any(),
x: None,
y: Some(rest(1)),
});
let mut span = Span::empty(Dir::DOWN);
span.push(filler.any());
span.push(capped.any());
let root = rsc.ui.widgets.add_strong(span).any();
let mut render = UiRenderState::new();
render.resize((800.0, 600.0));
render.set_density(2.5);
render.update(&root, &mut rsc);
render.update(&root, &mut rsc);
let box_px = render.window_region(&capped_w, &rsc).unwrap();
let height = box_px.bot_right.y - box_px.top_left.y;
assert!(
(height - 250.0).abs() < 0.01,
"expected the 100dp cap at density 2.5 to be a 250px slot, got {height} ({box_px:?})"
);
}
/// The sibling of `a_panned_widgets_own_hit_box_moves_exactly_once`, on
/// the branch that fix had no reason to touch: `draw_inner`'s
/// size-independent fast path rewrites a widget's primitives *in place*
/// and leaves its move slot alone, so unlike `mov` there is no slot delta
/// for `region` to have absorbed. Counting one there anyway makes
/// `resolved_region` subtract a delta the chain never held, and the
/// widget's hit box lands short of where it is drawn by exactly the
/// distance it just moved -- with nothing on screen to say so, since the
/// primitives are in the right place.
#[test]
fn a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let top = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let spacer = rsc.ui.widgets.add_strong(Sized {
inner: top.any(),
x: None,
y: Some(Len::abs(100.0)),
});
let spacer_w = spacer.weak();
// `Rect` is `is_size_independent`, so growing the spacer above it
// offers this one a region that changed *both* position and size --
// the one shape that reaches the branch under test.
let below = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let below_w = below.weak();
let mut span = Span::empty(Dir::DOWN);
span.push(spacer.any());
span.push(below.any());
let root = rsc.ui.widgets.add_strong(span).any();
let mut render = UiRenderState::new();
render.resize((800.0, 600.0));
render.update(&root, &mut rsc);
// `Span` draws each child once at the full region to measure it and
// then places it, so this widget has already been through the branch
// once by the end of the very first frame.
let first = render.window_region(&below_w, &rsc).unwrap();
assert!(
(first.top_left.y - 100.0).abs() < 0.01,
"hit box at {:?}, drawn at y=100",
first.top_left
);
rsc.ui.widgets.get_mut(&spacer_w).unwrap().y = Some(Len::abs(250.0));
render.update(&root, &mut rsc);
let after = render.window_region(&below_w, &rsc).unwrap();
assert!(
(after.top_left.y - 250.0).abs() < 0.01,
"hit box at {:?}, drawn at y=250",
after.top_left
);
}
+12
View File
@@ -781,6 +781,12 @@ impl VelocityTracker {
/// Record one frame's motion. `delta` is this frame's movement since /// Record one frame's motion. `delta` is this frame's movement since
/// the last sample, not a cumulative position. /// the last sample, not a cumulative position.
pub fn add_sample(&mut self, delta: f32, at: Instant) { pub fn add_sample(&mut self, delta: f32, at: Instant) {
// A caller that samples out of order (a restored/replayed
// gesture, a test) would silently produce a negative `span` in
// `velocity`, handled only by its `span <= 0.0 => 0.0` catch-all
// -- masking the bug that produced it rather than surfacing it
// (docs/REVIEW-2026-09-06.md finding 4).
debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last));
self.samples.push_back((at, delta)); self.samples.push_back((at, delta));
while let Some(&(when, _)) = self.samples.front() { while let Some(&(when, _)) = self.samples.front() {
if at.duration_since(when) > VELOCITY_WINDOW { if at.duration_since(when) > VELOCITY_WINDOW {
@@ -956,6 +962,10 @@ impl FlingCalculator {
/// Total signed distance the fling travels before settling, in the /// Total signed distance the fling travels before settling, in the
/// same pixel units `velocity` was given in. /// same pixel units `velocity` was given in.
pub fn distance(&self, velocity: f32) -> f32 { pub fn distance(&self, velocity: f32) -> f32 {
// See `List::fling`'s matching assertion -- a non-finite velocity
// here silently produces a NaN distance rather than surfacing the
// bug that produced it (docs/REVIEW-2026-09-06.md finding 3).
debug_assert!(velocity.is_finite());
if velocity == 0.0 { if velocity == 0.0 {
return 0.0; return 0.0;
} }
@@ -968,6 +978,8 @@ impl FlingCalculator {
/// How long the fling takes to settle. /// How long the fling takes to settle.
pub fn duration(&self, velocity: f32) -> Duration { pub fn duration(&self, velocity: f32) -> Duration {
// See `distance`'s matching assertion, above.
debug_assert!(velocity.is_finite());
if velocity == 0.0 { if velocity == 0.0 {
return Duration::ZERO; return Duration::ZERO;
} }
+70
View File
@@ -246,3 +246,73 @@ fn capturing_one_widget_starves_every_other_widget_of_events() {
"while a's drag holds capture, b must see no hover at all" "while a's drag holds capture, b must see no hover at all"
); );
} }
/// IRIS_TODO.md's "the composer has no touch-drag scroll": `Scroll` only
/// answered a wheel, so a finger drag over overflowed text did nothing.
/// End-to-end over the real wiring -- `scrollable()`'s own registration,
/// `run_sensors`' dispatch, `Scroll::drag`, `DragGesture`'s arbitration and
/// pointer capture -- rather than only `Scroll::drag`'s own unit tests in
/// `scroll.rs`, because the registration is exactly the half those cannot
/// see.
#[test]
fn a_finger_drag_over_a_scroll_area_pans_it() {
let mut rsc = SenseRsc {
ui: UiData::default(),
events: EventManager::default(),
};
// 1000px of content in a 100px window: room to pan.
let scroll_strong = rect(UiColor::WHITE)
.height(Len::abs(1000.0))
.scrollable()
.add_strong(&mut rsc);
let scroll = scroll_strong.weak();
let root = scroll_strong.any();
let mut render = UiRenderState::new();
render.resize((100.0, 100.0));
render.update(&root, &mut rsc);
// `Scroll` reads its content length back from the draw it just did, so
// the frame after is the first one that knows there is anything to pan
// -- the one-frame lag LAYOUT.md section 4 documents. `scroll(0.0)` is
// how `layout_tests.rs` asks for that second frame, and it also drops
// `snap_end`, leaving this parked at the start of the content.
rsc.ui.widgets.get_mut(&scroll).unwrap().scroll(0.0);
render.update(&root, &mut rsc);
assert_eq!(rsc.ui.widgets.get(&scroll).unwrap().amt(), 0.0);
let mut state = ();
let mut down = cursor_at((50.0, 80.0).into());
down.buttons.left = ActivationState::Start;
render.run_sensors(&mut rsc, &mut state, down, (100.0, 100.0).into());
assert_eq!(
rsc.ui.widgets.get(&scroll).unwrap().amt(),
0.0,
"the touch-down alone must not move anything"
);
// Inside the slop: still a tap as far as anything can tell.
let mut nudge = cursor_at((50.0, 80.0 - (DRAG_SLOP - 1.0)).into());
nudge.buttons.left = ActivationState::On;
render.run_sensors(&mut rsc, &mut state, nudge, (100.0, 100.0).into());
assert_eq!(
rsc.ui.widgets.get(&scroll).unwrap().amt(),
0.0,
"a press inside DRAG_SLOP must not scroll"
);
// Past it, upward: the content follows the finger up, which for this
// widget means more `amt`.
let mut drag = cursor_at((50.0, 80.0 - (DRAG_SLOP + 40.0)).into());
drag.buttons.left = ActivationState::On;
render.run_sensors(&mut rsc, &mut state, drag, (100.0, 100.0).into());
let after = rsc.ui.widgets.get(&scroll).unwrap().amt();
assert!(
(after - 40.0).abs() < 0.01,
"expected the 40px past the slop to pan it, got {after}"
);
// And the gesture holds the pointer, so the rest of it reaches this
// widget even once the finger leaves its box.
assert_eq!(render.captured_pointer(), Some(scroll.id()));
}
+194 -2
View File
@@ -424,6 +424,13 @@ impl List {
/// pixels, so `1.0` here is not a placeholder for "unknown density," /// pixels, so `1.0` here is not a placeholder for "unknown density,"
/// it is the correct density for a self-consistent unit system. /// it is the correct density for a self-consistent unit system.
pub fn fling(&mut self, velocity_px_per_s: f32) { pub fn fling(&mut self, velocity_px_per_s: f32) {
// A NaN/inf velocity (a `VelocityTracker::velocity()` divide-by-
// near-zero span, or a caller passing a raw device value straight
// through) would propagate silently into `deceleration_for`'s
// `.ln()` -- the fling either never settles or jumps to NaN
// positions with nothing on screen saying why (docs/
// REVIEW-2026-09-06.md finding 3).
debug_assert!(velocity_px_per_s.is_finite());
if velocity_px_per_s == 0.0 || self.anchor.is_none() { if velocity_px_per_s == 0.0 || self.anchor.is_none() {
self.fling = None; self.fling = None;
return; return;
@@ -763,6 +770,17 @@ impl List {
/// one-frame lag `Scroll`'s own content-length cache accepts, per /// one-frame lag `Scroll`'s own content-length cache accepts, per
/// LAYOUT.md. /// LAYOUT.md.
fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) { fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) {
// Every current caller derives `slot` from `repair_anchor`/
// `prev_slot`/`next_slot`, which already check existence -- but
// that invariant is enforced by convention across three call
// sites, not by this function, which would otherwise fail with a
// bare "index out of bounds" and no context (docs/
// REVIEW-2026-09-06.md finding 2). `slot_widget`, called from
// here, is what actually indexes/`.expect`s on it.
debug_assert!(
self.slot_exists(slot),
"place() called with a slot that doesn't exist: {slot:?}"
);
let axis = self.axis; let axis = self.axis;
let output_len = painter.output_size().axis(axis); let output_len = painter.output_size().axis(axis);
let container_len = painter.region().axis(axis).len(); let container_len = painter.region().axis(axis).len();
@@ -1086,7 +1104,7 @@ mod tests {
.push_front(ListRow::new(key, w)); .push_front(ListRow::new(key, w));
} }
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
let (draws, _rewrites, _moves) = render.take_counters(); let (draws, _rewrites, _moves, _shapes) = render.take_counters();
// None of the already-visible rows (11, 12) were touched: the // None of the already-visible rows (11, 12) were touched: the
// extents for those keys are numerically unchanged, and the only // extents for those keys are numerically unchanged, and the only
@@ -1224,7 +1242,7 @@ mod tests {
rsc.ui.widgets.get_mut(&list_weak).unwrap().scroll(5.0); rsc.ui.widgets.get_mut(&list_weak).unwrap().scroll(5.0);
render.update(&root, &mut rsc); render.update(&root, &mut rsc);
let (draws, _rewrites, moves) = render.take_counters(); let (draws, _rewrites, moves, _shapes) = render.take_counters();
// The visible window is a fixed ~10 rows regardless of n; an // The visible window is a fixed ~10 rows regardless of n; an
// O(n) regression would show up as draws/moves scaling with // O(n) regression would show up as draws/moves scaling with
@@ -1287,6 +1305,52 @@ mod tests {
); );
} }
/// Neither `replacing_the_last_row_stays_pinned_to_the_bottom` nor
/// its sibling below ever asserts the *evicted* key's own bookkeeping
/// is actually gone -- both replace row 4 with another row also keyed
/// `4`, so `heights.remove(&old.key)` removing and re-inserting the
/// same key would pass either test even if it did nothing (docs/
/// REVIEW-2026-09-06.md finding 10; this is `Selection`'s finding 1
/// class of bug -- a stale handle outliving what it points to --
/// production-tested from `List`'s own side). Replacing with a
/// **different** key is what actually exercises the removal.
#[test]
fn replace_back_forgets_the_evicted_keys_own_height() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let mut list = List::new(Axis::Y);
push_rows(&mut rsc, &mut list, &[0, 1, 2, 3, 4], 20.0);
let (list_weak, root) = add_list(&mut rsc, list);
let mut render = UiRenderState::new();
render.resize((100.0, 60.0));
render.update(&root, &mut rsc);
assert!(
rsc.ui
.widgets
.get(&list_weak)
.unwrap()
.heights
.contains_key(&4)
);
let (_weak, new_row) = fixed_row(&mut rsc, 40.0);
let old = rsc
.ui
.widgets
.get_mut(&list_weak)
.unwrap()
.replace_back(ListRow::new(100, new_row));
let list_ref = rsc.ui.widgets.get(&list_weak).unwrap();
assert_eq!(old.map(|o| o.key), Some(4));
assert!(
!list_ref.heights.contains_key(&4),
"the evicted key's cached height must not outlive the row it measured"
);
}
/// The other half of the same fix's contract: replacing a row that is /// The other half of the same fix's contract: replacing a row that is
/// *not* on screen must not move anything that is. `replace_back` only /// *not* on screen must not move anything that is. `replace_back` only
/// touches the last slot's own widget and this file's own `heights`/ /// touches the last slot's own widget and this file's own `heights`/
@@ -1403,6 +1467,69 @@ mod tests {
); );
} }
/// The doubled `Compacted:` row from Iris's phone (docs/bench/
/// iris-phone-v2-2026-09-06.md), reproduced at its mechanism.
///
/// `replacing_the_last_row_many_times_does_not_leak_primitives` above
/// counts *widgets*, which is why it passed all along: the orphan's
/// owner is very much alive -- it is an earlier set of that same
/// widget's primitives that got stranded. What strands them is a row
/// marked dirty and then reached by its **ancestor's** redraw rather
/// than by its own: `draw_inner` only *read* the dirty mark, so the
/// whole branch that frees a redrawn widget's previous primitives was
/// skipped, and the fresh `ActiveData` overwrote the only handles that
/// could ever have freed them. `List` sets no mask, so that copy then
/// draws every frame at whatever region it last had -- including,
/// where the row was being measured at `GENEROUS_PADDING`, well below
/// the list's own box and under the composer.
///
/// Two rows, two shapes of the same fault: row 2 has a cached height
/// (one `widget_within`), row 4 is replaced so it has none (`place`'s
/// `draw_twice`, which reaches `draw_inner` twice for one id in one
/// frame and so orphans a copy even with no ancestor involved).
#[test]
fn an_ancestor_redrawing_a_dirty_row_leaves_no_stale_copy() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let mut list = List::new(Axis::Y);
// Rows that own a primitive *at their own id* (a background rect),
// not only through a child: an orphan is a widget's own primitive
// outliving its own redraw, so a row whose top-level widget paints
// nothing itself cannot show one however broken the path is.
let mut rows = Vec::new();
for key in 0..5u64 {
let (bg_id, row) = background_styled_row(&mut rsc, 20.0);
rows.push((row.id(), bg_id));
list.push_back(ListRow::new(key, row));
}
let (list_weak, root) = add_list(&mut rsc, list);
let mut render = UiRenderState::new();
render.resize((100.0, 100.0));
render.update(&root, &mut rsc);
assert!(render.orphaned_primitives().is_empty());
// A streamed row's content changing: the row is marked dirty (any
// `.set()` on it does this)...
let (row2, row2_bg) = rows[2];
rsc.ui.widgets.get_dyn_mut(row2).unwrap();
rsc.ui.widgets.get_dyn_mut(row2_bg).unwrap();
// Redraw the *list* by name, so the dirty row is reached by its
// ancestor's draw rather than by `redraw_updates` happening to
// pick it first -- which is the order `HashSet` iteration makes
// arbitrary, and the reason this went unnoticed.
render.redraw(list_weak.id(), &mut rsc);
let orphans = render.orphaned_primitives();
assert!(
orphans.is_empty(),
"{} primitive(s) survived their own widget's redraw: {orphans:?}",
orphans.len(),
);
}
/// Enough rows, tall enough, that a fling toward the start has real /// Enough rows, tall enough, that a fling toward the start has real
/// room to travel before `at_start` clamps it -- shared by the fling /// room to travel before `at_start` clamps it -- shared by the fling
/// tests below. /// tests below.
@@ -1480,6 +1607,71 @@ mod tests {
} }
} }
/// `fling_moves_the_list_and_then_settles`/
/// `fling_distance_is_positive_toward_the_end` only check that a fling
/// started, moved the right way and eventually stopped -- both
/// unaffected by *how* the interior ticks split up the total travel
/// (docs/REVIEW-2026-09-06.md finding 9). A regression that made
/// `tick_fling` apply the whole spline distance every tick instead of
/// just this tick's incremental slice would still pass both, while
/// being wildly wrong every intermediate frame -- this pins the
/// per-tick delta to a decelerating curve (`FlingCalculator::
/// position_at`'s own monotonic-and-clamped property, one level
/// down, already covers the calculator alone; this is the same
/// property through `List::tick_fling`'s `scroll`/`extents`
/// accumulation).
#[test]
fn tick_fling_applies_shrinking_incremental_deltas() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let (list_weak, root, mut render) = build_flingable_list(&mut rsc);
rsc.ui.widgets.get_mut(&list_weak).unwrap().jump_to_start();
render.update(&root, &mut rsc);
rsc.ui.widgets.get_mut(&list_weak).unwrap().fling(8000.0);
let start = Instant::now();
let mut prev_top = rsc.ui.widgets.get(&list_weak).unwrap().extents[&0].top;
let mut deltas = Vec::new();
for step in 1..600 {
let now = start + std::time::Duration::from_millis(step * 16);
let still = rsc.ui.widgets.get_mut(&list_weak).unwrap().tick_fling(now);
render.update(&root, &mut rsc);
let Some(top) = rsc
.ui
.widgets
.get(&list_weak)
.unwrap()
.extents
.get(&0)
.map(|e| e.top)
else {
break; // row 0 scrolled out of the loaded extents
};
deltas.push((prev_top - top).abs());
prev_top = top;
if !still {
break;
}
}
assert!(
deltas.len() >= 3,
"fling settled or left row 0's extent before collecting enough samples"
);
// Skip the first tick (the slop-transition jump the arbiter
// applies is a `List::fling`-adjacent concern, not this curve,
// but the very first frame can still carry rounding noise from
// `jump_to_start`'s own layout settling).
for w in deltas[1..].windows(2) {
assert!(
w[1] <= w[0] + 0.01,
"fling's per-tick delta grew instead of decelerating: {:?} then {:?}",
w[0],
w[1]
);
}
}
#[test] #[test]
fn cancel_fling_stops_it_with_no_further_movement() { fn cancel_fling_stops_it_with_no_further_movement() {
let mut rsc = TestRsc { let mut rsc = TestRsc {
+8 -1
View File
@@ -15,7 +15,14 @@ impl MaxSize {
}; };
let len_px = len.apply_rest(density).to_abs(output); let len_px = len.apply_rest(density).to_abs(output);
let max_px = max.apply_rest(density).to_abs(output); let max_px = max.apply_rest(density).to_abs(output);
if len_px > max_px { max } else { len } // `fold_dp`, not the caller's `max` as written: a reported `Len`
// may not carry an unresolved `dp` -- see `Len::fold_dp` for the
// collapsed composer bar this caused.
if len_px > max_px {
max.fold_dp(density)
} else {
len
}
} }
/// The span (in this widget's own local, `UiRegion::FULL`-relative /// The span (in this widget's own local, `UiRegion::FULL`-relative
+257 -5
View File
@@ -1,4 +1,6 @@
use crate::prelude::*; use crate::prelude::*;
use crate::sense::{DragGesture, GestureOutcome};
use std::time::Instant;
pub struct Scroll { pub struct Scroll {
inner: StrongWidget, inner: StrongWidget,
@@ -7,6 +9,12 @@ pub struct Scroll {
snap_end: bool, snap_end: bool,
container_len: f32, container_len: f32,
content_len: f32, content_len: f32,
/// Touch panning, from the same `DragGesture` `List` is driven by
/// (`transcript-ui::Selection::drag`) rather than a second copy of its
/// wiring: arbitration, `DRAG_SLOP` and pointer capture all live in
/// `sense.rs` and only what a committed pan *means* is decided here.
/// See [`Self::drag`].
gesture: DragGesture,
} }
impl Widget for Scroll { impl Widget for Scroll {
@@ -25,10 +33,20 @@ impl Widget for Scroll {
// length itself (read below from what was actually drawn) is never // length itself (read below from what was actually drawn) is never
// stale, so this self-corrects the next frame and never leaves the // stale, so this self-corrects the next frame and never leaves the
// scroll range wrong for long. See LAYOUT.md section 4. // scroll range wrong for long. See LAYOUT.md section 4.
//
// Every length here is resolved against the box this widget was
// **offered** (`px_size`), never `output_size`: a `Scroll` is
// routinely smaller than the window -- the composer's field is
// capped at six lines by a `MaxSize` around it -- and measuring
// the window instead would make the pan range, and so where the
// content sits, a function of the screen rather than of the box.
// (What the previous arithmetic here computed came to the same
// number by a longer route, through a `within_len` against a
// window-relative scalar; it read as if the window were the
// container and cost a session working out that it was not.)
let axis = self.axis; let axis = self.axis;
let output_len = painter.output_size().axis(axis); let container_len = painter.px_size().axis(axis);
let container_len = painter.region().axis(axis).len(); self.container_len = container_len;
self.container_len = container_len.to_abs(output_len);
if self.snap_end { if self.snap_end {
self.amt = self.content_len - self.container_len; self.amt = self.content_len - self.container_len;
@@ -41,12 +59,22 @@ impl Widget for Scroll {
let used = painter.widget_within(&self.inner, region); let used = painter.widget_within(&self.inner, region);
// A child reporting `rel` means "this fraction of what I was
// offered", and what it was offered is this scroll area -- so the
// container, again, is what that resolves against.
self.content_len = used self.content_len = used
.axis(axis) .axis(axis)
.apply_rest(painter.density()) .apply_rest(painter.density())
.within_len(container_len) .to_abs(container_len);
.to_abs(output_len);
// The **content's** size, not the container's. A parent that can
// grow (the composer's bar) should hug the text until its own cap
// stops it, and reporting the container instead would make this
// widget's answer a function of the answer -- the bar is sized
// from what is reported here, so it collapses to nothing and
// never recovers. What keeps the content inside the offered box
// is the mask a caller puts around it (`.scrollable().masked()`),
// not this number.
used used
} }
} }
@@ -60,6 +88,59 @@ impl Scroll {
snap_end: true, snap_end: true,
container_len: 0.0, container_len: 0.0,
content_len: 0.0, content_len: 0.0,
gesture: DragGesture::new(),
}
}
/// Feed one frame of a touch gesture over this scroll area through.
/// Wired by `WidgetLike::scrollable`; a caller building a `Scroll` by
/// hand registers the same senses and calls this.
///
/// `id` is this widget's own id, which `DragGesture` takes pointer
/// capture on once the gesture commits -- so the rest of the drag
/// reaches here even after the finger has left this area, and, just as
/// importantly, stops reaching whatever is *inside* it. That is what
/// resolves a vertical drag over a focused text field: the field sees
/// the first few frames, iris::attr's `on_press` gives up its pending
/// selection the moment they pass `DRAG_SLOP` vertically, and this
/// takes the gesture over. Android's own `EditText` behaves the same
/// way -- a vertical drag scrolls, and only a long press selects.
///
/// No fling: unlike `List`, `Scroll` has no per-frame tick to animate
/// one with (`List::set_redraw_handle`/`tick_fling`), and the areas
/// this wraps today -- a six-line composer, a diagnostics pane -- are
/// at most a screenful, where Android does not fling either. The
/// released velocity is deliberately dropped rather than approximated.
pub fn drag(
&mut self,
render: &UiRenderState,
id: WidgetId,
sense: CursorSense,
pos_window: Vec2,
now: Instant,
) {
// `already_selected: false` -- a scroll area has no selection of
// its own to extend, so a horizontal drag stays `Undecided` and a
// vertical one past the slop pans, which is the whole contract
// here. A caller that *does* own a selection (the transcript's
// `Selection`) drives `DragGesture` itself instead.
match self
.gesture
.handle(render, id, sense, pos_window, now, false)
{
// `scroll(dy)`, not `scroll(-dy)` -- `Selection::drag` passes
// `-dy` to `List::scroll` because a `List`'s anchor offset and
// this widget's `amt` run in *opposite* directions (offset is
// where the anchored edge sits; `amt` is how far the content
// has been pulled up past the top), even though `List::scroll`'s
// own doc claims to mirror this one's convention. The rule that
// holds for both, and the one to check a sign against, is that
// the content follows the finger.
GestureOutcome::Pan(dy) => self.scroll(dy),
GestureOutcome::Undecided
| GestureOutcome::SelectStart
| GestureOutcome::SelectExtend
| GestureOutcome::Released(_) => {}
} }
} }
@@ -70,8 +151,179 @@ impl Scroll {
self.snap_end = self.amt == len; self.snap_end = self.amt == len;
} }
/// How far the content has been pulled past the container's leading
/// edge, in pixels -- 0 at the start of the content. Read-only, for a
/// caller that needs to observe a pan (a test, a scroll indicator).
pub fn amt(&self) -> f32 {
self.amt
}
pub fn scroll(&mut self, amt: f32) { pub fn scroll(&mut self, amt: f32) {
self.amt -= amt; self.amt -= amt;
self.update_amt(); self.update_amt();
} }
} }
#[cfg(test)]
mod tests {
use super::*;
use crate::sense::{CursorButton, DRAG_SLOP};
use iris_core::UiData;
use std::time::Duration;
/// A scroll area with 1000px of content in a 100px box, already
/// settled somewhere in the middle so a drag has room in both
/// directions.
fn area() -> (UiData, Scroll, WidgetId) {
let mut ui = UiData::default();
let inner = ui.widgets.add_strong(Rect::new(UiColor::WHITE)).any();
let id = inner.id();
let mut s = Scroll::new(inner, Axis::Y);
s.content_len = 1000.0;
s.container_len = 100.0;
s.amt = 400.0;
s.snap_end = false;
(ui, s, id)
}
fn press(
s: &mut Scroll,
render: &UiRenderState,
id: WidgetId,
sense: CursorSense,
y: f32,
t: Instant,
) {
s.drag(render, id, sense, Vec2::new(0.0, y), t);
}
#[test]
fn a_vertical_finger_drag_pans_the_content_with_the_finger() {
let (_ui, mut s, id) = area();
let render = UiRenderState::new();
let t = Instant::now();
press(
&mut s,
&render,
id,
CursorSense::PressStart(CursorButton::Left),
0.0,
t,
);
// Finger down by well past the slop: the content follows it down,
// which for this widget means *less* `amt`.
press(
&mut s,
&render,
id,
CursorSense::Pressing(CursorButton::Left),
DRAG_SLOP + 30.0,
t + Duration::from_millis(20),
);
assert!(
(s.amt - 370.0).abs() < 0.01,
"expected the 30px past the slop to be applied downward, got amt={}",
s.amt
);
// ...and the next frame's motion is a plain per-frame delta.
press(
&mut s,
&render,
id,
CursorSense::Pressing(CursorButton::Left),
DRAG_SLOP + 50.0,
t + Duration::from_millis(40),
);
assert!((s.amt - 350.0).abs() < 0.01, "amt={}", s.amt);
}
/// The half the change had no reason to touch: a press that never
/// leaves the slop is a tap, and must move nothing at all -- otherwise
/// every tap on a scrollable field nudges its text.
#[test]
fn a_press_that_stays_inside_the_slop_does_not_scroll() {
let (_ui, mut s, id) = area();
let render = UiRenderState::new();
let t = Instant::now();
press(
&mut s,
&render,
id,
CursorSense::PressStart(CursorButton::Left),
0.0,
t,
);
for (i, y) in [1.0, -2.0, DRAG_SLOP - 0.5].into_iter().enumerate() {
press(
&mut s,
&render,
id,
CursorSense::Pressing(CursorButton::Left),
y,
t + Duration::from_millis(10 * (i as u64 + 1)),
);
}
press(
&mut s,
&render,
id,
CursorSense::PressEnd(CursorButton::Left),
DRAG_SLOP - 0.5,
t + Duration::from_millis(50),
);
assert!(
(s.amt - 400.0).abs() < 0.01,
"a tap scrolled: amt={}",
s.amt
);
}
/// A horizontal drag is not this widget's gesture: it must stay put
/// rather than pick up the vertical noise in a sideways swipe.
#[test]
fn a_horizontal_drag_does_not_scroll() {
let (_ui, mut s, id) = area();
let render = UiRenderState::new();
let t = Instant::now();
s.drag(
&render,
id,
CursorSense::PressStart(CursorButton::Left),
Vec2::new(0.0, 0.0),
t,
);
s.drag(
&render,
id,
CursorSense::Pressing(CursorButton::Left),
Vec2::new(120.0, 3.0),
t + Duration::from_millis(20),
);
assert!((s.amt - 400.0).abs() < 0.01, "amt={}", s.amt);
}
/// Panning stops at the ends of the content rather than running off,
/// which is `update_amt`'s clamp -- checked through `drag` so the two
/// cannot drift apart.
#[test]
fn a_pan_past_the_end_clamps_instead_of_running_off() {
let (_ui, mut s, id) = area();
let render = UiRenderState::new();
let t = Instant::now();
s.drag(
&render,
id,
CursorSense::PressStart(CursorButton::Left),
Vec2::new(0.0, 0.0),
t,
);
s.drag(
&render,
id,
CursorSense::Pressing(CursorButton::Left),
Vec2::new(0.0, 5000.0),
t + Duration::from_millis(20),
);
assert!((s.amt - 0.0).abs() < 0.01, "amt={}", s.amt);
}
}
+5 -2
View File
@@ -26,9 +26,12 @@ impl Widget for Sized {
region.y = y.apply_rest(density).align(AxisAlign::Neg); region.y = y.apply_rest(density).align(AxisAlign::Neg);
} }
let used = painter.widget_within(&self.inner, region); let used = painter.widget_within(&self.inner, region);
// `fold_dp` on the way out: a declared size is a `Len` the caller
// wrote (`.width(dp(48))`), and a *reported* one may not carry an
// unresolved `dp` -- see `Len::fold_dp`.
Size { Size {
x: self.x.unwrap_or(used.x), x: self.x.map(|x| x.fold_dp(density)).unwrap_or(used.x),
y: self.y.unwrap_or(used.y), y: self.y.map(|y| y.fold_dp(density)).unwrap_or(used.y),
} }
} }
} }
+68 -7
View File
@@ -246,7 +246,21 @@ impl<'a> TextEditCtx<'a> {
self.clear_span(); self.clear_span();
let at = match self.text.selection { let at = match self.text.selection {
Some(sel) => sel.focus().index(), Some(sel) => sel.focus().index(),
None => return, // No caret means nowhere to put the text, so this drops the
// keystroke -- which is invisible, and was the whole of the
// "typed text never appears" defect (see `select`'s comment).
// A field the IME is talking to has been focused, and focusing
// one places a caret, so reaching here is a bug in whoever
// routed the input rather than something to recover from.
None => {
debug_assert!(
false,
"insert into a text field with no caret: '{}' was given input \
without being focused, so the keystroke would be dropped silently",
text,
);
return;
}
}; };
let at = at.min(self.text.view.buf.text().len()); let at = at.min(self.text.view.buf.text().len());
self.text.view.buf.edit().insert_str(at, text); self.text.view.buf.edit().insert_str(at, text);
@@ -368,14 +382,28 @@ impl<'a> TextEditCtx<'a> {
// The layout borrows `self`, so the whole decision is made in here and // The layout borrows `self`, so the whole decision is made in here and
// only the answer escapes. // only the answer escapes.
//
// **A press that reaches here has already been hit-tested to this
// widget, so there is no "outside" to clear the selection for.**
// This used to compare `pos` against the *laid-out text's* box and
// set `selection = None` for anything beyond it -- but the laid-out
// text is smaller than the field (padding, and for an empty field a
// box of literally zero width), so tapping an **empty** composer
// granted focus, opened the keyboard, and left `selection` at
// `None` -- and `insert_str` returns early on `None`, so every
// keystroke after that was silently dropped and nothing ever
// appeared. That is RUST.md's P0 box item 2, "composed text never
// becomes visible at all": the buffer was empty the whole time, and
// Gboard's suggestion strip (its own composing state, not ours) is
// what made it look otherwise. Parley's `from_point`/
// `extend_to_point` already clamp a point outside the layout to the
// nearest cursor position, which is what a tap in a field's padding
// should do anyway. Losing focus is a separate path
// (`TextEditCtx::deselect`, called from the backend's focus
// handling), not this one.
let outcome = { let outcome = {
let layout = self.layout(); let layout = self.layout();
let inside = if drag {
pos.x >= 0.0 && pos.y >= 0.0 && pos.x <= layout.width() && pos.y <= layout.height();
if !inside {
if drag { None } else { Some((None, None)) }
} else if drag {
prev_sel.map(|sel| (Some(sel.extend_to_point(layout, pos.x, pos.y)), prev_hit)) prev_sel.map(|sel| (Some(sel.extend_to_point(layout, pos.x, pos.y)), prev_hit))
} else { } else {
let hit = Selection::from_point(layout, pos.x, pos.y); let hit = Selection::from_point(layout, pos.x, pos.y);
@@ -669,6 +697,39 @@ mod tests {
assert_eq!(t.selection.unwrap().focus().index(), 0); assert_eq!(t.selection.unwrap().focus().index(), 0);
} }
/// The defect itself: an empty field's laid-out text is a zero-sized
/// box, so a tap anywhere in it used to land "outside" and clear the
/// selection -- leaving a focused composer that silently swallowed
/// every keystroke (RUST.md's P0 box item 2).
#[test]
fn tapping_an_empty_field_places_a_caret_so_typing_lands() {
let (mut t, mut d) = edit("", EditMode::MultiLine);
ctx(&mut t, &mut d).select(vec2(40.0, 20.0), vec2(1080.0, 2400.0), false, false);
assert!(t.selection.is_some(), "a tap must leave a caret behind");
ctx(&mut t, &mut d).insert("hi");
assert_eq!(content(&t), "hi");
}
/// The half the fix had no reason to touch: a field that *does* hold
/// text, tapped past the end of it (a multi-line composer's padding
/// below the last line) keeps a caret rather than losing the one it
/// had, and the caret lands at the nearest position -- the end.
#[test]
fn tapping_past_the_end_of_the_text_clamps_to_the_end() {
let (mut t, mut d) = edit("abc", EditMode::MultiLine);
ctx(&mut t, &mut d).select(vec2(9000.0, 9000.0), vec2(1080.0, 2400.0), false, false);
assert_eq!(t.selection.unwrap().focus().index(), 3);
}
/// A drag still needs something to extend: with no previous selection
/// there is nothing to drag from, and one must not be invented.
#[test]
fn dragging_without_a_previous_selection_selects_nothing() {
let (mut t, mut d) = edit("abc", EditMode::MultiLine);
ctx(&mut t, &mut d).select(vec2(10.0, 10.0), vec2(1080.0, 2400.0), true, false);
assert!(t.selection.is_none());
}
#[test] #[test]
fn a_single_line_field_refuses_newlines() { fn a_single_line_field_refuses_newlines() {
let (mut t, mut d) = edit("", EditMode::SingleLine); let (mut t, mut d) = edit("", EditMode::SingleLine);
+6
View File
@@ -69,6 +69,12 @@ impl TextView {
} }
self.width = width; self.width = width;
let tex = painter.render_text(&mut self.buf, &self.attrs, width); let tex = painter.render_text(&mut self.buf, &self.attrs, width);
log::debug!(
"iris text render: chars={} width={width:?} glyphs={} size={:?}",
self.buf.text().chars().count(),
tex.glyphs.len(),
tex.size,
);
self.tex = Some(tex.clone()); self.tex = Some(tex.clone());
self.attrs.changed = false; self.attrs.changed = false;
self.buf.changed = false; self.buf.changed = false;
+15
View File
@@ -1,5 +1,6 @@
use super::*; use super::*;
use crate::prelude::*; use crate::prelude::*;
use std::time::Instant;
// these methods should "not require any context" (require unit) because they're in core // these methods should "not require any context" (require unit) because they're in core
widget_trait! { widget_trait! {
@@ -90,6 +91,20 @@ widget_trait! {
let delta = ctx.data.scroll_delta.y * 50.0; let delta = ctx.data.scroll_delta.y * 50.0;
ctx.widget(rsc).scroll(delta); ctx.widget(rsc).scroll(delta);
}) })
// A finger drag, through the same `DragGesture` the
// transcript's `List` is panned by -- `Scroll::drag`'s doc
// has the arbitration and why there is no fling. The wheel
// above and this are the two inputs of one scroll, so they
// are registered together rather than left to each caller.
.on(
CursorSense::click_or_drag() | CursorSense::unclick(),
|ctx, rsc| {
let id = ctx.widget.id();
let (sense, pos) = (ctx.data.sense, ctx.data.cursor.pos);
ctx.widget(rsc)
.drag(ctx.data.render, id, sense, pos, Instant::now());
},
)
.add(state) .add(state)
} }
} }
+12 -1
View File
@@ -88,10 +88,21 @@ where
// height-capped field -- not a background rect and a field drawn as // height-capped field -- not a background rect and a field drawn as
// two independent siblings, which is what let the two disagree on // two independent siblings, which is what let the two disagree on
// where the bar actually was. // where the bar actually was.
// `.scrollable().masked()`: the finger pan (`Scroll::drag`) plus the
// clip that keeps six lines' worth of a longer message inside the
// bar. The mask is the caller's job rather than `Scroll`'s own,
// because `Painter::set_mask` allows exactly one mask per widget and
// a `Scroll` nested under another masked area would abort on the
// second -- `.masked()` is the one mechanism for clipping and this is
// one more use of it (tabs-ui's message area is the other).
// Without it the overflow paints *above* the bar, over the
// transcript: measured before this change at 58px of stray text for a
// 475px message in a 417px box.
let content = field let content = field
.scrollable()
.masked()
.pad(dp(FIELD_PAD_DP)) .pad(dp(FIELD_PAD_DP))
.max_height(dp(APPROX_LINE_HEIGHT_DP * MAX_LINES + FIELD_PAD_DP * 2.0)) .max_height(dp(APPROX_LINE_HEIGHT_DP * MAX_LINES + FIELD_PAD_DP * 2.0))
.scrollable()
.width(rest(1)) .width(rest(1))
.background(rect(UiColor::new(40, 40, 46, 255))) .background(rect(UiColor::new(40, 40, 46, 255)))
.add(rsc); .add(rsc);
+361 -19
View File
@@ -66,6 +66,14 @@ pub struct TranscriptScreen {
/// interior mutability, per `push_row`'s existing `&self`). Drained by /// interior mutability, per `push_row`'s existing `&self`). Drained by
/// [`Self::take_rebuilds`]. /// [`Self::take_rebuilds`].
rebuilds: std::cell::Cell<usize>, rebuilds: std::cell::Cell<usize>,
/// The per-block widgets of the row at the live end of the list --
/// the only row a streamed delta ever lands in -- so
/// [`Self::apply`]'s `ReplaceLast` can replace one markdown block
/// instead of rebuilding the message
/// (`row::RowBlocks::apply_delta`). `None` for a tail that has no
/// delta path (a tool run) or before anything has been pushed. Its
/// removal is every path that replaces or drops the tail row, below.
tail: RefCell<Option<(RowKey, row::RowBlocks)>>,
} }
impl TranscriptScreen { impl TranscriptScreen {
@@ -77,8 +85,39 @@ impl TranscriptScreen {
where where
Rsc::State: FocusHost, Rsc::State: FocusHost,
{ {
let (key, widget) = row::build_row(rsc, self.list, self.selection.clone(), row); let (key, widget, blocks) = row::build_row(rsc, self.list, self.selection.clone(), row);
(self.list)(rsc).push_back(ListRow::new(key, widget)); (self.list)(rsc).push_back(ListRow::new(key, widget));
*self.tail.borrow_mut() = blocks.map(|b| (key, b));
}
/// The `ReplaceLast` fast path: update the tail row's blocks in place
/// if this really is a delta into the same message, and say whether
/// that worked. `false` for anything the caller must rebuild instead
/// -- a tail with no block state (a tool run), a row that is not a
/// `Single`, or a change `RowBlocks::apply_delta` will not take.
fn apply_tail_delta<Rsc: HasEvents>(&self, rsc: &mut Rsc, key: RowKey, row: &FoldedRow) -> bool
where
Rsc::State: FocusHost,
{
let FoldedRow::Single(item) = row else {
return false;
};
let mut tail = self.tail.borrow_mut();
let Some((tail_key, blocks)) = tail.as_mut() else {
return false;
};
if *tail_key != key {
return false;
}
let (sender, markdown_src) = row::item_content(item);
blocks.apply_delta(
rsc,
self.list,
self.selection.clone(),
key,
sender,
&markdown_src,
)
} }
/// Apply the effect of one more folded event without rebuilding the /// Apply the effect of one more folded event without rebuilding the
@@ -134,26 +173,52 @@ impl TranscriptScreen {
} }
} }
RowDiff::ReplaceLast { common } => { RowDiff::ReplaceLast { common } => {
// Only the tail row's content changed -- rebuild that one // Only the tail row's content changed. First try the
// row and swap it in place, keeping every row before it // delta path: the row is a column of one widget per
// untouched. // markdown block, so a delta that lands in the last block
// is one `set_with_spans` and the earlier blocks keep
// their layouts (`row::RowBlocks::apply_delta`, and
// docs/DECISIONS.md for why the row is shaped that way).
let old_key = row::row_key(&old_rows[common].key()); let old_key = row::row_key(&old_rows[common].key());
let (new_key, widget) = let new_key = row::row_key(&new_rows[common].key());
row::build_row(rsc, self.list, self.selection.clone(), &new_rows[common]); if new_key == old_key && self.apply_tail_delta(rsc, new_key, &new_rows[common]) {
if new_key != old_key { for row in &new_rows[common + 1..] {
self.selection.borrow_mut().unregister(old_key); self.push_row(rsc, row);
} }
return;
}
// Otherwise rebuild that one row and swap it in place,
// keeping every row before it untouched. `unregister`
// unconditionally, not only when the key changed: a
// rebuild with *fewer* blocks under the same key would
// otherwise leave the extra blocks in `Selection`
// pointing at widgets the `drop` below frees (the shape
// docs/REVIEW-2026-09-06.md's finding 1 called out).
self.selection.borrow_mut().unregister(old_key);
let (new_key, widget, blocks) =
row::build_row(rsc, self.list, self.selection.clone(), &new_rows[common]);
let evicted = (self.list)(rsc).replace_back(ListRow::new(new_key, widget)); let evicted = (self.list)(rsc).replace_back(ListRow::new(new_key, widget));
drop(evicted); // frees the old row's widget, same as a pop would drop(evicted); // frees the old row's widget, same as a pop would
*self.tail.borrow_mut() = blocks.map(|b| (new_key, b));
for row in &new_rows[common + 1..] { for row in &new_rows[common + 1..] {
self.push_row(rsc, row); self.push_row(rsc, row);
} }
} }
RowDiff::Rebuild => { RowDiff::Rebuild => {
// A row before the tail changed (a regroup) -- nothing // A row before the tail changed (a regroup) -- nothing
// short of a full rebuild expresses that. // short of a full rebuild expresses that. `Selection`
// gets cleared the same way `List` does, right before the
// rows it was pointing at go with it -- `push_row` below
// re-`register`s whatever survives as it rebuilds each
// row (docs/REVIEW-2026-09-06.md finding 1: a key that
// `group_tool_runs` regrouped away used to stay in
// `Selection` pointing at a widget this `clear()` had
// just freed, panicking the next long-press anywhere).
self.rebuilds.set(self.rebuilds.get() + 1); self.rebuilds.set(self.rebuilds.get() + 1);
self.selection.borrow_mut().clear();
(self.list)(rsc).clear(); (self.list)(rsc).clear();
*self.tail.borrow_mut() = None;
for row in &new_rows { for row in &new_rows {
self.push_row(rsc, row); self.push_row(rsc, row);
} }
@@ -205,9 +270,18 @@ where
let selection = Rc::new(RefCell::new(Selection::new())); let selection = Rc::new(RefCell::new(Selection::new()));
let list = List::new(Axis::Y).add(rsc); let list = List::new(Axis::Y).add(rsc);
// The last row's block widgets are kept for the same reason
// `push_row` keeps them: a reply that is *already* streaming when the
// screen is built takes its next delta through `apply`, and a `None`
// here would send that delta down the rebuild path instead -- the
// whole message re-shaped, which is exactly what the per-block column
// exists to avoid, and nothing on screen or in `take_rebuilds` would
// say so.
let mut tail = None;
for row in &rows { for row in &rows {
let (key, widget) = row::build_row(rsc, list, selection.clone(), row); let (key, widget, blocks) = row::build_row(rsc, list, selection.clone(), row);
list(rsc).push_back(ListRow::new(key, widget)); list(rsc).push_back(ListRow::new(key, widget));
tail = blocks.map(|b| (key, b));
} }
// Wheel/trackpad scrolling -- the same idiom `trait_fns.rs`'s // Wheel/trackpad scrolling -- the same idiom `trait_fns.rs`'s
@@ -234,15 +308,13 @@ where
list.on( list.on(
CursorSense::Pressing(CursorButton::Left) | CursorSense::Drop, CursorSense::Pressing(CursorButton::Left) | CursorSense::Drop,
move |ctx, rsc| { move |ctx, rsc| {
let pos = ctx.data.pos; // Which *block* the finger is over, resolved from its
let row = list(rsc).key_at(pos.y).and_then(|key| { // drawn box rather than from the row's extent -- a row is
let (top, bottom) = list(rsc).extent(key)?; // a column of one widget per markdown block now, and the
Some(( // block is what `Selection` selects (`SelKey`).
key, let row = selection
Vec2::new(pos.x, pos.y - top), .borrow()
Vec2::new(ctx.data.size.x, bottom - top), .locate(&*rsc, ctx.data.render, ctx.data.cursor.pos);
))
});
selection.borrow_mut().drag( selection.borrow_mut().drag(
rsc, rsc,
list, list,
@@ -266,6 +338,7 @@ where
( (
TranscriptScreen { TranscriptScreen {
tail: RefCell::new(tail),
list, list,
composer, composer,
selection, selection,
@@ -414,3 +487,272 @@ mod diff_tests {
assert_eq!(diff_rows(&old, &new), RowDiff::Rebuild); assert_eq!(diff_rows(&old, &new), RowDiff::Rebuild);
} }
} }
/// Exercises `TranscriptScreen::apply`'s `Rebuild` arm through a real
/// `Selection`, the gap docs/REVIEW-2026-09-06.md finding 8 named: the
/// pure `diff_rows` decision above and `selection.rs`'s own registration
/// tests each pass in isolation, and neither alone catches finding 1 (a
/// regrouped-away row's key surviving in `Selection` after `List::clear()`
/// has already freed its widget). This fails before `Selection::clear()`
/// existed and the `Rebuild` arm called it, with a panic from
/// `TextEditable::edit` resolving the freed slot.
#[cfg(test)]
mod apply_tests {
use super::*;
use client_core::transcript_fold::TranscriptItem;
struct TestFocus {
focus: Option<WeakWidget<TextEdit>>,
}
impl FocusHost for TestFocus {
fn recent_click(&mut self) -> bool {
false
}
fn set_focus(&mut self, id: Option<WeakWidget<TextEdit>>) {
self.focus = id;
}
fn focus_gained(&mut self, _region: Option<PixelRegion>) {}
fn is_focused(&self, id: WeakWidget<TextEdit>) -> bool {
self.focus == Some(id)
}
}
struct TestRsc {
ui: UiData,
events: EventManager<TestRsc>,
}
impl UiRsc for TestRsc {
fn ui(&self) -> &UiData {
&self.ui
}
fn ui_mut(&mut self) -> &mut UiData {
&mut self.ui
}
fn on_draw(&mut self, active: &ActiveData) {
self.events.draw(active);
}
fn on_undraw(&mut self, active: &ActiveData) {
self.events.undraw(active);
}
fn on_remove(&mut self, id: WidgetId) {
self.events.remove(id);
}
}
impl HasState for TestRsc {
type State = TestFocus;
}
impl HasEvents for TestRsc {
fn events(&self) -> &EventManager<Self> {
&self.events
}
fn events_mut(&mut self) -> &mut EventManager<Self> {
&mut self.events
}
}
fn user(seq: u64, text: &str) -> TranscriptItem {
TranscriptItem::UserMsg {
seq,
text: text.to_string(),
attachments: Vec::new(),
}
}
fn tool(seq: u64, run_id: &str) -> TranscriptItem {
TranscriptItem::ToolRun {
seq,
id: format!("id{seq}"),
run_id: run_id.to_string(),
tool: "grep".to_string(),
input: "x".to_string(),
output: String::new(),
done: false,
asks: Vec::new(),
images: Vec::new(),
}
}
fn assistant(seq: u64, text: &str) -> TranscriptItem {
TranscriptItem::AssistantMsg {
seq,
text: text.to_string(),
settled: false,
}
}
/// A reply of `paragraphs` paragraphs, the last one still growing.
fn reply(paragraphs: usize, tail: &str) -> String {
let mut out = String::new();
for i in 0..paragraphs {
out.push_str(&format!("Paragraph number {i} of a streamed reply.\n\n"));
}
out.push_str(tail);
out
}
/// `(Widget::draw` calls, text layouts) caused by one streamed delta
/// landing in the last paragraph of a reply that already has
/// `paragraphs` of them.
fn cost_of_one_delta(paragraphs: usize) -> (u64, u64) {
let mut rsc = TestRsc {
ui: UiData::default(),
events: EventManager::default(),
};
let old_items = vec![assistant(1, &reply(paragraphs, "and the last one is st"))];
let new_items = vec![assistant(
1,
&reply(paragraphs, "and the last one is still going."),
)];
let (screen, tree) = build_tree(
&mut rsc,
client_core::transcript_fold::group_tool_runs(&old_items),
);
let mut render = UiRenderState::new();
render.resize((1080.0, 20000.0));
render.update(&tree, &mut rsc);
render.take_counters();
screen.apply(&mut rsc, &old_items, &new_items);
render.update(&tree, &mut rsc);
assert_eq!(screen.take_rebuilds(), 0, "the delta path must be taken");
let (draws, _, _, shapes) = render.take_counters();
(draws, shapes)
}
/// The pass condition for docs/DECISIONS.md's per-block row: a delta
/// costs the **last block**, not the message. A 3,000-character reply
/// has a hundred paragraphs already laid out; redrawing one delta into it
/// must cost exactly what the same delta costs in a one-paragraph
/// reply, or the earlier blocks are being re-shaped.
///
/// Before the split this was one `TextEdit` for the whole message, so
/// the count was the same *number* of widgets but each redraw
/// re-shaped every paragraph through parley -- which a draw counter
/// cannot see. What it can see is that the count does not *grow* with
/// the message, which it now does not and could not before, since the
/// one widget's own layout was O(message).
#[test]
fn a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one() {
assert!(
reply(100, "").len() > 3_000,
"the long case must actually be a long message"
);
let (short_draws, short_shapes) = cost_of_one_delta(1);
let (long_draws, long_shapes) = cost_of_one_delta(100);
assert_eq!(
short_draws, long_draws,
"a delta into a 100-paragraph reply redrew {long_draws} widgets against \
{short_draws} for a one-paragraph reply -- the earlier blocks are not being kept"
);
// The half a draw counter cannot see, and the one the per-block
// row actually exists for: a redraw is free if the text engine
// hits its memo, and a re-shape is the expensive thing. One
// shape, whatever the message is worth -- the block the delta
// landed in. Before the split this was necessarily O(message),
// since the whole reply was one buffer.
assert_eq!(
(short_shapes, long_shapes),
(1, 1),
"a delta shaped {long_shapes} text layouts in a 100-paragraph reply and \
{short_shapes} in a one-paragraph one; it must be the last block and nothing else"
);
}
#[test]
fn a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection() {
use client_core::transcript_fold::group_tool_runs;
let mut rsc = TestRsc {
ui: UiData::default(),
events: EventManager::default(),
};
// Same regroup shape as diff_tests' regroup case, plus a trailing
// row (seq 4) that survives unchanged -- what a reader would tap
// on right after the regroup lands.
let old_items = vec![tool(1, "run-a"), user(2, "meanwhile"), user(4, "stable")];
let new_items = vec![tool(1, "run-a"), tool(3, "run-a"), user(4, "stable")];
assert_eq!(
diff_rows(&group_tool_runs(&old_items), &group_tool_runs(&new_items)),
RowDiff::Rebuild,
"test setup must actually exercise the Rebuild arm"
);
let (screen, _tree) = build_tree(&mut rsc, group_tool_runs(&old_items));
screen.apply(&mut rsc, &old_items, &new_items);
// The surviving row (seq 4) is what a reader's long-press would
// land on; `begin` deselects every *other* registered row first,
// which is exactly what used to resolve a stale `WeakWidget` left
// by the regrouped-away rows and panic.
let surviving_key = row::row_key(&client_core::transcript_fold::ItemKey::Seq(4));
screen.selection.borrow_mut().begin(
&mut rsc,
(surviving_key, 0),
Vec2::ZERO,
Vec2::new(10.0, 10.0),
);
}
/// The failure half of the per-block row, and the one
/// `a_row_dropped_by_a_regroup_...` cannot reach: the tail row is
/// rebuilt under the **same key** with *fewer* blocks than it had.
/// `Selection` is keyed by `(row, block)`, so the blocks that no
/// longer exist are left pointing at widgets `replace_back`'s drop
/// frees -- and `begin` resolves every registered handle on an
/// ordinary press, so the next tap anywhere in the transcript
/// panics. Nothing about the key changed, which is why the
/// `if new_key != old_key` guard this replaced could not see it.
#[test]
fn a_tail_rebuilt_with_fewer_blocks_leaves_none_of_them_in_selection() {
use client_core::transcript_fold::group_tool_runs;
let mut rsc = TestRsc {
ui: UiData::default(),
events: EventManager::default(),
};
// Three blocks, then one. The rewrite is of an *earlier* block
// (the heading), so `RowBlocks::apply_delta` refuses it and the
// rebuild path is the one taken -- assert that below.
let old_items = vec![user(1, "stable"), assistant(2, "# Head\n\npara\n\n- item")];
let new_items = vec![user(1, "stable"), assistant(2, "short")];
assert_eq!(
diff_rows(&group_tool_runs(&old_items), &group_tool_runs(&new_items)),
RowDiff::ReplaceLast { common: 1 },
"test setup must actually exercise the ReplaceLast arm"
);
let (screen, _tree) = build_tree(&mut rsc, group_tool_runs(&old_items));
let tail_key = row::row_key(&client_core::transcript_fold::ItemKey::Seq(2));
assert_eq!(
screen
.selection
.borrow()
.registered_blocks(tail_key)
.count(),
3,
"the fixture must start with more blocks than it ends with"
);
screen.apply(&mut rsc, &old_items, &new_items);
assert_eq!(
screen
.selection
.borrow()
.registered_blocks(tail_key)
.count(),
1,
"the blocks the rebuild dropped are still registered"
);
// What a reader does next: press the row that survived. `begin`
// resolves every registered handle, so a stale one panics here.
let surviving_key = row::row_key(&client_core::transcript_fold::ItemKey::Seq(1));
screen.selection.borrow_mut().begin(
&mut rsc,
(surviving_key, 0),
Vec2::ZERO,
Vec2::new(10.0, 10.0),
);
}
}
+198 -29
View File
@@ -1,11 +1,18 @@
//! One `iris::widget::list::ListRow` per folded transcript row //! One `iris::widget::list::ListRow` per folded transcript row
//! (`client_core::transcript_fold::TranscriptRow`). Each row's whole text //! (`client_core::transcript_fold::TranscriptRow`). A row is a **column of
//! -- headings, paragraphs, inline styling -- goes through `markdown` into //! one `TextEdit` per top-level markdown block** (paragraph, heading,
//! **one** `TextEdit`, which is what makes it one thing `Selection` //! fence, list, table -- `client_core::markdown_blocks`), each rendered
//! (`selection.rs`) can select and what lets it wrap and scroll as a //! with `markdown`'s inline spans, so that RUST.md's "hard to get back"
//! single buffer, matching RUST.md's "hard to get back" behaviour 2 (rich //! behaviour 2 (rich inline text) still holds within a block and
//! inline text) and half of behaviour 1 (selectable within a row; across //! behaviour 1 (selection) runs across blocks and rows alike through
//! rows is `selection.rs`'s job). //! `selection.rs`.
//!
//! It was one `TextEdit` for the whole message until 2026-09-06, which
//! meant a streamed delta re-shaped every paragraph of a long reply
//! through parley again -- the stream phase was the one place iris trailed
//! Compose on Iris's phone. [`RowBlocks::apply_delta`] is the other half
//! of the fix; docs/DECISIONS.md's entry has what the alternative shapes
//! were and why this one.
//! //!
//! A `TranscriptRow::Tools` (a run of adjacent tool calls, grouped by //! A `TranscriptRow::Tools` (a run of adjacent tool calls, grouped by
//! `client_core::transcript_fold::group_tool_runs`) is the row that proves //! `client_core::transcript_fold::group_tool_runs`) is the row that proves
@@ -17,11 +24,18 @@
//! `list.rs`'s module doc describes for `AGENTS.md`'s `holdTopEdge`. //! `list.rs`'s module doc describes for `AGENTS.md`'s `holdTopEdge`.
use crate::markdown::render_markdown; use crate::markdown::render_markdown;
use crate::selection::Selection; use crate::selection::{SelKey, Selection};
use client_core::markdown_blocks::{Block, BlockKind, common_prefix, split_blocks};
use client_core::transcript_fold::{QuestionCard, TranscriptItem, TranscriptRow as FoldedRow}; use client_core::transcript_fold::{QuestionCard, TranscriptItem, TranscriptRow as FoldedRow};
use iris::prelude::*; use iris::prelude::*;
use std::{cell::RefCell, rc::Rc, time::Instant}; use std::{cell::RefCell, rc::Rc, time::Instant};
/// The gap drawn between two markdown blocks of one message. A block used
/// to be separated by the blank line `markdown::render_markdown` put in
/// the single buffer; now that each block is its own widget, that spacing
/// has to be the column's.
const BLOCK_GAP_DP: f32 = 8.0;
/// The paragraph size every row's `TextEdit` is built at; markdown headings /// The paragraph size every row's `TextEdit` is built at; markdown headings
/// inside a row scale relative to a fixed set of sizes rather than this one /// inside a row scale relative to a fixed set of sizes rather than this one
/// (`markdown::heading_size`), since a heading is meant to look the same /// (`markdown::heading_size`), since a heading is meant to look the same
@@ -51,7 +65,7 @@ pub fn row_key(key: &client_core::transcript_fold::ItemKey) -> RowKey {
/// The sender label shown above a row's text, and the markdown source to /// The sender label shown above a row's text, and the markdown source to
/// render below it. `None` for a system-style note that has no sender. /// render below it. `None` for a system-style note that has no sender.
fn item_content(item: &TranscriptItem) -> (Option<&str>, String) { pub(crate) fn item_content(item: &TranscriptItem) -> (Option<&str>, String) {
match item { match item {
TranscriptItem::UserMsg { text, .. } => (Some("You"), text.clone()), TranscriptItem::UserMsg { text, .. } => (Some("You"), text.clone()),
TranscriptItem::AssistantMsg { text, .. } => (Some("Claude"), text.clone()), TranscriptItem::AssistantMsg { text, .. } => (Some("Claude"), text.clone()),
@@ -107,24 +121,53 @@ fn tool_call_markdown(tool: &str, input: &str, output: &str) -> String {
out out
} }
/// Build one `TextEdit` from a sender label plus markdown source, register /// The per-block text widgets of one row, kept by `TranscriptScreen` for
/// it with `selection` under `key`, and wire the pointer handlers that /// the row a reply is streaming into, so a delta can replace the block it
/// drive `Selection::drag` -- shared by every row variant below, since a /// lands in instead of re-shaping the whole message
/// selectable row is always "one TextEdit plus this wiring" regardless of /// (docs/DECISIONS.md, 2026-09-06). Nothing else needs it: a row that is
/// what folded it. `list` is threaded through so that same drag can pan /// not the tail never changes.
/// the list instead of selecting, per `Selection::drag`'s own doc. pub struct RowBlocks {
fn build_text_row<Rsc: HasEvents>( /// What each field was built from, in order -- compared against a
/// fresh split to decide what may be kept. See
/// `client_core::markdown_blocks`' module doc for why this is a
/// comparison and not an assumption.
blocks: Vec<Block>,
fields: Vec<WeakWidget<TextEdit>>,
column: WeakWidget<Span>,
/// The sender label the row was built with. A delta that changes it is
/// not a delta into the same message, so it falls back to a rebuild.
sender: Option<String>,
}
/// Split for display: never empty, so a row with nothing in it yet is
/// still one (empty) text widget rather than no widget at all -- an empty
/// column reports a zero size and the row would vanish from the list.
fn display_blocks(markdown_src: &str) -> Vec<Block> {
let blocks = split_blocks(markdown_src);
if blocks.is_empty() {
vec![Block {
kind: BlockKind::Paragraph,
source: markdown_src.to_string(),
}]
} else {
blocks
}
}
/// One block's own `TextEdit`, registered with `selection` under
/// `(row, block)` and wired to `Selection::drag` -- the block is the
/// selection unit (`selection::SelKey`).
fn build_block_field<Rsc: HasEvents>(
rsc: &mut Rsc, rsc: &mut Rsc,
list: WeakWidget<List>, list: WeakWidget<List>,
selection: Rc<RefCell<Selection>>, selection: Rc<RefCell<Selection>>,
key: RowKey, key: SelKey,
sender: Option<&str>, source: &str,
markdown_src: &str, ) -> WeakWidget<TextEdit>
) -> StrongWidget
where where
Rsc::State: FocusHost, Rsc::State: FocusHost,
{ {
let (text, spans) = render_markdown(markdown_src, BASE_SIZE); let (text, spans) = render_markdown(source, BASE_SIZE);
let field = wtext(text) let field = wtext(text)
.spans(spans) .spans(spans)
.editable(EditMode::MultiLine) .editable(EditMode::MultiLine)
@@ -137,7 +180,7 @@ where
field field
// `| CursorSense::unclick()` on top of the usual click-or-drag set // `| CursorSense::unclick()` on top of the usual click-or-drag set
// -- this row's own registration only ever needs to see a // -- this block's own registration only ever needs to see a
// gesture's *first* frame (`PressStart`, or a `Pressing` that // gesture's *first* frame (`PressStart`, or a `Pressing` that
// missed it -- `DragGesture::handle`'s idle-recovery branch); once // missed it -- `DragGesture::handle`'s idle-recovery branch); once
// it commits, `DragGesture` takes pointer capture on `list`'s own // it commits, `DragGesture` takes pointer capture on `list`'s own
@@ -161,6 +204,38 @@ where
}, },
) )
.add(rsc); .add(rsc);
field
}
/// Build a row from a sender label plus markdown source: a column of one
/// `TextEdit` per top-level markdown block, under the sender's own label.
///
/// One widget per block rather than one per message is what makes a
/// streamed delta cost the last block instead of the whole reply -- see
/// [`RowBlocks::apply_delta`] for the other half, and
/// `client_core::markdown_blocks` for the split. Selection still runs
/// across the whole transcript; the unit it steps in is a block now rather
/// than a row (`selection::SelKey`).
fn build_text_row<Rsc: HasEvents>(
rsc: &mut Rsc,
list: WeakWidget<List>,
selection: Rc<RefCell<Selection>>,
key: RowKey,
sender: Option<&str>,
markdown_src: &str,
) -> (StrongWidget, RowBlocks)
where
Rsc::State: FocusHost,
{
let blocks = display_blocks(markdown_src);
let mut column = Span::empty(Dir::DOWN).gap(dp(BLOCK_GAP_DP));
let mut fields = Vec::with_capacity(blocks.len());
for (i, block) in blocks.iter().enumerate() {
let field = build_block_field(rsc, list, selection.clone(), (key, i as u32), &block.source);
fields.push(field);
column.push(field.width(rest(1)).add_strong(rsc).any());
}
let column = column.add(rsc);
// `.add` (weak), not `.add_strong` -- `header` is about to be embedded // `.add` (weak), not `.add_strong` -- `header` is about to be embedded
// as a child of the `.span(Dir::DOWN)` below, whose own composition is // as a child of the `.span(Dir::DOWN)` below, whose own composition is
@@ -178,12 +253,91 @@ where
None => Span::empty(Dir::DOWN).add(rsc), None => Span::empty(Dir::DOWN).add(rsc),
}; };
(header, field.width(rest(1))) let widget = (header, column.width(rest(1)))
.span(Dir::DOWN) .span(Dir::DOWN)
.gap(dp(4)) .gap(dp(4))
.pad(dp(10)) .pad(dp(10))
.add_strong(rsc) .add_strong(rsc)
.any() .any();
(
widget,
RowBlocks {
blocks,
fields,
column,
sender: sender.map(str::to_string),
},
)
}
impl RowBlocks {
/// Bring this row up to date with `markdown_src` **without** re-laying
/// out the blocks that did not change, and say whether that was
/// possible. `false` means the caller must rebuild the row the
/// ordinary way: an earlier block was rewritten (markdown allows it --
/// a trailing `---` turns the paragraph above into a heading), the
/// sender changed, or the message got shorter.
///
/// This is the whole point of the per-block column: a delta arriving
/// in a 3,000-character reply touches one `set_with_spans` on the last
/// block, so parley re-shapes that block and nothing else.
pub fn apply_delta<Rsc: HasEvents>(
&mut self,
rsc: &mut Rsc,
list: WeakWidget<List>,
selection: Rc<RefCell<Selection>>,
key: RowKey,
sender: Option<&str>,
markdown_src: &str,
) -> bool
where
Rsc::State: FocusHost,
{
if self.sender.as_deref() != sender {
return false;
}
let new_blocks = display_blocks(markdown_src);
let common = common_prefix(&self.blocks, &new_blocks);
// Everything already drawn must either be kept whole (`common ==
// len`, a pure append) or be kept except for the last block, which
// is the one a delta lands in. Anything else means an already
// laid-out block is no longer what it was.
if new_blocks.len() < self.blocks.len() || common + 1 < self.blocks.len() {
return false;
}
debug_assert!(
self.fields.len() == self.blocks.len(),
"one field per block: {} fields, {} blocks",
self.fields.len(),
self.blocks.len()
);
for (i, block) in new_blocks.iter().enumerate().skip(common) {
let (text, spans) = render_markdown(&block.source, BASE_SIZE);
match self.fields.get(i) {
Some(field) => field.edit(rsc).set_with_spans(&text, spans),
None => {
let field = build_block_field(
rsc,
list,
selection.clone(),
(key, i as u32),
&block.source,
);
self.fields.push(field);
let child = field.width(rest(1)).add_strong(rsc).any();
// `get_mut` marks the column dirty, which is what gets
// the new block drawn; its removal half is the row's
// own, since the column owns the child strongly.
if let Some(column) = rsc.ui_mut().widgets.get_mut(&self.column) {
column.push(child);
}
}
}
}
self.blocks = new_blocks;
true
}
} }
fn build_single<Rsc: HasEvents>( fn build_single<Rsc: HasEvents>(
@@ -192,7 +346,7 @@ fn build_single<Rsc: HasEvents>(
selection: Rc<RefCell<Selection>>, selection: Rc<RefCell<Selection>>,
key: RowKey, key: RowKey,
item: &TranscriptItem, item: &TranscriptItem,
) -> StrongWidget ) -> (StrongWidget, RowBlocks)
where where
Rsc::State: FocusHost, Rsc::State: FocusHost,
{ {
@@ -249,8 +403,15 @@ where
where where
Rsc::State: FocusHost, Rsc::State: FocusHost,
{ {
// Every block of the previous content goes first: collapsing a
// five-block expansion back to a one-line summary registers only
// `(key, 0)`, and blocks 1..5 would be left in `Selection`
// pointing at widgets `ptr.replace` is about to free -- the same
// class of bug docs/REVIEW-2026-09-06.md's finding 1 found in the
// `Rebuild` arm, reached the other way.
selection.borrow_mut().unregister(key);
let text = if expanded { full } else { summary }; let text = if expanded { full } else { summary };
build_text_row(rsc, list, selection, key, Some("Tools"), text) build_text_row(rsc, list, selection, key, Some("Tools"), text).0
} }
let content = build_content( let content = build_content(
@@ -299,18 +460,26 @@ pub fn build_row<Rsc: HasEvents>(
list: WeakWidget<List>, list: WeakWidget<List>,
selection: Rc<RefCell<Selection>>, selection: Rc<RefCell<Selection>>,
row: &FoldedRow, row: &FoldedRow,
) -> (RowKey, StrongWidget) ) -> (RowKey, StrongWidget, Option<RowBlocks>)
where where
Rsc::State: FocusHost, Rsc::State: FocusHost,
{ {
match row { match row {
FoldedRow::Single(item) => { FoldedRow::Single(item) => {
let key = row_key(&item.key()); let key = row_key(&item.key());
(key, build_single(rsc, list, selection, key, item)) let (widget, blocks) = build_single(rsc, list, selection, key, item);
(key, widget, Some(blocks))
} }
FoldedRow::Tools(calls) => { FoldedRow::Tools(calls) => {
let key = row_key(&calls[0].key()); let key = row_key(&calls[0].key());
(key, build_tools(rsc, list, selection, key, calls.clone())) // `None`: a run of tool calls is never what a reply streams
// into, and its own expand/collapse replaces the whole
// content anyway, so there is no delta path to keep state for.
(
key,
build_tools(rsc, list, selection, key, calls.clone()),
None,
)
} }
} }
} }
+96 -21
View File
@@ -33,9 +33,17 @@
use iris::prelude::*; use iris::prelude::*;
use std::{collections::BTreeMap, time::Instant}; use std::{collections::BTreeMap, time::Instant};
/// What this selects between: a row's `RowKey` and the index of one
/// markdown **block** inside it. A row is a column of one text widget per
/// block since 2026-09-06 (`client_core::markdown_blocks`, and
/// docs/DECISIONS.md for why), so the block, not the row, is the unit --
/// `(row, block)` compares lexicographically, which is reading order for
/// both levels, so every range query below is unchanged.
pub type SelKey = (RowKey, u32);
pub struct Selection { pub struct Selection {
rows: BTreeMap<RowKey, WeakWidget<TextEdit>>, rows: BTreeMap<SelKey, WeakWidget<TextEdit>>,
anchor: Option<(RowKey, Vec2)>, anchor: Option<(SelKey, Vec2)>,
/// One gesture shared by every row's drag handler -- RUST.md's I5 /// One gesture shared by every row's drag handler -- RUST.md's I5
/// gesture conflict (a row's own `click_or_drag()` and a list-level /// gesture conflict (a row's own `click_or_drag()` and a list-level
/// pan wanting the same touch gesture). See `drag` below, and /// pan wanting the same touch gesture). See `drag` below, and
@@ -63,16 +71,38 @@ impl Selection {
} }
/// A row's selectable text became visible/known. Every addition here /// A row's selectable text became visible/known. Every addition here
/// needs its removal (`unregister`) -- called when `List` evicts the /// needs its removal (`unregister`, or `clear` for all of them at
/// row (`pop_front`/`pop_back`), so this map never outgrows however /// once) -- called when `List` evicts the row (`pop_front`/
/// many rows are actually loaded. /// `pop_back`/`clear`), so this map never outgrows however many rows
pub fn register(&mut self, key: RowKey, text: WeakWidget<TextEdit>) { /// are actually loaded. `List::place` guards the twin of this same
/// class of bug on the list's own side (`list.rs`'s `slot_exists`
/// assertion) -- a derived handle that silently outlives what it
/// points to; the next caller adding a third row-keyed side table
/// should read both.
pub fn register(&mut self, key: SelKey, text: WeakWidget<TextEdit>) {
self.rows.insert(key, text); self.rows.insert(key, text);
} }
pub fn unregister(&mut self, key: RowKey) { /// Drops every registration at once -- the same shape `List::clear()`
self.rows.remove(&key); /// clears the list, and what `TranscriptScreen::apply`'s `Rebuild` arm
if self.anchor.map(|(k, _)| k) == Some(key) { /// calls right before it, since a full rebuild drops every row's old
/// widget and `push_row` re-`register`s each surviving key's new one
/// as it goes (review docs/REVIEW-2026-09-06.md finding 1: the
/// `Rebuild` arm used to call only `List::clear()`, leaving any key
/// dropped by the regroup -- present in the old rows, absent from the
/// new ones -- pointing at a widget the list had just freed, so the
/// next long-press anywhere panicked in `begin`'s deselect loop).
pub fn clear(&mut self) {
self.rows.clear();
self.anchor = None;
}
/// Forgets every block of one row -- a row is registered block by
/// block, so its removal has to take all of them, and taking only the
/// first is how a freed widget would be left behind in this map.
pub fn unregister(&mut self, row: RowKey) {
self.rows.retain(|&(k, _), _| k != row);
if self.anchor.map(|((k, _), _)| k) == Some(row) {
self.anchor = None; self.anchor = None;
} }
} }
@@ -82,8 +112,8 @@ impl Selection {
/// gives `key`'s row a collapsed caret at `pos` -- a plain click that /// gives `key`'s row a collapsed caret at `pos` -- a plain click that
/// never turns into a drag leaves exactly this and nothing else /// never turns into a drag leaves exactly this and nothing else
/// selected. /// selected.
pub fn begin(&mut self, ui: &mut impl UiRsc, key: RowKey, pos: Vec2, size: Vec2) { pub fn begin(&mut self, ui: &mut impl UiRsc, key: SelKey, pos: Vec2, size: Vec2) {
let rows: Vec<RowKey> = self.rows.keys().copied().collect(); let rows: Vec<SelKey> = self.rows.keys().copied().collect();
for k in rows { for k in rows {
if k != key if k != key
&& let Some(w) = self.rows.get(&k) && let Some(w) = self.rows.get(&k)
@@ -99,7 +129,7 @@ impl Selection {
/// The drag continues, now over `key`'s row at `pos`. See the module /// The drag continues, now over `key`'s row at `pos`. See the module
/// doc for the anchor-row shortcut. /// doc for the anchor-row shortcut.
pub fn extend(&mut self, ui: &mut impl UiRsc, key: RowKey, pos: Vec2, size: Vec2) { pub fn extend(&mut self, ui: &mut impl UiRsc, key: SelKey, pos: Vec2, size: Vec2) {
let Some((anchor_key, _anchor_pos)) = self.anchor else { let Some((anchor_key, _anchor_pos)) = self.anchor else {
return; return;
}; };
@@ -114,7 +144,7 @@ impl Selection {
} else { } else {
(key, anchor_key) (key, anchor_key)
}; };
let in_range: Vec<RowKey> = self.rows.range(lo..=hi).map(|(&k, _)| k).collect(); let in_range: Vec<SelKey> = self.rows.range(lo..=hi).map(|(&k, _)| k).collect();
for k in &in_range { for k in &in_range {
let Some(w) = self.rows.get(k).copied() else { let Some(w) = self.rows.get(k).copied() else {
continue; continue;
@@ -130,7 +160,7 @@ impl Selection {
w.edit(ui).select_all(); w.edit(ui).select_all();
} }
} }
let outside: Vec<RowKey> = self let outside: Vec<SelKey> = self
.rows .rows
.keys() .keys()
.copied() .copied()
@@ -143,6 +173,47 @@ impl Selection {
} }
} }
/// The block indices currently registered for `row`, in order. For a
/// test asserting that a row's removal or rebuild took every one of
/// its blocks with it -- the contract `unregister` states and the one
/// a caller can get wrong silently, since a stale handle only shows
/// up as a panic on some later, unrelated press.
#[cfg(test)]
pub fn registered_blocks(&self, row: RowKey) -> impl Iterator<Item = u32> + '_ {
self.rows
.keys()
.filter(move |(k, _)| *k == row)
.map(|&(_, b)| b)
}
/// Which registered block is under `pos_window`, with the position
/// and size that block's own `TextEdit` wants (block-local, the way
/// `begin`/`extend` are given them by a block's own pointer handler).
///
/// For the pointer-captured half of a drag, where the event no longer
/// reaches the widget under the finger and the list-level handler has
/// to say where the finger is. It asks the render state for each
/// block's drawn box rather than doing the arithmetic from the row's
/// extent -- the box is what a hit test resolves against anyway, and
/// it means this and a block's own handler cannot disagree about
/// where a block is. O(blocks loaded), on one frame of a drag.
pub fn locate(
&self,
ui: &impl UiRsc,
render: &UiRenderState,
pos_window: Vec2,
) -> Option<(SelKey, Vec2, Vec2)> {
for (&key, w) in &self.rows {
let Some(px) = render.window_region(w, ui) else {
continue;
};
if px.contains(pos_window) {
return Some((key, pos_window - px.top_left, px.size()));
}
}
None
}
/// Whether any row currently has a non-empty selection -- what a fresh /// Whether any row currently has a non-empty selection -- what a fresh
/// press consults so `drag` knows whether an early horizontal move is /// press consults so `drag` knows whether an early horizontal move is
/// "start dragging the selection handle" rather than an ordinary tap. /// "start dragging the selection handle" rather than an ordinary tap.
@@ -178,7 +249,7 @@ impl Selection {
&mut self, &mut self,
ui: &mut impl UiRsc, ui: &mut impl UiRsc,
list: WeakWidget<List>, list: WeakWidget<List>,
row: Option<(RowKey, Vec2, Vec2)>, row: Option<(SelKey, Vec2, Vec2)>,
pos_window: Vec2, pos_window: Vec2,
sense: CursorSense, sense: CursorSense,
now: Instant, now: Instant,
@@ -321,7 +392,7 @@ mod tests {
let list = rsc.ui.widgets.add_strong(List::new(Axis::Y)).weak(); let list = rsc.ui.widgets.add_strong(List::new(Axis::Y)).weak();
let mut sel = Selection::new(); let mut sel = Selection::new();
sel.register(1, field); sel.register((1, 0), field);
assert!(sel.gesture.is_idle()); assert!(sel.gesture.is_idle());
let render = UiRenderState::new(); let render = UiRenderState::new();
@@ -332,7 +403,7 @@ mod tests {
sel.drag( sel.drag(
&mut rsc, &mut rsc,
list, list,
Some((1, Vec2::ZERO, size)), Some(((1, 0), Vec2::ZERO, size)),
Vec2::new(540.0, 700.0), Vec2::new(540.0, 700.0),
CursorSense::Pressing(CursorButton::Left), CursorSense::Pressing(CursorButton::Left),
now, now,
@@ -346,7 +417,7 @@ mod tests {
} }
#[test] #[test]
fn unregister_forgets_the_row_and_clears_a_matching_anchor() { fn unregister_forgets_every_block_of_the_row_and_clears_a_matching_anchor() {
let mut rsc = TestRsc { let mut rsc = TestRsc {
ui: UiData::default(), ui: UiData::default(),
}; };
@@ -360,9 +431,13 @@ mod tests {
.weak(); .weak();
let mut sel = Selection::new(); let mut sel = Selection::new();
sel.register(5, field); // Two blocks of the same row, which is what `unregister` has to
sel.anchor = Some((5, Vec2::ZERO)); // take together -- removing only the first is how a freed widget
assert_eq!(sel.rows.len(), 1); // gets left in this map.
sel.register((5, 0), field);
sel.register((5, 1), field);
sel.anchor = Some(((5, 1), Vec2::ZERO));
assert_eq!(sel.rows.len(), 2);
sel.unregister(5); sel.unregister(5);
assert!(sel.rows.is_empty()); assert!(sel.rows.is_empty());