diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index f087aa8..24b15b4 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -12,6 +12,49 @@ and six phone-report sections went on 2026-09-08 for that reason. ## Fix +- [ ] **`Span` draws every child twice, and making the first one a + measurement moves the layout.** Found 2026-09-09; the safe half landed + and this is the part that needs a decision. + + `Span::draw`'s phase 1 draws each child at `UiRegion::FULL` purely to + learn its length along the axis, then phase 2 draws it at its real + share. The provisional slot is the whole span, so it is wrong by + construction for every child, and the doubling compounds through nested + spans: one streamed frame of the bench fixture made **1,083 + `Widget::draw` calls over 113 distinct widgets** before `ScrollArea`'s + probe became a `Painter::measure`, and **453** after, with 102 of the + 113 still at 4 draws (2 real, 2 measurements). + + Changing phase 1 to `painter.measure(child, UiRegion::FULL)` takes it + to 2, and **it renders differently**: 28,771 pixels, and the headless + phone shot shows the transcript shifted a few pixels vertically. The + layout is intact -- panels, code fences and text all draw correctly -- + so this is a position difference, not a broken frame, but which of the + two is *right* was not established and it must be before this lands. + + The mechanism to check first. With phase 1 drawing, the child's + `active.region` is the full span when phase 2 asks, so phase 2 always + finds a different size and does a real redraw. With phase 1 measuring, + `active.region` is still *last frame's* share, which usually matches, + so phase 2 takes `draw_inner`'s `mov` branch -- one `move_offsets` + write instead of a redraw, which is the intended win. But `mov` + accumulates (`entry.delta += delta`) where a redraw recomputes from + scratch, so the suspicion is float drift that phase 1's unconditional + redraw was hiding. If that is it, the fix is in `mov`, not in `Span`. + + Iris's framing, which is the target shape (2026-09-09): *"if you draw + one child in a list of fixed sized children, then you know immediately + where the second one must be and shouldn't need to probe its size + again. The only time redraws should actually be needed are if you're + using a `rest` length, where you don't know how long it's gonna be + until you draw everything else first, and so you have to move things. + Even then it should just be moving, not redrawing."* So the endpoint is + no probe phase at all for `abs` children -- draw each in turn at + `[cursor, end]`, read its length, advance the cursor -- with a `rest` + child forcing a reposition pass over what follows it rather than a + redraw. `Aligned` already has exactly this shape (one draw, then + `Painter::reposition`) and is the example to copy. + - [ ] **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 diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 740008c..bd9208a 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -509,14 +509,39 @@ impl UiRenderState { // and, with `LazySpan` setting no mask, outside the list's own bounds: // 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. - // A measurement consumes no redraw mark and takes none of the - // fast paths: it is not the redraw the mark asked for, and - // "already drawn at this region" would make it return without - // reporting a size at all. - let dirty = !mode.measuring() && rsc.widgets_mut().needs_redraw.remove(&id); - if let Some(active) = self.active.get_mut(&id) + // A measurement **peeks** at the mark rather than consuming it: it + // is not the redraw the mark asked for, and swallowing it would + // leave the widget stale until something else marked it again. + let dirty = if mode.measuring() { + rsc.widgets().needs_redraw.contains(&id) + } else { + rsc.widgets_mut().needs_redraw.remove(&id) + }; + + // What a measurement can answer without drawing at all. + // + // A widget's reported size is a function of its own state and the + // size it was offered -- not of where it was offered. So an + // undirtied widget already drawn at a region of this size has + // *already answered this question*, and `active.size` is that + // answer. This is the same assumption `mov` below already makes + // (same offered size, therefore identical output, therefore a + // translation rather than a redraw); it is only stated here as a + // size rather than acted on as a move. + // + // Without it a measurement costs a full recursive walk of the + // subtree, and since the containers that measure nest, that walk + // is what made one streamed frame 1,083 `Widget::draw` calls over + // 113 distinct widgets. + if mode.measuring() { + if let Some(active) = self.active.get(&id) + && !dirty + && active.region.size() == region.size() + { + return active.size; + } + } else if let Some(active) = self.active.get_mut(&id) && !dirty - && !mode.measuring() { // check to see if we can skip drawing first if active.region == region { diff --git a/iris/src/widget/position/scroll_area.rs b/iris/src/widget/position/scroll_area.rs index 42fbf2e..ff2bdbd 100644 --- a/iris/src/widget/position/scroll_area.rs +++ b/iris/src/widget/position/scroll_area.rs @@ -107,7 +107,11 @@ impl Widget for ScrollArea { // has been measured: a zero-length region on the first frame would // place the child's primitives against a box of no size. let hint = self.content_len.unwrap_or(container_len); - let used = painter.widget_within(&self.inner, self.child_region(hint)); + // A **measurement**: this asks how long the content is, and the + // box it asks about is built from a hint that the answer below is + // about to correct. Drawing it here painted the whole content at a + // provisional offset and then painted it again at the real one. + let used = painter.measure(&self.inner, self.child_region(hint)); // A child reporting `rel` means "this fraction of what I was // offered", and what it was offered is this scroll area -- so the diff --git a/iris/src/widget/position/span.rs b/iris/src/widget/position/span.rs index 9ecca40..a896b7a 100644 --- a/iris/src/widget/position/span.rs +++ b/iris/src/widget/position/span.rs @@ -17,14 +17,17 @@ impl Widget for Span { let axis = self.dir.axis; let gap = self.gap.apply_rest(painter.density()).abs; - // Phase 1: draw each child once, at the ambient (unmodified, full) - // region a size-only query used to see before this migration, to - // learn its length along the layout axis. This paints real - // primitives at a provisional slot; phase 2 below places each - // child for real via the normal `widget_within` dispatch, which - // only actually redraws it when that slot's *size* differs from - // this provisional one (most children: a resize, since the - // provisional slot is the whole span, not this child's share). + // Phase 1: ask each child how long it is along the layout axis, + // at the ambient (unmodified, full) region. + // + // A **measurement**, so it writes nothing: the provisional slot + // this asks about is the whole span rather than the child's own + // share, so it is almost never where the child ends up, and + // painting it there meant every child of every `Span` was drawn + // twice -- once at a slot that was wrong by construction and once + // where it belongs. With containers nested that doubling + // compounds, which is where a streamed frame's 1,083 draws over + // 113 widgets came from. let lens: Vec = self .children .iter() @@ -34,11 +37,13 @@ impl Widget for Span { let gap_total = gap * self.children.len().saturating_sub(1) as f32; let total = lens.iter().fold(Len::abs(gap_total), |s, &l| s + l); - // Phase 2: place each child for real, using the lengths just - // learned -- the same arithmetic this loop always used. The cross- - // axis length of *this* draw (used for `Span`'s own reported size - // below) falls out of each child's real, resolved-width `used` - // here for free -- this is what replaces `desired_ortho`'s former + // Phase 2: draw each child for real, using the lengths just + // learned -- the same arithmetic this loop always used. This is + // the only draw a child gets; where its size is unchanged and only + // its position moved, `draw_inner` turns it into one `move_offsets` + // write rather than a redraw. The cross-axis length of *this* draw + // (used for `Span`'s own reported size below) falls out of each + // child's real, resolved-width `used` here for free -- this is what replaces `desired_ortho`'s former // duplicate simulation of this same loop (see LAYOUT.md section 4). let mut start = UiScalar::rel_min(); let mut ortho_len = Len::ZERO;