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 <noreply@anthropic.com>
This commit is contained in:
1 parent
aea878d141
commit
60367d806e
1 file changed
+38
-8
+38
-8
@@ -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;
|
use std::ops::RangeInclusive;
|
||||||
|
|
||||||
/// The lengths of a box, in pixels, that one drawing of a widget holds for:
|
/// The lengths of a box, in pixels, that one drawing of a widget holds for:
|
||||||
@@ -55,15 +55,25 @@ impl Holds {
|
|||||||
if rel == 0 {
|
if rel == 0 {
|
||||||
return Self::ANY;
|
return Self::ANY;
|
||||||
}
|
}
|
||||||
// Three half steps either side -- one for the rounding on the way
|
// Half steps either side: two for the difference between a length
|
||||||
// in, two for the difference between a length composed down the
|
// composed down the chain and the same length measured against the
|
||||||
// chain and the same length measured against the window -- and half
|
// window, and one more for the rounding on the way in -- which the
|
||||||
// of what a `Rel` counts in, to divide by the fraction. Exact until
|
// whole of a box did not have, however many pixels were added to it,
|
||||||
// the division takes it back to the grid.
|
// 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 px = len.px.raw() as i64;
|
||||||
let half_rel = REL_SHIFT - 1;
|
let half_rel = REL_SHIFT - 1;
|
||||||
let lo = ((self.lo.raw() as i64 - px) * 2 - 3) << half_rel;
|
let slack = match rel == Rel::ONE.raw() as i64 {
|
||||||
let hi = ((self.hi.raw() as i64 - px) * 2 + 3) << half_rel;
|
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
|
// Dividing by a negative turns the ends around, so which end each
|
||||||
// bound comes from is decided before dividing rather than by taking
|
// bound comes from is decided before dividing rather than by taking
|
||||||
// the min and max of four divisions.
|
// 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]
|
#[test]
|
||||||
fn a_boundary_the_next_step_along_does_not_admit_it() {
|
fn a_boundary_the_next_step_along_does_not_admit_it() {
|
||||||
let boundary = Px::from_int(10);
|
let boundary = Px::from_int(10);
|
||||||
|
|||||||
Reference in new issue
Block a user