From 167862ca1b3edd4e28058ad7892162151eacbae6 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 6 Sep 2026 17:17:42 -0400 Subject: [PATCH] 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 --- docs/DECISIONS.md | 25 ++++ docs/IRIS.md | 44 ++++++ docs/IRIS_TODO.md | 39 +++-- docs/RUST.md | 77 ++++++++++ iris/android-app/run-bench.sh | 9 +- iris/core/src/orientation/len.rs | 23 +++ iris/core/src/ui/active.rs | 34 ++++- iris/core/src/ui/painter.rs | 29 +++- iris/core/src/ui/render_state.rs | 47 +++++- iris/src/layout_tests.rs | 209 +++++++++++++++++++++++++++ iris/src/widget/position/max_size.rs | 9 +- iris/src/widget/position/scroll.rs | 30 +++- iris/src/widget/position/sized.rs | 7 +- iris/transcript-ui/src/composer.rs | 24 +-- 14 files changed, 567 insertions(+), 39 deletions(-) diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index a6ceeb3..2fd36f6 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -5,6 +5,31 @@ 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 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 diff --git a/docs/IRIS.md b/docs/IRIS.md index 784a255..308206e 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,6 +8,50 @@ 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 it helps judge the change without the session that made it. Newest first. +## 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 diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index ec3a621..c602040 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -201,19 +201,32 @@ agent takes them without colliding with that pass's `bench_client.rs`/ a capped/scrollable height, bottom padding tied to the IME/nav-bar inset) -- structurally in place and unit-tested, but its own visual correctness cannot be screenshotted until text actually renders. -- [~] **The composer has no touch-drag scroll for overflowing text.** - **The mechanism is in, the composer is not, 2026-09-06.** `Scroll::drag` - takes its pan from the same `sense::DragGesture` `List` uses and - `WidgetLike::scrollable()` registers it beside the wheel handler, so - every scroll area in the codebase now pans on a finger (no fling -- see - IRIS.md). A vertical drag inside a focused field no longer extends a - selection, matching Android's `EditText`. But the composer field is - **not** wrapped in `.scrollable()` -- the note above was describing - intent, not the code -- and wrapping it was tried and reverted: `Scroll` - measures against the window rather than its own offered box, so inside - the `MaxSize` that caps the field at six lines it pans itself out of the - bar entirely (emulator, 474 characters, the bar collapsed to its - padding). docs/RUST.md's plan box has the numbers and the next step. +- [x] **The composer has no touch-drag scroll for overflowing text.** + **Done 2026-09-06.** `field.scrollable().masked()` in + `transcript-ui/src/composer.rs`: a finger drag inside the bar pans the + message, the bar stays capped at six lines, and a vertical drag in the + focused field no longer extends a selection (Android `EditText`'s own + 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. + **Still open, and pre-existing:** the bar's own grey background is not + drawn on this build (the `Stack{StackSize::Child(1)}` behind the field), + so the message reads as white text over the transcript. Present in the + build *before* this change too, so it is not the scroll area's doing. ## From the phone, 2026-09-06, 11:39 (build delivered 02:07, commit 543f6d9) diff --git a/docs/RUST.md b/docs/RUST.md index 5be05dc..88cee51 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -43,6 +43,83 @@ 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 closes it. +### Task A, closed 2026-09-06: the composer scrolls on a finger + +`iris/transcript-ui/src/composer.rs` is `field.scrollable().masked()` now. +Verified on this checkout's emulator -- the evidence and the numbers are +in docs/IRIS_TODO.md's ticked "composer has no touch-drag scroll" item. + +**The premise the task was given under was wrong, and that is worth +recording**: `Scroll` did *not* measure against the window. Its +`used.within_len(container).to_abs(output_size)` came to exactly +`abs + rel * container_px` -- the right number by a route that reads as if +the window were the container, which is what cost a session. It is +`painter.px_size()` and `to_abs(container_len)` now: same arithmetic, +stated the way the invariant is. `Scroll` also still reports its +**content's** size upward, deliberately -- reporting the container makes +the answer a function of itself (the bar is sized *from* that report, so +it collapses to nothing and never recovers; measured in the headless +harness before the shape was settled). + +What actually broke the composer was three separate defects, each now +carrying a headless regression test in `iris/src/layout_tests.rs` that was +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. + +### Bench, before 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. + + stream: 202 frames over 21.0s + late: 197 (97.5%) + total p50 61.5ms p90 211.7ms p99 342.6ms + worst 403.6ms + + - [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 diff --git a/iris/android-app/run-bench.sh b/iris/android-app/run-bench.sh index 44b9b43..3947e9a 100755 --- a/iris/android-app/run-bench.sh +++ b/iris/android-app/run-bench.sh @@ -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 # 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. +# 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 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 break fi @@ -66,4 +71,4 @@ if [ -z "$LINE" ]; then fi # -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. -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" diff --git a/iris/core/src/orientation/len.rs b/iris/core/src/orientation/len.rs index 84c194d..599e37b 100644 --- a/iris/core/src/orientation/len.rs +++ b/iris/core/src/orientation/len.rs @@ -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 { Self { abs: abs.to_f32(), diff --git a/iris/core/src/ui/active.rs b/iris/core/src/ui/active.rs index 9fbb712..3b702cc 100644 --- a/iris/core/src/ui/active.rs +++ b/iris/core/src/ui/active.rs @@ -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 #[derive(Debug)] @@ -9,7 +11,22 @@ pub struct ActiveData { pub textures: Vec, pub primitives: Vec, pub children: Vec, + /// 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, + /// 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, /// What `Widget::draw` returned the last time this widget was actually /// 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 /// LAYOUT.md section 2. 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, } diff --git a/iris/core/src/ui/painter.rs b/iris/core/src/ui/painter.rs index 09cd0cf..7a4a061 100644 --- a/iris/core/src/ui/painter.rs +++ b/iris/core/src/ui/painter.rs @@ -13,6 +13,10 @@ pub struct Painter<'a> { pub(super) region: UiRegion, pub(super) mask: MaskIdx, 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, pub(super) primitives: Vec, pub(super) children: Vec, @@ -48,12 +52,32 @@ impl<'a> Painter<'a> { 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) { assert!(self.mask == MaskIdx::NONE); - self.mask = self.rsc.ui_mut().masks.push(Mask { + let mask = Mask { region, 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 @@ -86,6 +110,7 @@ impl<'a> Painter<'a> { self.mask, None, None, + crate::render::MaskIdx::NONE, self.rsc, ); self.state diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 929d3a7..2429adf 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -156,6 +156,7 @@ impl UiRenderState { MaskIdx::NONE, None, None, + MaskIdx::NONE, rsc, ); } @@ -190,10 +191,12 @@ impl UiRenderState { mask: MaskIdx, old_children: Option>, old_move_slot: Option, + old_own_mask: MaskIdx, rsc: &mut dyn UiRsc, ) { let mut old_children = old_children.unwrap_or_default(); 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 @@ -207,6 +210,7 @@ impl UiRenderState { // 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); + let output_size = self.output_size; if let Some(active) = self.active.get_mut(&id) && !dirty { @@ -235,6 +239,13 @@ impl UiRenderState { *r = r.outside(&from).within(®ion); self.region_mut_count += 1; } + // Same bookkeeping `mov` does below and for the same + // reason: `region` moves, this widget's own slot delta + // does not, so the part of that delta `region` accounts + // for grows by exactly this step. See + // `ActiveData::move_applied`. + active.move_applied += + region.top_left().to_abs(output_size) - from.top_left().to_abs(output_size); active.region = region; return; } @@ -242,6 +253,7 @@ impl UiRenderState { let active = self.remove(id, false, rsc).unwrap(); old_children = active.children; 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 @@ -250,6 +262,7 @@ impl UiRenderState { 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 @@ -302,6 +315,7 @@ impl UiRenderState { region, mask, move_slot, + own_mask, layer, id, textures: Vec::new(), @@ -313,6 +327,16 @@ impl UiRenderState { let mut widget = painter.rsc.widgets().get_dyn_dynamic(id); painter.state.draw_count += 1; 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); painter.state.draw_started.remove(&id); @@ -322,6 +346,7 @@ impl UiRenderState { region, mask: _, move_slot, + own_mask, textures, primitives, children, @@ -341,6 +366,8 @@ impl UiRenderState { layer, size, move_slot, + own_mask, + move_applied: Vec2::ZERO, }; // remove old children that weren't kept @@ -368,6 +395,7 @@ impl UiRenderState { let from_px = from.top_left().to_abs(self.output_size); let to_px = to.top_left().to_abs(self.output_size); let delta = to_px - from_px; + active.move_applied += delta; let entry = rsc.ui_mut().move_offsets.get_mut(slot); entry.delta[0] += delta.x; entry.delta[1] += delta.y; @@ -402,6 +430,12 @@ impl UiRenderState { let Some(active) = self.active.get(&id) else { 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 .size .to_uivec2(self.density) @@ -443,6 +477,11 @@ impl UiRenderState { // the parent's own `ActiveData` may already be gone by the // time a deep descendant is retired (see LAYOUT.md // 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; rsc.ui_mut().move_offsets.remove(active.move_slot); if parent_slot != MoveOffset::NONE_PARENT { @@ -628,7 +667,12 @@ impl UiRenderState { /// section 2b. pub fn resolved_region(&self, id: &impl IdLike, rsc: &dyn UiRsc) -> Option { 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))) } @@ -691,6 +735,7 @@ impl UiRenderState { active.mask, Some(active.children), Some(active.move_slot), + active.own_mask, rsc, ); // If this widget's own reported size changed, its parent's layout diff --git a/iris/src/layout_tests.rs b/iris/src/layout_tests.rs index 757939f..a1bc959 100644 --- a/iris/src/layout_tests.rs +++ b/iris/src/layout_tests.rs @@ -299,3 +299,212 @@ 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:?}" ); } + +/// `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:?})" + ); +} diff --git a/iris/src/widget/position/max_size.rs b/iris/src/widget/position/max_size.rs index 45fed7f..22c5766 100644 --- a/iris/src/widget/position/max_size.rs +++ b/iris/src/widget/position/max_size.rs @@ -15,7 +15,14 @@ impl MaxSize { }; let len_px = len.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 diff --git a/iris/src/widget/position/scroll.rs b/iris/src/widget/position/scroll.rs index f1e9588..2b93486 100644 --- a/iris/src/widget/position/scroll.rs +++ b/iris/src/widget/position/scroll.rs @@ -33,10 +33,20 @@ impl Widget for Scroll { // length itself (read below from what was actually drawn) is never // stale, so this self-corrects the next frame and never leaves the // 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 output_len = painter.output_size().axis(axis); - let container_len = painter.region().axis(axis).len(); - self.container_len = container_len.to_abs(output_len); + let container_len = painter.px_size().axis(axis); + self.container_len = container_len; if self.snap_end { self.amt = self.content_len - self.container_len; @@ -49,12 +59,22 @@ impl Widget for Scroll { 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 .axis(axis) .apply_rest(painter.density()) - .within_len(container_len) - .to_abs(output_len); + .to_abs(container_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 } } diff --git a/iris/src/widget/position/sized.rs b/iris/src/widget/position/sized.rs index 968748b..ff7897d 100644 --- a/iris/src/widget/position/sized.rs +++ b/iris/src/widget/position/sized.rs @@ -26,9 +26,12 @@ impl Widget for Sized { region.y = y.apply_rest(density).align(AxisAlign::Neg); } 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 { - x: self.x.unwrap_or(used.x), - y: self.y.unwrap_or(used.y), + x: self.x.map(|x| x.fold_dp(density)).unwrap_or(used.x), + y: self.y.map(|y| y.fold_dp(density)).unwrap_or(used.y), } } } diff --git a/iris/transcript-ui/src/composer.rs b/iris/transcript-ui/src/composer.rs index 9780add..c0ebf7a 100644 --- a/iris/transcript-ui/src/composer.rs +++ b/iris/transcript-ui/src/composer.rs @@ -88,19 +88,19 @@ where // height-capped field -- not a background rect and a field drawn as // two independent siblings, which is what let the two disagree on // where the bar actually was. - // **Not** `.scrollable()` here yet, though IRIS_TODO.md's "the composer - // has no touch-drag scroll" wants it: `Scroll` resolves its own - // `content_len`/`container_len` against `Painter::output_size` -- the - // whole window -- so inside a `MaxSize` that has clamped the offered - // region to six lines the two are in different spaces and the field - // pans itself entirely out of the bar. Measured on this checkout's - // emulator 2026-09-06 with 474 characters in the field (`iris text - // render: ... size=(1016.7, 623.7)`, a 441px cap): the bar collapsed to - // its padding with no text in it. `Scroll::drag` -- the finger pan the - // TODO actually asks for -- is in place and exercised by the bench - // shell's report pane; what is left is `Scroll` measuring against its - // own offered box rather than the window. See docs/RUST.md. + // `.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 + .scrollable() + .masked() .pad(dp(FIELD_PAD_DP)) .max_height(dp(APPROX_LINE_HEIGHT_DP * MAX_LINES + FIELD_PAD_DP * 2.0)) .width(rest(1))