From 05e6ced31d0a9ac167b8b9f41cfe42247d47ac27 Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Sun, 20 Sep 2026 21:07:19 -0400 Subject: [PATCH] Hold a request in the arena its nodes are allocated in A size request was a second expression shape beside the one the layout pass already has. `SizeRequest` held `Sum`/`Min`/`Max` over `Arc` pairs; `RequestArena` held the same three operators as `Node { op, a, b }` in a `Vec`, with the same fold over `independent_order` written a second time, and `import` walked the first rebuilding it as the second. There is one node type now. An expression is the pass's nodes in an arena of its own that lasts as long as the rule holding it, and `import` grafts those nodes into the pass's arena, resolving fractions as they land. The fold is `Nodes::combine`, which both the builder and the importer call. So the `Arc` goes, and no refcount replaces it: nothing shares a request and nothing outside widget code holds one. A plain length stays inline, so `size_of::()` is 40 either way and only an actual expression allocates. A node's operand is a number within its own arena, and `RequestedLen` -- the only form that leaves one -- is that plus the epoch saying which pass numbered it, so the epoch is now checked once where a handle comes back in rather than at every level of the walk it starts. `SizeRule::at_least`/`at_most` were the only clones of a request in the framework, and both read a rule out, moved one end of its bound, and wrote it back into the slot it came from. `Widgets::edit_bound` does it where it sits, so nothing copies an expression to cap it. `SizeRequest` grew a `Display`, since the shrinker prints one and a derived `Debug` of an arena is not something a tree can be rebuilt from: `min(30 px;1 leftover;, 2 leftover;)<0.5 rel;`. Measured, medians of three release runs under `perf stat -e instructions:u`, each set within 0.005% of its median: bounds_cost MODE=cap FRAMES=2000 is 5.665B against 5.743B (-1.34%), and revision_cost resize ROWS=40 FRAMES=500 is 4.855B against 4.893B (-0.79%). Format, workspace clippy under -D warnings with and without layout-diagnostics, 206 ordinary and 210 diagnostic tests, the cold dump byte-identical to 2ac0843 across all 34,986 boxes, 400 depth-5 trees in 64.24s, 1,000 depth-6 in 160.35s, 2,000 depth-4 in 298.82s, and 400 depth-5 trees in each of the three deferred-request corpora in 205.42s. --- core/src/widget/request.rs | 385 ++++++++++++++++++++++++----------- core/src/widget/size_rule.rs | 38 ++-- core/src/widget/widgets.rs | 44 ++-- tests/scenario/mod.rs | 9 +- 4 files changed, 318 insertions(+), 158 deletions(-) diff --git a/core/src/widget/request.rs b/core/src/widget/request.rs index 9ed66db..764773e 100644 --- a/core/src/widget/request.rs +++ b/core/src/widget/request.rs @@ -2,7 +2,7 @@ use crate::{ ActiveData, Axis, Bound, LayoutLen, Len, Px, Rel, StrongWidget, UiNum, Weight, WidgetId, Widgets, util::HashMap, }; -use std::{cmp::Ordering, sync::Arc}; +use std::cmp::Ordering; impl From for SizeRequest { fn from(value: N) -> Self { @@ -24,14 +24,188 @@ impl LayoutLen { } } +#[derive(Clone, Copy, Debug, PartialEq)] +enum Op { + Sum, + Min, + Max, +} + +/// One operand of a [`Node`]: a length, or another node. Node numbers are an +/// arena's own, so an operand says nothing about which arena it came from -- +/// [`RequestedLen`] is the form that does, and the only one that leaves one. +#[derive(Clone, Copy, Debug, PartialEq)] +enum Operand { + Linear(LayoutLen), + Node(u32), +} + +/// One sum or comparison, with its two operands. Whether anything under it +/// divides leftover space is carried on the node rather than walked for, +/// because every caller of one asks. +#[derive(Clone, Copy, Debug, PartialEq)] +struct Node { + op: Op, + a: Operand, + b: Operand, + leftover: bool, +} + +/// The nodes of one expression, numbered from zero, and the folding that +/// happens as each is added. There are two kinds of owner and one kind of +/// arena: a rule's [`SizeRequest`] holds a small one for as long as the rule +/// lasts, and [`RequestArena`] holds the layout pass's. +#[derive(Clone, Debug, Default, PartialEq)] +struct Nodes(Vec); + +impl Nodes { + fn node(&self, index: u32) -> Node { + self.0[index as usize] + } + + /// The length itself, where no comparison is waiting on an allocation. + fn linear(&self, at: Operand) -> Option { + match at { + Operand::Linear(len) => Some(len), + Operand::Node(_) => None, + } + } + + fn leftover(&self, at: Operand) -> bool { + match at { + Operand::Linear(len) => len.leftover > Weight::ZERO, + Operand::Node(index) => self.node(index).leftover, + } + } + + /// `a op b`, which is a node only where the answer needs one. Two + /// lengths that keep their order whatever the room comes to are already + /// decided, and so are two operands that are the same thing. + fn combine(&mut self, op: Op, a: Operand, b: Operand) -> Operand { + if let (Some(x), Some(y)) = (self.linear(a), self.linear(b)) { + if matches!(op, Op::Sum) { + return Operand::Linear(x + y); + } + if let Some(order) = independent_order(x, y) { + let take_a = match op { + Op::Min => !order.is_gt(), + _ => !order.is_lt(), + }; + return if take_a { a } else { b }; + } + } + if a == b && !matches!(op, Op::Sum) { + return a; + } + let index = u32::try_from(self.0.len()).expect("more nodes than one arena can number"); + let leftover = self.leftover(a) || self.leftover(b); + self.0.push(Node { op, a, b, leftover }); + Operand::Node(index) + } + + /// Copies `at` and everything under it out of `from`, with every length + /// it holds passed through `resolve`. Folded again on the way in, since + /// resolving a fraction can settle a comparison that was open before it. + fn graft( + &mut self, + from: &Self, + at: Operand, + resolve: impl Copy + Fn(LayoutLen) -> LayoutLen, + ) -> Operand { + let index = match at { + Operand::Linear(len) => return Operand::Linear(resolve(len)), + Operand::Node(index) => index, + }; + let Node { op, a, b, .. } = from.node(index); + let a = self.graft(from, a, resolve); + let b = self.graft(from, b, resolve); + self.combine(op, a, b) + } + + fn write(&self, f: &mut std::fmt::Formatter<'_>, at: Operand) -> std::fmt::Result { + let index = match at { + Operand::Linear(len) => return write!(f, "{len}"), + Operand::Node(index) => index, + }; + let Node { op, a, b, .. } = self.node(index); + write!( + f, + "{}(", + match op { + Op::Sum => "sum", + Op::Min => "min", + Op::Max => "max", + } + )?; + self.write(f, a)?; + write!(f, ", ")?; + self.write(f, b)?; + write!(f, ")") + } +} + /// A size request before a container has divided its leftover space. /// Comparisons keep both operands until the share is known. +/// +/// An expression is the same nodes the layout pass allocates, in an arena of +/// its own: importing one copies those nodes into the pass's arena, so there +/// is no second shape to keep in step and one place where folding is decided. #[derive(Clone, Debug, PartialEq)] pub enum SizeRequest { Linear(LayoutLen), - Sum(Arc<(Self, Self)>), - Min(Arc<(Self, Self)>), - Max(Arc<(Self, Self)>), + /// Behind a pointer, because a plain length is what nearly every rule + /// holds and an expression should cost those rules nothing. + Expr(Box), +} + +/// An expression's own arena, and which of its nodes is the whole of it. +/// `root` is a node number rather than an operand, so an expression that +/// folded all the way down to a length cannot be written as one: that is a +/// [`SizeRequest::Linear`]. +#[derive(Clone, Debug, PartialEq)] +pub struct Expr { + nodes: Nodes, + root: u32, +} + +impl SizeRequest { + pub fn min(self, other: impl Into) -> Self { + self.join(Op::Min, other.into()) + } + + pub fn max(self, other: impl Into) -> Self { + self.join(Op::Max, other.into()) + } + + pub fn clamp(self, min: impl Into, max: impl Into) -> Self { + self.max(min).min(max) + } + + /// `self op other`, keeping `self`'s arena and copying `other`'s into + /// it, so the two sets of node numbers become one. Two requests that are + /// the same request compare to the same thing whatever the room is, which + /// is a comparison worth not building -- but adding something to itself + /// is twice it. + fn join(self, op: Op, other: Self) -> Self { + if self == other && !matches!(op, Op::Sum) { + return self; + } + let (mut nodes, a) = match self { + Self::Linear(len) => (Nodes::default(), Operand::Linear(len)), + Self::Expr(expr) => { + let Expr { nodes, root } = *expr; + (nodes, Operand::Node(root)) + } + }; + let b = match other { + Self::Linear(len) => Operand::Linear(len), + Self::Expr(expr) => nodes.graft(&expr.nodes, Operand::Node(expr.root), |len| len), + }; + match nodes.combine(op, a, b) { + Operand::Linear(len) => Self::Linear(len), + Operand::Node(root) => Self::Expr(Box::new(Expr { nodes, root })), + } + } } impl From for SizeRequest { @@ -46,47 +220,19 @@ impl From for SizeRequest { } } -impl SizeRequest { - pub fn min(self, other: impl Into) -> Self { - let other = other.into(); - if let (Self::Linear(a), Self::Linear(b)) = (&self, &other) - && let Some(order) = independent_order(*a, *b) - { - return if !order.is_gt() { self } else { other }; - } - if self == other { - self - } else { - Self::Min(Arc::new((self, other))) - } - } - - pub fn max(self, other: impl Into) -> Self { - let other = other.into(); - if let (Self::Linear(a), Self::Linear(b)) = (&self, &other) - && let Some(order) = independent_order(*a, *b) - { - return if !order.is_lt() { self } else { other }; - } - if self == other { - self - } else { - Self::Max(Arc::new((self, other))) - } - } - - pub fn clamp(self, min: impl Into, max: impl Into) -> Self { - self.max(min).min(max) - } -} - impl std::ops::Add for SizeRequest { type Output = Self; fn add(self, other: Self) -> Self { - match (self, other) { - (Self::Linear(a), Self::Linear(b)) => Self::Linear(a + b), - (a, b) => Self::Sum(Arc::new((a, b))), + self.join(Op::Sum, other) + } +} + +impl std::fmt::Display for SizeRequest { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Linear(len) => write!(f, "{len}"), + Self::Expr(expr) => expr.nodes.write(f, Operand::Node(expr.root)), } } } @@ -100,7 +246,7 @@ pub struct RequestedLen(RequestValue); enum RequestValue { Linear(LayoutLen), Deferred { - index: usize, + index: u32, epoch: u64, leftover: bool, }, @@ -132,42 +278,61 @@ impl RequestedLen { } } -#[derive(Clone, Copy)] -enum Op { - Sum, - Min, - Max, -} -struct Node { - op: Op, - a: RequestedLen, - b: RequestedLen, -} - #[derive(Default)] pub(crate) struct RequestArena { - nodes: Vec, + nodes: Nodes, epoch: u64, } impl RequestArena { pub(crate) fn reset(&mut self) { - self.nodes.clear(); + self.nodes.0.clear(); self.epoch = self .epoch .checked_add(1) .expect("layout generation exhausted"); } + + /// An operand of this pass's arena as the handle that leaves it. The + /// epoch says which pass numbered the node, so a handle a widget kept + /// past its pass is caught rather than answering about whatever node + /// took its place. + fn handle(&self, at: Operand) -> RequestedLen { + RequestedLen(match at { + Operand::Linear(len) => RequestValue::Linear(len), + Operand::Node(index) => RequestValue::Deferred { + index, + epoch: self.epoch, + leftover: self.nodes.leftover(at), + }, + }) + } + + /// The other direction, checked once where a handle comes back in rather + /// than again at every level of the walk it starts. + fn operand(&self, request: RequestedLen) -> Operand { + match request.0 { + RequestValue::Linear(len) => Operand::Linear(len), + RequestValue::Deferred { index, epoch, .. } => { + assert_eq!(epoch, self.epoch, "request retained beyond its layout pass"); + Operand::Node(index) + } + } + } + + /// A rule's expression in this pass's arena, with its fractions resolved + /// against the rel base the widget is being asked with. pub(crate) fn import(&mut self, request: &SizeRequest, base: Len) -> RequestedLen { - let (op, pair) = match request { - SizeRequest::Linear(len) => return len.within_len(base).into(), - SizeRequest::Sum(pair) => (Op::Sum, pair), - SizeRequest::Min(pair) => (Op::Min, pair), - SizeRequest::Max(pair) => (Op::Max, pair), + let at = match request { + SizeRequest::Linear(len) => Operand::Linear(len.within_len(base)), + SizeRequest::Expr(expr) => { + self.nodes + .graft(&expr.nodes, Operand::Node(expr.root), |len| { + len.within_len(base) + }) + } }; - let a = self.import(&pair.0, base); - let b = self.import(&pair.1, base); - self.combine(op, a, b) + self.handle(at) } pub(crate) fn bounded(&mut self, request: RequestedLen, bound: Bound) -> RequestedLen { @@ -182,73 +347,55 @@ impl RequestArena { } fn combine(&mut self, op: Op, a: RequestedLen, b: RequestedLen) -> RequestedLen { - if let (Some(x), Some(y)) = (a.linear(), b.linear()) { - if matches!(op, Op::Sum) { - return (x + y).into(); - } - let order = independent_order(x, y); - if let Some(order) = order { - let take_a = match op { - Op::Min => !order.is_gt(), - _ => !order.is_lt(), - }; - return if take_a { a } else { b }; - } - } - if a == b && !matches!(op, Op::Sum) { - return a; - } - let index = self.nodes.len(); - self.nodes.push(Node { op, a, b }); - RequestedLen(RequestValue::Deferred { - index, - epoch: self.epoch, - leftover: a.has_leftover() || b.has_leftover(), - }) + let (a, b) = (self.operand(a), self.operand(b)); + let at = self.nodes.combine(op, a, b); + self.handle(at) } + pub(crate) fn minimum(&self, request: RequestedLen, window: Px) -> Px { - Px::from_raw(self.segment(request, Ratio::ZERO, window).fixed as i32) + let at = self.operand(request); + Px::from_raw(self.segment(at, Ratio::ZERO, window).fixed as i32) } - fn segment(&self, request: RequestedLen, at: Ratio, window: Px) -> Segment { - match request.0 { - RequestValue::Linear(len) => { + + fn segment(&self, of: Operand, at: Ratio, window: Px) -> Segment { + let index = match of { + Operand::Linear(len) => { debug_assert!( len.leftover >= Weight::ZERO, "a leftover weight cannot be negative" ); - Segment { + return Segment { fixed: i64::from(len.without_leftover().to_px(window).raw()), weight: i64::from(len.leftover.raw()), end: None, - } - } - RequestValue::Deferred { index, epoch, .. } => { - assert_eq!(epoch, self.epoch, "request retained beyond its layout pass"); - let Node { op, a, b } = self.nodes[index]; - let a = self.segment(a, at, window); - let b = self.segment(b, at, window); - if matches!(op, Op::Sum) { - return a + b; - } - // At a crossing choose the branch to its right, so the next - // iteration advances rather than selecting that crossing again. - let order = a.value(at).cmp(&b.value(at)).then(a.weight.cmp(&b.weight)); - let take_a = match op { - Op::Min => !order.is_gt(), - _ => !order.is_lt(), }; - let mut selected = if take_a { a } else { b }; - selected.end = first(a.end, b.end); - if a.weight != b.weight { - let crossing = Ratio::new(b.fixed - a.fixed, a.weight - b.weight); - if crossing > at { - selected.end = first(selected.end, Some(crossing)); - } - } - selected + } + Operand::Node(index) => index, + }; + let Node { op, a, b, .. } = self.nodes.node(index); + let a = self.segment(a, at, window); + let b = self.segment(b, at, window); + if matches!(op, Op::Sum) { + return a + b; + } + // At a crossing choose the branch to its right, so the next + // iteration advances rather than selecting that crossing again. + let order = a.value(at).cmp(&b.value(at)).then(a.weight.cmp(&b.weight)); + let take_a = match op { + Op::Min => !order.is_gt(), + _ => !order.is_lt(), + }; + let mut selected = if take_a { a } else { b }; + selected.end = first(a.end, b.end); + if a.weight != b.weight { + let crossing = Ratio::new(b.fixed - a.fixed, a.weight - b.weight); + if crossing > at { + selected.end = first(selected.end, Some(crossing)); } } + selected } + /// 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 @@ -263,7 +410,7 @@ impl RequestArena { let mut at = Ratio::ZERO; loop { let total = requests.iter().fold(Segment::ZERO, |total, request| { - total + self.segment(*request, at, window) + total + self.segment(self.operand(*request), at, window) }); if total.value(at) >= i128::from(room.raw()) * i128::from(at.den) { break; @@ -283,7 +430,7 @@ impl RequestArena { let mut prefix = 0_i128; let mut previous = 0_i128; requests.iter().map(move |request| { - prefix += self.segment(*request, at, window).value(at); + prefix += self.segment(self.operand(*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 diff --git a/core/src/widget/size_rule.rs b/core/src/widget/size_rule.rs index a65a238..b31b016 100644 --- a/core/src/widget/size_rule.rs +++ b/core/src/widget/size_rule.rs @@ -16,35 +16,39 @@ impl SizeRule { }; pub fn min(min: Len) -> Self { - Self::FREE.at_least(min) + Self::bounded(Bound { + min: Some(min), + max: None, + }) } pub fn max(max: Len) -> Self { - Self::FREE.at_most(max) + Self::bounded(Bound { + min: None, + max: Some(max), + }) } pub fn clamp(min: Len, max: Len) -> Self { - Self::min(min).at_most(max) + Self::bounded(Bound { + min: Some(min), + max: Some(max), + }) + } + + /// A bound and no preferred length, so whatever the widget draws is held + /// to it. + fn bounded(bound: Bound) -> Self { + Self { + request: None, + bound, + } } pub fn has_fraction(&self) -> bool { self.exact().is_some_and(|len| len.rel != Rel::ZERO) || self.bound.has_fraction() } - /// Replaces the floor while preserving the request and cap. - pub fn at_least(&self, min: Len) -> Self { - let mut rule = self.clone(); - rule.bound.min = Some(min); - rule - } - - /// Replaces the cap while preserving the request and floor. - pub fn at_most(&self, max: Len) -> Self { - let mut rule = self.clone(); - rule.bound.max = Some(max); - rule - } - /// A linear preferred length, before applying the independent bounds. pub fn exact(&self) -> Option { match self.request { diff --git a/core/src/widget/widgets.rs b/core/src/widget/widgets.rs index cfc6620..5369a14 100644 --- a/core/src/widget/widgets.rs +++ b/core/src/widget/widgets.rs @@ -1,8 +1,8 @@ use std::sync::mpsc::{Receiver, Sender, channel}; use crate::{ - Axis, AxisAlign, IdLike, Len, RegionAlign, SizeRequest, SizeRule, SizeRules, StrongWidget, - WeakWidget, Widget, WidgetData, WidgetId, + Axis, AxisAlign, Bound, IdLike, Len, RegionAlign, SizeRequest, SizeRule, SizeRules, + StrongWidget, WeakWidget, Widget, WidgetData, WidgetId, util::{DynBorrower, HashSet, SlotVec, forget_mut, to_mut}, }; @@ -145,30 +145,40 @@ impl Widgets { self.needs_redraw.insert(id); } - /// Changes the preferred length without changing its bounds. + /// Changes the preferred length, leaving the bounds beside it alone. pub fn set_len(&mut self, id: impl IdLike, axis: Axis, len: impl Into) { let id = id.id(); - let rule = SizeRule { - request: Some(len.into()), - bound: self.size_rules(id)[axis].bound, - }; - self.set_size_rule(id, axis, rule); + let request = Some(len.into()); + let rule = &mut self.data_mut(id).unwrap().size[axis]; + if rule.request == request { + return; + } + rule.request = request; + self.needs_redraw.insert(id); } /// Puts a floor under this widget's length on one axis, keeping a cap it - /// already had. See [`SizeRule::at_least`]. + /// already had and the preferred length beside it. pub fn set_min_len(&mut self, id: impl IdLike, axis: Axis, min: Len) { - let id = id.id(); - let rule = self.size_rules(id)[axis].at_least(min); - self.set_size_rule(id, axis, rule); + self.edit_bound(id.id(), axis, |bound| bound.min = Some(min)); } - /// Puts a cap over it, keeping a floor it already had. See - /// [`SizeRule::at_most`]. + /// Puts a cap over it, keeping a floor it already had. pub fn set_max_len(&mut self, id: impl IdLike, axis: Axis, max: Len) { - let id = id.id(); - let rule = self.size_rules(id)[axis].at_most(max); - self.set_size_rule(id, axis, rule); + self.edit_bound(id.id(), axis, |bound| bound.max = Some(max)); + } + + /// Edits one axis's bound where it sits, rather than reading the whole + /// rule out and writing it back: an expression beside the bound is not + /// this edit's business, and copying it to move one end would be the + /// only thing here that ever copies one. + fn edit_bound(&mut self, id: WidgetId, axis: Axis, edit: impl FnOnce(&mut Bound)) { + let bound = &mut self.data_mut(id).unwrap().size[axis].bound; + let before = *bound; + edit(bound); + if *bound != before { + self.needs_redraw.insert(id); + } } /// Where this widget sits in a box longer than the length it takes. diff --git a/tests/scenario/mod.rs b/tests/scenario/mod.rs index 4ffa29b..b6f1e87 100644 --- a/tests/scenario/mod.rs +++ b/tests/scenario/mod.rs @@ -354,11 +354,10 @@ fn describe(id: WidgetId, h: &Harness) -> String { // every part of a rule prints: the preferred length and the bounds are // independent, and a bound lumped into "no rule" could not be rebuilt. let rule = |r: &SizeRule| { - let mut out = match &r.request { - None => String::new(), - Some(SizeRequest::Linear(len)) => format!("{len}"), - Some(request) => format!("{request:?}"), - }; + let mut out = r + .request + .as_ref() + .map_or_else(String::new, |r| format!("{r}")); if let Some(min) = r.bound.min { out += &format!(">{}", LayoutLen::from(min)); }