Cover joining two expressions, and print a request one way
Four findings over05e6ced, which no earlier round reviewed. `SizeRequest::join` grafts the other side's nodes into this side's arena, which happens only when both sides are expressions -- nothing in the suite did that, so the whole thing passed with a `panic!` in that arm. It is where a missed renumbering would be silent, since an operand copied without remapping still names a node that exists. A fixture at two window widths with absolute geometry covers it now, and the deferred corpus puts an expression on both sides of one arm; both were checked to reach it by instrumenting again. The path was already right. `SizeRequest` grew a `Display` because a derived `Debug` of an arena is not a tree anyone can write out again, and `describe` moved onto it -- but `Debug` stayed derived, so the `assert_eq!`s in `cases/deferred.rs`, the only place a request is compared, still printed the arena on failure. `Debug` forwards to `Display`. `Nodes::linear` asked nothing of the arena beside it: it is `Operand`'s question, the way `RequestedLen::linear` is `RequestedLen`'s. `describe`'s `|r| format!("{r}")` shadowed the `r: &SizeRule` four lines above it. Format, workspace clippy under -D warnings with and without layout-diagnostics, 207 ordinary and 211 diagnostic tests, the cold dump byte-identical to05e6cedacross all 34,986 boxes, and 400 depth-5 trees in each of the three deferred corpora in 200.95s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
05e6ced31d
commit
f48e04ed36
4 files changed
+45
-11
No files matched your search
+20
-10
@@ -40,6 +40,16 @@ enum Operand {
|
|||||||
Node(u32),
|
Node(u32),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl Operand {
|
||||||
|
/// The length itself, where no comparison is waiting on an allocation.
|
||||||
|
fn linear(&self) -> Option<LayoutLen> {
|
||||||
|
match *self {
|
||||||
|
Self::Linear(len) => Some(len),
|
||||||
|
Self::Node(_) => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// One sum or comparison, with its two operands. Whether anything under it
|
/// One sum or comparison, with its two operands. Whether anything under it
|
||||||
/// divides leftover space is carried on the node rather than walked for,
|
/// divides leftover space is carried on the node rather than walked for,
|
||||||
/// because every caller of one asks.
|
/// because every caller of one asks.
|
||||||
@@ -63,14 +73,6 @@ impl Nodes {
|
|||||||
self.0[index as usize]
|
self.0[index as usize]
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The length itself, where no comparison is waiting on an allocation.
|
|
||||||
fn linear(&self, at: Operand) -> Option<LayoutLen> {
|
|
||||||
match at {
|
|
||||||
Operand::Linear(len) => Some(len),
|
|
||||||
Operand::Node(_) => None,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn leftover(&self, at: Operand) -> bool {
|
fn leftover(&self, at: Operand) -> bool {
|
||||||
match at {
|
match at {
|
||||||
Operand::Linear(len) => len.leftover > Weight::ZERO,
|
Operand::Linear(len) => len.leftover > Weight::ZERO,
|
||||||
@@ -82,7 +84,7 @@ impl Nodes {
|
|||||||
/// lengths that keep their order whatever the room comes to are already
|
/// lengths that keep their order whatever the room comes to are already
|
||||||
/// decided, and so are two operands that are the same thing.
|
/// decided, and so are two operands that are the same thing.
|
||||||
fn combine(&mut self, op: Op, a: Operand, b: Operand) -> Operand {
|
fn combine(&mut self, op: Op, a: Operand, b: Operand) -> Operand {
|
||||||
if let (Some(x), Some(y)) = (self.linear(a), self.linear(b)) {
|
if let (Some(x), Some(y)) = (a.linear(), b.linear()) {
|
||||||
if matches!(op, Op::Sum) {
|
if matches!(op, Op::Sum) {
|
||||||
return Operand::Linear(x + y);
|
return Operand::Linear(x + y);
|
||||||
}
|
}
|
||||||
@@ -150,7 +152,7 @@ impl Nodes {
|
|||||||
/// An expression is the same nodes the layout pass allocates, in an arena of
|
/// 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
|
/// 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.
|
/// is no second shape to keep in step and one place where folding is decided.
|
||||||
#[derive(Clone, Debug, PartialEq)]
|
#[derive(Clone, PartialEq)]
|
||||||
pub enum SizeRequest {
|
pub enum SizeRequest {
|
||||||
Linear(LayoutLen),
|
Linear(LayoutLen),
|
||||||
/// Behind a pointer, because a plain length is what nearly every rule
|
/// Behind a pointer, because a plain length is what nearly every rule
|
||||||
@@ -237,6 +239,14 @@ impl std::fmt::Display for SizeRequest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The same, since an arena printed as a struct is not a tree anyone can
|
||||||
|
/// write out again, which is what a request is printed for.
|
||||||
|
impl std::fmt::Debug for SizeRequest {
|
||||||
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||||
|
std::fmt::Display::fmt(self, f)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// A discovered length. Deferred values belong to the current layout pass;
|
/// A discovered length. Deferred values belong to the current layout pass;
|
||||||
/// widgets must not retain them. Ordinary requests remain inline lengths.
|
/// widgets must not retain them. Ordinary requests remain inline lengths.
|
||||||
#[derive(Clone, Copy, Debug, PartialEq)]
|
#[derive(Clone, Copy, Debug, PartialEq)]
|
||||||
|
|||||||
@@ -132,6 +132,27 @@ fn a_deferred_comparison_can_compare_two_different_weights() {
|
|||||||
assert_corners!(h, b, (200, 0), (300, 100));
|
assert_corners!(h, b, (200, 0), (300, 100));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_comparison_between_two_comparisons_keeps_both_of_them() {
|
||||||
|
// Joining two expressions is the one path that copies a request's nodes
|
||||||
|
// into another's arena; the floor puts a node under the copied one, so
|
||||||
|
// its operands have to be renumbered as they land.
|
||||||
|
let capped = leftover(1).min(px(40));
|
||||||
|
let floored = leftover(2).min(px(70)).max(px(10));
|
||||||
|
let mut h = Harness::new((90, 100));
|
||||||
|
let both = rect(Color::RED).width(capped.max(floored)).add(&mut h.rsc);
|
||||||
|
let rest = rect(Color::BLUE).add(&mut h.rsc);
|
||||||
|
h.set_root((both, rest).span(Dir::RIGHT));
|
||||||
|
// Under either cap, so the doubled share is the longer of the two.
|
||||||
|
assert_corners!(h, both, (0, 0), (60, 100));
|
||||||
|
assert_corners!(h, rest, (60, 0), (90, 100));
|
||||||
|
h.resize((300, 100));
|
||||||
|
h.frame();
|
||||||
|
// Over both caps, so the comparison is between 40 and 70.
|
||||||
|
assert_corners!(h, both, (0, 0), (70, 100));
|
||||||
|
assert_corners!(h, rest, (70, 0), (300, 100));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_length_expression_is_resolved_before_wrapping_text() {
|
fn a_length_expression_is_resolved_before_wrapping_text() {
|
||||||
let mut h = Harness::new((300, 500));
|
let mut h = Harness::new((300, 500));
|
||||||
|
|||||||
@@ -38,6 +38,9 @@ fn deferred_requests_agree_warm_and_cold() {
|
|||||||
0 => leftover(1).clamp(20, 120).into(),
|
0 => leftover(1).clamp(20, 120).into(),
|
||||||
1 => leftover(1).min(rel(0.5)).into(),
|
1 => leftover(1).min(rel(0.5)).into(),
|
||||||
2 => (leftover(1) + px(30)).min(leftover(2)).into(),
|
2 => (leftover(1) + px(30)).min(leftover(2)).into(),
|
||||||
|
// Both sides an expression, the one shape that
|
||||||
|
// copies a request's nodes into another's.
|
||||||
|
3 => leftover(1).min(px(40)).max(leftover(2).min(px(70))).into(),
|
||||||
_ => rules[axis].clone(),
|
_ => rules[axis].clone(),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -357,7 +357,7 @@ fn describe(id: WidgetId, h: &Harness) -> String {
|
|||||||
let mut out = r
|
let mut out = r
|
||||||
.request
|
.request
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.map_or_else(String::new, |r| format!("{r}"));
|
.map_or_else(String::new, ToString::to_string);
|
||||||
if let Some(min) = r.bound.min {
|
if let Some(min) = r.bound.min {
|
||||||
out += &format!(">{}", LayoutLen::from(min));
|
out += &format!(">{}", LayoutLen::from(min));
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user