From 3bf22935ceb1a943317a173daf880b8213427060 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Thu, 17 Sep 2026 19:16:40 -0400 Subject: [PATCH] Take the deepest dirty widget from an ordered queue, not by scanning The walk found the next widget to settle with `max_by_key` over the whole `needs_redraw` set, and `depth` is a hash lookup, so a frame did a lookup per marked widget per pop -- 131 depth reads for nine marks at seed 1 depth 8, 1,314 for 34, and 14,611 for 145. The set is scanned once now and kept in a `BTreeSet` keyed by depth, and every mark made while the walk runs goes through `mark`, which puts itself in place. The same three counts become 57, 160 and 436. Two things the scan gave for free are paid for explicitly: a widget that was settled inside an ancestor's draw, or deferred to one, is dropped when its entry comes up, and an entry whose widget has since changed depth -- a subtree that moved under a new parent -- is re-queued at the depth it now has. What is drawn does not change: widget draws are identical at every load measured. Median frame at seed 1, depth 8: 0.955 -> 0.843 ms with 145 marks, 0.668 -> 0.666 with 34, and seed 13's default load 5.19 -> 4.86 ms. Ties between equal depths now break by widget id rather than by hash order, which makes the walk deterministic; nothing in the order within one depth was ever relied on, since a widget at the same depth as another cannot contain it. Co-Authored-By: Claude Opus 5 --- core/src/ui/render_state.rs | 53 +++++++++++++++++++++++++++---------- core/src/util/slot.rs | 2 +- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index 4d169c2..b62d68b 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -69,6 +69,10 @@ pub struct UiRenderState { /// Widgets waiting for an ancestor to draw them, so the walk down the /// depths does not pick one up again at its own depth. deferred: crate::util::HashSet, + /// What the walk has left to settle, deepest last. Ordered rather than + /// searched: the set is scanned once a frame, and every mark made while + /// the walk runs puts itself in place. + pending: std::collections::BTreeSet<(usize, WidgetId)>, pub moves: Moves, } @@ -81,6 +85,7 @@ impl UiRenderState { old_root: None, slots: Default::default(), deferred: Default::default(), + pending: Default::default(), moves: Default::default(), resized: false, } @@ -994,15 +999,25 @@ impl UiRenderState { // something below is about to change it -- which is the whole class // of defect where a widget settles inside its parent's draw, clears // its mark there, and tells nobody its answer moved. - loop { - let next = rsc - .widgets() - .needs_redraw - .iter() - .copied() - .filter(|id| !self.deferred.contains(id)) - .max_by_key(|&id| self.depth(id)); - let Some(id) = next else { break }; + let marked: Vec = rsc.widgets().needs_redraw.iter().copied().collect(); + for id in marked { + let depth = self.depth(id); + self.pending.insert((depth, id)); + } + while let Some(&(depth, id)) = self.pending.last() { + self.pending.remove(&(depth, id)); + // A widget settled inside an ancestor's draw, or deferred to one, + // is left here by the mark that queued it. + if self.deferred.contains(&id) || !rsc.widgets().needs_redraw.contains(&id) { + continue; + } + // A subtree that changed hands takes its descendants' depths with + // it, so an entry queued before that move names the wrong one. + let now = self.depth(id); + if now != depth { + self.pending.insert((now, id)); + continue; + } #[cfg(feature = "layout-diagnostics")] diag::bump(Counter::QueuePops); if !self.redraw(id, rsc) { @@ -1012,6 +1027,16 @@ impl UiRenderState { self.deferred.clear(); } + /// Marks a widget for the walk to settle. Every mark made while a frame + /// is being laid out goes through here, so the queue holds what the set + /// holds without being searched again. + fn mark(&mut self, id: WidgetId, widgets: &mut Widgets) { + if widgets.needs_redraw.insert(id) && !self.deferred.contains(&id) { + let depth = self.depth(id); + self.pending.insert((depth, id)); + } + } + fn depth(&self, id: WidgetId) -> usize { #[cfg(feature = "layout-diagnostics")] diag::bump(Counter::DepthReads); @@ -1115,8 +1140,8 @@ impl UiRenderState { // Both stay marked: the parent because it has this to draw, and // this because the parent must draw it rather than keep what it // has. The mark comes off in `draw_at`, where the parent draws. - rsc.widgets_mut().needs_redraw.insert(id); - rsc.widgets_mut().needs_redraw.insert(parent); + self.mark(id, rsc.widgets_mut()); + self.mark(parent, rsc.widgets_mut()); return false; } if !active.drawn { @@ -1144,8 +1169,8 @@ impl UiRenderState { // mark left on. Lengths and not whole boxes: what a drawing depends // on is its lengths, so the same lengths elsewhere is one question. if given_px != offered_px { - rsc.widgets_mut().needs_redraw.insert(id); - rsc.widgets_mut().needs_redraw.insert(parent); + self.mark(id, rsc.widgets_mut()); + self.mark(parent, rsc.widgets_mut()); return false; } let info = DrawInfo { @@ -1200,7 +1225,7 @@ impl UiRenderState { diag::bump(Counter::SizeChanges); diag::bump(Counter::ReaderEdges); } - rsc.widgets_mut().needs_redraw.insert(parent); + self.mark(parent, rsc.widgets_mut()); } true } diff --git a/core/src/util/slot.rs b/core/src/util/slot.rs index 94fd2a1..707b0bf 100644 --- a/core/src/util/slot.rs +++ b/core/src/util/slot.rs @@ -1,4 +1,4 @@ -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct SlotId { idx: u32, genr: u32,