Drop a multiply to the step below rather than rounding it

Bryan's call, 2026-09-16, taken for the cycles: a share now lands a
thousandth of a pixel short of its row instead of on it, which is less than
an even number of pixels draws.

`Fixed::mul` is a widening multiply and a shift, with the sign branch and the
half-step add gone. The two short-circuits priced against the old multiply go
with it: `UiSpan::within`'s test for a span that is the whole of its parent,
and `Fixed::scaled`'s test for nothing scaled by something, which was the
whole of `scaled` -- both cases come out of the truncating multiply unchanged,
and the bodies the comparisons cost were what kept the inliner from taking
`within` at all. `nm` is the check: `<UiSpan>::within` is a symbol in the
rounding head and in neither the float head nor this one.

`Holds::through` inverts the multiply, so its widening is re-derived: each
rounding now drops a whole step where it dropped half of one, which doubles
the allowance for the two routes to a length, and the multiply on the way in
drops only downward, so its own step goes at the top of the range alone. The
derived allowance for one truncation either side is measurably too narrow --
it excludes boxes drawings were made in, in eleven generated cases -- because
each route is a chain of multiplies rather than one.

Measured on the fixed-shape fixture (`Edits::fixed_branches`), seed 1 depth 8,
500 frames of `many`, medians of 25 runs of uninstrumented release binaries
with this VM's garbage `perf` readings dropped:

| | instructions | cycles | IPC |
| --- | ---: | ---: | ---: |
| `5ed9e87`, the float head | 1,761M | 688M | 2.561 |
| `60367d8`, rounding | 1,915M | 777M | 2.465 |
| this | 1,800M | 715M | 2.516 |

-6.0% instructions and -8.0% cycles against `60367d8`, whose twenty-five work
counters are identical to this one's, so that pair is the same work at a
different speed. It leaves +2.2% and +3.9% against the float head, from
+8.7% and +12.9% -- but the float head draws 100 widgets to this one's 97 and
writes 4,272 primitives to 3,951, so that pair is not, and the remainder is
not all arithmetic.

Checked: fmt, clippy, 80 suite tests and 18 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, unchanged), and depth-6 oracle seeds 18
and 190 passing with 326 still failing. `view`, `minimal`, `text`, `random`
and the tab replay render byte-identical at 1920x1200; `tabs` differs on
4,664 of 2,304,000 pixels, single-pixel-wide runs along 80 columns of one
band of rounded rects, which is an antialiased edge moved less than a pixel.

Three tests say what changed rather than being relaxed: a multiply drops on
both sides of zero, a division cannot put back what it dropped, and an
unevenly nested row's shares stay contiguous and end at its edge with each
edge on the even division or one step below.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
iris-aiandClaude Opus 5 committed 2026-09-16 17:03:45 -04:00
1 parent 60367d806e
commit 08c9d5aa32
5 files changed
+101 -73

No files matched your search

+42 -26
View File
@@ -55,25 +55,31 @@ impl Holds {
if rel == 0 {
return Self::ANY;
}
// 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.
// In half steps. The box a length was composed down the chain from
// and the box the same length is measured against the window in are
// two routes to one number, each rounding where the other does not,
// and each rounding drops a whole step since `Fixed::mul` truncates:
// two steps either side. The multiply on the way in drops a step of
// its own, and only downward, so it is one more step at the top and
// nothing at the bottom -- and the whole of a box has no multiply in
// it, 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, a step a level down a chain of widgets
// each taking the whole of its parent, which is the unsound
// direction: 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.
const ROUTES: i64 = 4;
let px = len.px.raw() as i64;
let half_rel = REL_SHIFT - 1;
let slack = match rel == Rel::ONE.raw() as i64 {
true => 2,
false => 3,
let way_in = match rel == Rel::ONE.raw() as i64 {
true => 0,
false => 2,
};
let lo = ((self.lo.raw() as i64 - px) * 2 - slack) << half_rel;
let hi = ((self.hi.raw() as i64 - px) * 2 + slack) << half_rel;
let lo = ((self.lo.raw() as i64 - px) * 2 - ROUTES) << half_rel;
let hi = ((self.hi.raw() as i64 - px) * 2 + ROUTES + way_in) << 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.
@@ -127,25 +133,35 @@ 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.
/// taken off it, brings no multiply of its own: only the two routes to
/// the same length are left to allow for, and not a rounding that did
/// not happen. Widening for it 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() {
fn the_whole_of_a_box_widens_by_the_routes_alone() {
let at = Px::from_int(956);
let step = |len: Px| Holds {
lo: len - Px::STEP,
hi: len + Px::STEP,
let two_steps = |len: Px| Holds {
lo: len - Px::from_raw(2),
hi: len + Px::from_raw(2),
};
assert_eq!(Holds::at(at).through(Len::FULL), step(at));
assert_eq!(Holds::at(at).through(Len::FULL), two_steps(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))
two_steps(at + Px::from_int(8))
);
}
/// A truncating multiply only ever drops, so the step it needs allowing
/// for on the way in belongs at the top of the range and not the bottom.
#[test]
fn a_fraction_widens_further_up_than_down() {
let half = Len::from_parts(Rel::from_f32(0.5), Px::ZERO);
let holds = Holds::at(Px::from_int(100)).through(half);
let box_len = Px::from_int(200);
assert!(holds.hi - box_len > box_len - holds.lo, "{holds:?}");
}
#[test]
fn a_boundary_the_next_step_along_does_not_admit_it() {
let boundary = Px::from_int(10);
+3 -3
View File
@@ -1147,9 +1147,9 @@ impl AxisRemap {
true => offset,
false => offset / scale.extent,
};
let from_px = scale.from_px + scale.from_px_span.scaled(fraction);
let to_rel = scale.to_rel + scale.to_rel_span.scaled(fraction);
let to_px = scale.to_px + scale.to_px_span.scaled(fraction);
let from_px = scale.from_px + scale.from_px_span.mul(fraction);
let to_rel = scale.to_rel + scale.to_rel_span.mul(fraction);
let to_px = scale.to_px + scale.to_px_span.mul(fraction);
Len::from_parts(to_rel, scalar.px - from_px + to_px)
}
}