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 <noreply@anthropic.com>
This commit is contained in:
1 parent
ff1d6ea932
commit
a6a100edc6
4 files changed
+32
-15
No files matched your search
@@ -84,7 +84,7 @@ var<storage> masks: array<Mask>;
|
||||
@group(3) @binding(1)
|
||||
var<storage> move_offsets: array<MoveOffset>;
|
||||
|
||||
// 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<storage> move_offsets: array<MoveOffset>;
|
||||
// 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<f32> {
|
||||
var total = vec2<f32>(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;
|
||||
}
|
||||
|
||||
@@ -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!(
|
||||
"{}({}, {})",
|
||||
|
||||
+5
-2
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user