Settle a frame strictly bottom-up rather than escalating into a parent

The queue was already deepest-first, but a widget that could not settle
where it was called `redraw` on its parent from inside itself. That drew a
shallow widget while dirty widgets deeper in other subtrees were still
pending, and a parent drawing over a subtree that has not settled reads
answers about to move: the one that settles does so inside the parent's
draw, where its mark comes off and nothing compares what it now answers.
Seed 564 was exactly that, and it is the second time this shape has been
found.

So a widget that cannot settle defers instead. It marks its parent, stays
marked itself, and waits in `deferred` until the walk down the depths
reaches the parent -- which cannot be before everything deeper has settled,
because the walk always takes the deepest widget that is not waiting. The
category stops being something to check for. (Bryan, 2026-09-17.)

`dirty_size_under` stays in `draw_inner` for now: `update` draws the root
for a resize before `redraw_updates` runs at all, so the ordering does not
cover that entry.

Green on the suite, the shrinker at 400 seeds of depth 5, the oracle at 1000
seeds of depth 6, and 2000 seeds at depth 4 over all fifteen cases. Drawn
widgets, widget draws and primitive writes are unchanged on every rig phase;
`many` pays 51 queue pops for 27 and 1059 depth reads for 410, which is the
deferring and nothing else.
This commit is contained in:
iris-ai committed 2026-09-17 04:29:41 -04:00
1 parent c8beca5753
commit a92c6acdbf
1 file changed
+43 -21
+43 -21
View File
@@ -55,6 +55,9 @@ pub struct UiRenderState {
/// Whether this frame contains a declared-length change, so any dirty /// Whether this frame contains a declared-length change, so any dirty
/// dependent replaces its answer too. /// dependent replaces its answer too.
replace_answers: bool, replace_answers: bool,
/// 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<WidgetId>,
pub moves: Moves, pub moves: Moves,
} }
@@ -68,6 +71,7 @@ impl UiRenderState {
slots: Default::default(), slots: Default::default(),
answer_invalid: Default::default(), answer_invalid: Default::default(),
replace_answers: false, replace_answers: false,
deferred: Default::default(),
moves: Default::default(), moves: Default::default(),
resized: false, resized: false,
} }
@@ -827,18 +831,33 @@ impl UiRenderState {
pub fn redraw_updates(&mut self, rsc: &mut dyn UiRsc) { pub fn redraw_updates(&mut self, rsc: &mut dyn UiRsc) {
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
let _layout = diag::timer(TimerKind::IncrementalLayout); let _layout = diag::timer(TimerKind::IncrementalLayout);
// Deepest first: a reader whose children have all settled asks each // Deepest first, and strictly: a widget that cannot settle where it
// once, where any other order has it lay out again for whatever // is defers to its parent rather than drawing the parent from
// settles under it afterwards. Equal-depth widgets are independent, // inside itself. It marks the parent, stays marked, and waits here
// so their order does not matter. // until the walk reaches its parent's depth.
while let Some(id) = { //
let dirty = rsc.widgets().needs_redraw.iter().copied(); // What that buys is that nothing shallower is ever drawn while
dirty.max_by_key(|&id| self.depth(id)) // anything deeper is still dirty. A parent drawing can therefore
} { // trust every answer it reads without descending to check whether
// 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 };
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
diag::bump(Counter::QueuePops); diag::bump(Counter::QueuePops);
self.redraw(id, rsc); if !self.redraw(id, rsc) {
self.deferred.insert(id);
}
} }
self.deferred.clear();
} }
fn depth(&self, id: WidgetId) -> usize { fn depth(&self, id: WidgetId) -> usize {
@@ -923,11 +942,13 @@ impl UiRenderState {
} }
/// Settles a dirty widget: asks it again where its parent asked, and /// Settles a dirty widget: asks it again where its parent asked, and
/// tells the parent if the answer changed. /// tells the parent if the answer changed. `false` where the question is
pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) { /// its parent's rather than its own, which leaves it marked for the
/// parent to draw when the walk reaches that depth.
pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) -> bool {
rsc.widgets_mut().needs_redraw.remove(&id); rsc.widgets_mut().needs_redraw.remove(&id);
let Some(active) = self.active.get(&id) else { let Some(active) = self.active.get(&id) else {
return; return true;
}; };
// Its parent resolved its declared lengths into its box and decided // Its parent resolved its declared lengths into its box and decided
// whether to draw it at all, so a change to either is the parent's // whether to draw it at all, so a change to either is the parent's
@@ -947,14 +968,15 @@ impl UiRenderState {
at = self.active[&next].parent; at = self.active[&next].parent;
} }
} }
// 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(id);
self.redraw(parent, rsc); rsc.widgets_mut().needs_redraw.insert(parent);
// Whatever the parent did not draw again is nothing it holds now. return false;
rsc.widgets_mut().needs_redraw.remove(&id);
return;
} }
if !active.drawn { if !active.drawn {
return; return true;
} }
// Nothing above the root resolved its rules or its alignment, so its // Nothing above the root resolved its rules or its alignment, so its
// box is its own to work out again against the output. Every other // box is its own to work out again against the output. Every other
@@ -969,7 +991,7 @@ impl UiRenderState {
diag::bump(Counter::LocalRedraws); diag::bump(Counter::LocalRedraws);
let old = self.remove(id, false, rsc); let old = self.remove(id, false, rsc);
self.draw_inner(id, region, info, old, rsc); self.draw_inner(id, region, info, old, rsc);
return; return true;
}; };
let (given_px, offered_px) = self.asked_px(id); let (given_px, offered_px) = self.asked_px(id);
// Asked again in the box its parent gave it, which is the question // Asked again in the box its parent gave it, which is the question
@@ -979,9 +1001,8 @@ impl UiRenderState {
// on is its lengths, so the same lengths elsewhere is one question. // on is its lengths, so the same lengths elsewhere is one question.
if given_px != offered_px { if given_px != offered_px {
rsc.widgets_mut().needs_redraw.insert(id); rsc.widgets_mut().needs_redraw.insert(id);
self.redraw(parent, rsc); rsc.widgets_mut().needs_redraw.insert(parent);
rsc.widgets_mut().needs_redraw.remove(&id); return false;
return;
} }
let info = DrawInfo { let info = DrawInfo {
layer: active.layer, layer: active.layer,
@@ -1014,6 +1035,7 @@ impl UiRenderState {
} }
rsc.widgets_mut().needs_redraw.insert(parent); rsc.widgets_mut().needs_redraw.insert(parent);
} }
true
} }
} }