Dirty many widgets at once, which nothing was checking

Every generated case changes one thing: four declared sizes, or one
span's children, or the output. A frame settling one dependency path
says nothing about a frame settling a set of them that overlap, which
is the case the settle order exists for.

So two more: every declared size in the tree changing at once, and a
spread of widgets marked for redraw together. The second changes
nothing, which is the point -- no box may move, and the order the
dirty set is taken in is all that can make one. The hundred-seed sweep
is 1,000 comparisons now rather than 800, and passes.

`IRIS_PHASE=many` is the same load for the diagnostics rig, with
`IRIS_DIRTY` widgets marked per frame. It says what one repainting leaf
cannot: at 130 of 260 widgets, choosing which dirty widget to settle
next is 24.5% of the frame, because the dirty set is scanned once per
widget settled and a hash set is walked by capacity rather than by
length. Memoizing the depth walk inside one scan does not pay -- it
trades parent lookups for memo lookups and costs 4% more instructions --
so the fix is to stop rescanning, which changes the order widgets
settle in and wants agreeing first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

core/src/layout_diagnostics.rs

@@ -54,6 +54,7 @@ pub(crate) enum Counter {
     TextRenders,
     TextShapeHits,
     TextShapes,
+    TextBreaks,
     GlyphPlacements,
 }
 
@@ -90,6 +91,7 @@ impl Counter {
         "text renders",
         "text shape hits",
         "text shapes",
+        "text line breaks",
         "glyph placements",
     ];
 }
@@ -102,6 +104,7 @@ pub(crate) enum TimerKind {
     IncrementalLayout,
     TextRender,
     TextShape,
+    TextBreak,
     GlyphPlacement,
 }
 
@@ -115,6 +118,7 @@ impl TimerKind {
         "incremental layout",
         "text render",
         "text shape",
+        "text line break",
         "glyph placement",
     ];
 }

core/src/primitive/text.rs

@@ -22,30 +22,31 @@ pub struct TextData {
     pub layout_ctx: LayoutContext<UiColor>,
     scale_ctx: ScaleContext,
     pub atlas: GlyphAtlas,
-    spare: VecDeque<Shaping>,
+    spare: VecDeque<Placed>,
 }
 
-/// One shaping of some text, and the glyphs placed from it. A buffer holds
-/// the one it is drawn as; these are the ones it had before, kept because a
-/// container measures a child by drawing it in a box it may not keep, and so
-/// comes back to widths it has already asked for.
-struct Shaping {
-    /// A shaping is a function of these three and nothing else, so no widget
-    /// or buffer identity is involved and two texts of the same words share
-    /// an answer.
+/// The glyphs of one text at one width. A buffer holds the ones it is drawn
+/// as; these are the ones it had before, kept because a container measures a
+/// child by drawing it in a box it may not keep, and so comes back to widths
+/// it has already asked for.
+struct Placed {
+    /// Where the glyphs land is a function of these three and nothing else,
+    /// so no widget or buffer identity is involved and two texts of the same
+    /// words share an answer.
     text: String,
     key: LayoutKey,
-    layout: Layout<UiColor>,
-    placed: Option<RenderedText>,
+    glyphs: RenderedText,
 }
 
-/// How many to keep. Bounding the whole cache rather than each buffer is
-/// what makes this a fixed cost instead of one a tree of ten thousand texts
-/// pays ten thousand times; the re-asks come from laying out one subtree, so
-/// they are close together and few are needed. On `tests/revision_cost.rs`
-/// under `SWEEP=1`, the case that cannot hit across frames, 32 is not enough
-/// (6.6 ms) and 64 is (4.4 ms).
-const SPARE_SHAPINGS: usize = 128;
+/// How many to keep. Bounding the whole store rather than each buffer is what
+/// makes this a fixed cost instead of one a tree of ten thousand texts pays
+/// ten thousand times; the re-asks come from laying out one subtree, so they
+/// are close together and few are needed. Instructions over 500 resize frames
+/// of `tests/revision_cost.rs`, both the repeating widths and the sweep that
+/// cannot hit across frames: 13.7B at 32, 12.1B at 64, 10.4B and 12.1B at 128,
+/// and nothing past that -- so 128, which is no worse in the case that never
+/// repeats and better in the one that does.
+const SPARE_PLACED: usize = 128;
 
 impl Default for TextData {
     fn default() -> Self {
@@ -188,21 +189,31 @@ impl TextBuffer {
             diag::bump(Counter::TextShapeHits);
             return;
         }
-        let kept = data.take_shaping(&self.text, &layout_key);
-        if let Some(key) = self.layout_key.take() {
-            data.keep_shaping(Shaping {
+        let same_shaping = self
+            .layout_key
+            .as_ref()
+            .is_some_and(|key| key.attrs == *attrs);
+        let old_key = self.layout_key.replace(layout_key);
+        // The glyphs it holds are of the width it held, which the layout may
+        // well come back to.
+        if let Some(key) = old_key
+            && let Some(glyphs) = self.placed.take()
+        {
+            data.keep_placed(Placed {
                 text: self.text.clone(),
                 key,
-                layout: std::mem::replace(&mut self.layout, Layout::new()),
-                placed: self.placed.take(),
+                glyphs,
             });
         }
-        if let Some(shaping) = kept {
+        // Only the line breaking depends on the width: the shaped runs under
+        // it are a function of the text and the attrs, and parley re-breaks
+        // them in place. So a new width is a break, not a shaping.
+        if same_shaping {
             #[cfg(feature = "layout-diagnostics")]
-            diag::bump(Counter::TextShapeHits);
-            self.layout = shaping.layout;
-            self.placed = shaping.placed;
-            self.layout_key = Some(shaping.key);
+            diag::bump(Counter::TextBreaks);
+            #[cfg(feature = "layout-diagnostics")]
+            let _break = diag::timer(TimerKind::TextBreak);
+            self.break_lines(width);
             return;
         }
         #[cfg(feature = "layout-diagnostics")]
@@ -219,10 +230,13 @@ impl TextBuffer {
         )));
         builder.push_default(StyleProperty::Brush(attrs.color));
         builder.build_into(&mut self.layout, &self.text);
+        self.break_lines(width);
+    }
+
+    fn break_lines(&mut self, width: Option<f32>) {
         self.layout.break_all_lines(width);
         self.layout
             .align(Alignment::Start, AlignmentOptions::default());
-        self.layout_key = Some(layout_key);
     }
 }
 
@@ -334,21 +348,21 @@ pub struct RenderedText {
 }
 
 impl TextData {
-    /// The shaping for this text at this width, taken out of what is kept.
-    fn take_shaping(&mut self, text: &str, key: &LayoutKey) -> Option<Shaping> {
+    /// The glyphs of this text at this width, taken out of what is kept.
+    fn take_placed(&mut self, text: &str, key: &LayoutKey) -> Option<RenderedText> {
         // From the newest, since a re-ask is usually of something recent.
         let at = self
             .spare
             .iter()
             .rposition(|spare| spare.key == *key && spare.text == text)?;
-        self.spare.remove(at)
+        self.spare.remove(at).map(|spare| spare.glyphs)
     }
 
-    fn keep_shaping(&mut self, shaping: Shaping) {
-        if self.spare.len() >= SPARE_SHAPINGS {
+    fn keep_placed(&mut self, placed: Placed) {
+        if self.spare.len() >= SPARE_PLACED {
             self.spare.pop_front();
         }
-        self.spare.push_back(shaping);
+        self.spare.push_back(placed);
     }
 
     pub fn render<'b>(
@@ -362,7 +376,13 @@ impl TextData {
         #[cfg(feature = "layout-diagnostics")]
         let _render = diag::timer(TimerKind::TextRender);
         buffer.shape(self, attrs, width);
-        let placed = match buffer.placed.take() {
+        // Only asked for when the buffer no longer holds them: taking one out
+        // of the store to then drop it would throw an answer away.
+        let placed = buffer.placed.take().or_else(|| {
+            let key = buffer.layout_key.as_ref()?;
+            self.take_placed(&buffer.text, key)
+        });
+        let placed = match placed {
             Some(placed) => placed,
             None => {
                 #[cfg(feature = "layout-diagnostics")]

tests/generated.rs

@@ -44,24 +44,37 @@ fn plant(h: &mut Harness, seed: u64, edits: &Edits) -> Tree {
     tree
 }
 
+fn resize_one(h: &mut Harness, tree: &Tree, idx: usize, rng: &mut Rng) -> Lens {
+    let lens = [
+        Some(Len::abs(20.0 + rng.below(180) as f32)),
+        Some(Len::abs(20.0 + rng.below(180) as f32)),
+    ];
+    let sized = &mut h.rsc[tree.sized[idx]];
+    sized.x = lens[0];
+    sized.y = lens[1];
+    lens
+}
+
 /// Changes a few of the declared sizes, and says which, so the cold tree can
 /// be grown with the same ones.
 fn edit(h: &mut Harness, tree: &Tree, rng: &mut Rng) -> HashMap<usize, Lens> {
     let mut edits = HashMap::new();
     for _ in 0..4 {
         let idx = rng.below(tree.sized.len());
-        let lens = [
-            Some(Len::abs(20.0 + rng.below(180) as f32)),
-            Some(Len::abs(20.0 + rng.below(180) as f32)),
-        ];
-        edits.insert(idx, lens);
-        let sized = &mut h.rsc[tree.sized[idx]];
-        sized.x = lens[0];
-        sized.y = lens[1];
+        edits.insert(idx, resize_one(h, tree, idx, rng));
     }
     edits
 }
 
+/// Every declared size at once, so every reader of a size in the tree has a
+/// changed descendant in the same frame and the whole dirty set has to settle
+/// together.
+fn edit_every(h: &mut Harness, tree: &Tree, rng: &mut Rng) -> HashMap<usize, Lens> {
+    (0..tree.sized.len())
+        .map(|idx| (idx, resize_one(h, tree, idx, rng)))
+        .collect()
+}
+
 /// A way of changing what a span holds. Each is a shape worth its own case:
 /// taking a child out of the middle is not the same as emptying a span, and
 /// adding one is not the same as adding three.
@@ -239,6 +252,52 @@ fn reshuffled(seed: u64, shuffle: Shuffle) {
     assert_same(seed, &what, (&warm, &grown), (&cold, &same));
 }
 
+fn changed_every_size(seed: u64) {
+    let mut warm = Harness::new((900, 1200));
+    let grown = plant(&mut warm, seed, &Edits::default());
+    if grown.sized.is_empty() {
+        return;
+    }
+
+    let mut rng = Rng::new(seed ^ 0xa11);
+    let sizes = edit_every(&mut warm, &grown, &mut rng);
+    warm.frame();
+
+    let mut cold = Harness::new((900, 1200));
+    let same = plant(
+        &mut cold,
+        seed,
+        &Edits {
+            sizes,
+            ..Default::default()
+        },
+    );
+
+    assert_same(seed, "every size at once", (&warm, &grown), (&cold, &same));
+}
+
+/// Marks a spread of widgets for redraw at once. Nothing changes, so no box
+/// may either; what this exercises is the order a frame settles a dirty set
+/// in, which the other cases reach one dependency path at a time.
+fn repainted_together(seed: u64) {
+    let mut warm = Harness::new((900, 1200));
+    let grown = plant(&mut warm, seed, &Edits::default());
+    for &id in grown.ids.iter().step_by(5) {
+        warm.rsc.widgets_mut().get_dyn_mut(id);
+    }
+    assert!(
+        !warm.rsc.widgets().needs_redraw.is_empty(),
+        "seed {seed}: nothing was marked"
+    );
+    warm.frame();
+
+    let mut cold = Harness::new((900, 1200));
+    let same = plant(&mut cold, seed, &Edits::default());
+
+    let what = "many repaints at once";
+    assert_same(seed, what, (&warm, &grown), (&cold, &same));
+}
+
 fn resized(seed: u64) {
     let mut warm = Harness::new((1920, 1200));
     let grown = plant(&mut warm, seed, &Edits::default());
@@ -280,6 +339,16 @@ fn a_changed_size_lands_where_growing_it_that_way_would() {
     SEEDS.into_iter().for_each(changed_size);
 }
 
+#[test]
+fn every_size_changing_at_once_lands_where_growing_it_that_way_would() {
+    SEEDS.into_iter().for_each(changed_every_size);
+}
+
+#[test]
+fn many_widgets_redrawing_at_once_leaves_every_box_where_it_was() {
+    SEEDS.into_iter().for_each(repainted_together);
+}
+
 #[test]
 fn a_resize_lands_where_starting_at_that_size_would() {
     SEEDS.into_iter().for_each(resized);
@@ -328,6 +397,8 @@ fn a_long_run_of_seeds_agrees() {
         .unwrap_or(1..=100);
     for seed in seeds {
         changed_size(seed);
+        changed_every_size(seed);
+        repainted_together(seed);
         resized(seed);
         resized_then_changed(seed);
         for shuffle in SHUFFLES {

tests/layout_diagnostics.rs

@@ -11,8 +11,9 @@
 //!       -e cycles:u,instructions:u cargo test --release \
 //!       --test layout_diagnostics -- --ignored --nocapture
 //!
-//! `IRIS_PHASE` is `cold`, `repaint`, `size`, `scroll`, `resize`, or `all`.
-//! `IRIS_SEED`, `IRIS_DEPTH`, and `IRIS_FRAMES` select the load.
+//! `IRIS_PHASE` is `cold`, `repaint`, `many`, `size`, `scroll`, `resize`, or
+//! `all`. `IRIS_SEED`, `IRIS_DEPTH`, and `IRIS_FRAMES` select the load, and
+//! `IRIS_DIRTY` how many widgets `many` marks at once.
 
 use iris::harness::Harness;
 use iris::prelude::*;
@@ -165,7 +166,7 @@ fn layout_cost() {
     assert!(frames > 0, "IRIS_FRAMES must be greater than zero");
     let phase = env("IRIS_PHASE", String::from("all"));
     assert!(
-        ["all", "cold", "repaint", "size", "scroll", "resize"].contains(&phase.as_str()),
+        ["all", "cold", "repaint", "many", "size", "scroll", "resize"].contains(&phase.as_str()),
         "unknown IRIS_PHASE {phase:?}"
     );
     let selected = |name| phase == "all" || phase == name;
@@ -194,6 +195,22 @@ fn layout_cost() {
         });
     }
 
+    if selected("many") {
+        let (mut harness, tree) = warm(seed, depth);
+        trace_selected(&tree);
+        // Spread through the tree rather than taken from one subtree, so the
+        // dependency paths the frame settles overlap.
+        let wanted = env("IRIS_DIRTY", 32_usize).max(1);
+        let step = (tree.ids.len() / wanted).max(1);
+        let dirty: Vec<_> = tree.ids.iter().copied().step_by(step).collect();
+        println!("marking {} of {} widgets", dirty.len(), tree.ids.len());
+        run("many", frames, &mut harness, move |harness, _| {
+            for &id in &dirty {
+                harness.rsc.widgets_mut().get_dyn_mut(id);
+            }
+        });
+    }
+
     if selected("size") {
         let (mut harness, tree) = warm(seed, depth);
         trace_selected(&tree);