Compare commits

...
Author SHA1 Message Date
iris-aiandClaude Fable 5.1 62a16b5608 Re-ask a dirty widget at its offer locally instead of deferring to its parent
A local redraw refused any widget whose given box was not as long as its
offer and marked its parent instead. Under the frame/extent protocol that
is nearly every widget beneath a self-sized container: a span hands its
children its own placement across itself, which is `FULL` while the span
is measured and its answer once it is placed, so the children's offer and
given frames differ on every such axis. A `many` frame at seed 13, depth
8 escalated 43 marks along chains up to seven levels and redrew 508 of
583 active widgets where e44dea3 redraws 159.

Retain the offer's frame beside the given one and ask the offer question
locally: the offer frame composed where the given one is, at the offer's
lengths and placement, then place at the given box where the two differ.
Seed 13 `many` goes from 4.73 ms to 1.28 ms against e44dea3's 0.90, and
294 distinct widgets a frame; size, scroll and repaint are unchanged.

Not sound yet: the suite, the debug oracle and the shrinker at 400 trees
of depth 5 pass, but the oracle at 1000 seeds of depth 6 diverges on seed
532 under reorder and seed 398 under every-size. Both reduce to a
self-sized container whose answer changes under a local redraw; the
reduced plans are in docs/HANDOFF.md of ai-app-2.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-17 20:51:36 -04:00
iris-aiandClaude Opus 5 34cafb6edc Read the marks rather than the queue to decide the walk is done
Review of the two commits above. The queue was the walk's only record of
what was left, so a mark that reached `needs_redraw` without going through
`mark` -- an `on_undraw` handler is the reachable one -- would have waited
for the next frame. The set is read again once the queue drains, which is
what the scan it replaced did for free. `pop_last` takes the deepest entry
in one step rather than reading and then removing it.

The rest is comments: nine lines shorter, and the arm that takes an
ordinary ask said only what it does for a declared length.

Unchanged by all of it: 109 suite and 20 core tests, the four fuzzer runs
(100 seeds, 400 trees at depth 5, 1000 at depth 6, 2000 at depth 4), the
five reference renders and the resized `tabs`, and every counter on the
diagnostics rig.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-17 19:31:29 -04:00
3 changed files with 83 additions and 50 deletions

No files matched your search

+5 -2
View File
@@ -18,6 +18,9 @@ pub struct ActiveData {
/// The original frame in its parent widget's coordinates. Recomposition /// The original frame in its parent widget's coordinates. Recomposition
/// and pixel-length evaluation both follow this chain. /// and pixel-length evaluation both follow this chain.
pub given_region: UiRegion, pub given_region: UiRegion,
/// The frame it was first asked in, in the same coordinates: the offer's
/// frame, which its parent's placing draw may since have narrowed.
pub offer_region: UiRegion,
/// The lengths of the box its parent first asked about it in, as /// The lengths of the box its parent first asked about it in, as
/// lengths of the box the parent was itself offered. Any later box it /// lengths of the box the parent was itself offered. Any later box it
/// was given was decided knowing its answer, so this is the question /// was given was decided knowing its answer, so this is the question
@@ -43,8 +46,8 @@ pub struct ActiveData {
pub mask_region: Option<DrawRegion>, pub mask_region: Option<DrawRegion>,
/// The children whose box is a part of this widget's extent rather than /// The children whose box is a part of this widget's extent rather than
/// of its frame, and which part each was given. Moving the extent /// of its frame, and which part each was given. Moving the extent
/// re-places them, so this widget's drawing does not have to depend on /// re-places them through that part, so the drawing need not depend on
/// where its own drawing sits. /// where it sits.
pub(crate) extent_children: Vec<(WidgetId, ExtentPlacement)>, pub(crate) extent_children: Vec<(WidgetId, ExtentPlacement)>,
pub children: Vec<WidgetId>, pub children: Vec<WidgetId>,
/// The children whose size this widget read while drawing. /// The children whose size this widget read while drawing.
+25 -21
View File
@@ -185,12 +185,10 @@ impl<'a> Painter<'a> {
/// resolves declared lengths and reports against that frame, then places /// resolves declared lengths and reports against that frame, then places
/// its drawing by its own alignment. /// its drawing by its own alignment.
/// ///
/// `DrawRegion::Extent` gives a part of where this widget's own drawing /// `DrawRegion::Extent` gives a part of where this widget's drawing sits
/// sits instead, which is what a container whose children belong inside /// instead, for a container whose children belong inside that rather than
/// its drawing rather than inside the box it was offered wants. The /// inside the box it was offered. The part is what is kept, so moving the
/// child's box then follows the extent without this widget's drawing /// extent re-places the child rather than drawing this widget again.
/// depending on where that extent is, so moving it re-places the child
/// rather than drawing this widget again.
pub fn widget_within<'s, W: ?Sized>( pub fn widget_within<'s, W: ?Sized>(
&'s mut self, &'s mut self,
id: &'s StrongWidget<W>, id: &'s StrongWidget<W>,
@@ -273,6 +271,14 @@ impl<'a> Painter<'a> {
.get(&id.id()) .get(&id.id())
.map_or(given_len, |a| a.offer_len), .map_or(given_len, |a| a.offer_len),
}; };
let offer_region = match first_ask {
true => local,
false => self
.state
.active
.get(&id.id())
.map_or(local, |a| a.offer_region),
};
let offer_placement = if first_ask { let offer_placement = if first_ask {
placement placement
} else { } else {
@@ -297,6 +303,7 @@ impl<'a> Painter<'a> {
region_node, region_node,
mask: self.mask, mask: self.mask,
given_region: local, given_region: local,
offer_region,
offer_len, offer_len,
offer_placement, offer_placement,
px, px,
@@ -322,20 +329,18 @@ impl<'a> Painter<'a> {
result.placement = Some(self.placement); result.placement = Some(self.placement);
} }
} }
// Its box is a part of this widget's extent, so both what // Its box is a part of this widget's extent, so what it
// it was given and what it took of that are ranges on the // holds for is a range on that extent and none of it a
// extent and none of them a range on the frame. That is // range on the frame. Only the part's length reaches it,
// what lets this widget's drawing move without being made // which is what lets the extent move without a redraw.
// again: only the part's length reaches the child, and
// where the part sits is re-placed rather than redrawn.
Some(ExtentPlacement::Within(part)) if declared[n].is_none() => { Some(ExtentPlacement::Within(part)) if declared[n].is_none() => {
result.extent[n] = holds.frame[n] result.extent[n] = holds.frame[n]
.and(holds.extent[n].through(chosen)) .and(holds.extent[n].through(chosen))
.through(part.axis(axis).len()); .through(part.axis(axis).len());
} }
// A declared length is a length of this widget's frame // Its box is a length of this widget's frame: an
// wherever the box it sits in came from, so what the child // ordinary ask, or a declared length, which is that
// holds for is a range on the frame either way. // length wherever the box it sits in came from.
_ => { _ => {
result.frame[n] = holds.frame[n].through(local.axis(axis).len()).and( result.frame[n] = holds.frame[n].through(local.axis(axis).len()).and(
holds.extent[n] holds.extent[n]
@@ -349,12 +354,10 @@ impl<'a> Painter<'a> {
}; };
self.under = self.under.and(in_parent(holds)); self.under = self.under.and(in_parent(holds));
let mut answer_holds = in_parent(answer_holds); let mut answer_holds = in_parent(answer_holds);
// What it reports is a fraction of the box it was given, and that box // What it reports is a fraction of the box it was given, which is a
// is a part of this widget's extent -- so the same fraction is a // part of this widget's extent -- so the same fraction is a different
// different length once the extent is. Only the report: where the // length once that extent is, and pixels are not. The answer only:
// extent moved without changing what it holds, the drawing under it // the drawing this holds is re-placed rather than made again.
// is re-placed rather than made again, which is what the extent ask
// is for. Pixels come up unchanged and say nothing.
if matches!(extent, Some(ExtentPlacement::Within(_))) if matches!(extent, Some(ExtentPlacement::Within(_)))
&& AXES.into_iter().any(|axis| { && AXES.into_iter().any(|axis| {
declared[axis as usize].is_none() && size.axis(axis).rel != crate::Rel::ZERO declared[axis as usize].is_none() && size.axis(axis).rel != crate::Rel::ZERO
@@ -433,6 +436,7 @@ impl<'a> Painter<'a> {
self.offered.push(child.id()); self.offered.push(child.id());
let active = self.state.active.get_mut(&child.id()).unwrap(); let active = self.state.active.get_mut(&child.id()).unwrap();
active.offer_len = local.size(); active.offer_len = local.size();
active.offer_region = local;
active.offer_placement = placement; active.offer_placement = placement;
} }
let placement = UiRegion { let placement = UiRegion {
+53 -27
View File
@@ -24,6 +24,8 @@ pub(super) struct DrawInfo {
pub given_region: UiRegion, pub given_region: UiRegion,
/// The original offer's lengths relative to the parent's own offer. /// The original offer's lengths relative to the parent's own offer.
pub offer_len: UiVec2, pub offer_len: UiVec2,
/// The offer's frame in the parent widget's coordinates.
pub offer_region: UiRegion,
pub offer_placement: [Option<UiSpan>; 2], pub offer_placement: [Option<UiSpan>; 2],
/// This ask's box in pixels, and the offer's: one multiply from the /// This ask's box in pixels, and the offer's: one multiply from the
/// parent's own, which is where every pixel length in layout comes from. /// parent's own, which is where every pixel length in layout comes from.
@@ -70,8 +72,7 @@ pub struct UiRenderState {
/// depths does not pick one up again at its own depth. /// depths does not pick one up again at its own depth.
deferred: crate::util::HashSet<WidgetId>, deferred: crate::util::HashSet<WidgetId>,
/// What the walk has left to settle, deepest last. Ordered rather than /// 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 /// searched for, so finding the next one is not a pass over the marks.
/// the walk runs puts itself in place.
pending: std::collections::BTreeSet<(usize, WidgetId)>, pending: std::collections::BTreeSet<(usize, WidgetId)>,
pub moves: Moves, pub moves: Moves,
} }
@@ -135,6 +136,7 @@ impl UiRenderState {
region_node: false, region_node: false,
mask: MaskIdx::NONE, mask: MaskIdx::NONE,
given_region: region, given_region: region,
offer_region: region,
offer_len: UiVec2::FULL_SIZE, offer_len: UiVec2::FULL_SIZE,
offer_placement: [None; 2], offer_placement: [None; 2],
px, px,
@@ -275,6 +277,7 @@ impl UiRenderState {
// same question again from these. // same question again from these.
active.region = region; active.region = region;
active.given_region = info.given_region; active.given_region = info.given_region;
active.offer_region = info.offer_region;
active.offer_len = info.offer_len; active.offer_len = info.offer_len;
if info.placement == info.offer_placement && info.px == info.offered_px { if info.placement == info.offer_placement && info.px == info.offered_px {
active.answer = Some(answer); active.answer = Some(answer);
@@ -479,6 +482,7 @@ impl UiRenderState {
region_node: false, region_node: false,
mask, mask,
given_region: UiRegion::FULL, given_region: UiRegion::FULL,
offer_region: UiRegion::FULL,
offer_len: UiVec2::FULL_SIZE, offer_len: UiVec2::FULL_SIZE,
offer_placement: [None; 2], offer_placement: [None; 2],
px, px,
@@ -496,6 +500,7 @@ impl UiRenderState {
region, region,
placement, placement,
given_region: info.given_region, given_region: info.given_region,
offer_region: info.offer_region,
offer_len: info.offer_len, offer_len: info.offer_len,
offer_placement: info.offer_placement, offer_placement: info.offer_placement,
// Whoever asked writes the answer, if this was the asking. // Whoever asked writes the answer, if this was the asking.
@@ -725,6 +730,7 @@ impl UiRenderState {
let active = self.active.get_mut(&id).unwrap(); let active = self.active.get_mut(&id).unwrap();
active.region = region; active.region = region;
active.given_region = info.given_region; active.given_region = info.given_region;
active.offer_region = info.offer_region;
active.offer_len = info.offer_len; active.offer_len = info.offer_len;
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
{ {
@@ -796,6 +802,7 @@ impl UiRenderState {
region_node: active.move_idx != active.parent_move, region_node: active.move_idx != active.parent_move,
mask, mask,
given_region: child_local, given_region: child_local,
offer_region: active.offer_region,
offer_len: active.offer_len, offer_len: active.offer_len,
offer_placement: active.offer_placement, offer_placement: active.offer_placement,
px: child_local.size().to_px(info.px), px: child_local.size().to_px(info.px),
@@ -935,6 +942,7 @@ impl UiRenderState {
region: UiRegion::FULL, region: UiRegion::FULL,
placement: UiRegion::FULL, placement: UiRegion::FULL,
given_region: UiRegion::FULL, given_region: UiRegion::FULL,
offer_region: UiRegion::FULL,
offer_len: UiVec2::FULL_SIZE, offer_len: UiVec2::FULL_SIZE,
offer_placement: [None; 2], offer_placement: [None; 2],
answer: None, answer: None,
@@ -999,20 +1007,29 @@ impl UiRenderState {
// something below is about to change it -- which is the whole class // something below is about to change it -- which is the whole class
// of defect where a widget settles inside its parent's draw, clears // of defect where a widget settles inside its parent's draw, clears
// its mark there, and tells nobody its answer moved. // its mark there, and tells nobody its answer moved.
let marked: Vec<WidgetId> = rsc.widgets().needs_redraw.iter().copied().collect(); // The queue is that set, ordered: a mark made while the walk runs
for id in marked { // queues itself through `mark`. What ends the walk is still the set
// being spent, not the queue, so a mark that reached it another way
// cannot be left for the next frame.
loop {
for &id in rsc.widgets().needs_redraw.iter() {
if !self.deferred.contains(&id) {
let depth = self.depth(id); let depth = self.depth(id);
self.pending.insert((depth, id)); self.pending.insert((depth, id));
} }
while let Some(&(depth, id)) = self.pending.last() { }
self.pending.remove(&(depth, id)); if self.pending.is_empty() {
// A widget settled inside an ancestor's draw, or deferred to one, break;
// is left here by the mark that queued it. }
while let Some((depth, id)) = self.pending.pop_last() {
// Settled inside an ancestor's draw, or deferred to one,
// since the mark that queued it.
if self.deferred.contains(&id) || !rsc.widgets().needs_redraw.contains(&id) { if self.deferred.contains(&id) || !rsc.widgets().needs_redraw.contains(&id) {
continue; continue;
} }
// A subtree that changed hands takes its descendants' depths with // A subtree that changed hands takes its descendants' depths
// it, so an entry queued before that move names the wrong one. // with it, so an entry queued before that move names the
// depth it had under the parent it left.
let now = self.depth(id); let now = self.depth(id);
if now != depth { if now != depth {
self.pending.insert((now, id)); self.pending.insert((now, id));
@@ -1024,12 +1041,11 @@ impl UiRenderState {
self.deferred.insert(id); self.deferred.insert(id);
} }
} }
}
self.deferred.clear(); self.deferred.clear();
} }
/// Marks a widget for the walk to settle. Every mark made while a frame /// Marks a widget for the walk to settle, and queues it at its depth.
/// 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) { fn mark(&mut self, id: WidgetId, widgets: &mut Widgets) {
if widgets.needs_redraw.insert(id) && !self.deferred.contains(&id) { if widgets.needs_redraw.insert(id) && !self.deferred.contains(&id) {
let depth = self.depth(id); let depth = self.depth(id);
@@ -1163,16 +1179,6 @@ impl UiRenderState {
return true; 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
// its parent asked only while that box is as long as the offer. Any
// other box is a different question, so the parent asks it, with the
// 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 {
self.mark(id, rsc.widgets_mut());
self.mark(parent, rsc.widgets_mut());
return false;
}
let info = DrawInfo { let info = DrawInfo {
layer: active.layer, layer: active.layer,
parent: active.parent, parent: active.parent,
@@ -1181,6 +1187,7 @@ impl UiRenderState {
region_node: rsc.widgets().is_region_node(id), region_node: rsc.widgets().is_region_node(id),
mask: active.parent_mask, mask: active.parent_mask,
given_region: active.given_region, given_region: active.given_region,
offer_region: active.offer_region,
offer_len: active.offer_len, offer_len: active.offer_len,
offer_placement: active.offer_placement, offer_placement: active.offer_placement,
px: given_px, px: given_px,
@@ -1195,14 +1202,33 @@ impl UiRenderState {
diag::bump(Counter::LocalRedraws); diag::bump(Counter::LocalRedraws);
let old = self.remove(id, false, rsc); let old = self.remove(id, false, rsc);
// Refresh the original measurement before restoring the assigned slot. // Asked again where its parent asked: the offer's frame, composed
// Its lengths may differ even though the fraction reference is unchanged. // where the given one is, at the offer's lengths and placement. That
// is the question its answer came from, whatever box the parent then
// chose from the answer -- which is often a different frame, since a
// span hands its children its own placement across itself. The
// parent draws in its own frame, or in `FULL` where it is a region
// node.
let parent_frame = match self.active.get(&parent) {
Some(p) if p.move_idx == p.parent_move => p.region,
_ => UiRegion::FULL,
};
let offer_frame = match info.offer_region == UiRegion::FULL {
true => parent_frame,
false => info.offer_region.within(&parent_frame),
};
let offered = DrawInfo { let offered = DrawInfo {
placement: info.offer_placement, placement: info.offer_placement,
given_region: info.offer_region,
px: offered_px,
..info ..info
}; };
let answer = self.draw_inner(id, given, offered, old, false, rsc); // Where the given differs from the offer, the first draw is only the
if info.placement != offered.placement { // measurement and the second puts the drawing where the parent did.
let placed_apart =
info.placement != offered.placement || info.px != offered.px || given != offer_frame;
let answer = self.draw_inner(id, offer_frame, offered, old, placed_apart, rsc);
if placed_apart {
self.draw_inner(id, given, info, None, false, rsc); self.draw_inner(id, given, info, None, false, rsc);
} }
let active = self.active.get_mut(&id).unwrap(); let active = self.active.get_mut(&id).unwrap();