Answer "have I asked this child?" in one read

A container's draw asked it once per child by searching the list of children
it had added so far, and four other per-child steps searched a list too, so
one draw cost the square of its children: 70% of a 1,600-child redraw was
those searches. A draw takes a DrawId and leaves it on every widget it asks
about; one note per widget is enough because the handle a container holds a
child by cannot be cloned. tests/children_cost.rs is the rig that shows it,
and it is the only one here that varies width: 3.680 ms to 0.811 ms at 1,600
children, and flat per child.

Beside it, the rest of the fourteenth sweep of #19: a mask's rectangle
resolved once per fragment instead of once per instance, which takes the
storage buffers out of the fragment stage and is 8.8x on a screenful of
deeply nested clips; TextBuffer::shape copying its attrs before the check
that would not need them, which allocated once per named-family text per
frame; a should_panic test on a debug assertion that made cargo test
--release fail; Fixed::div, reached only by its own test; Moves::remove
re-uploading an array it cannot have changed; and two comments the branch
itself falsified.

docs/LAYOUT_LOG.md has all eight with their measurements, the five things
looked at and left, and what was verified.

core/src/fixed.rs

@@ -1,7 +1,7 @@
 use crate::{UiNum, util::Vec2};
 use std::{
     fmt::{Debug, Display, Formatter},
-    ops::{Add, AddAssign, Div, Mul, Neg, Sub, SubAssign},
+    ops::{Add, AddAssign, Mul, Neg, Sub, SubAssign},
 };
 
 /// A number held as a whole count of `1 / 2^SHIFT`.
@@ -175,21 +175,6 @@ impl<const SHIFT: u32> Fixed<SHIFT> {
         Self(div_round(self.0 as i64, by as i64) as i32)
     }
 
-    /// Divided by a number on any grid. A zero divisor is a caller bug -- a
-    /// box of no length has no fraction of itself -- and answers with the end
-    /// of the range so that a release build lays out something absurd rather
-    /// than dying.
-    pub const fn div<const BY: u32>(self, by: Fixed<BY>) -> Self {
-        debug_assert!(by.0 != 0, "dividing by a length of zero");
-        if by.0 == 0 {
-            return match self.0 < 0 {
-                true => Self::MIN,
-                false => Self::MAX,
-            };
-        }
-        Self(div_round((self.0 as i64) << BY, by.0 as i64) as i32)
-    }
-
     /// `num / den` on *this* grid rather than on theirs, for weights coarser
     /// than the share they divide.
     pub const fn ratio<const OF: u32>(num: Fixed<OF>, den: Fixed<OF>) -> Self {
@@ -320,14 +305,6 @@ const impl<const SHIFT: u32, const BY: u32> Mul<Fixed<BY>> for Fixed<SHIFT> {
     }
 }
 
-const impl<const SHIFT: u32, const BY: u32> Div<Fixed<BY>> for Fixed<SHIFT> {
-    type Output = Self;
-
-    fn div(self, rhs: Fixed<BY>) -> Self {
-        Fixed::div(self, rhs)
-    }
-}
-
 impl<const SHIFT: u32> Display for Fixed<SHIFT> {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         Display::fmt(&self.to_f32(), f)
@@ -478,19 +455,6 @@ mod tests {
         assert_eq!(Px::ONE.neg() * step_and_a_half, Px::from_raw(-2));
     }
 
-    /// A division rounds to the nearest step, so it cannot put back the
-    /// steps a truncating multiply dropped: a round trip comes back short,
-    /// never long, and by the few steps the two operations gave up.
-    #[test]
-    fn dividing_by_a_fraction_cannot_undo_a_truncating_multiply() {
-        let third = Rel::ONE / Rel::from_int(3);
-        let len = Px::from_int(300);
-        let back = len * third / third;
-        assert!(back <= len, "{back:?} is longer than {len:?}");
-        assert!(len - back <= Px::from_raw(3), "{back:?} against {len:?}");
-        assert_eq!(Px::from_int(100) / Rel::from_f32(0.5), Px::from_int(200));
-    }
-
     /// The bound a greedy line break needs: the width it was measured at is
     /// not on the grid, and the narrowest box the break still holds for is
     /// the step at or above it, never the one below.

core/src/primitive/text.rs

@@ -194,41 +194,25 @@ impl TextBuffer {
     }
 
     pub fn shape(&mut self, data: &mut TextData, attrs: &TextAttrs, width: Option<f32>) {
-        let layout_key = LayoutKey {
-            attrs: attrs.clone(),
-            max_width: width,
-        };
-        if self.layout_key.as_ref() == Some(&layout_key) {
-            #[cfg(feature = "layout-diagnostics")]
-            diag::bump(Counter::TextShapeHits);
-            return;
-        }
-        // A greedy break at one width is the same break at every width down
-        // to the longest line it produced: each line still fits, and none can
-        // take a word that would not fit in the wider box. So the layout in
-        // hand already answers, and re-breaking would only be work.
-        //
-        // At the longest line exactly, with no margin below it. A narrower
-        // width really does break differently, so answering one from the
-        // break in hand is how a warm tree keeps lines a cold tree would
-        // never produce. The margin was here because a text reports the
-        // width it used and a parent hands that back; the report is the step
-        // at or above its longest line now, so what comes back fits.
-        if let Some(key) = &self.layout_key
-            && key.attrs == *attrs
-            && let (Some(broke_at), Some(want)) = (key.max_width, width)
-            && want <= broke_at
-            && want >= self.layout.width()
+        // Asked of the attrs it was given rather than of a copy: copying one
+        // allocates wherever its family is named, and the hit below is what
+        // this cache is for.
+        let same_shaping = self
+            .layout_key
+            .as_ref()
+            .is_some_and(|key| key.attrs == *attrs);
+        if same_shaping
+            && let Some(key) = &self.layout_key
+            && self.breaks_the_same(key.max_width, width)
         {
             #[cfg(feature = "layout-diagnostics")]
             diag::bump(Counter::TextShapeHits);
             return;
         }
-        let same_shaping = self
-            .layout_key
-            .as_ref()
-            .is_some_and(|key| key.attrs == *attrs);
-        let old_key = self.layout_key.replace(layout_key);
+        let old_key = self.layout_key.replace(LayoutKey {
+            attrs: attrs.clone(),
+            max_width: width,
+        });
         // The glyphs it holds are of the width it held, which the layout may
         // well come back to.
         if let Some(key) = old_key
@@ -268,6 +252,28 @@ impl TextBuffer {
         self.break_lines(width);
     }
 
+    /// Whether the break in hand is the break `want` would make. The attrs
+    /// are the caller's to compare; this is about the width alone.
+    ///
+    /// A greedy break at one width is the same break at every width down to
+    /// the longest line it produced: each line still fits, and none can take a
+    /// word that would not fit in the wider box. So the layout in hand already
+    /// answers, and re-breaking would only be work.
+    ///
+    /// At the longest line exactly, with no margin below it. A narrower width
+    /// really does break differently, so answering one from the break in hand
+    /// is how a warm tree keeps lines a cold tree would never produce. The
+    /// margin was here because a text reports the width it used and a parent
+    /// hands that back; the report is the step at or above its longest line
+    /// now, so what comes back fits.
+    fn breaks_the_same(&self, broke_at: Option<f32>, want: Option<f32>) -> bool {
+        match (broke_at, want) {
+            (broke_at, want) if broke_at == want => true,
+            (Some(broke_at), Some(want)) => want <= broke_at && want >= self.layout.width(),
+            _ => false,
+        }
+    }
+
     fn break_lines(&mut self, width: Option<f32>) {
         self.layout.break_all_lines(width);
         self.layout

core/src/render/mod.rs

@@ -285,7 +285,9 @@ impl UiRenderNode {
     }
 
     /// What every draw in the ui is given: the window, the masks and the
-    /// move chain every position is resolved through.
+    /// move chain every position is resolved through. The last two are the
+    /// vertex stage's alone -- a mask's rectangle is the same for every
+    /// fragment of one instance, so it is resolved once and handed on.
     fn shared_layout(device: &Device) -> BindGroupLayout {
         device.create_bind_group_layout(&BindGroupLayoutDescriptor {
             entries: &[
@@ -301,7 +303,7 @@ impl UiRenderNode {
                 },
                 BindGroupLayoutEntry {
                     binding: 1,
-                    visibility: ShaderStages::FRAGMENT,
+                    visibility: ShaderStages::VERTEX,
                     ty: BindingType::Buffer {
                         ty: BufferBindingType::Storage { read_only: true },
                         has_dynamic_offset: false,
@@ -311,7 +313,7 @@ impl UiRenderNode {
                 },
                 BindGroupLayoutEntry {
                     binding: 2,
-                    visibility: ShaderStages::VERTEX | ShaderStages::FRAGMENT,
+                    visibility: ShaderStages::VERTEX,
                     ty: BindingType::Buffer {
                         ty: BufferBindingType::Storage { read_only: true },
                         has_dynamic_offset: false,

core/src/render/shader/prelude.wgsl

@@ -126,9 +126,25 @@ struct VertexOutput {
     @location(2) uv: vec2<f32>,
     @location(3) @interpolate(flat) mask_idx: u32,
     @location(4) @interpolate(flat) idx: u32,
+    // The mask's rectangle in output pixels, resolved here because it is the
+    // same rectangle for every fragment of one instance and resolving it is a
+    // walk up a chain. Its own chain, not the drawn primitive's, so a
+    // stationary viewport clips content that moves inside it.
+    @location(5) @interpolate(flat) mask_top_left: vec2<f32>,
+    @location(6) @interpolate(flat) mask_bot_right: vec2<f32>,
     @builtin(position) clip_position: vec4<f32>,
 };
 
+// The pixel corners of a region, which is where every coordinate the CPU
+// decided becomes one.
+fn corners(r: Region) -> mat2x2<f32> {
+    let top_left = snap_floor(vec2(r.x.start.rel, r.y.start.rel) * window.dim
+        + vec2(r.x.start.px, r.y.start.px));
+    let bot_right = snap_floor(vec2(r.x.end.rel, r.y.end.rel) * window.dim
+        + vec2(r.x.end.px, r.y.end.px));
+    return mat2x2<f32>(top_left, bot_right);
+}
+
 @vertex
 fn vs_main(
     @builtin(vertex_index) vi: u32,
@@ -141,16 +157,17 @@ fn vs_main(
         UiSpan(scalar_of_pair(in.x_start), scalar_of_pair(in.x_end)),
         UiSpan(scalar_of_pair(in.y_start), scalar_of_pair(in.y_end)),
     );
-    let r = resolve_move(in.move_idx, local);
-    let top_left_rel = vec2(r.x.start.rel, r.y.start.rel);
-    let top_left_px = vec2(r.x.start.px, r.y.start.px);
-    let bot_right_rel = vec2(r.x.end.rel, r.y.end.rel);
-    let bot_right_px = vec2(r.x.end.px, r.y.end.px);
-
-    let top_left = snap_floor(top_left_rel * window.dim + top_left_px);
-    let bot_right = snap_floor(bot_right_rel * window.dim + bot_right_px);
+    let own = corners(resolve_move(in.move_idx, local));
+    let top_left = own[0];
+    let bot_right = own[1];
     let size = bot_right - top_left;
 
+    var mask = mat2x2<f32>(vec2<f32>(0.0), vec2<f32>(0.0));
+    if in.mask_idx != MASK_NONE {
+        let m = masks[in.mask_idx];
+        mask = corners(resolve_move(m.move_idx, Region(span_of(m.x), span_of(m.y))));
+    }
+
     let uv = vec2<f32>(
         f32(vi % 2u),
         f32(vi / 2u)
@@ -161,6 +178,8 @@ fn vs_main(
     out.top_left = top_left;
     out.bot_right = bot_right;
     out.mask_idx = in.mask_idx;
+    out.mask_top_left = mask[0];
+    out.mask_bot_right = mask[1];
     out.idx = ii;
 
     return out;
@@ -170,17 +189,8 @@ fn masked(in: VertexOutput, color: vec4<f32>) -> vec4<f32> {
     if in.mask_idx == MASK_NONE {
         return color;
     }
-    let mask = masks[in.mask_idx];
-    // Its own chain, not the drawn primitive's, so a stationary viewport
-    // clips content that moves inside it.
-    let m = resolve_move(mask.move_idx, Region(span_of(mask.x), span_of(mask.y)));
-    let tl = vec2(m.x.start.rel, m.y.start.rel);
-    let tl_px = vec2(m.x.start.px, m.y.start.px);
-    let br = vec2(m.x.end.rel, m.y.end.rel);
-    let br_px = vec2(m.x.end.px, m.y.end.px);
-
-    let top_left = snap_floor(tl * window.dim + tl_px);
-    let bot_right = snap_floor(br * window.dim + br_px);
+    let top_left = in.mask_top_left;
+    let bot_right = in.mask_bot_right;
     let pos = in.clip_position.xy;
     if pos.x < top_left.x || pos.x > bot_right.x || pos.y < top_left.y || pos.y > bot_right.y {
         return color * 0.0;

core/src/ui/active.rs

@@ -3,6 +3,23 @@ use crate::{
     RetainedPrimitive, Size, TextureHandle, UiRegion, UiVec2, WidgetId,
 };
 
+/// One draw of one widget, so that a widget it asked about can carry which
+/// draw that was. Its own type beside the indices here because it names an
+/// occasion rather than a slot: nothing is stored per draw.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct DrawId(u64);
+
+impl DrawId {
+    /// No draw at all, which is what a widget nothing has asked about carries.
+    pub const NONE: Self = Self(0);
+
+    /// The next one after this. Handed out in order and never reused, so a
+    /// note left by an earlier draw can never be read as this one's.
+    pub(crate) fn next(self) -> Self {
+        Self(self.0 + 1)
+    }
+}
+
 /// What is kept of a widget its parent has asked about. `drawn` says whether
 /// it currently draws; one that does not is kept so that a change to it, or
 /// under it, still reaches whoever asked.
@@ -28,6 +45,12 @@ pub struct ActiveData {
     /// The measured answer and its dependencies. A hint-only dependency or
     /// a widget first encountered during placement has no measurement yet.
     pub answer: Option<Answer>,
+    /// The draw that last asked about this widget, which is what says whether
+    /// the draw now running has already asked -- a question the child list can
+    /// only answer by a search, and so in the children a container has rather
+    /// than in one read. Written by whoever asked, so a draw of this widget
+    /// itself carries it across rather than setting it.
+    pub(crate) asked_by: DrawId,
     /// Asked more than once in its parent's last draw -- measured in one box
     /// and then asked in the one the parent decided. The parent's layout
     /// rests on the first answer and its drawing on the last, so only the

core/src/ui/mod.rs

@@ -57,8 +57,11 @@ impl Moves {
         }
     }
 
+    /// Frees a slot. Not a change to the entries: the slot keeps the bytes it
+    /// had, nothing names it until it is handed out again, and whoever is
+    /// handed it writes it then -- so re-uploading the array here would send
+    /// the GPU what it already has.
     pub fn remove(&mut self, idx: MoveIdx) {
-        self.changed = true;
         self.arena.remove(Id::preset(idx.idx() as u32));
     }
 

core/src/ui/painter.rs

@@ -1,8 +1,8 @@
 #[cfg(feature = "layout-diagnostics")]
 use crate::layout_diagnostics::{self as diag, Counter};
 use crate::{
-    Axis, Bound, Bounds, Declared, DrawScratch, Holds, LayoutHolds, LayoutLen, Len, PlaceDesc,
-    PlaceFit, Px, PxVec2, RegionAlign, Rel, RenderedText, RequestArena, RequestedLen,
+    Axis, Bound, Bounds, Declared, DrawId, DrawScratch, Holds, LayoutHolds, LayoutLen, Len,
+    PlaceDesc, PlaceFit, Px, PxVec2, RegionAlign, Rel, RenderedText, RequestArena, RequestedLen,
     RetainedPrimitive, Size, SizeRequests, StrongWidget, TextAttrs, TextBuffer, TextureHandle,
     UiRegion, UiRenderState, UiRsc, UiVec2, Weight, WidgetId, Widgets,
     render::{
@@ -61,6 +61,8 @@ pub struct Painter<'a> {
     /// counted from however far `layer` has walked.
     pub(super) own_layer: usize,
     pub(super) depth: usize,
+    /// This draw's own id, which it leaves on every widget it asks about.
+    pub(super) draw: DrawId,
     pub(super) id: WidgetId,
 }
 
@@ -339,7 +341,12 @@ impl<'a> Painter<'a> {
             diag::region_node(id.id(), self.id, region);
         }
         // A child listed twice would be moved twice.
-        let re_asked = self.children.contains(&id.id());
+        let re_asked = self.state.asked_in(id.id(), self.draw);
+        debug_assert_eq!(
+            re_asked,
+            self.children.contains(&id.id()),
+            "the note on a child disagrees with the list it says it is in",
+        );
         if !re_asked {
             self.children.push(id.id());
         }
@@ -364,11 +371,27 @@ impl<'a> Painter<'a> {
             None,
             self.rsc,
         );
+        // Written now the draw has happened, since a widget drawn here has no
+        // record of its own until it has.
+        self.state
+            .active
+            .get_mut(&id.id())
+            .expect("a widget that was drawn has a record")
+            .asked_by = self.draw;
         let holds = self.in_parent(drawn.drawing_holds, region, place, declared);
         let answer_holds = self.in_parent(drawn.answer.holds, region, place, declared);
-        match self.under.iter_mut().find(|(child, _)| *child == id.id()) {
-            Some((_, kept)) => *kept = holds,
-            None => self.under.push((id.id(), holds)),
+        // Added in step with the child list, so the one search here is the
+        // rare case of a child asked about twice.
+        match re_asked {
+            true => {
+                let kept = self
+                    .under
+                    .iter_mut()
+                    .find(|(child, _)| *child == id.id())
+                    .expect("a child asked about twice was added the first time");
+                kept.1 = holds;
+            }
+            false => self.under.push((id.id(), holds)),
         }
         DrawResult {
             child: id,
@@ -385,6 +408,11 @@ impl<'a> Painter<'a> {
         self.children.retain(|child| *child != id.id());
         self.under.retain(|(child, _)| *child != id.id());
         self.state.undraw_rec(id.id(), self.rsc);
+        // Taken out of the child list, so the note saying it is in there goes
+        // with it: placing it again is asking again.
+        if let Some(active) = self.state.active.get_mut(&id.id()) {
+            active.asked_by = DrawId::NONE;
+        }
     }
 
     /// Puts a child in `place` of this widget's box, where that box is the
@@ -406,7 +434,7 @@ impl<'a> Painter<'a> {
         let states_rel_base = Axis::BOTH
             .iter()
             .any(|&axis| matches!(place[axis].rel_base, RelBase::Len(_)));
-        if states_rel_base || !self.children.contains(&id.id()) {
+        if states_rel_base || !self.state.asked_in(id.id(), self.draw) {
             return self.widget_at(id, place);
         }
         let at = self.placing();
@@ -470,10 +498,12 @@ impl<'a> Painter<'a> {
         resolved
     }
 
+    /// Records that this draw read a child's length. Listed once per read
+    /// rather than once per child: a child that answered with a hint may have
+    /// no record to note it on until this draw ends, and what the list drives
+    /// asks the same of a widget twice as of it once.
     fn depend_on<W: ?Sized>(&mut self, child: &StrongWidget<W>) {
-        if !self.size_deps.contains(&child.id()) {
-            self.size_deps.push(child.id());
-        }
+        self.size_deps.push(child.id());
     }
 
     pub fn render_text<'b>(

core/src/ui/render_state.rs

@@ -1,9 +1,9 @@
 #[cfg(feature = "layout-diagnostics")]
 use crate::layout_diagnostics::{self as diag, Counter, ReuseOutcome, TimerKind};
 use crate::{
-    ActiveData, Answer, Axis, Bounds, Declared, DrawLayers, IdLike, LayoutHolds, LayoutLen, Len,
-    MaskIdx, MoveIdx, Moves, Painter, PixelRegion, PlaceDesc, PxVec2, Size, StrongWidget, UiRegion,
-    UiRsc, UiSpan, UiVec2, Weight, WidgetId, Widgets,
+    ActiveData, Answer, Axis, Bounds, Declared, DrawId, DrawLayers, IdLike, LayoutHolds, LayoutLen,
+    Len, MaskIdx, MoveIdx, Moves, Painter, PixelRegion, PlaceDesc, PxVec2, Size, StrongWidget,
+    UiRegion, UiRsc, UiSpan, UiVec2, Weight, WidgetId, Widgets,
     ui::painter::Ask,
     util::{HashMap, Vec2},
 };
@@ -97,6 +97,10 @@ pub struct UiRenderState {
     pending: std::collections::BinaryHeap<(usize, WidgetId)>,
     pub(super) requests: crate::RequestArena,
     changed: Vec<WidgetId>,
+    /// The last draw id handed out. Each draw takes a fresh one and leaves it
+    /// on every widget it asks about, which is how it knows in one read
+    /// whether it has asked already.
+    last_draw: DrawId,
     request_readers: HashMap<WidgetId, crate::util::HashSet<WidgetId>>,
     pub moves: Moves,
 }
@@ -113,6 +117,7 @@ impl UiRenderState {
             pending: Default::default(),
             requests: Default::default(),
             changed: Vec::new(),
+            last_draw: DrawId::NONE,
             request_readers: Default::default(),
             moves: Default::default(),
             resized: false,
@@ -322,6 +327,14 @@ impl UiRenderState {
         old: Option<ActiveData>,
         rsc: &mut dyn UiRsc,
     ) -> Answer {
+        let draw = self.next_draw();
+        // Whoever asked about this widget wrote this, and a draw of the widget
+        // itself is not that: the record is rebuilt below, so it is carried
+        // across rather than reset.
+        let asked_by = old
+            .as_ref()
+            .or_else(|| self.active.get(&id))
+            .map_or(DrawId::NONE, |active| active.asked_by);
         let rel_base = info.rel_base;
         let (move_idx, region, retired_move) = match info.region_node {
             // A node entry is only a translation. Its local box keeps the
@@ -383,6 +396,7 @@ impl UiRenderState {
             answer_under: LayoutHolds::ANY,
             depth: info.depth,
             move_idx,
+            draw,
             rsc,
         };
 
@@ -419,6 +433,7 @@ impl UiRenderState {
             layer,
             own_layer: _,
             depth: _,
+            draw: _,
             id,
         } = painter;
 
@@ -507,7 +522,7 @@ impl UiRenderState {
             region.to_px(window),
         );
         for c in &old_children {
-            if !children.contains(c) {
+            if !self.asked_in(*c, draw) {
                 self.undraw_rec(*c, rsc);
             }
         }
@@ -531,7 +546,7 @@ impl UiRenderState {
         // and a change there has to reach it. Asking answered whatever mark
         // it had: a hint is read live, and a drawing is not kept past one.
         for &dep in &size_deps {
-            if !children.contains(&dep) {
+            if !self.asked_in(dep, draw) {
                 self.asked(
                     dep,
                     DrawInfo {
@@ -574,6 +589,7 @@ impl UiRenderState {
             region,
             // Whoever asked writes the answer.
             answer: None,
+            asked_by,
             re_asked: info.re_asked,
             size,
             holds,
@@ -603,6 +619,21 @@ impl UiRenderState {
         }
     }
 
+    fn next_draw(&mut self) -> DrawId {
+        self.last_draw = self.last_draw.next();
+        self.last_draw
+    }
+
+    /// Whether `draw` has asked about this widget, which is what it left on
+    /// the widget's own record when it did. One widget is asked about by one
+    /// container, since the handle a container holds a child by cannot be
+    /// cloned, so one note per widget is enough to answer this.
+    pub(super) fn asked_in(&self, id: WidgetId, draw: DrawId) -> bool {
+        self.active
+            .get(&id)
+            .is_some_and(|active| active.asked_by == draw)
+    }
+
     /// Keeps a region node's entry across redraws because descendants retain
     /// its index.
     fn move_slot(&mut self, id: WidgetId, parent: MoveIdx, region: UiRegion) -> MoveIdx {
@@ -956,6 +987,7 @@ impl UiRenderState {
                 asked: PlaceDesc::WHOLE,
                 region: UiRegion::FULL,
                 answer: None,
+                asked_by: DrawId::NONE,
                 re_asked: false,
                 size,
                 holds: LayoutHolds::ANY,