From 992482414f05cc9b168533d2617904adfe16a5bc Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Wed, 9 Sep 2026 15:11:57 -0400 Subject: [PATCH] Redesign span layout around retained placement --- core/src/render/primitive.rs | 18 +- core/src/ui/active.rs | 54 +---- core/src/ui/painter.rs | 174 ++++++--------- core/src/ui/render_state.rs | 315 +++++++++------------------ core/src/widget/mod.rs | 48 ++--- src/layout_tests.rs | 68 ++++-- src/widget/image.rs | 11 +- src/widget/mask.rs | 23 +- src/widget/position/align.rs | 32 +-- src/widget/position/layer.rs | 4 +- src/widget/position/lazy_span.rs | 336 ++--------------------------- src/widget/position/max_size.rs | 17 +- src/widget/position/offset.rs | 4 +- src/widget/position/pad.rs | 24 ++- src/widget/position/scroll_area.rs | 106 +-------- src/widget/position/sized.rs | 23 +- src/widget/position/span.rs | 69 +++--- src/widget/position/stack.rs | 55 +++-- src/widget/ptr.rs | 4 +- src/widget/rect.rs | 37 +--- src/widget/text/edit.rs | 4 + src/widget/text/mod.rs | 4 + src/widget/trait_fns.rs | 1 + 23 files changed, 426 insertions(+), 1005 deletions(-) diff --git a/core/src/render/primitive.rs b/core/src/render/primitive.rs index d8bacae..e4e4af7 100644 --- a/core/src/render/primitive.rs +++ b/core/src/render/primitive.rs @@ -264,22 +264,8 @@ impl Primitives { slot as u32 } - /// Rewrites a slot this widget already owns, instead of freeing it - /// and allocating another -- the recycle path - /// (`Painter::write_primitive`). - /// - /// **Why a redraw must be able to do this.** Freed slots do not - /// become reusable until the end of the frame (`freed`), so a widget - /// that frees its primitives and immediately draws again takes fresh - /// slots every time. Since a container learns a child's size by - /// drawing it (`Painter::measure`, and the real draw that follows) - /// and containers nest, that made the arena's - /// high-water the *transient* push count rather than the live one: - /// measured over the bench fixture's 401 streamed deltas - /// (`scripts/rigs/ui-profile`'s `arena_churn`) at 17 million pushes - /// and 127,443 slots for 11,569 live primitives, growing linearly - /// with the transcript. - /// + /// Rewrites a slot this widget already owns. Freed slots are not reusable + /// until the end of a frame, so nested provisional redraws must recycle. /// The caller has already checked that `h` is the same kind of /// primitive in the same layer, which is what makes the slot, its /// entry in the per-primitive data, and its position in the layer's diff --git a/core/src/ui/active.rs b/core/src/ui/active.rs index 468bfd3..d1cdbeb 100644 --- a/core/src/ui/active.rs +++ b/core/src/ui/active.rs @@ -11,59 +11,15 @@ pub struct ActiveData { pub textures: Vec, pub primitives: Vec, pub children: Vec, - /// The mask this widget was drawn **under** (its parent's), not the - /// one it set for itself -- see `own_mask` for that. + /// The inherited mask, not `own_mask`. pub mask: MaskIdx, - /// The mask slot this widget allocated for *itself* with - /// `Painter::set_mask`, or `MaskIdx::NONE`. Kept across redraws and - /// rewritten in place, the way `move_slot` is: a `Masked` that pushed - /// a fresh slot each draw left every already-drawn descendant -- - /// which `draw_inner`'s unchanged-region fast path does not revisit -- - /// clipping to the *old* slot's region, so a composer whose bar had - /// since been placed at the bottom of the screen was still being - /// clipped to a box at the top of it and drew nothing (measured - /// 2026-09-06: four mask entries live, none of them the widget's - /// current region). Its path out is the `undraw` branch of - /// `UiRenderState::remove`, which drops the self-ownership ref taken - /// when the slot was allocated. + /// The widget's retained mask slot, or `MaskIdx::NONE`. pub own_mask: MaskIdx, pub layer: LayerId, - /// What `Widget::draw` returned the last time this widget was actually - /// drawn -- read by a parent placing this widget again without - /// redrawing it, replacing `Cache.size`'s old role. See LAYOUT.md - /// section 5. + /// The last `Widget::draw` result. pub size: Size, - /// This widget's slot in `UiData::move_offsets`, assigned on its first - /// draw and kept for the rest of its life (redraws reuse it in place - /// so a retained child's `parent` link never goes stale). See - /// LAYOUT.md section 2. + /// Retained so descendants' parent links stay valid across redraws. pub move_slot: MoveIdx, - /// How much of this widget's own `move_slot` delta is already folded - /// into `region` above, in window pixels. The two mechanisms that - /// write that slot disagree about this and cannot be told apart from - /// the slot alone: `UiRenderState::mov` shifts `region` and the delta - /// together (the *offered* region genuinely moved), while - /// `Painter::reposition` writes only the delta (`region` stays the - /// offered box and the delta says where inside it the content was - /// placed). So anything that wants the widget's real position -- - /// `resolved_region`, and through it every hit test -- must subtract - /// this from the chain sum. Without it a panned widget's own hit box - /// sits at twice the pan while its descendants' are correct, which is - /// how it went unnoticed: the composer's field became untappable - /// after a finger pan (2026-09-06). Reset to zero whenever the widget - /// is really redrawn, since `draw_inner` zeroes the slot then too. + /// The part of this widget's move delta already folded into `region`. pub move_applied: Vec2, - /// The offset the last `Painter::reposition` placed this widget's - /// content at *within* `region`, in window pixels. The move slot has - /// exactly one owner and one meaning: - /// `move_offsets[move_slot] == move_applied + repositioned`. `mov` - /// adds to the first, `reposition` overwrites the second (it - /// recomputes `from` afresh every call, so repeating it must land on - /// the same answer rather than drifting), and both then rewrite the - /// slot from the sum -- which is what lets a parent both move a child - /// with its own layout and place it inside that moved region in one - /// frame. `LazySpan::place`'s Bottom-known branch does exactly that once a - /// row's blocks wrap. Reset to zero on a real redraw, with - /// `move_applied` and the slot itself. - pub repositioned: Vec2, } diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index 199966d..e2dda90 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -1,6 +1,6 @@ use crate::{ - Color, DrawMode, RenderedText, Size, StrongWidget, TextAttrs, TextBuffer, TextData, - TextureHandle, UiRegion, UiRenderState, UiRsc, UiScalar, UiVec2, WidgetId, + Axis, Color, Len, 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, @@ -9,7 +9,6 @@ use crate::{ util::Vec2, }; -/// makes your surfaces look pretty pub struct Painter<'a> { pub(super) state: &'a mut UiRenderState, pub(super) rsc: &'a mut dyn UiRsc, @@ -17,43 +16,19 @@ pub struct Painter<'a> { pub(super) region: UiRegion, pub(super) mask: MaskIdx, pub(super) move_slot: MoveIdx, - /// This widget's own mask slot, reused across redraws -- see - /// `ActiveData::own_mask`. `MaskIdx::NONE` until `set_mask` is called - /// for the first time in this widget's life. + /// This widget's retained mask slot. pub(super) own_mask: MaskIdx, pub(super) textures: Vec, pub(super) primitives: Vec, - /// The handles this widget owned before *this* draw, offered back to - /// it in the order it wrote them last time -- see - /// [`Self::take_recycled`]. Empty for a widget being drawn for the - /// first time. Whatever is left when the draw ends is genuinely gone, - /// and `UiRenderState::draw_inner` frees the remainder. - /// - /// An iterator rather than a vec and a cursor because a - /// `PrimitiveHandle` is an ownership token and deliberately not - /// `Clone`: `peek` asks whether the next one fits without taking it, - /// `next` takes it, and what is left is exactly what nothing claimed. + /// Previous handles, consumed in draw order and freed if left over. pub(super) recycle: std::iter::Peekable>, pub(super) children: Vec, + pub(super) reuse_child_sizes: bool, pub layer: usize, pub(super) id: WidgetId, - /// Whether this draw produces what goes on screen or only a size -- - /// see [`crate::DrawMode`]. Inherited by every child this widget - /// draws, so one `measure` at the top makes the whole subtree - /// write-free. - pub(super) mode: DrawMode, } impl<'a> Painter<'a> { - /// True while this draw is only being asked how big the widget would - /// be. **Every method here that writes anything must return early on - /// it** -- a widget's own `draw` never has to check, which is the - /// point: measuring is a property of the painter, not something each - /// widget re-implements. - pub fn measuring(&self) -> bool { - self.mode == DrawMode::Measure - } - fn primitive_at(&mut self, primitive: P, region: UiRegion) { self.write_primitive(primitive, region, Drawn::Yes); } @@ -74,8 +49,6 @@ impl<'a> Painter<'a> { let h = self.recycle.peek()?; let drawn_matches = (h.pos == NOT_DRAWN) == (drawn == Drawn::No); if h.binding != binding || h.layer != self.layer || !drawn_matches { - // Left un-taken deliberately: this handle and everything after - // it is freed together when the draw ends. return None; } self.recycle.next() @@ -89,9 +62,6 @@ impl<'a> Painter<'a> { region: UiRegion, drawn: Drawn, ) -> u32 { - if self.measuring() { - return u32::MAX; - } let inst = PrimitiveInst { id: self.id, primitive, @@ -107,7 +77,6 @@ impl<'a> Painter<'a> { None => self.state.write_primitive(self.layer, drawn, inst), }; if self.mask != MaskIdx::NONE { - // TODO: I have no clue if this works at all :joy: self.rsc.ui_mut().masks.push_ref(self.mask); } let slot = h.slot; @@ -157,13 +126,6 @@ impl<'a> Painter<'a> { /// so keeps pointing at whichever slot it was drawn under. See /// `ActiveData::own_mask` for what pushing a fresh one cost. pub fn set_mask(&mut self, region: UiRegion) { - // Clipping changes no widget's reported size, so a measurement - // skips it whole -- not just the shape primitive, but the mask - // slot and its refs, which would otherwise be a leaked slot per - // masked widget per measured frame. - if self.measuring() { - return; - } let shape = self.write_primitive(RectPrimitive::color(Color::NONE), region, Drawn::No); self.set_mask_to(shape); } @@ -176,12 +138,6 @@ impl<'a> Painter<'a> { /// with no radius argument anywhere that could fall out of step with /// the one being drawn. pub fn set_mask_to_widget(&mut self, shape: &StrongWidget) { - // Same as `set_mask`, and doubly so: a measurement leaves nothing - // in `active`, so the shape widget has drawn no primitive to - // point at and this would panic on its own message. - if self.measuring() { - return; - } let slot = self.state.first_primitive(shape.id()).unwrap_or_else(|| { panic!( "'{}' was given as a mask's shape but drew no primitive, so there is nothing to \ @@ -261,6 +217,16 @@ impl<'a> Painter<'a> { self.widget_at(id, region.within(&self.region)) } + 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())); + } + if !self.reuse_child_sizes || self.rsc.widgets().needs_redraw.contains(&id.id()) { + return None; + } + self.state.active.get(&id.id()).map(|a| a.size.axis(axis)) + } + fn widget_at(&mut self, id: &StrongWidget, region: UiRegion) -> Size { self.children.push(id.id()); // Passed directly rather than looked up from `self.active`: this @@ -277,66 +243,65 @@ impl<'a> Painter<'a> { Some(self.id), self.move_slot.idx() as u32, self.mask, - self.mode, Retained::default(), self.rsc, ) } - /// Ask `widget` how big it would be in `region`, **writing nothing** - /// -- see [`DrawMode::Measure`]. For the container that cannot choose - /// what to offer a child without already knowing the child's size: - /// measure, work out the real region, then draw it for real. - /// - /// ```ignore - /// let used = painter.measure(&child, generous); - /// painter.widget_within(&child, self.box_for(used)); - /// ``` - /// - /// This replaced a `draw_twice(child, first, |used| second)`, which - /// made the same two draws but had the caller express the second - /// region as a closure returning it -- so the interesting arithmetic - /// happened inside a callback and anything it wanted to keep had to - /// be written out through a captured `&mut`. Two statements say the - /// same thing in the order it happens (CODE_RULES' "compose - /// linearly"), and the measurement costs no arena slot now rather - /// than allocating one and freeing it. - /// - /// The measured widget is left exactly as it was -- not in `active` - /// if it was not there before, and untouched if it was -- so the draw - /// that follows is an ordinary one and cannot be short-circuited by - /// the measurement having "already drawn" it at that region. - pub fn measure(&mut self, id: &StrongWidget, region: UiRegion) -> Size { - self.state.draw_inner( - self.layer, - id.id(), - region.within(&self.region), - Some(self.id), - self.move_slot.idx() as u32, - self.mask, - DrawMode::Measure, - Retained::default(), - self.rsc, - ) - } - - /// Move an already-drawn child from wherever it currently sits to - /// `region` (resolved against this widget's own region, matching - /// `widget_within`) without a second draw -- an O(1) offset write via - /// `UiRenderState::mov`. For a container that draws a child - /// provisionally to learn its size (e.g. `Aligned`) and then places it - /// for real. Only valid when the target keeps the child's drawn size; - /// if the shape actually changes, the normal `widget_within` dispatch - /// (which detects that from the stored region) does the right thing - /// instead. - pub fn reposition(&mut self, id: &StrongWidget, region: UiRegion) { - // Moves an *already-drawn* child, of which a measurement has - // none. - if self.measuring() { - return; - } + /// Place an already-drawn child's used area, redrawing only if its size changes. + pub fn place(&mut self, id: &StrongWidget, region: UiRegion) -> Size { let region = region.within(&self.region); - self.state.reposition(id.id(), region, self.rsc); + let retained = self + .state + .active + .get(&id.id()) + .map(|active| (active.layer, active.mask)); + if let Some(size) = self.state.place(id.id(), region, self.rsc) { + size + } else if let Some((layer, mask)) = retained { + self.children.push(id.id()); + self.rsc.widgets_mut().needs_redraw.insert(id.id()); + self.state.draw_inner( + layer, + id.id(), + region, + Some(self.id), + self.move_slot.idx() as u32, + mask, + Retained::default(), + self.rsc, + ) + } else { + self.widget_at(id, region) + } + } + + pub fn place_used( + &mut self, + id: &StrongWidget, + used: Size, + within: UiRegion, + ) -> Size { + let region = self.fit_region(used, within); + self.place(id, region) + } + + pub fn fit_region(&mut self, used: Size, mut within: UiRegion) -> UiRegion { + let mut region = used + .to_uivec2(self.density()) + .align(RegionAlign::TOP_LEFT) + .within(&within); + let output = self.output_size(); + for axis in [Axis::X, Axis::Y] { + let mut actual = region.within(&self.region); + let mut available = within.within(&self.region); + if actual.axis(axis).len().to_abs(output.axis(axis)) + > available.axis(axis).len().to_abs(output.axis(axis)) + { + *region.axis_mut(axis) = *within.axis(axis); + } + } + region } pub fn texture_within(&mut self, handle: &TextureHandle, region: UiRegion) { @@ -358,9 +323,6 @@ impl<'a> Painter<'a> { /// the layer's one instanced draw, so it goes through /// `Primitives::write_image` instead of `primitive_at`/`Primitive::vec`. fn write_image(&mut self, texture_idx: u32, region: UiRegion) { - if self.measuring() { - return; - } let h = match self.take_recycled(IMAGE_BINDING, Drawn::Yes) { Some(h) => { self.state.primitives.recycle_image( diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index bd9208a..340ae78 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -2,8 +2,8 @@ use std::sync::Mutex; use std::time::{Duration, Instant}; use crate::{ - ActiveData, IdLike, MaskIdx, MoveIdx, Painter, PixelRegion, PrimitiveLayers, RegionAlign, Size, - StrongWidget, UiRegion, UiRsc, UiVec2, WidgetId, Widgets, + ActiveData, Axis, IdLike, MaskIdx, MoveIdx, Painter, PixelRegion, PrimitiveLayers, RegionAlign, + Size, StrongWidget, UiRegion, UiRsc, UiVec2, WidgetId, Widgets, render::{ Drawn, MoveOffset, NOT_DRAWN, Primitive, PrimitiveHandle, PrimitiveInst, Primitives, RectPrimitive, rounded_rect_coverage, @@ -94,65 +94,19 @@ pub struct UiRenderState { last_input_at: Mutex>, } -/// Whether a draw is producing what goes on screen, or only asking a -/// widget how big it would be. -/// -/// **There is no size query without a draw** (LAYOUT.md section 5): -/// `Widget::draw` reports the size it used, and nothing else can answer -/// it. A container that cannot choose what to offer a child without -/// already knowing the child's size therefore has to draw it -- so -/// [`Self::Measure`] is that draw with everything it *writes* switched -/// off. It allocates no arena slot, no mask, no move slot, leaves nothing -/// in `active` and marks nothing dirty; the widget is walked and its text -/// is shaped (which is memoized, and is the expensive half anyway), and -/// only the returned `Size` survives. -/// -/// Because it leaves no trace, the real draw that follows is an ordinary -/// first draw or redraw and cannot be short-circuited by the measurement -/// having "already drawn" the widget at that region -- which is the trap -/// the discarded-draw approach it replaced had to work around. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum DrawMode { - /// Write primitives, keep the result in `active`. - Draw, - /// Report a size and write nothing. - Measure, -} - -impl DrawMode { - fn measuring(self) -> bool { - self == Self::Measure - } -} - -/// What a widget being redrawn keeps from the draw it is replacing. -/// -/// These four always travel together -- they are read off one -/// `ActiveData` that was just taken out of `active` and handed straight -/// to the draw that replaces it -- and they were four positional -/// parameters of [`UiRenderState::draw_inner`] until 2026-09-09, next to -/// six others. [`Default`] is the "nothing to keep" case: a widget drawn -/// for the first time, and the root of a full relayout. +/// State retained while replacing one draw with another. pub(crate) struct Retained { - /// So children this draw does not draw again can be retired. + pub region: Option, pub children: Vec, - /// Reused in place with its delta reset, never reallocated: a - /// descendant that is not itself redrawn still points at it. See - /// LAYOUT.md section 2. pub move_slot: Option, pub own_mask: MaskIdx, - /// Slots the draw may write into instead of allocating -- see - /// `Painter::take_recycled`. Anything it does not claim is freed when - /// the draw ends. pub primitives: Vec, } impl Default for Retained { - /// Nothing kept: no children to retire, no move slot to reuse, no - /// mask of its own yet, nothing to recycle. Hand-written because - /// `MaskIdx`'s zero is a real slot rather than "none". fn default() -> Self { Self { + region: None, children: Vec::new(), move_slot: None, own_mask: MaskIdx::NONE, @@ -453,7 +407,6 @@ impl UiRenderState { None, MoveOffset::NONE_PARENT, MaskIdx::NONE, - DrawMode::Draw, Retained::default(), rsc, ); @@ -477,7 +430,6 @@ impl UiRenderState { .unwrap_or(MoveOffset::NONE_PARENT) } - // TODO: should prolly make a DrawInfo struct or smth for everything other than rsc #[allow(clippy::too_many_arguments)] pub(super) fn draw_inner( &mut self, @@ -487,120 +439,83 @@ impl UiRenderState { parent: Option, parent_move_slot: u32, mask: MaskIdx, - mode: DrawMode, retained: Retained, rsc: &mut dyn UiRsc, ) -> Size { let Retained { + region: mut old_region, children: mut old_children, move_slot: mut old_move_slot, mut own_mask, primitives: mut recycle, } = retained; - // Consumed here, not merely read: this call *is* the redraw the mark - // asked for, and leaving the mark set is what stranded a widget's - // primitives. A measure-then-draw reaches this twice for the same id - // in one frame (`LazySpan::place`'s measurement pass), and on the second - // call the still-set mark took the whole `if let` below -- including - // the `remove` that frees the first draw's primitives -- out of play, - // so `active.insert` at the end overwrote the only handles that could - // ever have freed them. The result is a full second copy of the row, - // drawn every frame from then on at the oversized measurement region - // 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 **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) - }; + let dirty = rsc.widgets_mut().needs_redraw.remove(&id); + let requires_exact_region = rsc + .widgets() + .get_dyn(id) + .is_some_and(|widget| widget.requires_exact_region()); - // 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) + if let Some(active) = self.active.get_mut(&id) && !dirty + && active.layer == layer + && active.mask == mask { - // check to see if we can skip drawing first if active.region == region { return active.size; - } else if active.region.size() == region.size() { - // TODO: epsilon? + } else if Self::same_size(active.region, region, self.output_size) { let from = active.region; let size = active.size; self.mov(id, from, region, rsc); return size; + } else if !requires_exact_region + && Self::same_size( + active + .size + .to_uivec2(self.density) + .align(RegionAlign::TOP_LEFT) + .within(&active.region), + region, + self.output_size, + ) + { + return self.place(id, region, rsc).unwrap(); } else if rsc .widgets() .get_dyn(id) .map(|w| w.is_size_independent()) .unwrap_or(false) { - // The offered region changed shape, but this widget's own - // drawn output does not depend on it (a fixed-size leaf) -- - // rewrite its own primitives' regions in place (O(primitives - // owned directly by this widget, which for a leaf is O(1)) - // instead of redrawing. See LAYOUT.md section 3. let from = active.region; for h in &active.primitives { let r = self.primitives.region_mut(h); *r = r.outside(&from).within(®ion); self.region_mut_count += 1; } - // `move_applied` is deliberately **not** touched here, - // unlike in `mov`: it counts the part of this widget's own - // move-slot delta that `region` has already absorbed, and - // this branch writes no delta at all -- the primitives were - // moved directly. Counting one would make - // `resolved_region` subtract a distance the chain never - // held, putting the hit box short of the drawing by - // exactly this step. See `ActiveData::move_applied`, and - // `a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at`. active.region = region; return active.size; } - // if not, then maintain resize and track old children to remove unneeded let active = self.remove(id, false, true, rsc).unwrap(); + old_region = Some(active.region); old_children = active.children; old_move_slot = Some(active.move_slot); own_mask = active.own_mask; recycle = active.primitives; - } else if dirty && self.active.contains_key(&id) { - debug_assert!(!mode.measuring()); - // Dirty and already drawn: none of the fast paths above may be - // taken (the widget's own content changed, so its old primitives - // say nothing about its new ones), but they are also the only - // thing that frees them. Same two lines, reached the other way. + } else if self.active.contains_key(&id) { + let layer_changed = self + .active + .get(&id) + .is_some_and(|active| active.layer != layer); let active = self.remove(id, false, true, rsc).unwrap(); + if layer_changed { + rsc.on_undraw(&active); + } + old_region = Some(active.region); old_children = active.children; old_move_slot = Some(active.move_slot); own_mask = active.own_mask; recycle = active.primitives; } - // draw widget let reentrant = !self.draw_started.insert(id); debug_assert!( !reentrant, @@ -608,23 +523,11 @@ impl UiRenderState { the second draw's primitives would orphan the first's" ); - // A measurement writes no primitive, so nothing ever reads this - // -- and allocating one would leak a slot per measured widget per - // frame, since `move_offsets` only frees on a widget's removal. - let move_slot = match mode { - DrawMode::Measure => Id::preset(MoveOffset::NONE_PARENT), - DrawMode::Draw => Self::move_slot_for(old_move_slot, parent_move_slot, rsc), - }; + let move_slot = Self::move_slot_for(old_move_slot, parent_move_slot, rsc); - // The mask this widget was drawn *under*, kept aside because - // `Painter::set_mask` overwrites `painter.mask` with the widget's - // own new one -- and `ActiveData::mask`'s only consumer is - // `redraw`, which feeds it back in as the *inherited* mask. Storing - // the set one instead handed a `Masked` its own mask on every - // targeted redraw -- an abort the first time the composer's scroll - // area was redrawn on the emulator, and now (masks nest) a mask - // whose parent is itself, which `set_mask`'s own assert names. let inherited_mask = mask; + let reuse_child_sizes = + old_region.is_some_and(|old| Self::same_size(old, region, self.output_size)); let mut painter = Painter { state: self, region, @@ -637,23 +540,34 @@ impl UiRenderState { primitives: Vec::new(), recycle: recycle.into_iter().peekable(), children: Vec::new(), - mode, + reuse_child_sizes, rsc, }; let mut widget = painter.rsc.widgets().get_dyn_dynamic(id); + let density = painter.density(); + let hints = [ + widget.size_hint(Axis::X).map(|len| len.fold_dp(density)), + widget.size_hint(Axis::Y).map(|len| len.fold_dp(density)), + ]; painter.state.draw_count += 1; let size = widget.draw(&mut painter); - // A reported length is consumed by containers that read `abs`, - // `rel` and `rest` straight off it (`Span`'s placement, `Pad`'s - // addition), so an unresolved `dp` in one is silently worth zero - // -- see `Len::fold_dp`, which is what a widget reporting a - // caller-declared size has to put it through. debug_assert!( size.x.dp == 0.0 && size.y.dp == 0.0, "widget {id:?} reported an unresolved `dp` size ({size:?}); \ report `Len::fold_dp(painter.density())` instead" ); + for (axis, hint) in [Axis::X, Axis::Y] + .into_iter() + .zip(hints) + .filter_map(|(axis, hint)| hint.map(|hint| (axis, hint))) + { + debug_assert_eq!( + size.axis(axis), + hint, + "widget {id:?}'s {axis:?} size hint differs from its draw result" + ); + } drop(widget); painter.state.draw_started.remove(&id); @@ -668,29 +582,11 @@ impl UiRenderState { primitives, recycle, children, + reuse_child_sizes: _, layer, id, - mode: _, } = painter; - if mode.measuring() { - // Nothing to unwind: a measurement allocates no slot, no - // mask, no move offset and no `ActiveData`, so the size is - // the whole of what it produced. Asserted rather than - // assumed, because a `Painter` method that forgot to check - // the mode would otherwise leak silently -- one primitive per - // measured widget per frame, which a screen redrawn every - // frame turns into an arena that grows without bound. - debug_assert!( - primitives.is_empty() && textures.is_empty(), - "measuring {id:?} wrote {} primitive(s) and {} texture(s); \ - every `Painter` write must check `Painter::measuring`", - primitives.len(), - textures.len(), - ); - return size; - } - // Whatever the draw did not claim is genuinely gone: this draw // wrote fewer primitives than the last one, or stopped matching // part way. Freeing it here rather than in `remove` is what lets @@ -714,7 +610,6 @@ impl UiRenderState { move_slot, own_mask, move_applied: Vec2::ZERO, - repositioned: Vec2::ZERO, }; // remove old children that weren't kept @@ -782,67 +677,64 @@ impl UiRenderState { self.mov_count += 1; } - /// Move an already-active widget to `to`. Used by `Painter::reposition`, - /// for a parent that drew a child provisionally (at the whole region it - /// was offered) and now knows where the child actually belongs. - /// - /// Unlike `mov` (called by `draw_inner`'s own dispatch, where the - /// *offered* region really did move and `active.region` already tracks - /// it), the child here was not offered a smaller region -- it was - /// offered everything and chose, on its own, to occupy only - /// `active.size` of it. By convention every widget in this crate that - /// does that anchors its own content at the top-left of whatever it - /// was given (`Rect`/`Image`/`Sized`/`MaxSize` -- see their `draw` - /// bodies), so that is where this assumes the child was actually - /// painted, not `active.region` itself (which is the *offered* box, - /// usually bigger). A nested `Aligned` whose own child is not top-left - /// anchored -- i.e. `Aligned` wrapping `Aligned` -- is the one shape - /// this does not cover; none of iris's widgets or examples build that - /// today. See LAYOUT.md's "Rejected, and why" / deviations for the - /// full reasoning. - /// - /// The delta is overwritten, not accumulated like `mov`'s: `from` is - /// recomputed fresh from `active.size`/`active.region` every call, so - /// repeating the same `reposition` (e.g. an unrelated redraw elsewhere - /// re-running this widget's parent without its own layout changing) - /// must land on the same answer, not drift further each time. - pub(super) fn reposition(&mut self, id: WidgetId, to: UiRegion, rsc: &mut dyn UiRsc) { - let Some(active) = self.active.get(&id) else { - return; - }; - let move_applied = active.move_applied; - let repositioned = active.repositioned; + fn same_size(a: UiRegion, b: UiRegion, output: Vec2) -> bool { + let a = a.size().to_abs(output); + let b = b.size().to_abs(output); + (a.x - b.x).abs() < 0.01 && (a.y - b.y).abs() < 0.01 + } + + pub(super) fn place( + &mut self, + id: WidgetId, + to: UiRegion, + rsc: &mut dyn UiRsc, + ) -> Option { + let active = self.active.get(&id)?; + if rsc + .widgets() + .get_dyn(id) + .is_some_and(|widget| widget.requires_exact_region()) + && !Self::same_size(active.region, to, self.output_size) + { + return None; + } let from = active .size .to_uivec2(self.density) .align(RegionAlign::TOP_LEFT) .within(&active.region); + if !Self::same_size(from, to, self.output_size) { + return None; + } + let size = active.size; + if active.region == to { + return Some(size); + } + if Self::same_size(active.region, to, self.output_size) { + let region = active.region; + self.mov(id, region, to, rsc); + return Some(size); + } + + let move_applied = active.move_applied; let slot = active.move_slot; let from_px = from.top_left().to_abs(self.output_size); let to_px = to.top_left().to_abs(self.output_size); let delta = to_px - from_px; - // Not `delta` alone: a parent may have `mov`ed this widget to a - // region that itself moved earlier in the same frame, and that - // part of the slot is `move_applied`'s, not this call's. Writing - // `delta` on its own dropped it and put the content back at the - // pre-move position. `from` is computed against `active.region`, - // which `mov` already updated, so `delta` is purely the placement - // inside the region and the two summands never overlap. + if delta.x.abs() < 0.01 && delta.y.abs() < 0.01 { + if let Some(active) = self.active.get_mut(&id) { + active.region = to; + } + return Some(size); + } let entry = rsc.ui_mut().move_offsets.get_mut(slot); - debug_assert_eq!( - entry.delta, - [ - move_applied.x + repositioned.x, - move_applied.y + repositioned.y - ], - "widget {id:?}'s move slot was written by something other than `mov`/`reposition`; \ - the slot is theirs and means `move_applied + repositioned` -- see `ActiveData`" - ); entry.delta = [move_applied.x + delta.x, move_applied.y + delta.y]; if let Some(active) = self.active.get_mut(&id) { - active.repositioned = delta; + active.region = to; + active.move_applied += delta; } self.mov_count += 1; + Some(size) } /// Retires `id`'s primitives (unless `keep_primitives`, in which case @@ -1073,6 +965,7 @@ impl UiRenderState { /// against [`Self::orphaned_primitives`]'s O(primitives), which on a /// transcript is tens of thousands and made a debug build on a phone /// too slow to finish a benchmark run. + #[cfg(debug_assertions)] fn primitive_counts_agree(&self) -> bool { let live: usize = self.primitives.live_count(); let owned: usize = self.active.values().map(|a| a.primitives.len()).sum(); @@ -1316,8 +1209,8 @@ impl UiRenderState { parent, parent_move_slot, active.mask, - DrawMode::Draw, Retained { + region: Some(active.region), children: active.children, move_slot: Some(active.move_slot), own_mask: active.own_mask, diff --git a/core/src/widget/mod.rs b/core/src/widget/mod.rs index 9b180bd..d22e4b1 100644 --- a/core/src/widget/mod.rs +++ b/core/src/widget/mod.rs @@ -1,4 +1,4 @@ -use crate::{Painter, Size}; +use crate::{Axis, Len, Painter, Size}; use std::any::Any; mod data; @@ -16,46 +16,28 @@ pub use view::*; pub use widgets::*; pub trait Widget: Any { - /// Draw within `painter.region()` (the space the parent offered) and - /// report how much of it was actually used, per axis. fn draw(&mut self, painter: &mut Painter) -> Size; - /// True if `draw`'s output (both the primitives it writes and the - /// `Size` it returns) is the same for any `painter.region()` of the - /// same *content* -- an icon, a fixed-size rect, an already-decoded - /// image at its natural size. Default `false` (redraw on any change to - /// the offered region) because assuming independence wrongly produces - /// a stale draw; a widget must opt in. See LAYOUT.md. + /// An exact, context-free length known without drawing or inspecting children. + fn size_hint(&self, _axis: Axis) -> Option { + None + } + + /// Whether the draw result is independent of the offered region. fn is_size_independent(&self) -> bool { false } - /// What kind of control this is, for the AccessKit tree `ui::access` - /// builds (RUST.md's I4). Only consulted for a widget that also has an - /// explicit `.label()` -- an unnamed widget is never visited by that - /// tree at all, named or not, so the default here costs nothing except - /// at the handful of call sites that opt in. Default `Unknown` (a - /// generic control with no more specific semantics); a widget with a - /// real platform equivalent -- `TextEdit`'s `MultilineTextInput` -- - /// overrides it. + fn requires_exact_region(&self) -> bool { + false + } + + /// The AccessKit role for a labelled widget. fn access_role(&self) -> accesskit::Role { accesskit::Role::Unknown } - /// Advance whatever this widget is animating to `now`, and say whether - /// it is still animating afterwards. Default: nothing is, so a widget - /// opts in by overriding this *and* by something calling - /// [`crate::UiData::animate`] with its id when the animation starts -- - /// which is that animation's path out, since the driver - /// ([`crate::UiData::tick_animations`]) drops every id whose `tick` - /// answers `false`. - /// - /// Called once per frame, before the frame's draw, by whichever - /// backend owns the surface; a `true` answer is what makes that - /// backend ask for another frame. So this is the only thing in iris - /// that moves without an input event, and a widget that animates - /// without registering simply never moves -- which is exactly how a - /// finger fling looked on Iris's phone before this existed. + /// Advance an animation and report whether it needs another frame. #[allow(unused_variables)] fn tick(&mut self, now: std::time::Instant) -> bool { false @@ -70,6 +52,10 @@ impl Widget for () { fn is_size_independent(&self) -> bool { true } + + fn size_hint(&self, _axis: Axis) -> Option { + Some(Len::ZERO) + } } impl dyn Widget { diff --git a/src/layout_tests.rs b/src/layout_tests.rs index 2f3a1fe..69348ad 100644 --- a/src/layout_tests.rs +++ b/src/layout_tests.rs @@ -23,6 +23,51 @@ impl UiRsc for TestRsc { } } +struct FixedRect(f32); + +impl Widget for FixedRect { + fn draw(&mut self, painter: &mut Painter) -> Size { + let size = Size::from_axis(Axis::Y, Len::abs(self.0), Len::REST); + painter.primitive_within( + RectPrimitive::color(UiColor::WHITE), + size.to_uivec2(painter.density()) + .align(RegionAlign::TOP_LEFT), + ); + size + } +} + +#[test] +fn a_hinted_rest_draws_once_and_only_moves_the_fixed_child_after_it() { + let mut rsc = TestRsc { + ui: UiData::default(), + }; + let first = rsc.ui.widgets.add_strong(FixedRect(40.0)); + let fill = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE)); + let fill = rsc.ui.widgets.add_strong(Sized { + inner: fill.any(), + x: None, + y: Some(Len::REST), + }); + let last = rsc.ui.widgets.add_strong(FixedRect(40.0)); + let last_w = last.weak(); + let mut span = Span::empty(Dir::DOWN); + span.push(first.any()); + span.push(fill.any()); + span.push(last.any()); + let root = rsc.ui.widgets.add_strong(span).any(); + + let mut render = UiRenderState::new(); + render.resize((200.0, 300.0)); + render.update(&root, &mut rsc); + let (draws, _rewrites, moves, _shapes) = render.take_counters(); + + assert_eq!(draws, 5); + assert_eq!(moves, 1); + let last = render.window_region(&last_w, &rsc).unwrap(); + assert!((last.top_left.y - 260.0).abs() < 0.01, "{last:?}"); +} + /// A `ScrollArea` over a `Span` of `n` fixed-height rects -- N primitives large /// enough that an O(N) regression in the move path would show up as a /// non-trivial counter rather than being lost in noise (LAYOUT.md section @@ -227,7 +272,7 @@ fn a_mask_stays_put_while_its_scrolled_content_moves() { let masked_slot_after = render.active.get(&masked_id).unwrap().move_slot; let mask_delta_after = rsc.ui.move_offsets[masked_slot_after.idx()].delta; - // `Masked` itself is never the target of a `mov`/`reposition` here -- + // `Masked` itself is never the target of a `mov`/`place` here -- // only its scrolled child is -- so the slot its own mask references // (`Painter::set_mask` bakes in `self.move_slot`, i.e. this one) must // still read zero after the scroll. The visible counterpart of this @@ -307,7 +352,7 @@ fn composing_text_after_a_keyboard_resize_lands_in_the_bars_own_region() { // The keyboard opens: a real `surface_changed`/`resize` to a shorter // window, then a further keystroke -- the redraw that must land in the // bar's new (also short) region, not whatever region a provisional - // measurement pass used along the way. + // provisional placement used along the way. render.resize((1080.0, 1478.0)); render.update(&root, &mut rsc); field.edit(&mut rsc).insert("b"); @@ -583,9 +628,6 @@ fn a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at( let mut render = UiRenderState::new(); render.resize((800.0, 600.0)); render.update(&root, &mut rsc); - // `Span` draws each child once at the full region to measure it and - // then places it, so this widget has already been through the branch - // once by the end of the very first frame. let first = render.window_region(&below_w, &rsc).unwrap(); assert!( (first.top_left.y - 100.0).abs() < 0.01, @@ -604,7 +646,7 @@ fn a_size_independent_widget_moved_by_its_parent_has_the_hit_box_it_is_drawn_at( } /// A parent that both `mov`s a child (its own layout moved the box it -/// offers) and `reposition`s it inside that box in the same frame -- what +/// offers) and places it inside that box in the same frame -- what /// `LazySpan::place`'s Bottom-known branch does once a row's cached height /// stops matching what the row reports, which is reachable as soon as a /// transcript row's blocks wrap (docs/IRIS_TODO.md's "Found by P1a"). @@ -634,21 +676,12 @@ impl Widget for MoveThenPlace { UiScalar::abs(self.place_top + 40.0), ), ); - painter.reposition(&self.inner, place); + painter.place(&self.inner, place); Size::default() } } -/// `mov` accumulates a delta onto a widget's move slot and `reposition` -/// overwrites it, and both can legitimately land on one widget in one -/// frame (see `MoveThenPlace`). `reposition` used to write its own delta -/// alone, which dropped the move and put the child back at the position -/// the offered box had *before* it moved; a `debug_assert!` that -/// `move_applied` was zero hid that behind a panic instead of fixing it. -/// The slot has one owner and one meaning now -- -/// `move_applied + repositioned` -- so the child stays where it was -/// placed however its offered box moves. Fails at the offer's position -/// (200) rather than the placement's (100) without that. +/// Placement must preserve a move already applied in the same frame. #[test] fn a_widget_moved_by_its_parent_and_then_placed_inside_it_lands_at_the_placement() { let mut rsc = TestRsc { @@ -1025,6 +1058,7 @@ fn a_span_of_padded_children_inside_a_span_draws_each_where_its_box_is() { let padded = rsc.ui.widgets.add_strong(Pad { padding: Padding::uniform(PAD), inner: sized.any(), + exact_region: false, }); let fill = rsc.ui.widgets.add_strong(Rect::new(UiColor::BLUE)).any(); let card = rsc.ui.widgets.add_strong(Stack { diff --git a/src/widget/image.rs b/src/widget/image.rs index 7f2c131..615a5c8 100644 --- a/src/widget/image.rs +++ b/src/widget/image.rs @@ -7,18 +7,17 @@ pub struct Image { impl Widget for Image { fn draw(&mut self, painter: &mut Painter) -> Size { - // Drawn at its own natural size, anchored top-left of whatever it - // was offered, not stretched to fill it -- its primitive is - // independent of the offered region, matching `is_size_independent` - // below. A caller that wants it placed differently wraps it (e.g. - // `.center()`, `.align(...)`). let size = self.handle.size(); painter.texture_within(&self.handle, size.align(Align::TOP_LEFT)); Size::abs(size) } + fn size_hint(&self, axis: Axis) -> Option { + Some(Len::abs(self.handle.size().axis(axis))) + } + fn is_size_independent(&self) -> bool { - true // a decoded image's primitive never depends on the region it is offered + true } } diff --git a/src/widget/mask.rs b/src/widget/mask.rs index 1373599..241921f 100644 --- a/src/widget/mask.rs +++ b/src/widget/mask.rs @@ -1,18 +1,6 @@ use crate::prelude::*; -/// Clips `inner` -- and everything below it -- to a shape. -/// -/// The shape is a **primitive**, never a rectangle or a radius stored -/// here: with `shape`, the widget named there is drawn behind `inner` -/// filling the same box and the clip is its first primitive, so a rounded -/// container's corner and the corner its content is cut to are the same -/// arithmetic and cannot fall out of step. Without one, this writes an -/// undrawn rect at its own region, which is the plain "clip to my box" -/// every list and scroll area wants. See docs/LAYOUT.md's "Masks with a -/// shape". pub struct Masked { - /// The widget whose first primitive is the clip, drawn behind - /// `inner`, or `None` for this widget's own box. pub shape: Option, pub inner: StrongWidget, } @@ -20,9 +8,6 @@ pub struct Masked { impl Widget for Masked { fn draw(&mut self, painter: &mut Painter) -> Size { match &self.shape { - // Layered the way `Stack` layers a background under its - // content, and for the same reason: within one layer the draw - // order is undefined once anything has been freed. Some(shape) => { painter.child_layer(); painter.widget(shape); @@ -31,6 +16,12 @@ impl Widget for Masked { } None => painter.set_mask(painter.region()), } - painter.widget(&self.inner) + let used = painter.widget(&self.inner); + painter.place_used(&self.inner, used, UiRegion::FULL); + used + } + + fn requires_exact_region(&self) -> bool { + true } } diff --git a/src/widget/position/align.rs b/src/widget/position/align.rs index feb3c06..69625ce 100644 --- a/src/widget/position/align.rs +++ b/src/widget/position/align.rs @@ -7,30 +7,18 @@ pub struct Aligned { impl Widget for Aligned { fn draw(&mut self, painter: &mut Painter) -> Size { - // Draw once at the whole region this widget was offered to learn - // the child's real size -- this placement is provisional and - // corrected below without a second draw. `painter.widget` (not - // `widget_within(..., painter.region())`) is what "my whole, - // already-resolved region, unmodified" means: `widget_within` - // composes its argument as a *local*, `UiRegion::FULL`-relative - // box against `painter.region()`, so handing it the - // already-resolved region double-applies that composition and is - // wrong for any widget nested below the root. let used = painter.widget(&self.inner); let density = painter.density(); - let region = match self.align.tuple() { - (Some(x), Some(y)) => used.to_uivec2(density).align(RegionAlign { x, y }), - (Some(x), None) => { - let x = used.x.apply_rest(density).align(x); - UiRegion::new(x, UiSpan::FULL) - } - (None, Some(y)) => { - let y = used.y.apply_rest(density).align(y); - UiRegion::new(UiSpan::FULL, y) - } - (None, None) => UiRegion::FULL, - }; - painter.reposition(&self.inner, region); // O(1): one offset write, no second draw + let (x, y) = self.align.tuple(); + let region = UiRegion::new( + used.x + .apply_rest(density) + .align(x.unwrap_or(AxisAlign::Neg)), + used.y + .apply_rest(density) + .align(y.unwrap_or(AxisAlign::Neg)), + ); + painter.place(&self.inner, region); used } } diff --git a/src/widget/position/layer.rs b/src/widget/position/layer.rs index 93f7616..e3d63c2 100644 --- a/src/widget/position/layer.rs +++ b/src/widget/position/layer.rs @@ -10,6 +10,8 @@ impl Widget for LayerOffset { for _ in 0..self.offset { painter.next_layer(); } - painter.widget(&self.inner) + let used = painter.widget(&self.inner); + painter.place_used(&self.inner, used, UiRegion::FULL); + used } } diff --git a/src/widget/position/lazy_span.rs b/src/widget/position/lazy_span.rs index 87f9f49..11b1a8b 100644 --- a/src/widget/position/lazy_span.rs +++ b/src/widget/position/lazy_span.rs @@ -1,169 +1,9 @@ -//! `LazySpan`: a virtualised span of variable-height rows, laid out from -//! an anchor rather than eagerly like `Span`. +//! A virtualised, variable-height span laid out from an anchor. //! -//! **`docs/SCROLL.md` is the overview** -- how this widget and `ScrollArea` -//! divide the work, the one sign convention, the `Widget` handoff, and -//! what is still open. Read it first; this file is the detail. -//! RUST.md's I3. Read LAYOUT.md first -- this widget is built entirely out -//! of primitives that design already provides (`Painter::widget`/ -//! `widget_within`/`reposition`, and `draw_inner`'s own old-children -//! diffing) rather than adding a second move mechanism. -//! -//! ## Design -//! -//! **It knows nothing about masks.** What it does is *cull*: a row that -//! falls entirely outside the region this widget was offered is never -//! drawn (`intersects_viewport`). A row that *straddles* an edge is drawn -//! in full, because virtualisation decides which rows are drawn and never -//! how much of one -- so the overhang past this list's box reaches the -//! screen unless something clips it, and clipping is `.masked()`, which -//! the caller adds when it wants one. Iris, 2026-09-08: **"Why does the -//! mask matter at all. If you want a mask then you add `.masked()`. It -//! should just prevent rows that aren't in its region at all from drawing -//! ... Just like the opt in scrollable, masking should be opt in."** -//! -//! Two shapes this file went through before that, both worse. It asserted -//! `Painter::is_masked` and refused to draw otherwise, which made an -//! ordinary full-screen list -- every benchmark, every simple app -- -//! panic for want of ceremony it did not need; and the case that actually -//! bites passed the check anyway, since a mask *larger* than the list's -//! box satisfies `is_masked` while still letting the overhang through. -//! Then it set a mask of its own, which is this widget deciding something -//! that is not its to decide: a caller that wants the overhang (or that -//! is already clipped by something bigger) has no way to say so, and the -//! transcript ended up double-masked. -//! -//! The fault a caller is opting *out* of, when it leaves `.masked()` off, -//! is the transcript panned to its top edge drawing code through the -//! header bar above it, on Iris's phone (docs/IRIS_TODO.md, 2026-09-07). -//! -//! **Rows are keyed by a `u64` (`RowKey`), not a generic type.** Every real -//! row source in this codebase (a transcript's monotonic sequence number, a -//! chat message id) is already an integer; a generic key would cost every -//! call site a type parameter for a capability nothing here needs yet -- -//! the simplest thing that works, per the code rules. -//! -//! **Composed only while visible, for free.** `LazySpan` does not maintain its -//! own "which widgets are alive" bookkeeping. Its `draw` calls -//! `painter.widget`/`widget_within` only for the rows currently in view; -//! `UiRenderState::draw_inner` already diffs a redrawn widget's new -//! `children` against its old ones and frees (`remove_rec`) whatever is no -//! longer called (LAYOUT.md section on caching, and the "old_children" -//! removal in `draw_inner`). A row that scrolls out is therefore dropped -//! and its primitives freed the very next time `LazySpan` redraws -- no new -//! mechanism, just relying on the one LAYOUT.md already built. -//! -//! **Rows draw once and are moved, not re-laid-out, on scroll.** A `LazySpan` -//! is laid out outward from one **anchor** row (`Anchor { slot, edge, -//! offset }`: a slot index, which of its edges is pinned, and that edge's -//! pixel offset from the viewport's leading edge) rather than from a -//! single scroll amount measured from the top of all content -- there is -//! no "top of all content" to measure without walking every row, which is -//! exactly the O(N) cost virtualisation exists to avoid. Rows below the -//! anchor are placed **top-known** (`Placement::Leading`): offered an exact -//! top and a generous, oversized bottom, drawn once with -//! `painter.widget_within`, and their real height read back from the -//! returned `Size`. Rows above the anchor are placed **bottom-known** -//! (`Placement::Trailing`): since every widget in this crate paints itself -//! anchored top-left of whatever it is offered (LAYOUT.md's deviation 2), -//! placing a row so its *bottom* lands at an exact pixel needs the same -//! "learn the size, then move" trick `Aligned` already uses -- -//! `painter.widget` at the full offered region to measure, then -//! `painter.reposition` (an O(1) offset write, no second draw) to the -//! exact box. On an ordinary scroll tick only `Anchor::offset` changes; -//! every already-visible row keeps the same *size* it was offered last -//! frame (top-known rows: same generous bottom bound; bottom-known rows: -//! the same full-region measurement, which `draw_inner`'s own -//! `active.region == region` check turns into a **no draw at all**, its -//! cached `Size` returned for free) so the per-row cost of a tick is one -//! `mov()`/`reposition()` write, never a redraw -- verified in this file's -//! `moves_stay_o1_across_list_size` test and in `benches/message_lazy_span.rs`. -//! -//! **The scroll anchor survives a row inserted above it.** The anchor -//! names a row by its *slot index*, not by an absolute content offset -//! measured from the top -- so `push_front` only has to shift the -//! anchor's slot by one (`+= 1`, an O(1) write) to keep it pointing at the -//! same logical row; nothing about where that row is drawn changes, and -//! rows outside the loaded window are never touched. This is the same -//! reason `push_back`/`pop_front`/`pop_back` are all O(1): the widget -//! never computes "total content height," only the local heights of the -//! rows it is actively placing. -//! -//! **"More" sentinels are two ordinary optional widgets, not a second -//! data model.** `more_before`/`more_after` are each `Option` -//! set by the caller (`WidgetPtr`'s own idiom); when present, the walk -//! outward from the anchor treats the sentinel as one more slot past the -//! real rows (`BEFORE_SLOT`/`AFTER_SLOT`, reserved `isize` values below -//! `0`/above any real index) rather than special-casing it, so a sentinel -//! costs nothing extra to place or to virtualise away. -//! -//! **"Hold the edge nearest the tap," done in the layout pass.** -//! `note_tap(viewport_pos)` records where the user last touched the list, -//! in viewport-relative pixels, without forcing a redraw by itself -- the -//! app is expected to call it and then mutate whatever row is expanding -//! (e.g. toggling a collapsed message), which is what actually marks that -//! row (and, by the existing resize-bubble in `UiRenderState::redraw`, -//! `LazySpan` itself) dirty. The *next* time `LazySpan::draw` runs, before placing -//! anything, it looks at `extents` (each visible row's on-screen box as of -//! the *previous* frame, cached while walking) to find which row contains -//! the tap, decides whether the tap was nearer that row's top or bottom -//! edge, and re-anchors to exactly that row/edge/pixel -- so the row this -//! frame draws at its *new* height with the chosen edge pinned to the same -//! screen position it already occupied, and only the far side visibly -//! grows or shrinks. This is a layout decision made before any primitive -//! is written for the frame, not a correction applied to an already-drawn -//! wrong frame. -//! -//! **A row's height is cached by key once measured**, and reused directly -//! (one `widget_within` at the exact box, no re-measurement) on every later -//! placement of that row -- not merely an optimisation: see `place`'s doc -//! for why a row that fills whatever it is offered (a `.background(rect -//! (...))`) needs this to ever be placed at the right size at all, and why -//! re-measuring every frame instead would defeat `draw_inner`'s own -//! skip-or-move caching. Only a row's first-ever appearance pays the -//! two-draw measurement; nothing here estimates a height for an off-screen -//! row that has never been measured, so this stays independent of how many -//! rows exist outside the loaded window. -//! -//! **Only what overlaps the viewport is drawn, and it is drawn whole.** -//! One rule, `intersects_viewport`, used by both halves of that sentence: -//! a row straddling either edge is drawn in full and clipped by the -//! `.masked()` its caller must place it in (`LazySpan::draw` asserts that), -//! and a row that has left the viewport is not drawn at all. The walk -//! still traverses whatever lies between the anchor and the viewport, and -//! `rehome_anchor` moves the anchor back onto a visible row every frame so -//! that "whatever lies between" stays empty however far the list is -//! panned. -//! -//! **This widget scrolls itself, and everything that is not its layout -//! lives in a [`ScrollController`] it owns** -- the position, the gesture, -//! the fling and the pin, the same struct a `ScrollArea` holds -//! (`docs/SCROLL.md`). It is not wrapped in one of those and must not be: -//! a scroll tick offers a moved region of the same size, `draw_inner` -//! takes the `mov` path, and a virtualising child inside it would never -//! update which rows it shows. `.scrollable()` here is the span's own -//! inherent one, registering the wheel and the drag against that -//! controller. -//! -//! What is left in this file is the layout: an anchor, a walk outward -//! from it, and an honest answer about how far it can go -//! ([`Self::travel`]). -//! -//! **Overscroll cannot be entered by scrolling, and is taken back within -//! the frame when something else causes it.** The controller clamps a -//! delta to the travel the last walk reported, so a delta that runs off a -//! wall already in view is simply cut short. What that cannot cover is a -//! wall this span has not walked to yet -- with rows loaded past an edge -//! there is no bound to report -- or the content or viewport changing -//! under a settled anchor, and for both -//! `overscroll_gap` measures the gap from the ends the walk already -//! placed and `draw` moves the anchor by it and walks a second time -//! **before the frame ends** -- layout is a pure function of the state, -//! not of how many frames have been drawn (Iris, 2026-09-08), the same -//! rule `ScrollArea::draw` follows. A span shorter than its viewport is not -//! overscrolled and is left alone, still pinned to the end it was built -//! with. - +//! It owns a `ScrollController`, draws only rows overlapping the viewport, +//! and leaves clipping to an optional `.masked()` wrapper. Cached row heights +//! let retained rows move directly; a new or resized row is drawn and then +//! placed at the exact extent it reports. See `docs/SCROLL.md`. use crate::prelude::*; use iris_core::util::HashMap; use std::collections::VecDeque; @@ -299,13 +139,7 @@ pub struct LazySpan { last_viewport_len: f32, pending_tap: Option, extents: HashMap, - /// Each row's height as of its last real draw, kept across frames so - /// an already-measured row is placed directly at its exact box next - /// time (one `widget_within`, no oversized measurement pass) -- - /// see `place`'s doc for why a fresh measurement can't be skipped - /// merely by translating an already-drawn primitive. Pruned when a - /// row is evicted (`pop_front`/`pop_back`) so this cannot grow past - /// however many rows are currently loaded. + /// Last reported height, pruned when its row is evicted. heights: HashMap, /// Whether the last walk found no more content before the leading /// edge *and* nothing left to give back there -- what @@ -1003,62 +837,8 @@ impl LazySpan { region } - /// Place one slot (a real row or a sentinel) per `placement`, caching - /// its resolved extent (for the next frame's `note_tap` resolution) and - /// height (for its own next placement, see below), and return its - /// resolved `(leading, trailing)` edges in viewport pixels. - /// - /// A row already measured on some earlier frame is placed directly at - /// its cached height's exact box -- one `widget_within`/`reposition` - /// pass, the same as any other widget placed by an already-known - /// region. A row seen for the first time has no cached height to place - /// it *at*, so it is measured first (an oversized, fixed-size region) - /// and then drawn a *second* time at the tight box that measurement - /// implies, via `Painter::measure` -- not `reposition` (a pure - /// translation, no resize). This distinction is required, not just an - /// optimisation: a row is not always plain wrapped text -- - /// `.background(rect(tint))` is an ordinary way to style one, and - /// `Rect::draw` is `is_size_independent` specifically because it fills - /// *whatever region it is given* (`Size::REST`, see `rect.rs`). - /// Measuring such a row at the oversized box has it paint an oversized - /// rect there; `reposition` only ever writes an offset, never a size, - /// so an every-frame reposition-only scheme would leave that primitive - /// oversized forever. Measuring on *every* frame would fix - /// that but break the opposite property: its two calls use two - /// different regions, so whichever one `ActiveData.region` ends up - /// holding always disagrees with the *next* frame's first call, - /// forcing a real redraw every single frame instead of the cheap - /// skip-or-move `draw_inner` already provides for an unchanged or - /// merely-translated widget. Caching the height once measured is what - /// lets an already-seen row go back to that cheap path while a - /// first-seen one still gets a correctly-sized initial paint. - /// - /// **A row whose measurement disagrees with the box it was offered is - /// drawn again, this frame, at the box its own height implies** -- - /// both placements, since both offer a cached height and both can be - /// wrong the frame a row's content changes size. This is not an - /// optimisation to skip: a row is routinely `.background(rect(..))` - /// (a tool card *is* one), and `Rect::draw` fills whatever region it - /// is handed, so a row offered last frame's height paints its - /// background at last frame's height while its text lays out at the - /// new one -- Iris's 2026-09-08 report that "collapsing and opening an - /// edit card draws the card background a frame late, so it looks - /// closed even when there's text". A `reposition` does not fix it - /// (it writes an offset, never a size), which is what the bottom- - /// anchored half used to do. The extra draw happens only on the frame - /// a row actually changes height, which is a frame that was already - /// redrawing that row. + /// Place a row and remember its height for later walks. fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) { - // Every current caller derives `slot` from `repair_anchor`/ - // `prev_slot`/`next_slot`, which already check existence -- but - // that invariant is enforced by convention across three call - // sites, not by this function, which would otherwise fail with a - // bare "index out of bounds" and no context (review, - // 2026-09-06). `slot_widget`, called from - // here, is what actually indexes/`.expect`s on it. Stays a - // `debug_assert!` under R1's rule: this runs once per row placed - // per frame, and its release failure is the `.expect` below rather - // than something silently wrong on screen. debug_assert!( self.slot_exists(slot), "place() called with a slot that doesn't exist: {slot:?}" @@ -1077,16 +857,6 @@ impl LazySpan { let key = self.slot_key(slot); let cached = key.and_then(|k| self.heights.get(&k).copied()); - // A row entirely outside the viewport is traversed but not drawn - // -- see `intersects_viewport`. The walk still has to *pass - // through* it, because its height is what says where the rows - // behind it land, but nothing about it reaches the screen, so - // drawing it costs a redraw (and, unclipped, paints over whatever - // is above the list) for content nobody can see. Only possible - // for a row whose height is already known: a first-time row has - // to be drawn to be measured at all, which is why the extent - // below is recorded from the intersection test rather than from - // "was this drawn". if let Some(h) = cached { let (lead, trail) = placement.edges(h); if !self.intersects_viewport(lead, trail) { @@ -1096,56 +866,31 @@ impl LazySpan { let widget = self.slot_widget(slot); let height = match cached { - // Offered a box sized to the *cached* height (cheap to compare - // against last frame's offer, see `place`'s doc), but the - // height kept is what this draw actually reported -- if the - // row's real content grew since it was cached (and was - // therefore redrawn: an unchanged widget never disagrees with - // its own cache), it is drawn again here, this frame, at the - // box its own height implies rather than waiting a frame to - // self-correct. Some(h) => { let (lead, trail) = placement.edges(h); let used = painter.widget_within(widget, Self::abs_region(dir, lead, trail)); let height = resolve(used); if height != h { let (lead, trail) = placement.edges(height); - painter.widget_within(widget, Self::abs_region(dir, lead, trail)); + painter.place(widget, Self::abs_region(dir, lead, trail)); } height } - // Never measured, so there is no height to place it at: it is - // measured at an oversized region first and drawn again at the - // box that measurement implies (`Painter::measure` then a - // real draw, not `reposition`, which writes an offset and - // never a size). - // - // A bottom-known row measures at a *zero-anchored* region - // rather than at its own box: using the real box would make - // the measurement's offered size track this list's own height, - // so a sibling growing taller (the input-box case) would look - // like a resize to every bottom-known row and force a full - // redraw of each -- despite a row's content depending only on - // width. 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.measure(widget, first)); + let height = resolve(painter.widget_within(widget, first)); let (lead, trail) = placement.edges(height); - painter.widget_within(widget, Self::abs_region(dir, lead, trail)); + painter.place(widget, Self::abs_region(dir, lead, trail)); height } }; let (lead, trail) = placement.edges(height); if let Some(k) = key { self.heights.insert(k, height); - // `extents` is what is *on screen* (`key_at`'s doc, and - // `rehome_anchor` below reads it as exactly that), so a - // first-time row that had to be drawn to be measured and - // turned out to be off-screen does not go in it. if self.intersects_viewport(lead, trail) { self.extents.insert(k, RowExtent { slot, lead, trail }); } @@ -1154,16 +899,7 @@ impl LazySpan { } } -/// The oversized bound offered along the primary axis when a row's real -/// extent isn't known yet (a fresh top-known placement) or is deliberately -/// discarded (a bottom-known measurement, see `place`). Large enough that -/// no real row's content is taller than this -- rows do not clip to the -/// height they're offered, only width drives a wrapped row's height -- and -/// a fixed module constant rather than derived from `viewport_len`, since -/// deriving it from a value that changes whenever the list itself resizes -/// (a sibling growing) would make the offered region's *size* change too, -/// defeating the same-size-different-position fast path `place` depends -/// on for an O(1) move. +/// Primary-axis room for a row whose extent is not cached yet. const GENEROUS_PADDING: f32 = 100_000.0; impl LazySpan { @@ -1321,6 +1057,10 @@ impl Widget for LazySpan { self.ctl.set_travel(self.travel()); Size::REST } + + fn size_hint(&self, _axis: Axis) -> Option { + Some(Len::REST) + } } #[cfg(test)] @@ -1719,7 +1459,7 @@ mod tests { fn a_fill_shaped_background_is_not_left_oversized() { // Regression test for a real bug found building the I3 example: // `place`'s Bottom-known branch used to measure a row at an - // oversized, fixed-size region and `reposition` (a pure + // oversized, fixed-size region and placement (a pure // translation) it into its final box. A row's own natural height // is independent of that oversized offer (true for wrapped text), // but a `Rect` background is *defined* to fill whatever it is @@ -1751,7 +1491,7 @@ mod tests { assert!( (height - 20.0).abs() < 0.5, "background rect should be exactly the row's height (20px), got {height}px \ - -- an oversized measurement region leaking through would show as ~100000px" + -- an oversized provisional region leaking through would show as ~100000px" ); } } @@ -2094,23 +1834,7 @@ mod tests { } } - /// RUST.md's P0 phone report (Iris's screenshot, 2026-09-06): a - /// replaced row's primitives drawn a second time, overlapping the - /// replacement. Reproduces the exact path `TranscriptScreen::apply`'s - /// `ReplaceLast` case drives up to 400 times during a streamed reply - /// (`bench_client.rs`'s stream phase): the last slot's widget is - /// swapped for a brand-new one, same key, and (since a fresh widget - /// has no cached height) placed via `place`'s measure-then-draw path every - /// time -- the provisional-then-real two-draw sequence LAYOUT.md - /// documents as the one place in this crate that deliberately draws a - /// widget twice. If `draw_inner`'s old-children diffing or - /// `UiRenderState::remove`'s primitive freeing ever failed to retire - /// the evicted widget (or the provisional draw's own primitives), it - /// would show up here as `active_widgets` growing without bound. - /// **Passes as written** -- this pins the widget-arena layer as - /// correct in isolation; see the P0 box for where the duplicate was - /// actually chased to instead (`Span`'s two-phase draw and the - /// `redraw_all`-vs-`redraw_updates` split, still open). + /// Replacing a streamed row must not grow the active widget set. #[test] fn replacing_the_last_row_many_times_does_not_leak_primitives() { let mut rsc = TestRsc { @@ -2129,9 +1853,6 @@ mod tests { let before = render.active_widgets(); for i in 0..400u32 { - // A varying height keeps every replace on the measure-then-draw - // (cache-miss) path rather than settling into the O(1) - // same-size `mov` fast path once the height happens to repeat. let (_bg_id, new_row) = background_styled_row(&mut rsc, 20.0 + (i % 3) as f32); rsc.ui .widgets @@ -2151,26 +1872,7 @@ mod tests { ); } - /// The doubled `Compacted:` row from Iris's phone (docs/bench/ - /// iris-phone-v2-2026-09-06.md), reproduced at its mechanism. - /// - /// `replacing_the_last_row_many_times_does_not_leak_primitives` above - /// counts *widgets*, which is why it passed all along: the orphan's - /// owner is very much alive -- it is an earlier set of that same - /// widget's primitives that got stranded. What strands them is a row - /// marked dirty and then reached by its **ancestor's** redraw rather - /// than by its own: `draw_inner` only *read* the dirty mark, so the - /// whole branch that frees a redrawn widget's previous primitives was - /// skipped, and the fresh `ActiveData` overwrote the only handles that - /// could ever have freed them. `LazySpan` sets no mask, so that copy then - /// draws every frame at whatever region it last had -- including, - /// where the row was being measured at `GENEROUS_PADDING`, well below - /// the list's own box and under the composer. - /// - /// Two rows, two shapes of the same fault: row 2 has a cached height - /// (one `widget_within`), row 4 is replaced so it has none (`place`'s - /// measure-then-draw, which reaches `draw_inner` twice for one id in one - /// frame and so orphans a copy even with no ancestor involved). + /// An ancestor redraw must retire every replaced primitive. #[test] fn an_ancestor_redrawing_a_dirty_row_leaves_no_stale_copy() { let mut rsc = TestRsc { diff --git a/src/widget/position/max_size.rs b/src/widget/position/max_size.rs index 22c5766..f1df2f2 100644 --- a/src/widget/position/max_size.rs +++ b/src/widget/position/max_size.rs @@ -7,17 +7,12 @@ pub struct MaxSize { } impl MaxSize { - /// Caps a reported length at `max`, comparing in pixels since `Len`'s - /// rel/abs/rest components are not otherwise comparable. fn clamp(len: Len, max: Option, output: f32, density: f32) -> Len { let Some(max) = max else { return len; }; let len_px = len.apply_rest(density).to_abs(output); let max_px = max.apply_rest(density).to_abs(output); - // `fold_dp`, not the caller's `max` as written: a reported `Len` - // may not carry an unresolved `dp` -- see `Len::fold_dp` for the - // collapsed composer bar this caused. if len_px > max_px { max.fold_dp(density) } else { @@ -25,12 +20,6 @@ impl MaxSize { } } - /// The span (in this widget's own local, `UiRegion::FULL`-relative - /// terms) to actually offer the child: unconstrained if it already fits - /// within `max`, or a box of exactly `max`, anchored at this axis's - /// start, if it does not. Needed so the child is never painted bigger - /// than the size this widget reports for it -- see the identical - /// requirement noted on `Sized::draw`. fn clamp_region(offered_px: f32, max: Option, output: f32, density: f32) -> UiSpan { let Some(max) = max else { return UiSpan::FULL; @@ -55,9 +44,11 @@ impl Widget for MaxSize { y: Self::clamp_region(offered.y, self.y, output.y, density), }; let used = painter.widget_within(&self.inner, region); - Size { + let size = Size { x: Self::clamp(used.x, self.x, output.x, density), y: Self::clamp(used.y, self.y, output.y, density), - } + }; + painter.place_used(&self.inner, size, UiRegion::FULL); + size } } diff --git a/src/widget/position/offset.rs b/src/widget/position/offset.rs index c1df8d1..6e746fa 100644 --- a/src/widget/position/offset.rs +++ b/src/widget/position/offset.rs @@ -8,6 +8,8 @@ pub struct Offset { impl Widget for Offset { fn draw(&mut self, painter: &mut Painter) -> Size { let region = UiRegion::FULL.offset(self.amt); - painter.widget_within(&self.inner, region) + let used = painter.widget_within(&self.inner, region); + painter.place_used(&self.inner, used, region); + used } } diff --git a/src/widget/position/pad.rs b/src/widget/position/pad.rs index 8e2b3ec..30161c8 100644 --- a/src/widget/position/pad.rs +++ b/src/widget/position/pad.rs @@ -3,29 +3,37 @@ use crate::prelude::*; pub struct Pad { pub padding: Padding, pub inner: StrongWidget, + /// Cleared after the reported size fits the offered region. + pub exact_region: bool, } impl Widget for Pad { fn draw(&mut self, painter: &mut Painter) -> Size { let density = painter.density(); - let used = painter.widget_within(&self.inner, self.padding.region(density)); + let offered = painter.px_size(); + let region = self.padding.region(density); + let used = painter.widget_within(&self.inner, region); + painter.place_used(&self.inner, used, region); let width = self.padding.left.apply_rest(density).abs + self.padding.right.apply_rest(density).abs; let height = self.padding.top.apply_rest(density).abs + self.padding.bottom.apply_rest(density).abs; - Size { + let size = Size { x: used.x + Len::abs(width), y: used.y + Len::abs(height), + }; + let needed = size.to_uivec2(density).to_abs(painter.output_size()); + if needed.x <= offered.x + 0.01 && needed.y <= offered.y + 0.01 { + self.exact_region = false; } + size + } + + fn requires_exact_region(&self) -> bool { + self.exact_region } } -/// Each side is a `Len`, not a bare `f32`, so `.pad(dp(10))` resolves -/// against the display's density the same way any other size does -- see -/// `Len::dp`'s field doc. `.pad(10)` (a bare number) still works via -/// `From` below, unchanged: it becomes an `abs` (physical-pixel) -/// `Len`, exactly as a bare number always has meant elsewhere in this -/// crate. pub struct Padding { pub left: Len, pub right: Len, diff --git a/src/widget/position/scroll_area.rs b/src/widget/position/scroll_area.rs index ff2bdbd..e3cf6c1 100644 --- a/src/widget/position/scroll_area.rs +++ b/src/widget/position/scroll_area.rs @@ -7,34 +7,11 @@ use crate::prelude::*; use std::time::Instant; -/// A scrolling view over a child that is a fixed lump: it is measured -/// whole and then moved, which is what makes a scroll tick an O(1) move of -/// one subtree rather than a redraw. -/// -/// **"Area" because it only scrolls a predefined one** (Iris, 2026-09-08): -/// a child that lays out lazily cannot be measured whole or moved as a -/// lump, and virtualising it inside one of these would never update which -/// rows it shows, since a scroll tick offers a same-size moved region and -/// `draw_inner` never re-enters the child. That case is `LazySpan`, which -/// owns a controller of its own instead of being wrapped in one of these. +/// A scrolling view that moves one fixed child as a subtree. pub struct ScrollArea { inner: StrongWidget, - /// The position, the gesture, the fling and the pin -- everything - /// about scrolling that is not this widget's own layout, shared with - /// `LazySpan` rather than reimplemented beside it. ctl: ScrollController, container_len: f32, - /// How long the content is along the axis, as of the last draw -- - /// `None` until this widget has drawn once. - /// - /// An `Option` rather than a `0.0` that stands in for both, because - /// the two answers led somewhere different and the code could not tell - /// them apart: on the first frame the clamp computed a scroll range of - /// zero, concluded from `amt == range` that the area was sitting at - /// its end, and pinned it -- so the next frame, now knowing the real - /// length, jumped to it. A code fence therefore opened at the end of - /// its longest line, mid-word (`iris/run-headless.sh phone`, - /// 2026-09-08). content_len: Option, } @@ -49,73 +26,23 @@ impl Scrollable for ScrollArea { } impl Widget for ScrollArea { - /// A scroll area animates exactly one thing, its fling. The - /// registration that makes this run is `UiData::animate`, which - /// `WidgetLike::scrollable`'s own drag handler calls the frame a - /// release starts one. fn tick(&mut self, now: Instant) -> bool { self.tick_fling(now) } - /// Measure, then place -- the same idiom `LazySpan` uses, for the same - /// reason: nothing drawn may depend on a length measured last frame. - /// - /// **The child is drawn twice, and only the second decides anything.** - /// The first is handed last frame's length as a *hint*, and it exists - /// only so that the usual case, where the content's length did not - /// change, offers the same region twice: `draw_inner` then makes the - /// first call an O(1) `mov` and returns at the first line of the - /// second. A frame on which the content did grow or shrink pays one - /// real extra draw, and that is a frame on which the content was being - /// redrawn anyway. - /// - /// The alternative -- place against the hint and let the next frame - /// fix it -- is what Iris found on her phone (2026-09-08): every - /// newline typed into the composer drew the field in a box one line - /// short of its text, and since that text is centred in its box it - /// hung half a line past each end. There was no next frame: nothing - /// dirtied that subtree again, so the stale placement was the last one - /// drawn, until the keyboard closed and its inset rewrite forced a - /// redraw ("it fixes itself"). **Layout is a pure function of the - /// state, not of how many frames have been drawn** (Iris, 2026-09-08) - /// -- a correction that needs a second frame is a frame drawn wrong. fn draw(&mut self, painter: &mut Painter) -> Size { - // Every length here is resolved against the box this widget was - // **offered** (`px_size`), never `output_size`: a scroll area is - // routinely smaller than the window -- the composer's field is - // capped at six lines by a `MaxSize` around it -- and measuring - // the window instead would make the pan range, and so where the - // content sits, a function of the screen rather than of the box. let axis = self.ctl.axis(); let container_len = painter.px_size().axis(axis); self.container_len = container_len; - // Learned from the frame rather than passed in: a fling's - // deceleration is a physical quantity and needs the real display - // density, and `draw` is where this widget meets the only thing - // that knows it. self.ctl.set_density(painter.density()); - // Where the delta asked for since the last frame puts the content. - // Already inside the range the previous frame published, so it is - // the position to *measure* against; the clamp below is what the - // length just measured has to say about it. let delta = self.ctl.take_delta(); let travelled = self.ctl.amt() - delta; self.ctl.set_amt(travelled); - // The container's own length stands in as the hint until anything - // 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); - // 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)); + let used = painter.widget_within(&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 - // container, again, is what that resolves against. let measured = used .axis(axis) .apply_rest(painter.density()) @@ -123,18 +50,6 @@ impl Widget for ScrollArea { self.content_len = Some(measured); let range = (measured - container_len).max(0.0); - // The end-pin, and then the clamp, against the length just - // measured. Deliberately not also run before the measuring draw - // above -- clamping against the hint would let a stale length - // reduce `amt` in a way this pass cannot undo, and then where the - // content sits would depend on the previous frame after all. - // - // Only a frame with no delta of its own re-pins: the pin means - // "stay flush with the end as the content grows", and a reader who - // just scrolled away from that end has said otherwise. (A delta - // cannot be moving *toward* the end here -- the travel published - // below is zero that way while pinned, so `take_delta` has already - // clipped it.) let amt = if self.ctl.pinned_to_end() && delta == 0.0 { range } else { @@ -147,15 +62,7 @@ impl Widget for ScrollArea { fwd: range - amt, }); - // The **content's** size, not the container's. A parent that can - // grow (the composer's bar) should hug the text until its own cap - // stops it, and reporting the container instead would make this - // widget's answer a function of the answer -- the bar is sized - // from what is reported here, so it collapses to nothing and never - // recovers. What keeps the content inside the offered box is the - // mask a caller puts around it (`.scrollable(..).masked()`), not - // this number. - painter.widget_within(&self.inner, self.child_region(measured)) + painter.place(&self.inner, self.child_region(measured)) } } @@ -176,12 +83,7 @@ impl ScrollArea { } } - /// Where the child sits for a given content length: a box that long - /// along the scroll axis, pulled back by `amt`. The length is taken as - /// a parameter rather than read from `content_len`, because `draw` - /// places twice -- once against last frame's length and once against - /// the one it has just measured -- and the two must be the same - /// arithmetic. + /// A content-sized box offset by the current scroll amount. fn child_region(&self, content_len: f32) -> UiRegion { let axis = self.ctl.axis(); let mut region = UiRegion::FULL; diff --git a/src/widget/position/sized.rs b/src/widget/position/sized.rs index ff7897d..d6d17ed 100644 --- a/src/widget/position/sized.rs +++ b/src/widget/position/sized.rs @@ -8,15 +8,6 @@ pub struct Sized { impl Widget for Sized { fn draw(&mut self, painter: &mut Painter) -> Size { - // The child is drawn within a region that actually carves out the - // fixed axes, not whatever region this widget itself happened to - // be offered -- needed so the painted geometry matches the - // declared size returned below regardless of how much room a - // parent offers. `Aligned`'s single-draw pattern (LAYOUT.md - // section 6) draws its child once at its own *full* region to - // learn its size, then moves it into place with a pure - // translation; that translation is only valid if what got painted - // is already the reported size, anchored the same way both times. let density = painter.density(); let mut region = UiRegion::FULL; if let Some(x) = self.x { @@ -26,12 +17,18 @@ impl Widget for Sized { region.y = y.apply_rest(density).align(AxisAlign::Neg); } let used = painter.widget_within(&self.inner, region); - // `fold_dp` on the way out: a declared size is a `Len` the caller - // wrote (`.width(dp(48))`), and a *reported* one may not carry an - // unresolved `dp` -- see `Len::fold_dp`. - Size { + let size = Size { x: self.x.map(|x| x.fold_dp(density)).unwrap_or(used.x), y: self.y.map(|y| y.fold_dp(density)).unwrap_or(used.y), + }; + painter.place_used(&self.inner, size, UiRegion::FULL); + size + } + + fn size_hint(&self, axis: Axis) -> Option { + match axis { + Axis::X => self.x, + Axis::Y => self.y, } } } diff --git a/src/widget/position/span.rs b/src/widget/position/span.rs index a896b7a..2040186 100644 --- a/src/widget/position/span.rs +++ b/src/widget/position/span.rs @@ -4,11 +4,6 @@ use std::marker::PhantomData; pub struct Span { pub children: Vec, pub dir: Dir, - /// A `Len` (not a bare `f32`) so `dp(4)` resolves against the display's - /// density the same way any other size in the tree does -- see - /// `Len::dp`'s field doc. Only the `abs` component (folded from `dp` at - /// draw time, `Widget::draw` below) is meaningful here; `rel`/`rest` - /// were never supported for a gap and still are not. pub gap: Len, } @@ -17,38 +12,43 @@ impl Widget for Span { let axis = self.dir.axis; let gap = self.gap.apply_rest(painter.density()).abs; - // 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 + let mut lens: Vec> = self .children .iter() - .map(|child| painter.widget(child).axis(axis)) + .map(|child| painter.known_len(child, axis)) .collect(); + let mut drawn = vec![false; self.children.len()]; + + let mut cursor = UiScalar::rel_min(); + for (i, child) in self.children.iter().enumerate() { + let len = match lens[i] { + Some(len) => len, + None => { + let mut slot = UiSpan::new(cursor, UiScalar::rel_max()); + if self.dir.sign == Sign::Neg { + slot.flip(); + } + let region = UiRegion::from_axis(axis, slot, UiSpan::FULL); + let len = painter.widget_within(child, region).axis(axis); + lens[i] = Some(len); + drawn[i] = true; + len + } + }; + cursor.abs += len.abs + gap; + cursor.rel += len.rel; + } + + let lens: Vec = lens.into_iter().map(Option::unwrap).collect(); 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: 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; let mut ortho_mixed = false; - for (child, &len) in self.children.iter().zip(&lens) { + let mut placed = Vec::with_capacity(self.children.len()); + for (i, (child, &len)) in self.children.iter().zip(&lens).enumerate() { let mut span = UiSpan::FULL; span.start = start; if len.rest > 0.0 { @@ -64,7 +64,12 @@ impl Widget for Span { if self.dir.sign == Sign::Neg { child_region.flip(axis); } - let used = painter.widget_within(child, child_region); + let used = if drawn[i] { + painter.place(child, child_region) + } else { + painter.widget_within(child, child_region) + }; + placed.push(child_region); start.abs += gap; let ortho = used.axis(!axis); @@ -76,6 +81,14 @@ impl Widget for Span { } if ortho_mixed { ortho_len = Len::default(); + } else { + let ortho = ortho_len + .apply_rest(painter.density()) + .align(AxisAlign::Neg); + for (child, mut region) in self.children.iter().zip(placed) { + *region.axis_mut(!axis) = ortho; + painter.place(child, region); + } } let along = if total.rest == 0.0 && total.rel == 0.0 { diff --git a/src/widget/position/stack.rs b/src/widget/position/stack.rs index d927491..6c4fd40 100644 --- a/src/widget/position/stack.rs +++ b/src/widget/position/stack.rs @@ -9,25 +9,54 @@ pub struct Stack { impl Widget for Stack { fn draw(&mut self, painter: &mut Painter) -> Size { - let mut picked = None; - let mut iter = self.children.iter().enumerate(); - if let Some((i, child)) = iter.next() { + let density = painter.density(); + let known = match self.size { + StackSize::Default => Some(Size::REST), + StackSize::Child(i) => self.children.get(i).and_then(|child| { + Some(Size { + x: painter.known_len(child, Axis::X)?, + y: painter.known_len(child, Axis::Y)?, + }) + }), + }; + let region = known.map(|size| size.to_uivec2(density).align(RegionAlign::TOP_LEFT)); + let mut used = Vec::with_capacity(self.children.len()); + let mut iter = self.children.iter(); + if let Some(child) = iter.next() { painter.child_layer(); - let used = painter.widget(child); - if matches!(self.size, StackSize::Child(j) if j == i) { - picked = Some(used); - } + used.push(match region { + Some(region) => painter.widget_within(child, region), + None => painter.widget(child), + }); } - for (i, child) in iter { + for child in iter { painter.next_layer(); - let used = painter.widget(child); - if matches!(self.size, StackSize::Child(j) if j == i) { - picked = Some(used); + used.push(match region { + Some(region) => painter.widget_within(child, region), + None => painter.widget(child), + }); + } + let size = match self.size { + StackSize::Default => Size::default(), + StackSize::Child(i) => used.get(i).copied().unwrap_or_default(), + }; + if known.is_none() { + let final_region = size.to_uivec2(density).align(RegionAlign::TOP_LEFT); + for (child, child_size) in self.children.iter().zip(used) { + let child_region = child_size + .to_uivec2(density) + .align(RegionAlign::TOP_LEFT) + .within(&final_region); + painter.place(child, child_region); } } + size + } + + fn size_hint(&self, _axis: Axis) -> Option { match self.size { - StackSize::Default => Size::default(), - StackSize::Child(_) => picked.unwrap_or_default(), + StackSize::Default => Some(Len::REST), + StackSize::Child(_) => None, } } } diff --git a/src/widget/ptr.rs b/src/widget/ptr.rs index 5f1531f..c016c2c 100644 --- a/src/widget/ptr.rs +++ b/src/widget/ptr.rs @@ -8,7 +8,9 @@ pub struct WidgetPtr { impl Widget for WidgetPtr { fn draw(&mut self, painter: &mut Painter) -> Size { if let Some(id) = &self.inner { - painter.widget(id) + let used = painter.widget(id); + painter.place_used(id, used, UiRegion::FULL); + used } else { Size::ZERO } diff --git a/src/widget/rect.rs b/src/widget/rect.rs index 28ea415..68fe5fe 100644 --- a/src/widget/rect.rs +++ b/src/widget/rect.rs @@ -3,12 +3,6 @@ use crate::prelude::*; #[derive(Clone, Copy)] pub struct Rect { pub color: UiColor, - /// A `Len` rather than a raw `f32` so a corner can be written in `dp` - /// and come out the same physical size on every display -- resolved - /// against `Painter::density` in [`Rect::draw`], the same place every - /// other `dp` is resolved. A plain number still works and still means - /// physical pixels (`impl From for Len`), which is what - /// a hairline wants. pub radius: Len, pub thickness: f32, pub inner_radius: f32, @@ -37,40 +31,15 @@ impl Widget for Rect { fn draw(&mut self, painter: &mut Painter) -> Size { painter.primitive(RectPrimitive { color: self.color, - // `rel` has no meaning for a corner (a rect that fills its - // parent has no length of its own to take a fraction of), so - // only the `abs`/`dp` halves are folded. radius: self.radius.fold_dp(painter.density()).abs, thickness: self.thickness, inner_radius: self.inner_radius, }); - Size::REST // fills whatever it was given -- used == available + Size::REST } - /// **No** -- despite drawing one primitive and nothing else. - /// - /// `is_size_independent` asks whether the widget's *content* is - /// unaffected by how big a region it was given, so that - /// `draw_inner` may keep the primitives it already has and rewrite - /// their regions in place. A `Rect`'s content **is** its region: it - /// returns `Size::REST` and fills whatever it was handed, so the fast - /// path's `r.outside(&from).within(®ion)` remap has to reproduce - /// the whole of `draw` -- and it does not, because a region carries - /// `rel` and `abs` components that the round trip cannot recover - /// separately. - /// - /// What that looked like: a fenced code block's background - /// (`transcript-ui`'s `BlockFrame::Verbatim`, a `Rect` behind a - /// `Pad` in a `Stack`) kept the height of the *provisional* full- - /// region draw `Span` does in its first phase, so one fence's panel - /// covered every block below it -- and every row below that -- while - /// the text itself was laid out correctly. Visible in - /// `docs/bench/p1a-2026-09-06/`'s history and reproduced by this - /// crate's `transcript` example. Answering `false` costs a redraw of - /// one primitive when a rect is resized, which is what the fast path - /// was saving. - fn is_size_independent(&self) -> bool { - false + fn size_hint(&self, _axis: Axis) -> Option { + Some(Len::REST) } } diff --git a/src/widget/text/edit.rs b/src/widget/text/edit.rs index 2e30746..12eb8cf 100644 --- a/src/widget/text/edit.rs +++ b/src/widget/text/edit.rs @@ -125,6 +125,10 @@ impl Widget for TextEdit { used } + fn requires_exact_region(&self) -> bool { + true + } + /// I4 (RUST.md): the one override that exists so far -- everything /// else falls back to `Widget::access_role`'s default `Unknown`. fn access_role(&self) -> accesskit::Role { diff --git a/src/widget/text/mod.rs b/src/widget/text/mod.rs index 03b3068..dc44f12 100644 --- a/src/widget/text/mod.rs +++ b/src/widget/text/mod.rs @@ -145,6 +145,10 @@ impl Widget for Text { self.update_buf(); self.view.draw(painter) } + + fn requires_exact_region(&self) -> bool { + true + } } impl Deref for Text { diff --git a/src/widget/trait_fns.rs b/src/widget/trait_fns.rs index 13283ef..9bc30fa 100644 --- a/src/widget/trait_fns.rs +++ b/src/widget/trait_fns.rs @@ -9,6 +9,7 @@ widget_trait! { |state| Pad { padding: padding.into(), inner: self.add_strong(state), + exact_region: false, } }