From 5ece49b8d90bc70171678021aee0c6eabf61c692 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Wed, 9 Sep 2026 16:54:39 -0400 Subject: [PATCH] Settle growing layout branches in one frame --- AGENTS.md | 4 +- app-rust/tests/phone_screen.rs | 36 ++++++++++++----- docs/LAYOUT.md | 27 +++++++------ docs/RUST.md | 11 +++--- iris/core/src/ui/render_state.rs | 22 +++++++++-- iris/src/layout_tests.rs | 64 +++++++++++++++++++++++++++++++ iris/src/widget/position/sized.rs | 8 ++++ 7 files changed, 139 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fb11ecf..a59124d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -462,8 +462,8 @@ Each exists because something was invisible without it. 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. + Pinned growth now uploads instances at **1.1% against a 1.1% floor**, from + 71.9% against 71.8%; p50 instance upload is **1,488 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; diff --git a/app-rust/tests/phone_screen.rs b/app-rust/tests/phone_screen.rs index 583fef5..7d2b81e 100644 --- a/app-rust/tests/phone_screen.rs +++ b/app-rust/tests/phone_screen.rs @@ -258,17 +258,15 @@ fn a_newline_leaves_the_caret_inside_the_composers_padding() { screen.composer.field.edit(&mut h.rsc).insert("a\n"); h.frame(PHONE_FRAME_MS); } + let message = h + .render + .debug(h.rsc.widgets(), "Message") + .find(|a| !a.primitives.is_empty()) + .expect("the composer field is drawn"); // The caret is the last primitive `TextEdit::draw` emits. - let caret = { - let slot = *h - .render - .debug(h.rsc.widgets(), "Message") - .flat_map(|a| a.primitives.iter().map(|p| p.slot)) - .collect::>() - .last() - .expect("the focused field draws a caret"); - h.render.primitive_corners(slot, &h.rsc) - }; + let caret = h + .render + .primitive_corners(message.primitives.last().unwrap().slot, &h.rsc); // The bar sits directly on the IME, so its inside edge is one // `FIELD_PAD_DP` above `height - ime`. Stated in pixels rather than // read back from the composer, which is the thing under test. @@ -280,4 +278,22 @@ fn a_newline_leaves_the_caret_inside_the_composers_padding() { and its padding is {padding}px", caret.bot_right.y, ); + + // The old assertion only guarded the last line. A viewport one line + // shorter than the field still kept that caret above the bottom while + // moving the first line above the bar's mask, visibly slicing it off. + let mask = h.rsc.ui.masks[message.mask.idx()]; + let bar = h.render.primitive_corners(mask.primitive, &h.rsc); + let visible_content_top = message + .primitives + .iter() + .map(|p| h.render.primitive_corners(p.slot, &h.rsc)) + .filter(|r| r.bot_right.y > bar.top_left.y) + .map(|r| r.top_left.y) + .fold(f32::INFINITY, f32::min); + assert!( + visible_content_top > bar.top_left.y, + "the composer's first visible line is clipped above its bar: content starts at {visible_content_top}, bar starts at {}", + bar.top_left.y, + ); } diff --git a/docs/LAYOUT.md b/docs/LAYOUT.md index 4afbb2a..d37cc1a 100644 --- a/docs/LAYOUT.md +++ b/docs/LAYOUT.md @@ -281,12 +281,15 @@ always a leaf: `Rect`, `Image`, a fixed glyph). A widget that returns `false` (the default) is redrawn in full whenever `available` changes, which is correct always, just not free. -**Ancestor propagation** (a resized child changing its own reported size, -requiring its parent to re-lay-out) is unchanged in spirit from today's -`redraw` (`render_state.rs:270-305`), which already walks up exactly the -ancestors whose cached size differs from the new one and stops as soon as -a size is unchanged (`:274-286`). That loop moves from consulting -`Cache.size` to consulting `ActiveData.size` (§5) but keeps its shape. +**Size propagation goes both ways in the same frame.** A resized child first +walks upward through exactly the ancestors whose cached size changes. That +measurement pass gives each parent the new size but necessarily drew the +branch in its old boxes. As the recursion returns, `redraw_and_settle` revisits +those changed widgets from the outside in, after their parents have assigned +the final boxes. Otherwise a newly appended child can retain the provisional +(even inverted) region it was measured in until another update happens. The +downward work is confined to the branch that changed; unchanged descendants +still take `draw_inner`'s retained fast path. ### 4. Wrapped text, and "needs child height before choosing width" @@ -667,9 +670,11 @@ A move alone cannot fix a changed size; `Painter::place` redraws in that case. The cost is bounded and worth stating, because it is what makes the rule -safe to apply everywhere: the second draw happens only on the frame a +safe to apply everywhere: the settling draw happens only on the frame a widget's own size actually changes, which is a frame that was already -redrawing it. A widget whose reported size is a function of the box it -was *offered* would disagree every frame and redraw every frame — which -is why `LazySpan` requires content-sized rows, and has since long before -this. +redrawing it. `Sized` also requires its final region before retaining its +children: its own reported size may be known exactly while a descendant was +drawn in the provisional box, so moving only the wrapper is insufficient. A +widget whose reported size is a function of the box it was *offered* would +disagree every frame and redraw every frame — which is why `LazySpan` requires +content-sized rows, and has since long before this. diff --git a/docs/RUST.md b/docs/RUST.md index fb6f1d2..7a4fe39 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -682,10 +682,9 @@ report an exact `Len`; a debug assertion compares every hint with the real draw result. If final allocation changes a child's size, `Painter::place` redraws it in that box. Otherwise placement is one move-offset write. -Measured over the fixture's 401 streamed events: the busiest frame makes -176 `Widget::draw` calls and the worst widget is called four times. -Streamed-frame CPU p50 is 0.35ms, from 1.18ms before this layout change. -Arena size and upload floors are unchanged. +Measured over the fixture's 401 streamed events, streamed-frame CPU p50 is +0.12ms, from 1.18ms before this layout change. Arena size and upload floors +are unchanged. Pinned growth now uses the same subtree translation as scrolling. A container can retain a child-coordinate move slot through @@ -693,8 +692,8 @@ container can retain a child-coordinate move slot through 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 +upload is **1.1% against a 1.1% floor**, from 71.9% against 71.8%; median +instance bytes per frame are **1,488**, from 176,496. This is framework layout/rendering behaviour and the transcript screen contains no special case for it. diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 1009c74..0358ee0 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -1207,6 +1207,16 @@ impl UiRenderState { /// redraws a widget that's currently active (drawn) pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) { + self.redraw_and_settle(id, rsc); + } + + /// Measure a changed branch toward the root, then revisit each widget + /// whose reported size changed on the way back down. The upward pass gives + /// every parent the new child size; the downward pass is what lets those + /// children draw inside the final boxes their parents chose. Without it a + /// newly grown subtree can retain the provisional (even inverted) region + /// it was measured in until an unrelated later update redraws it. + fn redraw_and_settle(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) { rsc.widgets_mut().needs_redraw.remove(&id); // An ancestor is drawing this widget right now, and that draw is // about to write fresh primitives for it. Drawing it a second time @@ -1248,11 +1258,15 @@ impl UiRenderState { // relay out too. Checked after the real draw, not before it -- // there is no query left that answers "what size would this be" // without actually drawing (LAYOUT.md section 5). - if let Some(pid) = parent { - let new_size = self.active.get(&id).map(|a| a.size); - if new_size != Some(old_size) { - self.redraw(pid, rsc); + let changed = self.active.get(&id).map(|a| a.size) != Some(old_size); + if changed { + if let Some(pid) = parent { + self.redraw_and_settle(pid, rsc); } + // The parent pass above has now placed this widget in its final + // region. Draw it once more there; unchanged descendants still + // take draw_inner's retained fast path. + self.redraw_and_settle(id, rsc); } } } diff --git a/iris/src/layout_tests.rs b/iris/src/layout_tests.rs index 29b4e6e..beaad8d 100644 --- a/iris/src/layout_tests.rs +++ b/iris/src/layout_tests.rs @@ -1172,3 +1172,67 @@ fn a_span_of_padded_children_inside_a_span_draws_each_where_its_box_is() { ); } } + +/// Growing an already-drawn row first measures its new child against the +/// row's old height. That provisional box can end before it starts when the +/// old trailing edge is above the new child's cursor. The size must bubble to +/// `LazySpan` and the corrected allocation must travel back down before this +/// update is presented; a later stream event is not a layout pass. +#[test] +fn a_new_child_in_a_growing_lazy_row_uses_its_final_box_immediately() { + const FIRST: f32 = 30.0; + const SECOND: f32 = 70.0; + const GAP: f32 = 8.0; + + let mut rsc = TestRsc { + ui: UiData::default(), + }; + let first = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE)); + let first_id = first.id(); + let first = rsc.ui.widgets.add_strong(Sized { + inner: first.any(), + x: None, + y: Some(Len::abs(FIRST)), + }); + let mut contents = Span::empty(Dir::DOWN).gap(Len::abs(GAP)); + contents.push(first.any()); + let contents = rsc.ui.widgets.add_strong(contents); + let contents_w = contents.weak(); + let row = rsc.ui.widgets.add_strong(Sized { + inner: contents.any(), + x: Some(Len::rest(1.0)), + y: None, + }); + let mut list = LazySpan::new(Dir::DOWN, Pin::End); + list.push_back(LazyItem::new(0, row.any())); + let root = rsc.ui.widgets.add_strong(list).any(); + + let mut render = UiRenderState::new(); + render.resize((200.0, 200.0)); + render.update(&root, &mut rsc); + + let second = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE)); + let second_id = second.id(); + let second = rsc.ui.widgets.add_strong(Sized { + inner: second.any(), + x: None, + y: Some(Len::abs(SECOND)), + }); + rsc.ui + .widgets + .get_mut(&contents_w) + .unwrap() + .push(second.any()); + render.update(&root, &mut rsc); + + let first = render.primitive_corners(render.first_primitive(first_id).unwrap(), &rsc); + let second = render.primitive_corners(render.first_primitive(second_id).unwrap(), &rsc); + assert!( + (second.top_left.y - (first.bot_right.y + GAP)).abs() < 0.01, + "the new child should start after the old child and its gap: first={first:?} second={second:?}" + ); + assert!( + (second.bot_right.y - second.top_left.y - SECOND).abs() < 0.01, + "the new child retained its provisional box: {second:?}" + ); +} diff --git a/iris/src/widget/position/sized.rs b/iris/src/widget/position/sized.rs index d6d17ed..f34ff80 100644 --- a/iris/src/widget/position/sized.rs +++ b/iris/src/widget/position/sized.rs @@ -31,4 +31,12 @@ impl Widget for Sized { Axis::Y => self.y, } } + + fn requires_exact_region(&self) -> bool { + // `Sized` may be measured in a provisional box and then placed in + // the content-sized box it reported. Its own region can be corrected + // by a move, but its child consumed the original box and must see the + // final one too. + true + } }