From 60367d806ebd9947de1a42c0ed9ff745aa5a85ca Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Wed, 16 Sep 2026 16:46:44 -0400 Subject: [PATCH] Do not widen a validity range where nothing rounded `Holds::through` inverts `px + rel * box`, and allowed three half steps either side: one for that multiply's rounding and two for the difference between a length composed down the chain and the same length measured against the window. The whole of a box has no multiply in it -- `rel` is one and taking the pixels off again is exact -- so the first half step was being allowed for a rounding that did not happen, and it compounded: a chain of widgets each taking the whole of its parent grew the interval half a step a level. Traced while making the multiply truncate, where the same compounding moved the interval off the box the drawing was made in and fired the `Holds` assertion in eleven generated cases. A range wider than what a drawing holds for is one that admits reusing it where it does not hold, so this is the unsound direction to be loose in. Checked: fmt, clippy, 80 suite tests and 16 core unit tests, the release oracle at 100 seeds, all fifteen shrinker cases at 400 seeds of depth 5 (seed 288 on `region-node` still failing and unchanged by this), and `tabs`, `view`, `minimal`, `text`, `random` plus the tab replay byte-identical at 1920x1200. Co-Authored-By: Claude Opus 5 --- core/src/ui/holds.rs | 46 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/core/src/ui/holds.rs b/core/src/ui/holds.rs index 25e142c..b30ab6e 100644 --- a/core/src/ui/holds.rs +++ b/core/src/ui/holds.rs @@ -1,4 +1,4 @@ -use crate::{Len, Px, REL_SHIFT, fixed::div_toward, fixed::narrow}; +use crate::{Len, Px, REL_SHIFT, Rel, fixed::div_toward, fixed::narrow}; use std::ops::RangeInclusive; /// The lengths of a box, in pixels, that one drawing of a widget holds for: @@ -55,15 +55,25 @@ impl Holds { if rel == 0 { return Self::ANY; } - // Three half steps either side -- one for the rounding on the way - // in, two for the difference between a length composed down the - // chain and the same length measured against the window -- and half - // of what a `Rel` counts in, to divide by the fraction. Exact until - // the division takes it back to the grid. + // Half steps either side: two for the difference between a length + // composed down the chain and the same length measured against the + // window, and one more for the rounding on the way in -- which the + // whole of a box did not have, however many pixels were added to it, + // since multiplying by one is exact and taking the pixels off again + // is too. Allowing for it there anyway compounded, half a step a + // level down a chain of widgets each taking the whole of its parent, + // and a range wider than what a drawing holds for admits reusing it + // where it does not hold. Shifted by half of what a `Rel` counts in, + // to divide by the fraction: exact until the division takes it back + // to the grid. let px = len.px.raw() as i64; let half_rel = REL_SHIFT - 1; - let lo = ((self.lo.raw() as i64 - px) * 2 - 3) << half_rel; - let hi = ((self.hi.raw() as i64 - px) * 2 + 3) << half_rel; + let slack = match rel == Rel::ONE.raw() as i64 { + true => 2, + false => 3, + }; + let lo = ((self.lo.raw() as i64 - px) * 2 - slack) << half_rel; + let hi = ((self.hi.raw() as i64 - px) * 2 + slack) << half_rel; // Dividing by a negative turns the ends around, so which end each // bound comes from is decided before dividing rather than by taking // the min and max of four divisions. @@ -116,6 +126,26 @@ mod tests { } } + /// A widget handed the whole of its parent's box, with or without pixels + /// taken off it, brings no rounding of its own: only the step between a + /// length composed down the chain and the same length measured against + /// the window is left to allow for. Widening for the multiply as well + /// grew the interval a level at a time down a chain of them. + #[test] + fn the_whole_of_a_box_widens_by_the_one_step_it_has_to() { + let at = Px::from_int(956); + let step = |len: Px| Holds { + lo: len - Px::STEP, + hi: len + Px::STEP, + }; + assert_eq!(Holds::at(at).through(Len::FULL), step(at)); + let less_eight = Len::from_parts(Rel::ONE, Px::from_int(-8)); + assert_eq!( + Holds::at(at).through(less_eight), + step(at + Px::from_int(8)) + ); + } + #[test] fn a_boundary_the_next_step_along_does_not_admit_it() { let boundary = Px::from_int(10);