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>
This commit is contained in:
1 parent
3bf22935ce
commit
34cafb6edc
3 files changed
+50
-49
No files matched your search
+33
-26
@@ -70,8 +70,7 @@ pub struct UiRenderState {
|
||||
/// depths does not pick one up again at its own depth.
|
||||
deferred: crate::util::HashSet<WidgetId>,
|
||||
/// 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.
|
||||
/// searched for, so finding the next one is not a pass over the marks.
|
||||
pending: std::collections::BTreeSet<(usize, WidgetId)>,
|
||||
pub moves: Moves,
|
||||
}
|
||||
@@ -999,37 +998,45 @@ 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.
|
||||
let marked: Vec<WidgetId> = 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;
|
||||
// The queue is that set, ordered: a mark made while the walk runs
|
||||
// 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);
|
||||
self.pending.insert((depth, id));
|
||||
}
|
||||
}
|
||||
// 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;
|
||||
if self.pending.is_empty() {
|
||||
break;
|
||||
}
|
||||
#[cfg(feature = "layout-diagnostics")]
|
||||
diag::bump(Counter::QueuePops);
|
||||
if !self.redraw(id, rsc) {
|
||||
self.deferred.insert(id);
|
||||
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) {
|
||||
continue;
|
||||
}
|
||||
// A subtree that changed hands takes its descendants' depths
|
||||
// with it, so an entry queued before that move names the
|
||||
// depth it had under the parent it left.
|
||||
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) {
|
||||
self.deferred.insert(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
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.
|
||||
/// Marks a widget for the walk to settle, and queues it at its depth.
|
||||
fn mark(&mut self, id: WidgetId, widgets: &mut Widgets) {
|
||||
if widgets.needs_redraw.insert(id) && !self.deferred.contains(&id) {
|
||||
let depth = self.depth(id);
|
||||
|
||||
Reference in new issue
Block a user