diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index 48e2aea..84ac0d9 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -55,6 +55,9 @@ pub struct UiRenderState { /// Whether this frame contains a declared-length change, so any dirty /// dependent replaces its answer too. 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, pub moves: Moves, } @@ -68,6 +71,7 @@ impl UiRenderState { slots: Default::default(), answer_invalid: Default::default(), replace_answers: false, + deferred: Default::default(), moves: Default::default(), resized: false, } @@ -827,18 +831,33 @@ impl UiRenderState { pub fn redraw_updates(&mut self, rsc: &mut dyn UiRsc) { #[cfg(feature = "layout-diagnostics")] let _layout = diag::timer(TimerKind::IncrementalLayout); - // Deepest first: a reader whose children have all settled asks each - // once, where any other order has it lay out again for whatever - // settles under it afterwards. Equal-depth widgets are independent, - // so their order does not matter. - while let Some(id) = { - let dirty = rsc.widgets().needs_redraw.iter().copied(); - dirty.max_by_key(|&id| self.depth(id)) - } { + // Deepest first, and strictly: a widget that cannot settle where it + // is defers to its parent rather than drawing the parent from + // inside itself. It marks the parent, stays marked, and waits here + // until the walk reaches its parent's depth. + // + // What that buys is that nothing shallower is ever drawn while + // 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")] 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 { @@ -923,11 +942,13 @@ impl UiRenderState { } /// Settles a dirty widget: asks it again where its parent asked, and - /// tells the parent if the answer changed. - pub fn redraw(&mut self, id: WidgetId, rsc: &mut dyn UiRsc) { + /// tells the parent if the answer changed. `false` where the question is + /// 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); let Some(active) = self.active.get(&id) else { - return; + return true; }; // 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 @@ -947,14 +968,15 @@ impl UiRenderState { 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); - self.redraw(parent, rsc); - // Whatever the parent did not draw again is nothing it holds now. - rsc.widgets_mut().needs_redraw.remove(&id); - return; + rsc.widgets_mut().needs_redraw.insert(parent); + return false; } if !active.drawn { - return; + return true; } // 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 @@ -969,7 +991,7 @@ impl UiRenderState { diag::bump(Counter::LocalRedraws); let old = self.remove(id, false, rsc); self.draw_inner(id, region, info, old, rsc); - return; + return true; }; let (given_px, offered_px) = self.asked_px(id); // 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. if given_px != offered_px { rsc.widgets_mut().needs_redraw.insert(id); - self.redraw(parent, rsc); - rsc.widgets_mut().needs_redraw.remove(&id); - return; + rsc.widgets_mut().needs_redraw.insert(parent); + return false; } let info = DrawInfo { layer: active.layer, @@ -1014,6 +1035,7 @@ impl UiRenderState { } rsc.widgets_mut().needs_redraw.insert(parent); } + true } }