Hold a bound's length where it is decided, and say each thing once

A quality sweep over the deferred request system, which no earlier round
has reviewed.

`Bound::outside` said which end a length fell outside and left the caller
to look that end up through `Bound::at`, which `expect`s an end the value
it is given does not promise: only the pairing of the two calls kept
`at(Shorter)` off a bound with no floor. It already had the length in
hand, so it returns that, and `Outside` and `at` go with the state that
could panic.

`measured_request` pinned the rel base for any bound at all, so a measured
share under a cap in pixels was invalidated by a change to a base its
answer cannot depend on. That question is `Bound::has_fraction` now, which
is also the one `Placing::ask` and `SizeRule::has_fraction` were each
writing out over a bare array.

The rest is one name where there were several spellings: `Span::gaps`,
`Padding::along`, `Plan::drop_bounds` behind one `IRIS_UNBOUNDED` in both
rigs that had grown their own, and `Stack::size_request` resolving its
sizing child the way its draw already does. `Span`'s placement loop asked
three times whether the row was allocated, twice to decide one child's
length; one match answers all three, so the allocated and plain rules are
read side by side.

The buffers `draw_at` now reuses for their capacity are empty only because
every path to it drains them in `remove`; a `debug_assert` says so, since
a drawing over primitives left in one would record them twice.

Comments: `with_requests` named discovery as the hazard where it is a
child drawn mid-row, `Painter::allocate` documented the window it holds
for rather than what it does, `minimum_request` had none, and the note
saying a span carries its children's weight whole -- which is still what
the unallocated path does, and still the surprising part -- had been
replaced by one about the other path.

core/src/ui/holds.rs

@@ -1,4 +1,4 @@
-use crate::{Bound, Len, Outside, Px, REL_SHIFT, fixed::div_toward, fixed::narrow};
+use crate::{Bound, Len, Px, REL_SHIFT, fixed::div_toward, fixed::narrow};
 use std::ops::RangeInclusive;
 
 /// The lengths of a box, in pixels, that one drawing of a widget holds for:
@@ -40,40 +40,39 @@ impl Len {
 }
 
 impl Bound {
-    /// Which end of this bound `len` falls outside, and the windows that
-    /// answer holds for. Nothing where it is inside, which is the answer
-    /// wherever there is no bound at all.
+    /// The end of this bound `len` falls outside, which is the length it
+    /// gets instead of its own, and the windows that answer holds for.
+    /// Nothing where it is inside, which is the answer wherever there is no
+    /// bound at all.
     ///
     /// `len` and this bound are lengths of the same thing, whichever that
     /// is: a box in window lengths wants the bound resolved, and a length a
     /// widget declares of its rel base wants it as the rule wrote it. Both
     /// comparisons are in pixels, so each is a question about this window,
     /// and the box is decided again on the other side of a crossing.
-    pub fn outside(&self, len: Len, window: Px) -> (Option<Outside>, Holds) {
-        let mut outside = None;
+    pub fn outside(&self, len: Len, window: Px) -> (Option<Len>, Holds) {
+        let mut held = None;
         let mut holds = Holds::ANY;
-        let mut held = len;
         if let Some(min) = self.min {
-            let (shorter, kept) = min.longer_than(held, window);
+            let (shorter, kept) = min.longer_than(len, window);
             holds = holds.and(kept);
             if shorter {
-                outside = Some(Outside::Shorter);
-                held = min;
+                held = Some(min);
             }
         }
         if let Some(max) = self.max {
-            let (longer, kept) = held.longer_than(max, window);
+            let (longer, kept) = held.unwrap_or(len).longer_than(max, window);
             holds = holds.and(kept);
             if longer {
                 debug_assert!(
-                    outside.is_none(),
+                    held.is_none(),
                     "a floor of {:?} over a cap of {max:?} bounds nothing",
                     self.min,
                 );
-                outside = Some(Outside::Longer);
+                held = Some(max);
             }
         }
-        (outside, holds)
+        (held, holds)
     }
 }
 

core/src/ui/painter.rs

@@ -1,10 +1,10 @@
 #[cfg(feature = "layout-diagnostics")]
 use crate::layout_diagnostics::{self as diag, Counter};
 use crate::{
-    Axis, Bounds, Declared, Holds, LayoutHolds, LayoutLen, Len, PlaceDesc, Px, PxVec2, RegionAlign,
-    Rel, RenderedText, RequestArena, RequestedLen, RetainedPrimitive, Size, SizeRequests,
-    StrongWidget, TextAttrs, TextBuffer, TextureHandle, UiRegion, UiRenderState, UiRsc, UiVec2,
-    Weight, WidgetId, Widgets,
+    Axis, Bound, Bounds, Declared, DrawScratch, Holds, LayoutHolds, LayoutLen, Len, PlaceDesc,
+    PlaceFit, Px, PxVec2, RegionAlign, Rel, RenderedText, RequestArena, RequestedLen,
+    RetainedPrimitive, Size, SizeRequests, SizeRule, StrongWidget, TextAttrs, TextBuffer,
+    TextureHandle, UiRegion, UiRenderState, UiRsc, UiVec2, Weight, WidgetId, Widgets,
     render::{
         GlyphPrimitive, Mask, MaskIdx, MoveIdx, Primitive, PrimitiveInst, PrimitiveKind,
         TexturePrimitive,
@@ -43,7 +43,7 @@ pub struct Painter<'a> {
     /// The children whose size this widget read while drawing.
     pub(super) size_deps: Vec<WidgetId>,
     pub(super) request_deps: Vec<WidgetId>,
-    pub(super) scratch: crate::DrawScratch,
+    pub(super) scratch: DrawScratch,
     /// What this draw itself reads, as against what its children's drawings
     /// hold for: every window and every length of its own region until it
     /// reads one, then that one unless it says otherwise, and the rel base or
@@ -65,8 +65,9 @@ pub struct Painter<'a> {
 }
 
 impl<'a> Painter<'a> {
-    /// Reuses this widget's allocation buffers across draws. Nested painters
-    /// have independent buffers, so discovering a child cannot overwrite them.
+    /// Reuses this widget's allocation buffers across draws. A child drawn
+    /// part way through gets a painter of its own, with buffers of its own,
+    /// so nothing it does while this one is mid-row can reach these.
     pub fn with_requests<T>(
         &mut self,
         f: impl FnOnce(&mut Self, &mut Vec<RequestedLen>, &mut Vec<Px>) -> T,
@@ -87,7 +88,7 @@ impl<'a> Painter<'a> {
         child: &StrongWidget<W>,
         axis: Axis,
     ) -> Option<RequestedLen> {
-        if self.rsc.widgets().size_rules(child.id())[axis].bound() == crate::Bound::ANY
+        if self.rsc.widgets().size_rules(child.id())[axis].bound() == Bound::ANY
             && let Some(len) = self.size_hint(child, axis)
         {
             self.request_deps.push(child.id());
@@ -107,7 +108,7 @@ impl<'a> Painter<'a> {
             request.has_leftover()
                 || matches!(
                     self.rsc.widgets().size_rules(child.id())[axis],
-                    crate::SizeRule::Request(_)
+                    SizeRule::Request(_)
                 )
         });
         if request.is_some() {
@@ -144,19 +145,24 @@ impl<'a> Painter<'a> {
             self.rel_base(axis);
             return request;
         }
-        let request = if len.leftover > Weight::ZERO {
-            requests.bounded(len.into(), bound)
-        } else {
-            len.into()
+        let shares = len.leftover > Weight::ZERO;
+        let request = match shares {
+            true => requests.bounded(len.into(), bound),
+            false => len.into(),
         };
         self.request_deps.truncate(start);
-        if len.leftover > Weight::ZERO && bound != crate::Bound::ANY {
+        // Only a bound that is a fraction was read against the rel base; one
+        // in pixels binds at the same length under any of them.
+        if shares && bound.has_fraction() {
             self.rel_base(axis);
         }
         request
     }
 
-    /// A deferred comparison reads this window when the allocation is solved.
+    /// Divides `room` between `requests`, one length per request in
+    /// `output`. A deferred comparison is decided here, against this window:
+    /// which side of a crossing the solution falls is a question in pixels,
+    /// so the drawing holds only for the window that answered it.
     pub fn allocate(
         &mut self,
         requests: &[RequestedLen],
@@ -174,6 +180,8 @@ impl<'a> Painter<'a> {
         )
     }
 
+    /// The least a request can come to, which is what it takes of the row
+    /// before anything is divided. A comparison is read at no share at all.
     pub fn minimum_request(&mut self, request: &RequestedLen, axis: Axis) -> Len {
         match request.linear() {
             Some(len) => len.without_leftover(),
@@ -553,7 +561,7 @@ impl<'a> Painter<'a> {
     pub fn has_exact_size(&self, axis: Axis) -> bool {
         matches!(
             self.rsc.widgets().size_rules(self.id)[axis],
-            crate::SizeRule::Exact(_) | crate::SizeRule::Request(_)
+            SizeRule::Exact(_) | SizeRule::Request(_)
         )
     }
 
@@ -797,7 +805,11 @@ impl Widgets {
     /// share included, since a share is a length only to whoever divides one,
     /// and that is the parent rather than this widget.
     fn exact_len(&self, id: WidgetId, axis: Axis) -> Option<LayoutLen> {
-        if matches!(self.size_rules(id)[axis], crate::SizeRule::Request(_)) {
+        // A request is a length the rule gives, and the hint below must not
+        // narrow the box in its place: what the request comes to is not known
+        // until the parent allocates, and it is the parent's answer, not this
+        // widget's.
+        if matches!(self.size_rules(id)[axis], SizeRule::Request(_)) {
             return None;
         }
         self.size_rules(id)[axis].exact().or_else(|| {
@@ -868,23 +880,22 @@ impl Placing {
         let mut bounds = Bounds::ANY;
         for axis in Axis::BOTH {
             let base = place.base(axis, self.rel_base);
-            if let crate::SizeRule::Request(request) = &rules[axis] {
+            if let SizeRule::Request(request) = &rules[axis] {
                 inputs[axis].rel_base = Some(self.rel_base[axis]);
                 inputs[axis].region_len = Some(self.region[axis].len());
                 inputs[axis].window = Holds::at(window[axis]);
                 let offer = place.of(self.region, align)[axis].len();
-                let len = if place[axis].fit == crate::PlaceFit::Allocated {
-                    offer
+                let px = if place[axis].fit == PlaceFit::Allocated {
+                    offer.to_px(window[axis])
                 } else {
                     let request = requests.import(request, base);
-                    let len = requests
+                    holds[axis].window = Holds::at(window[axis]);
+                    requests
                         .allocate(&[request], offer.to_px(window[axis]), window[axis])
                         .next()
-                        .unwrap();
-                    holds[axis].window = Holds::at(window[axis]);
-                    Len::from_parts(Rel::ZERO, len)
+                        .unwrap()
                 };
-                let len = Len::from_parts(Rel::ZERO, len.to_px(window[axis]));
+                let len = Len::from_parts(Rel::ZERO, px);
                 place[axis].rel_base = RelBase::Len(len);
                 declared[axis] = Some(len);
                 holds[axis].rel_base = Some(len);
@@ -909,11 +920,7 @@ impl Placing {
             // a fraction in it is of. `MaxSize` is the box version, and it is
             // a widget because a widget is drawn again when its box changes.
             let bound = rules[axis].bound();
-            if [bound.min, bound.max]
-                .into_iter()
-                .flatten()
-                .any(|len| len.rel != Rel::ZERO)
-            {
+            if bound.has_fraction() {
                 inputs[axis].rel_base = Some(self.rel_base[axis]);
             }
             bounds[axis] = bound.within_len(base);

core/src/ui/place.rs

@@ -25,8 +25,8 @@ pub enum PlaceFit {
 }
 
 impl PlaceFit {
-    pub fn fills(self) -> bool {
-        self != Self::Align
+    pub fn fills(&self) -> bool {
+        !matches!(self, Self::Align)
     }
 }
 

core/src/ui/render_state.rs

@@ -349,6 +349,14 @@ impl UiRenderState {
             ),
             None => Default::default(),
         };
+        // Every one of these is a buffer this widget's last draw filled and
+        // `remove` emptied, kept for its capacity alone. A drawing whose
+        // primitives were still in it would record them twice.
+        debug_assert!(
+            textures.is_empty() && primitives.is_empty() && request_deps.is_empty(),
+            "'{}' ({id:?}) was drawn again over what its last draw left",
+            rsc.widgets().label(id)
+        );
         let children = std::mem::take(&mut scratch.children);
         let size_deps = std::mem::take(&mut scratch.size_deps);
         let under = std::mem::take(&mut scratch.under);
@@ -468,11 +476,10 @@ impl UiRenderState {
             if answer.leftover != Weight::ZERO {
                 continue;
             }
-            let (outside, kept) =
-                info.bounds[axis].outside(answer.without_leftover(), window[axis]);
+            let (held, kept) = info.bounds[axis].outside(answer.without_leftover(), window[axis]);
             bounded[axis].window = kept;
-            if let Some(outside) = outside {
-                size[axis] = info.bounds[axis].at(outside).into();
+            if let Some(held) = held {
+                size[axis] = held.into();
             }
         }
         // A widget that clipped its contents to its box drew nothing outside
@@ -1031,10 +1038,10 @@ 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.
-        // 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.
+        // A mark made while the walk runs queues itself through `mark`. What
+        // ends the walk is the marks being spent rather than the queue being
+        // empty, so a mark that reached the queue twice, or that was settled
+        // another way, costs a pop and nothing else.
         loop {
             for &id in rsc.widgets().needs_redraw.iter() {
                 if !self.deferred.contains(&id) {

core/src/widget/request.rs

@@ -1,6 +1,10 @@
-use crate::{Axis, LayoutLen, Len, Px, StrongWidget, Weight, WidgetId, Widgets};
+use crate::{
+    ActiveData, Axis, Bound, LayoutLen, Len, Px, Rel, SizeRule, StrongWidget, UiNum, Weight,
+    WidgetId, Widgets, util::HashMap,
+};
+use std::{cmp::Ordering, sync::Arc};
 
-impl<N: crate::UiNum> From<N> for SizeRequest {
+impl<N: UiNum> From<N> for SizeRequest {
     fn from(value: N) -> Self {
         LayoutLen::px(value).into()
     }
@@ -25,9 +29,9 @@ impl LayoutLen {
 #[derive(Clone, Debug, PartialEq)]
 pub enum SizeRequest {
     Linear(LayoutLen),
-    Sum(std::sync::Arc<(Self, Self)>),
-    Min(std::sync::Arc<(Self, Self)>),
-    Max(std::sync::Arc<(Self, Self)>),
+    Sum(Arc<(Self, Self)>),
+    Min(Arc<(Self, Self)>),
+    Max(Arc<(Self, Self)>),
 }
 
 impl From<LayoutLen> for SizeRequest {
@@ -53,7 +57,7 @@ impl SizeRequest {
         if self == other {
             self
         } else {
-            Self::Min(std::sync::Arc::new((self, other)))
+            Self::Min(Arc::new((self, other)))
         }
     }
 
@@ -67,7 +71,7 @@ impl SizeRequest {
         if self == other {
             self
         } else {
-            Self::Max(std::sync::Arc::new((self, other)))
+            Self::Max(Arc::new((self, other)))
         }
     }
 
@@ -82,7 +86,7 @@ impl std::ops::Add for SizeRequest {
     fn add(self, other: Self) -> Self {
         match (self, other) {
             (Self::Linear(a), Self::Linear(b)) => Self::Linear(a + b),
-            (a, b) => Self::Sum(std::sync::Arc::new((a, b))),
+            (a, b) => Self::Sum(Arc::new((a, b))),
         }
     }
 }
@@ -113,13 +117,14 @@ impl From<Len> for RequestedLen {
     }
 }
 impl RequestedLen {
-    pub fn linear(self) -> Option<LayoutLen> {
+    /// The length itself, where no comparison is waiting on an allocation.
+    pub fn linear(&self) -> Option<LayoutLen> {
         match self.0 {
             RequestValue::Linear(len) => Some(len),
             _ => None,
         }
     }
-    pub fn has_leftover(self) -> bool {
+    pub fn has_leftover(&self) -> bool {
         match self.0 {
             RequestValue::Linear(len) => len.leftover > Weight::ZERO,
             RequestValue::Deferred { leftover, .. } => leftover,
@@ -195,7 +200,7 @@ impl RequestArena {
     fn segment(&self, request: RequestedLen, at: Ratio, window: Px) -> Segment {
         match request.0 {
             RequestValue::Linear(len) => {
-                assert!(
+                debug_assert!(
                     len.leftover >= Weight::ZERO,
                     "a leftover weight cannot be negative"
                 );
@@ -232,8 +237,11 @@ impl RequestArena {
             }
         }
     }
-    /// Allocates one scope of nonnegative shares. Floors can overflow; caps
-    /// can leave unused room. Prefix rounding keeps adjacent slot edges equal.
+    /// Divides `room` between requests whose weights are nonnegative, one
+    /// length per request. A floor can overflow the room and a cap can leave
+    /// part of it unused, so the lengths need not come to `room`. Each edge
+    /// is rounded from the running total rather than from the length before
+    /// it, so two neighbouring slots meet exactly.
     pub(crate) fn allocate<'a>(
         &'a self,
         requests: &'a [RequestedLen],
@@ -265,6 +273,9 @@ impl RequestArena {
         requests.iter().map(move |request| {
             prefix += self.segment(*request, at, window).value(at);
             let den = i128::from(at.den);
+            // Half away from zero, which is what `Fixed` rounds a division
+            // to: the two decide the same edge, and a change to one of them
+            // is a change to the other.
             let edge = prefix.signum() * ((prefix.abs() + den / 2) / den);
             let len = Px::from_raw((edge - previous) as i32);
             previous = edge;
@@ -289,7 +300,7 @@ impl Ratio {
     const ZERO: Self = Self { num: 0, den: 1 };
 
     fn new(num: i64, den: i64) -> Self {
-        assert_ne!(den, 0);
+        debug_assert_ne!(den, 0, "a ratio of nothing");
         if den < 0 {
             Self {
                 num: -num,
@@ -302,14 +313,14 @@ impl Ratio {
 }
 
 impl Ord for Ratio {
-    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+    fn cmp(&self, other: &Self) -> Ordering {
         (i128::from(self.num) * i128::from(other.den))
             .cmp(&(i128::from(other.num) * i128::from(self.den)))
     }
 }
 
 impl PartialOrd for Ratio {
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
         Some(self.cmp(other))
     }
 }
@@ -355,7 +366,7 @@ fn first(a: Option<Ratio>, b: Option<Ratio>) -> Option<Ratio> {
 /// expressed in window lengths; `rel_base` supplies the base for declarations.
 pub struct SizeRequests<'a> {
     pub(crate) arena: &'a mut RequestArena,
-    pub(crate) measured: Option<&'a crate::util::HashMap<WidgetId, crate::ActiveData>>,
+    pub(crate) measured: Option<&'a HashMap<WidgetId, ActiveData>>,
     pub(crate) widgets: &'a Widgets,
     pub(crate) dependencies: &'a mut Vec<WidgetId>,
     pub(crate) rel_base: Len,
@@ -380,7 +391,7 @@ impl SizeRequests<'_> {
         self.dependencies.push(child.id());
         let rules = self.widgets.size_rules(child.id());
         let rule = &rules[axis];
-        if let crate::SizeRule::Request(request) = rule {
+        if let SizeRule::Request(request) = rule {
             return Some(self.arena.import(request, self.rel_base));
         }
         if let Some(exact) = rule.exact() {
@@ -396,7 +407,7 @@ impl SizeRequests<'_> {
         Some(self.bounded(request, rule.bound()))
     }
 
-    pub(crate) fn bounded(&mut self, request: RequestedLen, bound: crate::Bound) -> RequestedLen {
+    pub(crate) fn bounded(&mut self, request: RequestedLen, bound: Bound) -> RequestedLen {
         let bound = bound.within_len(self.rel_base);
         let request = match bound.min {
             Some(min) => self.max(request, min.into()),
@@ -422,12 +433,12 @@ impl SizeRequests<'_> {
         self.rel_base.px -= padding;
         let request = self.widget(child, axis);
         self.rel_base = base;
-        request.map(|request| self.sum(request, Len::from_parts(crate::Rel::ZERO, padding).into()))
+        request.map(|request| self.sum(request, Len::from_parts(Rel::ZERO, padding).into()))
     }
 }
 
 // Equal fractions keep this valid even when padding makes a rel base negative.
-fn independent_order(a: LayoutLen, b: LayoutLen) -> Option<std::cmp::Ordering> {
+fn independent_order(a: LayoutLen, b: LayoutLen) -> Option<Ordering> {
     if a.rel == b.rel && a.leftover == b.leftover {
         Some(a.px.cmp(&b.px))
     } else if a.px == b.px && a.rel == b.rel {

core/src/widget/size_rule.rs

@@ -1,5 +1,6 @@
 use crate::util::impl_axis_index;
 use crate::{Axis, LayoutLen, Len, Rel, SizeRequest};
+use std::sync::Arc;
 
 /// What a widget's length on one axis is, as a rule its parent applies where
 /// it draws it rather than an answer the widget gives about itself.
@@ -20,7 +21,7 @@ pub enum SizeRule {
     /// This length, whatever the widget reports.
     Exact(LayoutLen),
     /// An exact request whose comparisons await the parent's allocation.
-    Request(std::sync::Arc<SizeRequest>),
+    Request(Arc<SizeRequest>),
     /// At least this long, and otherwise whatever the box gives it.
     Min(Len),
     /// At most this long.
@@ -53,12 +54,7 @@ impl SizeRule {
     /// Whether what this rule says is a fraction of the rel base, so that
     /// the same rule against a different one is a different length.
     pub fn has_fraction(&self) -> bool {
-        let bound = self.bound();
-        self.exact().is_some_and(|len| len.rel != Rel::ZERO)
-            || [bound.min, bound.max]
-                .iter()
-                .flatten()
-                .any(|len| len.rel != Rel::ZERO)
+        self.exact().is_some_and(|len| len.rel != Rel::ZERO) || self.bound().has_fraction()
     }
 
     /// This rule with a floor under it, which is the whole of it where there
@@ -115,13 +111,6 @@ pub struct Bound {
     pub max: Option<Len>,
 }
 
-/// Which end of a bound a length fell outside.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum Outside {
-    Shorter,
-    Longer,
-}
-
 impl Bound {
     /// Every length.
     pub const ANY: Self = Self {
@@ -129,14 +118,13 @@ impl Bound {
         max: None,
     };
 
-    /// The end [`Outside`] names, which is the length a widget outside it
-    /// gets instead of its own.
-    pub fn at(&self, outside: Outside) -> Len {
-        let end = match outside {
-            Outside::Shorter => self.min,
-            Outside::Longer => self.max,
-        };
-        end.expect("an end nothing is outside of")
+    /// Whether either end is a fraction of the rel base, so that the same
+    /// bound against a different one binds at a different length.
+    pub fn has_fraction(&self) -> bool {
+        [self.min, self.max]
+            .into_iter()
+            .flatten()
+            .any(|len| len.rel != Rel::ZERO)
     }
 
     /// This bound as lengths of the window, from lengths of a rel base that
@@ -182,7 +170,7 @@ impl From<SizeRequest> for SizeRule {
     fn from(request: SizeRequest) -> Self {
         match request {
             SizeRequest::Linear(len) => Self::Exact(len),
-            request => Self::Request(std::sync::Arc::new(request)),
+            request => Self::Request(Arc::new(request)),
         }
     }
 }