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
+37
-36
No files matched your search
@@ -43,8 +43,8 @@ pub struct ActiveData {
|
||||
pub mask_region: Option<DrawRegion>,
|
||||
/// 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
|
||||
/// re-places them, so this widget's drawing does not have to depend on
|
||||
/// where its own drawing sits.
|
||||
/// re-places them through that part, so the drawing need not depend on
|
||||
/// where it sits.
|
||||
pub(crate) extent_children: Vec<(WidgetId, ExtentPlacement)>,
|
||||
pub children: Vec<WidgetId>,
|
||||
/// The children whose size this widget read while drawing.
|
||||
|
||||
+15
-21
@@ -185,12 +185,10 @@ impl<'a> Painter<'a> {
|
||||
/// resolves declared lengths and reports against that frame, then places
|
||||
/// its drawing by its own alignment.
|
||||
///
|
||||
/// `DrawRegion::Extent` gives a part of where this widget's own drawing
|
||||
/// sits instead, which is what a container whose children belong inside
|
||||
/// its drawing rather than inside the box it was offered wants. The
|
||||
/// child's box then follows the extent without this widget's drawing
|
||||
/// depending on where that extent is, so moving it re-places the child
|
||||
/// rather than drawing this widget again.
|
||||
/// `DrawRegion::Extent` gives a part of where this widget's drawing sits
|
||||
/// instead, for a container whose children belong inside that rather than
|
||||
/// inside the box it was offered. The part is what is kept, so moving the
|
||||
/// extent re-places the child rather than drawing this widget again.
|
||||
pub fn widget_within<'s, W: ?Sized>(
|
||||
&'s mut self,
|
||||
id: &'s StrongWidget<W>,
|
||||
@@ -322,20 +320,18 @@ impl<'a> Painter<'a> {
|
||||
result.placement = Some(self.placement);
|
||||
}
|
||||
}
|
||||
// Its box is a part of this widget's extent, so both what
|
||||
// it was given and what it took of that are ranges on the
|
||||
// extent and none of them a range on the frame. That is
|
||||
// what lets this widget's drawing move without being made
|
||||
// again: only the part's length reaches the child, and
|
||||
// where the part sits is re-placed rather than redrawn.
|
||||
// Its box is a part of this widget's extent, so what it
|
||||
// holds for is a range on that extent and none of it a
|
||||
// range on the frame. Only the part's length reaches it,
|
||||
// which is what lets the extent move without a redraw.
|
||||
Some(ExtentPlacement::Within(part)) if declared[n].is_none() => {
|
||||
result.extent[n] = holds.frame[n]
|
||||
.and(holds.extent[n].through(chosen))
|
||||
.through(part.axis(axis).len());
|
||||
}
|
||||
// A declared length is a length of this widget's frame
|
||||
// wherever the box it sits in came from, so what the child
|
||||
// holds for is a range on the frame either way.
|
||||
// Its box is a length of this widget's frame: an
|
||||
// ordinary ask, or a declared length, which is that
|
||||
// length wherever the box it sits in came from.
|
||||
_ => {
|
||||
result.frame[n] = holds.frame[n].through(local.axis(axis).len()).and(
|
||||
holds.extent[n]
|
||||
@@ -349,12 +345,10 @@ impl<'a> Painter<'a> {
|
||||
};
|
||||
self.under = self.under.and(in_parent(holds));
|
||||
let mut answer_holds = in_parent(answer_holds);
|
||||
// What it reports is a fraction of the box it was given, and that box
|
||||
// is a part of this widget's extent -- so the same fraction is a
|
||||
// different length once the extent is. Only the report: where the
|
||||
// extent moved without changing what it holds, the drawing under it
|
||||
// is re-placed rather than made again, which is what the extent ask
|
||||
// is for. Pixels come up unchanged and say nothing.
|
||||
// What it reports is a fraction of the box it was given, which is a
|
||||
// part of this widget's extent -- so the same fraction is a different
|
||||
// length once that extent is, and pixels are not. The answer only:
|
||||
// the drawing this holds is re-placed rather than made again.
|
||||
if matches!(extent, Some(ExtentPlacement::Within(_)))
|
||||
&& AXES.into_iter().any(|axis| {
|
||||
declared[axis as usize].is_none() && size.axis(axis).rel != crate::Rel::ZERO
|
||||
|
||||
+20
-13
@@ -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,20 +998,29 @@ 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 {
|
||||
// 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));
|
||||
}
|
||||
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.pending.is_empty() {
|
||||
break;
|
||||
}
|
||||
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 wrong one.
|
||||
// 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));
|
||||
@@ -1024,12 +1032,11 @@ impl UiRenderState {
|
||||
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