Compare commits

...
2 Commits
Author SHA1 Message Date
iris-ai e44dea34b4 Record which widget is drawing a subtree that changed hands
A subtree can be reused whole under a different parent -- same box, same
layer, same region node, clean -- and nothing in the drawing says it
moved. Two things read who its parent is, and both were wrong after one
of these.

The old parent still listed it as a child, and a parent's next draw
undraws whatever is missing from that list: two spans under one root,
with the root swapping which of them it holds, drew the subtree under
the new span and then erased it when the old one drew. The move is
recorded on both sides where `draw_inner` already writes what the ask
decided, rather than guarded at each reader.

Its depth was also the one it had under the old parent, which is what
the settling walk orders by, so a change made under it afterwards
settled at the wrong point in the frame. `try_reuse` re-walks the
subtree's depths, and only where the top of it moved, which is what
makes that free in the ordinary case.

Two tests: one shape where the subtree's box does not move and the span
it left erases it, one where it changes depth and the change made under
it has to reach the span it moved to. Each fails without one half.
2026-09-17 13:05:15 -04:00
iris-ai a0693acc56 Let a resize settle through the walk, and drop the stale-answer guard
A resize drew the root outside `redraw_updates`, top-down over a tree
with dirty widgets still in it, which is the one entry point
`dirty_size_under` was guarding: since `a92c6ac` settles a frame strictly
bottom-up, no fuzzer could tell whether that guard still did anything
anywhere else. Closing the entry point retires the guard rather than
keeping a check for a hole reasoned rather than measured.

The root is marked instead, and only where the new output falls outside
what its answer holds for. That range is the intersection of everything
under it, so admitting the new output says the whole tree still stands,
and nothing above the root moved -- the window is no entry to rewrite.
Marking it unconditionally would have cost the root its own `Holds`: a
leaf root that scales with its box was drawn again on every resize.

`dirty_size_under` goes at both call sites. `resize` takes `Widgets`
because a mark is what it now leaves behind.
2026-09-17 13:00:32 -04:00
5 changed files with 177 additions and 52 deletions

No files matched your search

+8
View File
@@ -74,4 +74,12 @@ impl ActiveData {
pub fn holds_at(&self, px: crate::PxVec2) -> bool { pub fn holds_at(&self, px: crate::PxVec2) -> bool {
self.holds[0].contains(px.x) && self.holds[1].contains(px.y) self.holds[0].contains(px.x) && self.holds[1].contains(px.y)
} }
/// Whether what it answered still stands for a box of these pixel
/// lengths -- the box it was asked in, where `holds` is about the box its
/// answer then chose.
pub fn answers_at(&self, px: crate::PxVec2) -> bool {
let (_, holds) = self.answer;
holds[0].contains(px.x) && holds[1].contains(px.y)
}
} }
+56 -48
View File
@@ -43,7 +43,9 @@ pub struct UiRenderState {
old_root: Option<WidgetId>, old_root: Option<WidgetId>,
/// Whether the output has changed since the last update. A frame is /// Whether the output has changed since the last update. A frame is
/// owed for that whether or not anything has to be drawn again. /// owed for that whether or not anything has to be drawn again: every
/// fraction becomes pixels against the output, in the shader's uniform
/// as well as here.
resized: bool, resized: bool,
/// A widget's move slot, which outlives any one `ActiveData`: a redraw /// A widget's move slot, which outlives any one `ActiveData`: a redraw
/// replaces that while its children go on pointing at the slot. /// replaces that while its children go on pointing at the slot.
@@ -83,13 +85,28 @@ impl UiRenderState {
/// size is applied where a fraction becomes pixels -- here in `to_px`, /// size is applied where a fraction becomes pixels -- here in `to_px`,
/// and in the shader by its uniform. A resize therefore rewrites no /// and in the shader by its uniform. A resize therefore rewrites no
/// retained entry at all. /// retained entry at all.
pub fn resize(&mut self, size: impl Into<Vec2>) { ///
/// The root is the only widget a resize marks, and only where the new
/// output falls outside what its answer holds for: that range is the
/// intersection of everything under it, so admitting the new output says
/// the whole tree still stands. Where it does not, the ordinary walk
/// draws the root, and each widget's own range decides how far down the
/// new length reaches.
pub fn resize(&mut self, size: impl Into<Vec2>, widgets: &mut Widgets) {
let size = PxVec2::from_f32(size.into()); let size = PxVec2::from_f32(size.into());
if size == self.output_size { if size == self.output_size {
return; return;
} }
self.output_size = size; self.output_size = size;
self.resized = true; self.resized = true;
let Some(root) = self.old_root else { return };
let stands = self
.active
.get(&root)
.is_some_and(|active| active.answers_at(active.given_len.to_px(size)));
if !stands {
widgets.needs_redraw.insert(root);
}
} }
/// The root is asked about in the output: the window is where a fraction /// The root is asked about in the output: the window is where a fraction
@@ -143,17 +160,6 @@ impl UiRenderState {
if self.root_changed(root) { if self.root_changed(root) {
self.redraw_all(root, rsc); self.redraw_all(root, rsc);
self.old_root = root.map(|r| r.id()); self.old_root = root.map(|r| r.id());
} else if let Some(root) = root
&& self.resized
{
// The output is the root's box, so a resize is that box changing
// length, found the way every other box change is found. Before
// anything dirty settles, so that whatever a new output draws
// again is drawn once, in the box it will have.
let region = Self::root_region(root.id(), rsc.widgets());
let info = self.root_info(region);
let answer = self.draw_inner(root.id(), region, info, None, rsc);
self.active.get_mut(&root.id()).unwrap().answer = answer;
} }
self.resized = false; self.resized = false;
if rsc.widgets().has_updates() { if rsc.widgets().has_updates() {
@@ -196,22 +202,10 @@ impl UiRenderState {
diag::draw_request(id, info.parent, region, info.px, info.region_node); diag::draw_request(id, info.parent, region, info.px, info.region_node);
} }
let align = rsc.widgets().alignment(id); let align = rsc.widgets().alignment(id);
// Nothing this widget has is an answer while something it measured // Nothing this widget measured can be dirty while it draws: layout is
// is dirty: settling that changes what it would report, and a widget // one bottom-up walk, so anything deeper has settled or deferred to
// settled inside its parent's draw tells nobody -- the comparison // its own parent, and a deferred one leaves that parent marked.
// that marks a reader is in `redraw`, which is not what asked here. let stale = rsc.widgets().needs_redraw.contains(&id);
// Both retained routes are an answer, so the question is asked once
// rather than by each of them.
//
// Since `a92c6ac` settles a frame strictly bottom-up, no fuzzer can
// tell whether the second half of this still does anything: dropping
// `dirty_size_under` passes the suite, the shrinker at 400 seeds of
// depth 5, the oracle at 1000 of depth 6 and 2000 seeds at depth 4.
// It stays because `update` draws the root for a resize before
// `redraw_updates` runs at all, which that ordering does not reach --
// a hole that is reasoned rather than measured.
let stale =
rsc.widgets().needs_redraw.contains(&id) || self.dirty_size_under(id, rsc.widgets());
let replace_answer = self.answer_invalid.remove(&id) || (self.replace_answers && stale); let replace_answer = self.answer_invalid.remove(&id) || (self.replace_answers && stale);
let retained = match replace_answer || stale { let retained = match replace_answer || stale {
true => None, true => None,
@@ -259,7 +253,18 @@ impl UiRenderState {
active.answer = settled; active.answer = settled;
active.decided = info.decided; active.decided = info.decided;
active.own_align = align; active.own_align = align;
active.depth = info.depth; // A subtree can be reused whole under a different parent -- same box,
// same layer, same region node -- and nothing in the drawing says it
// changed hands. Two things read who its parent is: a deferral, which
// marks whoever has it to draw, and the old parent's list of children,
// which its next draw undraws whatever is missing from.
let old_parent = std::mem::replace(&mut active.parent, info.parent);
if old_parent != info.parent
&& let Some(old_parent) = old_parent
&& let Some(old_parent) = self.active.get_mut(&old_parent)
{
old_parent.children.retain(|child| *child != id);
}
settled settled
} }
@@ -492,7 +497,7 @@ impl UiRenderState {
parent_move: MoveIdx, parent_move: MoveIdx,
widgets: &Widgets, widgets: &Widgets,
) -> Option<(Size, [Holds; 2])> { ) -> Option<(Size, [Holds; 2])> {
if widgets.needs_redraw.contains(&id) || self.dirty_size_under(id, widgets) { if widgets.needs_redraw.contains(&id) {
return None; return None;
} }
let active = self.active.get(&id)?; let active = self.active.get(&id)?;
@@ -517,22 +522,7 @@ impl UiRenderState {
{ {
return None; return None;
} }
let (size, holds) = active.answer; active.answers_at(info.px).then_some(active.answer)
(holds[0].contains(info.px.x) && holds[1].contains(info.px.y)).then_some((size, holds))
}
/// Whether anything whose size this widget's own size was read from is
/// dirty, which makes what it would answer not yet known. It also keeps
/// a reader that asks first from laying out twice, which is all it was
/// here for while a changed size was thought to reach its reader in any
/// order; it does not, where the change settles inside the reader's own
/// draw.
fn dirty_size_under(&self, id: WidgetId, widgets: &Widgets) -> bool {
self.active.get(&id).is_some_and(|active| {
active.size_deps.iter().any(|child| {
widgets.needs_redraw.contains(child) || self.dirty_size_under(*child, widgets)
})
})
} }
/// The pixel lengths of the box a widget was given and of the box it was /// The pixel lengths of the box a widget was given and of the box it was
@@ -646,12 +636,12 @@ impl UiRenderState {
self.remap_subtree(id, &remap, info.parent_move, rsc); self.remap_subtree(id, &remap, info.parent_move, rsc);
} }
} }
self.redepth(id, info.depth);
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; active.given = region;
active.given_len = info.given_len; active.given_len = info.given_len;
active.offer_len = info.offer_len; active.offer_len = info.offer_len;
active.depth = info.depth;
#[cfg(feature = "layout-diagnostics")] #[cfg(feature = "layout-diagnostics")]
{ {
match (moved, has_region_node) { match (moved, has_region_node) {
@@ -675,6 +665,24 @@ impl UiRenderState {
Some(answer) Some(answer)
} }
/// A reused subtree keeps its shape, so every widget in it moves by the
/// same amount -- and where the top of it did not move, none of it did,
/// which is what makes this free in the ordinary case.
fn redepth(&mut self, id: WidgetId, depth: usize) {
let Some(active) = self.active.get_mut(&id) else {
return;
};
if active.depth == depth {
return;
}
active.depth = depth;
let children = active.children.len();
for index in 0..children {
let child = self.active[&id].children[index];
self.redepth(child, depth + 1);
}
}
/// Re-expresses an ordinary retained subtree in a new parent region. /// Re-expresses an ordinary retained subtree in a new parent region.
/// An independently movable descendant needs only its own region changed; /// An independently movable descendant needs only its own region changed;
/// its contents stay in that region's coordinate space. /// its contents stay in that region's coordinate space.
+1 -1
View File
@@ -251,7 +251,7 @@ impl<State: DefaultAppState> AppState for DefaultApp<State> {
ui_state.renderer.draw(); ui_state.renderer.draw();
} }
WindowEvent::Resized(size) => { WindowEvent::Resized(size) => {
render.resize((size.width, size.height)); render.resize((size.width, size.height), rsc.widgets_mut());
ui_state.renderer.resize(size) ui_state.renderer.resize(size)
} }
WindowEvent::KeyboardInput { event, .. } => { WindowEvent::KeyboardInput { event, .. } => {
+3 -3
View File
@@ -144,9 +144,9 @@ impl Harness {
// bound that comes with `SyncSender` is far past anything a test // bound that comes with `SyncSender` is far past anything a test
// leaves unread. // leaves unread.
let (send, updates) = sync_channel(1024); let (send, updates) = sync_channel(1024);
let rsc = DefaultRsc::init(Arc::new(Queue(send))); let mut rsc = DefaultRsc::init(Arc::new(Queue(send)));
let mut render = UiRenderState::new(); let mut render = UiRenderState::new();
render.resize(size); render.resize(size, rsc.widgets_mut());
Self { Self {
rsc, rsc,
render, render,
@@ -161,7 +161,7 @@ impl Harness {
} }
pub fn resize(&mut self, size: impl Into<Vec2>) { pub fn resize(&mut self, size: impl Into<Vec2>) {
self.render.resize(size); self.render.resize(size, self.rsc.widgets_mut());
} }
/// Changes a length rule after the fact, the way `.width()` sets one. /// Changes a length rule after the fact, the way `.width()` sets one.
+109
View File
@@ -628,3 +628,112 @@ fn a_masked_widget_redrawn_on_its_own_sets_its_mask_again() {
h.frame(); h.frame();
assert_corners!(h, inner, (100, 0), (400, 200)); assert_corners!(h, inner, (100, 0), (400, 200));
} }
/// The two spans a subtree changes hands between, and the branch that is not
/// in the tree yet -- kept alive by the test until it is.
struct Handover {
leaf: WidgetId,
first: WeakWidget<Span>,
second: WeakWidget<Span>,
root: WeakWidget<Span>,
spare: StrongWidget,
}
/// A subtree that changes hands while its box does not move, so nothing about
/// reusing its drawing says it changed parents. `deeper` puts a span between
/// the root and `second`, so it changes depth by changing hands as well.
fn plant_handover(h: &mut Harness, moved: bool, deeper: bool, width: f32) -> Handover {
let leaf = rect(Color::RED).add(&mut h.rsc);
let sized = leaf.width(width).add(&mut h.rsc);
let holder = (sized,).span(Dir::RIGHT).add(&mut h.rsc);
let first = Span {
children: match moved {
true => Vec::new(),
false => vec![holder.add_strong(&mut h.rsc)],
},
dir: Dir::RIGHT,
gap: Px::ZERO,
}
.add(&mut h.rsc);
let second = Span {
children: match moved {
true => vec![holder.add_strong(&mut h.rsc)],
false => Vec::new(),
},
dir: Dir::RIGHT,
gap: Px::ZERO,
}
.add(&mut h.rsc);
let branch = match deeper {
true => (second,).span(Dir::RIGHT).add_strong(&mut h.rsc),
false => second.add_strong(&mut h.rsc),
};
let (in_tree, spare) = match moved {
true => (branch, first.add_strong(&mut h.rsc)),
false => (first.add_strong(&mut h.rsc), branch),
};
let root = Span {
children: vec![in_tree],
dir: Dir::RIGHT,
gap: Px::ZERO,
}
.add(&mut h.rsc);
h.state.root = Some(root.add_strong(&mut h.rsc));
Handover {
leaf: sized.id(),
first,
second,
root,
spare,
}
}
/// Moves the subtree and swaps the branch it sits in for the one it left.
fn hand_over(h: &mut Harness, tree: Handover) -> WidgetId {
let holder = h.rsc[tree.first].children.remove(0);
h.rsc[tree.second].children.push(holder);
h.rsc[tree.root].children.clear();
h.rsc[tree.root].children.push(tree.spare);
h.frame();
tree.leaf
}
#[test]
fn a_subtree_that_changed_parents_is_not_undrawn_by_the_one_it_left() {
let mut warm = Harness::new((400, 200));
let tree = plant_handover(&mut warm, false, false, 40.0);
warm.frame();
let leaf = hand_over(&mut warm, tree);
let mut cold = Harness::new((400, 200));
let grown = plant_handover(&mut cold, true, false, 40.0);
cold.frame();
assert_eq!(
warm.region(&leaf),
cold.region(&grown.leaf),
"the span it left still listed it and undrew it"
);
}
#[test]
fn a_subtree_that_changed_parents_settles_at_the_depth_it_moved_to() {
let mut warm = Harness::new((400, 200));
let tree = plant_handover(&mut warm, false, true, 40.0);
warm.frame();
let leaf = hand_over(&mut warm, tree);
// After it has changed hands, so what has to reach the new parent is a
// change made under the subtree it now holds.
warm.set_len(leaf, Axis::X, LayoutLen::px(90.0));
warm.frame();
let mut cold = Harness::new((400, 200));
let grown = plant_handover(&mut cold, true, true, 90.0);
cold.frame();
assert_eq!(
warm.region(&leaf),
cold.region(&grown.leaf),
"the span it moved to is the one the change has to reach"
);
}