diff --git a/core/src/ui/active.rs b/core/src/ui/active.rs index 826f7f4..b5f04a9 100644 --- a/core/src/ui/active.rs +++ b/core/src/ui/active.rs @@ -43,8 +43,8 @@ pub struct ActiveData { pub mask_region: Option, /// 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, /// The children whose size this widget read while drawing. diff --git a/core/src/ui/painter.rs b/core/src/ui/painter.rs index d256d89..c6b06a7 100644 --- a/core/src/ui/painter.rs +++ b/core/src/ui/painter.rs @@ -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, @@ -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 diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index b62d68b..00359e1 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -70,8 +70,7 @@ pub struct UiRenderState { /// depths does not pick one up again at its own depth. deferred: crate::util::HashSet, /// 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 = 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);