diff --git a/AGENTS.md b/AGENTS.md index a24bd2d..20a91b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -446,16 +446,17 @@ Each exists because something was invisible without it. are only reusable next frame and provisional layout nested -- grew the arena to **127,443 slots for 11,569 live primitives**; and nothing tracked *which* entries changed. Now: the stream arena is 11,569 slots - for 11,569 live, and every array uploads within a hair of its floor - (fling instances 1.0% against a 0.9% floor, stream instances 71.9% - against 71.8%, and stream glyphs 0.6% against 0.6%). The CPU half improved with it, since the freeing and renumbering + for 11,569 live, and every array uploads within a hair of its floor. + The CPU half improved with it, since the freeing and renumbering went away: a streamed frame is p50 1.39ms, from 2.20ms. - **Stream instances sit at 71.9%, against a 71.8% floor, and dirty - tracking is not the defect here.** The list is pinned to the newest end, so a growing reply moves - every row, and a row's instances carry an absolute region. That is a - `move_offsets` write the layout is not making -- the next thing to look - at, and a layout question rather than an upload one. + The remaining layout cost was then removed at the framework boundary: + `Painter::set_child_offset` gives a container one retained coordinate slot + for its child subtree, and `LazySpan` keeps row boxes stable behind it. + Pinned growth now uploads instances at **2.9% against a 2.9% floor**, from + 71.9% against 71.8%; p50 instance upload is **1,728 bytes**, from 176,496. + `Primitives` also cancels dirty marks for provisional writes restored before + upload, so CPU-only layout states never become GPU work. - **The emulator is a GLES rig, deliberately** (Iris, 2026-09-08; docs/RUST.md). Its guest has no hardware Vulkan -- only SwiftShader in software -- while its GLES *is* the host's real GPU through virgl at diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index a2a4ffe..41575f6 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -12,35 +12,6 @@ and six phone-report sections went on 2026-09-08 for that reason. ## Fix -- [ ] **A row moving because the list grew should be one `move_offsets` - write, and today it is a redraw.** Found 2026-09-09 by - `scripts/rigs/ui-profile`'s `arena_churn` and left for whoever picks - this up next; the upload half of it is done and this is the layout - half. - - The measurement. Over the bench fixture's 401 streamed deltas, the - instance arena uploads **71.9%** of itself per frame against a - **71.8%** floor -- those entries genuinely differ, so no amount of - better dirty-tracking touches it. The control that says it is wrong is - the fling phase on the same screen and the same content: it moves the - same primitives every frame and uploads **3.3%**, because a scroll - reaches `UiRenderState::mov` and writes one `move_offsets` delta for - the subtree (LAYOUT.md section 2) instead of rewriting every - primitive's absolute region. - - The list is pinned to the newest end, so a growing reply pushes every - earlier row up. `draw_inner` now treats sub-pixel size differences as a - move and the span refactor removed nearly all repeated draws, but the - instance floor remains 71.8%. The remaining question is why those - translations still rewrite primitive regions instead of stopping at - the rows' move slots. - - Done looks like: `arena_churn`'s `what_a_streamed_reply_uploads` shows - stream instances in the same range as the fling's, and its `whole` - column stops being the interesting one. The rig prints floor, - uploaded and whole per array precisely so this is checkable rather - than argued. - - [ ] **Where the scroll *pin* lives.** The rest of "scrolling moves out of the list" landed on 2026-09-08 -- `List` is `LazySpan`, the physics and the gesture live in one `ScrollController`, `.scrollable()` is the diff --git a/docs/LAYOUT.md b/docs/LAYOUT.md index 7403318..4afbb2a 100644 --- a/docs/LAYOUT.md +++ b/docs/LAYOUT.md @@ -94,6 +94,15 @@ resolved in the vertex shader.** - `mov(id, delta)` becomes: look up `id`'s slot, write `move_offsets[slot].delta += delta`. One write — no primitive touched, no recursion, since descendants already reference this slot transitively. +- A container may also retain one optional **child-coordinate slot** between + its own slot and every direct child's slot. `Painter::set_child_offset` + creates that boundary before the first child is drawn and can update it + after measuring a child on later redraws. The container's own primitives, + hit region and mask stay fixed; its whole child subtree moves through one + write and every existing GPU, hit-test and accessibility chain sees the + same result. `LazySpan` uses this while still walking visible rows for + virtualisation: row boxes stay in stable local coordinates and the shared + boundary carries the changing screen translation. - `shader.wgsl`'s vertex stage, after computing `top_left`/`bot_right` in pixels (after `:106`, before the clip-space divide at `:113`), walks `move_idx → move_offsets[i].parent` for a bounded number of steps (a @@ -120,6 +129,12 @@ for a resize that changes a region's `rel` component (a genuine reflow, §3) and for a size-independent widget's resize (§3), where the content's shape doesn't change and one field write already suffices. +Provisional layout can still write an instance at an intermediate position +and restore it before upload. `Primitives::set_instance` remembers the value +at the first write in a frame and clears the dirty bit when the final bytes +match it. The GPU therefore observes final layout state, not CPU-only +measurement work. + ### 2b. Two more readers of "where is this widget," and masks Moving the offset into the vertex shader means `ActiveData.region` is no diff --git a/docs/RUST.md b/docs/RUST.md index 9d60418..68ec9a7 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -670,8 +670,10 @@ The trap that only the rig could have caught: writing an entry is not the same as changing it. Recycling rewrote every glyph of every moved row with identical bytes, marking 73% of the glyph array against 0.6% genuinely changed. `PrimitiveVec::set` and `Primitives::set_instance` compare before -marking, and `arena_churn` prints both numbers so the gap cannot reopen -unnoticed. +marking. Layout can also write a provisional instance and restore it within +one frame; `Primitives` remembers the pre-frame bytes and cancels that dirty +bit when the GPU-visible result is unchanged. `arena_churn` prints both +numbers so either gap cannot reopen unnoticed. **Layout has no measurement mode.** A widget is drawn provisionally only when its size cannot be known yet, and that retained drawing is moved into @@ -685,12 +687,16 @@ Measured over the fixture's 401 streamed events: the busiest frame makes Streamed-frame CPU p50 is 0.35ms, from 1.18ms before this layout change. Arena size and upload floors are unchanged. -**What is left, and it is a layout question rather than an upload one.** -Stream instances upload 71.9%, against a 71.8% floor: the list is pinned to -the newest end, so a growing reply moves every row, and a row's instances -carry an absolute region. Moving a subtree is supposed to be one -`move_offsets` write (LAYOUT.md section 2); something on this path is -redrawing instead. +Pinned growth now uses the same subtree translation as scrolling. A +container can retain a child-coordinate move slot through +`Painter::set_child_offset`; `LazySpan` keeps retained rows in stable local +boxes and changes that one slot when its anchor moves. It still walks the +visible run to virtualise it, but unchanged rows no longer acquire new +absolute primitive regions. Over the fixture's 401 streamed events, instance +upload is **2.9% against a 2.9% floor**, from 71.9% against 71.8%; median +instance bytes per frame are **1,728**, from 176,496. This is framework +layout/rendering behaviour and the transcript screen contains no special +case for it. ### The Android release profile is `opt-level = 3`, not `"s"` (2026-09-09) diff --git a/docs/SCROLL.md b/docs/SCROLL.md index 232f121..1d85bad 100644 --- a/docs/SCROLL.md +++ b/docs/SCROLL.md @@ -272,18 +272,21 @@ cannot pan; there is a `debug_assert` in `drag` naming that. ## Measurements worth not re-taking -- A settled scroll tick of a `LazySpan` with 31 rows on screen: - **1 real draw and 31 move-slot writes**, no primitive rewrites and no - text reshaped. An idle frame is `(0, 0, 0, 0)` — `draw_inner` does not - even enter the widget. This is the number any "store the edges and only - recompute what changed" optimisation would have to beat, and it is why - the walk was left alone. +- A settled scroll tick of a `LazySpan`, for 20, 200 or 2,000 total rows: + **1 real draw and 1 child-coordinate move-slot write**, no primitive + rewrites and no text reshaped. The visible-row walk remains: it is what + admits and retires rows at the viewport boundary, and a newly admitted row + has real initial-placement work of its own. An idle frame is `(0, 0, 0, + 0)` — `draw_inner` does not even enter the widget. - A fully hinted `Span` draws each child once. Unknown fixed children draw provisionally and move; region-dependent children redraw if their final box has a different size. -- The one design that would collapse those 31 moves into a single delta - write is moving the content as a unit, which needs a content length — - which a lazy layout cannot supply. +- Moving the currently retained run as a unit does **not** require the lazy + span's unknowable total content length. Its anchor supplies the relation + between stable local row boxes and their desired screen boxes; one retained + child-coordinate slot carries that translation. The offset is occasionally + rebased after 65,536 pixels to preserve `f32` precision, a rare O(visible) + move-slot pass rather than steady-state work. ## Tests that pin the behaviour diff --git a/iris/core/src/render/primitive.rs b/iris/core/src/render/primitive.rs index e4e4af7..ebe66f4 100644 --- a/iris/core/src/render/primitive.rs +++ b/iris/core/src/render/primitive.rs @@ -143,6 +143,13 @@ macro_rules! primitives { /// compacted, so a `Mask` can hold a slot across frames. pub struct Primitives { instances: Vec, + /// The value a slot held before its first rewrite since the last + /// upload. Layout may place a widget provisionally and restore it in + /// the same frame; remembering the pre-frame value lets `set_instance` + /// clear that dirty bit instead of uploading a change the GPU never + /// needs to observe. Entries are overwritten on the next clean-to-dirty + /// transition, so no separate end-of-frame sweep is needed. + original_instances: Vec>, assoc: Vec, /// Where each slot's [`PrimitiveHandle`] sits in its owner's /// `ActiveData::primitives` -- the index that makes @@ -179,6 +186,7 @@ impl Default for Primitives { fn default() -> Self { Self { instances: Default::default(), + original_instances: Default::default(), assoc: Default::default(), handle_idx: Default::default(), freed: Vec::new(), @@ -251,11 +259,13 @@ impl Primitives { fn push(&mut self, inst: PrimitiveInstance, id: WidgetId) -> u32 { let slot = if let Some(i) = self.reusable.pop() { self.instances[i] = inst; + self.original_instances[i] = None; self.assoc[i] = id; self.handle_idx[i] = Self::NO_HANDLE; i } else { self.instances.push(inst); + self.original_instances.push(None); self.assoc.push(id); self.handle_idx.push(Self::NO_HANDLE); self.instances.len() - 1 @@ -342,8 +352,18 @@ impl Primitives { if bytemuck::bytes_of(&self.instances[slot]) == bytemuck::bytes_of(&inst) { return; } + if !self.dirty.contains(slot) { + self.original_instances[slot] = Some(self.instances[slot]); + } self.instances[slot] = inst; - self.dirty.mark(slot); + if self.original_instances[slot] + .is_some_and(|original| bytemuck::bytes_of(&original) == bytemuck::bytes_of(&inst)) + { + self.original_instances[slot] = None; + self.dirty.unmark(slot); + } else { + self.dirty.mark(slot); + } } /// Retires a slot, answering the mask it was drawn under so the caller @@ -392,6 +412,7 @@ impl Primitives { pub fn clear(&mut self) { self.dirty.mark_all(); self.instances.clear(); + self.original_instances.clear(); self.assoc.clear(); self.handle_idx.clear(); self.freed.clear(); @@ -464,7 +485,13 @@ impl Primitives { } pub fn region_mut(&mut self, h: &PrimitiveHandle) -> &mut UiRegion { - self.dirty.mark(h.slot as usize); + let slot = h.slot as usize; + // The caller receives unrestricted mutable access, so this path + // cannot tell whether a later write restored the prior bytes. Leave + // the entry dirty rather than letting a stale `set_instance` baseline + // cancel it. + self.original_instances[slot] = None; + self.dirty.mark(slot); &mut self.instances[h.slot as usize].region } } @@ -785,3 +812,45 @@ impl Deref for PrimitiveVec { &self.vec } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{UiVec2, Widgets, util::Id}; + + #[test] + fn an_instance_restored_before_upload_is_clean() { + let mut primitives = Primitives::default(); + let mut widgets = Widgets::new(); + let widget = widgets.add_strong(()); + let owner = widget.id(); + let original = PrimitiveInstance { + region: UiRegion::FULL, + binding: RectPrimitive::BINDING, + idx: 0, + mask_idx: MaskIdx::NONE, + move_idx: Id::preset(0), + }; + primitives.push(original, owner); + primitives.dirty.clear(); + + let mut moved = original; + moved.region = moved.region.offset(UiVec2::abs((0.0, 20.0))); + primitives.set_instance(0, moved, owner); + assert!(!primitives.dirty.is_clean()); + + primitives.set_instance(0, original, owner); + assert!( + primitives.dirty.is_clean(), + "the GPU never observes the provisional position" + ); + + // A subsequent frame takes its baseline from the value currently in + // the arena, rather than reusing the now-stale original above. + primitives.set_instance(0, moved, owner); + assert!(!primitives.dirty.is_clean()); + primitives.dirty.clear(); + primitives.set_instance(0, original, owner); + assert!(!primitives.dirty.is_clean()); + } +} diff --git a/iris/core/src/ui/active.rs b/iris/core/src/ui/active.rs index d1cdbeb..0050fe4 100644 --- a/iris/core/src/ui/active.rs +++ b/iris/core/src/ui/active.rs @@ -20,6 +20,10 @@ pub struct ActiveData { pub size: Size, /// Retained so descendants' parent links stay valid across redraws. pub move_slot: MoveIdx, + /// The optional coordinate boundary between this widget and its direct + /// children. Descendants retain links to it across redraws, just as they + /// do to `move_slot`. + pub child_move_slot: Option, /// The part of this widget's move delta already folded into `region`. pub move_applied: Vec2, } diff --git a/iris/core/src/ui/mod.rs b/iris/core/src/ui/mod.rs index 42d92df..e922019 100644 --- a/iris/core/src/ui/mod.rs +++ b/iris/core/src/ui/mod.rs @@ -18,11 +18,12 @@ pub struct UiData { pub textures: Textures, pub text: TextData, pub masks: TrackedArena, - /// One entry per widget ever drawn, forming the parent-linked chain - /// `resolve_move` walks in both shader stages. Allocated once on a - /// widget's first draw and reused for every later redraw of the same - /// id (never reallocated), so a retained descendant's `parent` index - /// never goes stale -- see LAYOUT.md section 2. + /// One entry per widget ever drawn, plus optional child-coordinate + /// boundaries owned by containers. Together they form the parent-linked + /// chain `resolve_move` walks in both shader stages. A widget's ordinary + /// entry is allocated once on its first draw and reused for every later + /// redraw of the same id, so a retained descendant's `parent` index never + /// goes stale -- see LAYOUT.md section 2. pub move_offsets: TrackedArena, /// Every widget whose [`crate::Widget::tick`] should run before the /// next frame -- today, a `LazySpan` coasting through a fling. Added by diff --git a/iris/core/src/ui/painter.rs b/iris/core/src/ui/painter.rs index e2dda90..c9ef46b 100644 --- a/iris/core/src/ui/painter.rs +++ b/iris/core/src/ui/painter.rs @@ -1,6 +1,7 @@ use crate::{ - Axis, Color, Len, RegionAlign, RenderedText, Size, StrongWidget, TextAttrs, TextBuffer, - TextData, TextureHandle, UiRegion, UiRenderState, UiRsc, UiScalar, UiVec2, WidgetId, + Axis, Color, Len, MoveOffset, RegionAlign, RenderedText, Size, StrongWidget, TextAttrs, + TextBuffer, TextData, TextureHandle, UiRegion, UiRenderState, UiRsc, UiScalar, UiVec2, + WidgetId, render::{ Drawn, GlyphPrimitive, IMAGE_BINDING, Mask, MaskIdx, MoveIdx, NOT_DRAWN, Primitive, PrimitiveHandle, PrimitiveInst, RectPrimitive, @@ -16,6 +17,7 @@ pub struct Painter<'a> { pub(super) region: UiRegion, pub(super) mask: MaskIdx, pub(super) move_slot: MoveIdx, + pub(super) child_move_slot: Option, /// This widget's retained mask slot. pub(super) own_mask: MaskIdx, pub(super) textures: Vec, @@ -217,6 +219,47 @@ impl<'a> Painter<'a> { self.widget_at(id, region.within(&self.region)) } + /// Translate this widget's children as one retained subtree, in output + /// pixels. The first call must happen before drawing a child, because the + /// slot becomes the parent of every direct child's ordinary move slot. + /// Once retained, it may be updated later in a redraw (for example after + /// measuring a changed child). All deeper descendants inherit it and the + /// CPU hit-test walk resolves the same translation as the shader. + /// + /// This offsets the child coordinate space, not this widget: its own + /// primitives and hit region remain fixed. Once allocated, the boundary + /// stays in the chain across redraws; set it to zero to return children to + /// their unshifted positions. + pub fn set_child_offset(&mut self, offset: Vec2) { + let slot = match self.child_move_slot { + Some(slot) => slot, + None => { + assert!( + self.children.is_empty(), + "a child offset must be created before drawing a child" + ); + let parent = self.move_slot.idx() as u32; + let slot = self + .rsc + .ui_mut() + .move_offsets + .push(MoveOffset::new([offset.x, offset.y], parent)); + // One ref for this widget's ownership and one on the + // up-link. Direct children take their own refs when their + // move slots are allocated. + self.rsc.ui_mut().move_offsets.push_ref(slot); + self.rsc.ui_mut().move_offsets.push_ref(self.move_slot); + self.child_move_slot = Some(slot); + return; + } + }; + let next = [offset.x, offset.y]; + if self.rsc.ui().move_offsets[slot.idx()].delta != next { + self.rsc.ui_mut().move_offsets.get_mut(slot).delta = next; + self.state.note_move(); + } + } + pub fn known_len(&self, id: &StrongWidget, axis: Axis) -> Option { if let Some(len) = self.rsc.widgets().get_dyn(id.id())?.size_hint(axis) { return Some(len.fold_dp(self.density())); @@ -236,12 +279,13 @@ impl<'a> Painter<'a> { // call -- would always find nothing. `self.move_slot` is this // widget's own slot, already known, and always correct regardless // of insertion order. See `UiRenderState::move_parent_of`. + let parent_move_slot = self.child_move_slot.unwrap_or(self.move_slot); self.state.draw_inner( self.layer, id.id(), region, Some(self.id), - self.move_slot.idx() as u32, + parent_move_slot.idx() as u32, self.mask, Retained::default(), self.rsc, @@ -261,12 +305,13 @@ impl<'a> Painter<'a> { } else if let Some((layer, mask)) = retained { self.children.push(id.id()); self.rsc.widgets_mut().needs_redraw.insert(id.id()); + let parent_move_slot = self.child_move_slot.unwrap_or(self.move_slot); self.state.draw_inner( layer, id.id(), region, Some(self.id), - self.move_slot.idx() as u32, + parent_move_slot.idx() as u32, mask, Retained::default(), self.rsc, diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 340ae78..1009c74 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -99,6 +99,7 @@ pub(crate) struct Retained { pub region: Option, pub children: Vec, pub move_slot: Option, + pub child_move_slot: Option, pub own_mask: MaskIdx, pub primitives: Vec, } @@ -109,6 +110,7 @@ impl Default for Retained { region: None, children: Vec::new(), move_slot: None, + child_move_slot: None, own_mask: MaskIdx::NONE, primitives: Vec::new(), } @@ -173,6 +175,10 @@ impl UiRenderState { ) } + pub(super) fn note_move(&mut self) { + self.mov_count += 1; + } + /// Writes a primitive into the arena and, unless it is /// [`Drawn::No`], into `layer`'s draw order. pub(super) fn write_primitive( @@ -446,6 +452,7 @@ impl UiRenderState { region: mut old_region, children: mut old_children, move_slot: mut old_move_slot, + mut child_move_slot, mut own_mask, primitives: mut recycle, } = retained; @@ -498,6 +505,7 @@ impl UiRenderState { old_region = Some(active.region); old_children = active.children; old_move_slot = Some(active.move_slot); + child_move_slot = active.child_move_slot; own_mask = active.own_mask; recycle = active.primitives; } else if self.active.contains_key(&id) { @@ -512,6 +520,7 @@ impl UiRenderState { old_region = Some(active.region); old_children = active.children; old_move_slot = Some(active.move_slot); + child_move_slot = active.child_move_slot; own_mask = active.own_mask; recycle = active.primitives; } @@ -533,6 +542,7 @@ impl UiRenderState { region, mask, move_slot, + child_move_slot, own_mask, layer, id, @@ -577,6 +587,7 @@ impl UiRenderState { region, mask: _, move_slot, + child_move_slot, own_mask, textures, primitives, @@ -608,6 +619,7 @@ impl UiRenderState { layer, size, move_slot, + child_move_slot, own_mask, move_applied: Vec2::ZERO, }; @@ -804,6 +816,18 @@ impl UiRenderState { rsc.ui_mut().masks.remove(outer); } } + // The self-ownership ref held by this widget's optional + // child-coordinate slot, plus that slot's link to this + // widget's ordinary move slot. Direct children may still + // hold the slot alive until `remove_rec` reaches them, the + // same lifetime shape as `move_slot` immediately below. + if let Some(slot) = active.child_move_slot { + let parent_slot = rsc.ui().move_offsets[slot.idx()].parent; + rsc.ui_mut().move_offsets.remove(slot); + if parent_slot != MoveOffset::NONE_PARENT { + rsc.ui_mut().move_offsets.remove(Id::preset(parent_slot)); + } + } 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 { @@ -1213,6 +1237,7 @@ impl UiRenderState { region: Some(active.region), children: active.children, move_slot: Some(active.move_slot), + child_move_slot: active.child_move_slot, own_mask: active.own_mask, primitives: active.primitives, }, diff --git a/iris/core/src/util/dirty.rs b/iris/core/src/util/dirty.rs index 2a77f06..54150dd 100644 --- a/iris/core/src/util/dirty.rs +++ b/iris/core/src/util/dirty.rs @@ -45,6 +45,26 @@ impl Dirty { self.words[word] |= 1 << (i % 64); } + pub fn contains(&self, i: usize) -> bool { + self.all + || self + .words + .get(i / 64) + .is_some_and(|word| word & (1 << (i % 64)) != 0) + } + + /// Clear one entry that was restored to the value already on the GPU. + /// `all` has no per-entry representation and is used only when every + /// byte must be uploaded regardless of later writes, so it stays set. + pub fn unmark(&mut self, i: usize) { + if self.all { + return; + } + if let Some(word) = self.words.get_mut(i / 64) { + *word &= !(1 << (i % 64)); + } + } + /// Everything must be written: the buffer was reallocated (its /// contents are undefined), or the array was cleared. pub fn mark_all(&mut self) { @@ -114,6 +134,17 @@ mod tests { assert_eq!(marked(&[3, 4, 5], 64, 0), vec![3..6]); } + #[test] + fn a_restored_entry_can_be_unmarked() { + let mut dirty = Dirty::default(); + dirty.mark(3); + dirty.mark(5); + assert!(dirty.contains(3)); + dirty.unmark(3); + assert!(!dirty.contains(3)); + assert_eq!(dirty.ranges(8, 0), vec![5..6]); + } + #[test] fn a_run_that_crosses_a_word_boundary_is_one_range() { assert_eq!(marked(&[62, 63, 64, 65], 128, 0), vec![62..66]); diff --git a/iris/src/layout_tests.rs b/iris/src/layout_tests.rs index 69348ad..29b4e6e 100644 --- a/iris/src/layout_tests.rs +++ b/iris/src/layout_tests.rs @@ -37,6 +37,67 @@ impl Widget for FixedRect { } } +struct ChildOffset { + child: StrongWidget, + offset: Vec2, +} + +impl Widget for ChildOffset { + fn draw(&mut self, painter: &mut Painter) -> Size { + painter.set_child_offset(self.offset); + painter.widget(&self.child) + } +} + +#[test] +fn a_child_coordinate_offset_moves_only_the_child_subtree() { + let mut rsc = TestRsc { + ui: UiData::default(), + }; + let child = rsc.ui.widgets.add_strong(FixedRect(40.0)); + let child_weak = child.weak(); + let parent = rsc.ui.widgets.add_strong(ChildOffset { + child: child.any(), + offset: vec2(0.0, 15.0), + }); + let parent_weak = parent.weak(); + // Keep the coordinate-owning widget below the root: a nested widget is + // normally redrawn when its parent visits it, which takes a different + // retained-state path from `UiRenderState::redraw` on the root itself. + let outer = rsc.ui.widgets.add_strong(Sized { + inner: parent.any(), + x: None, + y: None, + }); + let outer_weak = outer.weak(); + let root = outer.any(); + let mut render = UiRenderState::new(); + render.resize((100.0, 100.0)); + render.update(&root, &mut rsc); + render.take_counters(); + + let parent_before = render.window_region(&parent_weak, &rsc).unwrap(); + let child_before = render.window_region(&child_weak, &rsc).unwrap(); + assert!((parent_before.top_left.y - 0.0).abs() < 0.01); + assert!((child_before.top_left.y - 15.0).abs() < 0.01); + + rsc.ui.widgets.get_mut(&parent_weak).unwrap().offset.y = 35.0; + // Force the ordinary ancestor-redraw path rather than letting the + // renderer visit only the dirty descendant directly. + rsc.ui.widgets.get_mut(&outer_weak).unwrap().x = None; + render.update(&root, &mut rsc); + let (draws, rewrites, moves, _shapes) = render.take_counters(); + let parent_after = render.window_region(&parent_weak, &rsc).unwrap(); + let child_after = render.window_region(&child_weak, &rsc).unwrap(); + + assert_eq!((draws, rewrites, moves), (2, 0, 1)); + assert_eq!( + parent_after, parent_before, + "the container itself stays put" + ); + assert!((child_after.top_left.y - 35.0).abs() < 0.01); +} + #[test] fn a_hinted_rest_draws_once_and_only_moves_the_fixed_child_after_it() { let mut rsc = TestRsc { diff --git a/iris/src/widget/position/lazy_span.rs b/iris/src/widget/position/lazy_span.rs index 11b1a8b..552975d 100644 --- a/iris/src/widget/position/lazy_span.rs +++ b/iris/src/widget/position/lazy_span.rs @@ -141,6 +141,11 @@ pub struct LazySpan { extents: HashMap, /// Last reported height, pruned when its row is evicted. heights: HashMap, + /// Direction-relative translation applied to every row through one + /// retained child-coordinate slot. Row regions subtract this value, so + /// changing it moves the visible run without changing any retained row's + /// own geometry. + content_offset: f32, /// Whether the last walk found no more content before the leading /// edge *and* nothing left to give back there -- what /// [`Self::overscroll_gap`] reads. `false` by default, matching @@ -190,6 +195,7 @@ impl LazySpan { pending_tap: None, extents: HashMap::default(), heights: HashMap::default(), + content_offset: 0.0, } } @@ -309,6 +315,7 @@ impl LazySpan { self.ctl.set_pinned_to_end(true); self.heights.clear(); self.extents.clear(); + self.content_offset = 0.0; } /// Move the anchor's edge by `amt` pixels, where positive brings @@ -725,7 +732,7 @@ impl LazySpan { /// Called a second time in the same `draw` when the first pass lands /// off the end of the content -- see [`Self::overscroll_gap`] and /// `draw`. - fn lay_out(&mut self, painter: &mut Painter) -> (f32, f32) { + fn lay_out(&mut self, painter: &mut Painter, stable_anchor_lead: Option) -> (f32, f32) { let anchor = self .anchor .expect("lay_out with no anchor: `draw` returns before this without one"); @@ -733,14 +740,15 @@ impl LazySpan { Edge::Leading => Placement::Leading(anchor.offset), Edge::Trailing => Placement::Trailing(anchor.offset), }; - let (mut lead, mut trail) = self.place(painter, anchor.slot, placement); + let (mut lead, mut trail) = + self.place(painter, anchor.slot, placement, stable_anchor_lead, true); let mut idx_lead = anchor.slot; while lead > 0.0 { let Some(prev) = self.prev_slot(idx_lead) else { break; }; - let (l, _) = self.place(painter, prev, Placement::Trailing(lead)); + let (l, _) = self.place(painter, prev, Placement::Trailing(lead), None, false); lead = l; idx_lead = prev; } @@ -750,7 +758,7 @@ impl LazySpan { let Some(next) = self.next_slot(idx_trail) else { break; }; - let (_, t) = self.place(painter, next, Placement::Leading(trail)); + let (_, t) = self.place(painter, next, Placement::Leading(trail), None, false); trail = t; idx_trail = next; } @@ -837,8 +845,48 @@ impl LazySpan { region } + /// A row's stable local box. `lead`/`trail` are where it belongs on + /// screen; the child-coordinate move slot adds `content_offset` back in + /// the shader and in `resolved_region`. + fn row_region(&self, lead: f32, trail: f32) -> UiRegion { + Self::abs_region( + self.dir, + lead - self.content_offset, + trail - self.content_offset, + ) + } + + fn apply_content_offset(&self, painter: &mut Painter) { + let screen_offset = match self.dir.sign { + Sign::Pos => self.content_offset, + Sign::Neg => -self.content_offset, + }; + painter.set_child_offset(Vec2::from_axis(self.dir.axis, screen_offset, 0.0)); + } + + fn rebase_content_offset(&mut self) { + if self.content_offset.abs() >= CONTENT_OFFSET_REBASE { + self.content_offset = 0.0; + } + } + + /// Keep an anchor row's local leading edge unchanged while its desired + /// on-screen leading edge moves. Scrolling and end-pinned growth can then + /// move the visible run through one child-coordinate write. + fn stabilize_lead(&mut self, painter: &mut Painter, from: f32, to: f32) { + self.content_offset += to - from; + self.apply_content_offset(painter); + } + /// Place a row and remember its height for later walks. - fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) { + fn place( + &mut self, + painter: &mut Painter, + slot: isize, + placement: Placement, + stable_lead: Option, + is_anchor: bool, + ) -> (f32, f32) { debug_assert!( self.slot_exists(slot), "place() called with a slot that doesn't exist: {slot:?}" @@ -856,37 +904,69 @@ impl LazySpan { }; let key = self.slot_key(slot); let cached = key.and_then(|k| self.heights.get(&k).copied()); + // A dirty descendant is redrawn before its changed size bubbles up + // to this list. By the time the anchored row is placed, its retained + // draw can therefore already report the new height without another + // provisional row draw. This is deliberately anchor-only: the walk + // has no old on-screen reference with which to stabilize another + // row's coordinate space. + let known_anchor_height = (is_anchor && stable_lead.is_some()).then(|| { + painter.known_len(self.slot_widget(slot), axis).map(|len| { + len.apply_rest(density) + .within_len(container_len) + .to_abs(output_len) + }) + }); + let known_anchor_height = known_anchor_height.flatten(); if let Some(h) = cached { - let (lead, trail) = placement.edges(h); + let (lead, trail) = placement.edges(known_anchor_height.unwrap_or(h)); + if let Some(old_lead) = stable_lead { + self.stabilize_lead(painter, old_lead, lead); + } if !self.intersects_viewport(lead, trail) { return (lead, trail); } } - let widget = self.slot_widget(slot); - let height = match cached { - Some(h) => { + let height = match (cached, known_anchor_height) { + (Some(_), Some(height)) => { + let (lead, trail) = placement.edges(height); + let region = self.row_region(lead, trail); + painter.widget_within(self.slot_widget(slot), region); + height + } + (Some(h), None) => { let (lead, trail) = placement.edges(h); - let used = painter.widget_within(widget, Self::abs_region(dir, lead, trail)); + let region = self.row_region(lead, trail); + let used = painter.widget_within(self.slot_widget(slot), region); let height = resolve(used); if height != h { - let (lead, trail) = placement.edges(height); - painter.place(widget, Self::abs_region(dir, lead, trail)); + let (new_lead, new_trail) = placement.edges(height); + if stable_lead.is_some() { + self.stabilize_lead(painter, lead, new_lead); + } + let region = self.row_region(new_lead, new_trail); + painter.place(self.slot_widget(slot), region); } height } - None => { + (None, None) => { let measure_from = match placement { Placement::Leading(lead) => lead, Placement::Trailing(_) => 0.0, }; - let first = Self::abs_region(dir, measure_from, measure_from + GENEROUS_PADDING); - let height = resolve(painter.widget_within(widget, first)); + let first = self.row_region(measure_from, measure_from + GENEROUS_PADDING); + let height = resolve(painter.widget_within(self.slot_widget(slot), first)); let (lead, trail) = placement.edges(height); - painter.place(widget, Self::abs_region(dir, lead, trail)); + if is_anchor { + self.stabilize_lead(painter, measure_from, lead); + } + let region = self.row_region(lead, trail); + painter.place(self.slot_widget(slot), region); height } + (None, Some(_)) => unreachable!("an uncached row has no retained height"), }; let (lead, trail) = placement.edges(height); if let Some(k) = key { @@ -902,6 +982,11 @@ impl LazySpan { /// Primary-axis room for a row whose extent is not cached yet. const GENEROUS_PADDING: f32 = 100_000.0; +/// Keep stable local row coordinates precise over an indefinitely long +/// scroll. Crossing this costs one rare O(visible) rebase of row move slots; +/// ordinary ticks remain one child-coordinate write. +const CONTENT_OFFSET_REBASE: f32 = 65_536.0; + impl LazySpan { /// Make this span scrollable: the wheel and a finger drag, registered /// on the span itself. @@ -1022,9 +1107,17 @@ impl Widget for LazySpan { if let Some(tap) = self.pending_tap.take() { self.reanchor_at_tap(tap); } + self.rebase_content_offset(); + let stable_anchor_lead = self.anchor.and_then(|anchor| { + self.extents + .values() + .find(|extent| extent.slot == anchor.slot) + .map(|extent| extent.lead) + }); self.extents.clear(); + self.apply_content_offset(painter); - let (lead, trail) = self.lay_out(painter); + let (lead, trail) = self.lay_out(painter, stable_anchor_lead); // **The clamp is applied inside the frame that found it**, not // marked for the next one: layout is a pure function of the state @@ -1047,9 +1140,15 @@ impl Widget for LazySpan { // box at a new offset, which `draw_inner` dispatches as an O(1) // move. if let Some(gap) = self.overscroll_gap(lead, trail) { + let stable_anchor_lead = self.anchor.and_then(|anchor| { + self.extents + .values() + .find(|extent| extent.slot == anchor.slot) + .map(|extent| extent.lead) + }); self.move_anchor(gap); self.extents.clear(); - self.lay_out(painter); + self.lay_out(painter, stable_anchor_lead); } self.rehome_anchor(); @@ -1200,13 +1299,12 @@ mod tests { /// is `dir` -- and the newest row moves from the bottom of the screen /// to the top. /// - /// Asserts on **where each row was actually drawn** - /// (`UiRenderState::active`), not on `extents`: those are kept in the - /// walk's own direction-relative space and converted on the way out, - /// so an `extent()`-only test passes even with the flip in - /// `abs_region` deleted -- it would be checking the bookkeeping - /// against itself while every row painted at the mirror of where it - /// belongs. + /// Asserts on each row's **resolved window region**, not on `extents`: + /// those are kept in the walk's own direction-relative space and + /// converted on the way out, so an `extent()`-only test passes even with + /// the flip in `abs_region` deleted. The resolved region also includes + /// the child-coordinate offset that now keeps retained row geometry + /// stable. #[test] fn a_dir_up_span_grows_upward_from_item_zero() { let mut rsc = TestRsc { @@ -1221,7 +1319,9 @@ mod tests { render.update(&root, &mut rsc); let drawn = |render: &UiRenderState, row: &WeakWidget| { - let px = render.active[&row.id()].region.to_px((100.0, 100.0).into()); + let px = render + .window_region(row, &rsc) + .expect("every short-list row is drawn"); (px.top_left.y, px.bot_right.y) }; assert_eq!( @@ -1297,16 +1397,16 @@ mod tests { // Row 4 is on screen in both spans now, and stays drawn // across a move this small whichever way it goes. - let top = |render: &UiRenderState| { - render.active[&rows[4].id()] - .region - .to_px((100.0, 100.0).into()) + let top = |render: &UiRenderState, rsc: &TestRsc| { + render + .window_region(&rows[4], rsc) + .expect("row 4 is still drawn") .top_left .y }; - let before = top(&render); + let before = top(&render, &rsc); push(&mut rsc, &mut render, -10.0); - top(&render) - before + top(&render, &rsc) - before }; for dir in [Dir::DOWN, Dir::UP] { @@ -1445,7 +1545,14 @@ mod tests { rsc.ui.widgets.get_mut(&fg).unwrap().y = Some(Len::abs(new_height)); render.update(&root, &mut rsc); - let px = render.active[&bg_id].region.to_px((100.0, 100.0).into()); + let px = render + .active + .get(&bg_id) + .unwrap_or_else(|| { + panic!("row {key_to_change} resized to {new_height}px lost its background") + }) + .region + .to_px((100.0, 100.0).into()); let drawn = px.size().y; assert!( (drawn - new_height).abs() < 0.5, @@ -1664,19 +1771,79 @@ mod tests { // overscroll, and the clamp lays out a second time within the // frame to give it back -- a correct extra pass, but not the // ordinary scroll tick whose cost this test is about. + // Admit any row touching the viewport's leading edge first; its + // initial placement is real virtualization work, not movement of + // the already-retained run. rsc.ui.widgets.get_mut(&list_weak).unwrap().scroll(5.0); render.update(&root, &mut rsc); + render.take_counters(); + + rsc.ui.widgets.get_mut(&list_weak).unwrap().scroll(1.0); + render.update(&root, &mut rsc); let (draws, _rewrites, moves, _shapes) = render.take_counters(); - // The visible window is a fixed ~10 rows regardless of n; an - // O(n) regression would show up as draws/moves scaling with - // list size instead of staying flat. - assert!(draws <= 12, "n={n}: expected O(visible), got {draws} draws"); - assert!(moves >= 1, "n={n}: a scroll tick should move something"); - assert!(moves <= 12, "n={n}: expected O(visible) moves, got {moves}"); + // The visible window is a fixed ~10 rows regardless of n. The + // row walk still runs so virtualization can admit and retire + // rows, but every retained row shares the list's one child + // coordinate move. + assert_eq!(draws, 1, "n={n}: only the list should really draw"); + assert_eq!(moves, 1, "n={n}: the whole retained run should move once"); } } + #[test] + fn a_large_accumulated_offset_rebases_without_moving_the_content() { + let mut rsc = TestRsc { + ui: UiData::default(), + }; + let mut list = LazySpan::new(Dir::DOWN, Pin::End); + let keys: Vec = (0..100).collect(); + let rows = push_rows(&mut rsc, &mut list, &keys, 1_000.0); + let (list_weak, root) = add_list(&mut rsc, list); + + let mut render = UiRenderState::new(); + render.resize((100.0, 200.0)); + render.update(&root, &mut rsc); + + // Accumulate enough shared translation to cross the precision + // threshold while staying away from either end of the content. + rsc.ui + .widgets + .get_mut(&list_weak) + .unwrap() + .scroll(CONTENT_OFFSET_REBASE + 1.0); + render.update(&root, &mut rsc); + + let anchor_key = { + let list = rsc.ui.widgets.get(&list_weak).unwrap(); + let anchor = list.anchor.expect("the populated list has an anchor"); + list.slot_key(anchor.slot) + .expect("the anchor is a real row away from the sentinels") + }; + let anchor_row = &rows[anchor_key as usize]; + let before = render + .window_region(anchor_row, &rsc) + .expect("the rehomed anchor is retained") + .top_left + .y; + + // This draw first rebases the local coordinate space, then applies + // the requested pixel of movement. Neither operation may jump the + // rendered row. + rsc.ui.widgets.get_mut(&list_weak).unwrap().scroll(1.0); + render.update(&root, &mut rsc); + let after = render + .window_region(anchor_row, &rsc) + .expect("the anchor remains retained after the rebase") + .top_left + .y; + assert!( + (after - before - 1.0).abs() < 0.5, + "the rebase must be invisible; expected +1px, got {}px", + after - before, + ); + } + /// The streamed-reply case (RUST.md's "streaming still costs a full /// rebuild" fix, `transcript-ui::TranscriptScreen::apply`): a delta /// swaps the last row's widget for a taller one, same key, same slot.