From a6a100edc6f6c78c4d43d458b66c5eeab05c9614 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Mon, 7 Sep 2026 21:05:55 -0400 Subject: [PATCH] iris: the chain bound is named for the walk, and two nits from the review docs/REVIEW-2026-09-07.md's rule finding on `MOVE_CHAIN_LIMIT` plus both nits. `MOVE_CHAIN_LIMIT` bounds two different parent walks -- move offsets in the vertex stage and `Mask::parent` in the fragment stage -- under a name that says one, and the shader's own comment beside it already called it "the bound on the parent walk". Renamed to `PARENT_CHAIN_LIMIT` in both files at once (the constant has no other users), with the doc saying which two chains it governs. `DragGesture`'s release computed `self.velocity.velocity()` twice, once for the outcome and once for the `iris drag release:` line -- a full Lsq2 fit each. Once now, into a local both read. `transcript-ui`'s `selection.rs` called `ui.ui_mut().animate(id)` even when `List::fling` had bailed (Compose's `|v| <= 1.0`, or no anchor), so a frame was asked to advance an animation known not to exist. It is behind `is_scrolling()` now, which is the same answer `fling` itself reached. `phone_screen.rs`'s recorded flick still flings, which is the half that says the guard did not turn a working release off. Verified: `cargo test --lib -p iris` (104) and `-p transcript-fixture` (12), fmt and clippy clean, and layer 2 (`run-headless.sh phone --phone`) still renders with the mask chain intact -- code fences clipped to their rows, the list clipped at the composer -- which is what the wgsl rename needed looking at rather than compiling. Co-Authored-By: Claude Fable 5.1 --- iris/core/src/render/shader.wgsl | 8 ++++---- iris/core/src/ui/render_state.rs | 20 +++++++++++++------- iris/src/sense.rs | 7 +++++-- iris/transcript-ui/src/selection.rs | 12 ++++++++++-- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/iris/core/src/render/shader.wgsl b/iris/core/src/render/shader.wgsl index d658716..9b01ec7 100644 --- a/iris/core/src/render/shader.wgsl +++ b/iris/core/src/render/shader.wgsl @@ -84,7 +84,7 @@ var masks: array; @group(3) @binding(1) var move_offsets: array; -// The bound on the parent walk, kept in step with `MOVE_CHAIN_LIMIT` in +// The bound on the parent walk, kept in step with `PARENT_CHAIN_LIMIT` in // render_state.rs, which walks the identical chain on the CPU side for // hit-testing. Bounded so a malformed chain (a cyclic `parent`) cannot // hang the GPU -- not a claim about how deep a real tree gets. It was 16 @@ -94,7 +94,7 @@ var move_offsets: array; // Past the bound both walks simply stop summing, so the widget draws and // hit-tests short by whatever the outer slots held, with nothing on // screen to say so. -const MOVE_CHAIN_LIMIT: u32 = 64u; +const PARENT_CHAIN_LIMIT: u32 = 64u; /// Sums the pixel delta along the parent chain starting at `idx`, shared by /// the vertex stage (a primitive's own corners) and the fragment stage (its @@ -102,7 +102,7 @@ const MOVE_CHAIN_LIMIT: u32 = 64u; fn resolve_move(idx: u32) -> vec2 { var total = vec2(0.0, 0.0); var i = idx; - for (var step = 0u; step < MOVE_CHAIN_LIMIT; step++) { + for (var step = 0u; step < PARENT_CHAIN_LIMIT; step++) { let entry = move_offsets[i]; total += entry.delta; if entry.parent == 4294967295u { @@ -204,7 +204,7 @@ fn fs_main( // its own mask inside another is clipped by both, and each carries its // own move slot (`Mask::parent` in data.rs). var mask_idx = in.mask_idx; - for (var step = 0u; step < MOVE_CHAIN_LIMIT; step++) { + for (var step = 0u; step < PARENT_CHAIN_LIMIT; step++) { if mask_idx == 4294967295u { break; } diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index d18c504..46d687e 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -113,7 +113,12 @@ pub struct UiRenderState { /// prints the chain). A chain past the bound is not reported anywhere at /// run time; both walks just stop summing, so the widget is drawn and hit /// tested short by whatever the outer slots held. -pub const MOVE_CHAIN_LIMIT: usize = 64; +/// +/// Named for the walk rather than for one of its two subjects: it bounds +/// the move-offset chain *and* the mask chain (`Mask::parent`, walked in +/// the fragment stage), and `MOVE_CHAIN_LIMIT` said only the first +/// (docs/REVIEW-2026-09-07.md). +pub const PARENT_CHAIN_LIMIT: usize = 64; impl UiRenderState { pub fn new() -> Self { @@ -841,13 +846,13 @@ impl UiRenderState { /// The plain-Rust twin of `resolve_move` in shader.wgsl: sums the /// pixel delta along the parent chain starting at `slot`. Both walks - /// share `MOVE_CHAIN_LIMIT` as their bound so the two cannot disagree + /// share `PARENT_CHAIN_LIMIT` as their bound so the two cannot disagree /// about where the chain ends. fn resolve_move_chain(&self, slot: MoveIdx, rsc: &dyn UiRsc) -> Vec2 { let offsets = &rsc.ui().move_offsets; let mut delta = Vec2::ZERO; let mut at = slot; - for i in 0..MOVE_CHAIN_LIMIT { + for i in 0..PARENT_CHAIN_LIMIT { let entry = &offsets[at.idx()]; delta.x += entry.delta[0]; delta.y += entry.delta[1]; @@ -860,8 +865,9 @@ impl UiRenderState { // follow are different faults with different fixes, and the // slot numbers are the only thing that tells them apart. debug_assert!( - i + 1 < MOVE_CHAIN_LIMIT, - "move offset chain exceeded MOVE_CHAIN_LIMIT ({MOVE_CHAIN_LIMIT}): {chain} -- a \ + i + 1 < PARENT_CHAIN_LIMIT, + "move offset chain exceeded PARENT_CHAIN_LIMIT ({PARENT_CHAIN_LIMIT}): {chain} \ + -- a \ repeated slot means a `parent` link is cyclic, all-distinct slots mean the tree \ nests deeper than shader.wgsl's own walk of the same bound", chain = Self::move_chain_debug(slot, offsets) @@ -871,13 +877,13 @@ impl UiRenderState { } /// The parent chain from `slot`, as `slot(dx, dy) -> ...`, walked twice - /// `MOVE_CHAIN_LIMIT` so a cycle shows up as a repeated slot number + /// `PARENT_CHAIN_LIMIT` so a cycle shows up as a repeated slot number /// rather than as a chain that merely stops. Only ever called from the /// failed assertion above. fn move_chain_debug(slot: MoveIdx, offsets: &[MoveOffset]) -> String { let mut parts = Vec::new(); let mut at = slot; - for _ in 0..MOVE_CHAIN_LIMIT * 2 { + for _ in 0..PARENT_CHAIN_LIMIT * 2 { let entry = &offsets[at.idx()]; parts.push(format!( "{}({}, {})", diff --git a/iris/src/sense.rs b/iris/src/sense.rs index 5846d6d..32f88f1 100644 --- a/iris/src/sense.rs +++ b/iris/src/sense.rs @@ -919,8 +919,11 @@ impl DragGesture { self.dispatch(render, id, pos_window, now) } CursorSense::Drop | CursorSense::PressEnd(_) => { + // Once: a `velocity()` is a full Lsq2 fit, and the log + // line below wants the same number the outcome carries. + let released = self.velocity.velocity(); let outcome = if self.arbiter.is_panning() { - GestureOutcome::Released(Some(self.velocity.velocity())) + GestureOutcome::Released(Some(released)) } else if self.arbiter.is_undecided() { GestureOutcome::Tapped } else { @@ -937,7 +940,7 @@ impl DragGesture { "iris drag release: samples={} span={:.1}ms v={:.0} outcome={:?}", self.velocity.sample_count(), self.velocity.span().as_secs_f32() * 1000.0, - self.velocity.velocity(), + released, outcome, ); // The samples themselves, so a flick that felt wrong on diff --git a/iris/transcript-ui/src/selection.rs b/iris/transcript-ui/src/selection.rs index 69c0e73..5f35f09 100644 --- a/iris/transcript-ui/src/selection.rs +++ b/iris/transcript-ui/src/selection.rs @@ -299,8 +299,16 @@ impl Selection { // The half that actually makes it move -- see // `List::fling`'s doc. Without it the velocity is // computed, stored, and never advanced by anything. - let id = list.id(); - ui.ui_mut().animate(id); + // + // Only when `fling` actually took it: below Compose's + // `|v| <= 1.0`, or with no anchor, there is nothing to + // tick, and registering an animation for a widget that is + // not animating asks the next frame to find that out + // (docs/REVIEW-2026-09-07.md's second nit). + if list(ui).is_scrolling() { + let id = list.id(); + ui.ui_mut().animate(id); + } } // A tap is nobody's business here -- `row.rs` reads it from // the returned outcome and follows a link if one was under