diff --git a/core/src/fixed.rs b/core/src/fixed.rs index 72380d6..9e71070 100644 --- a/core/src/fixed.rs +++ b/core/src/fixed.rs @@ -10,10 +10,10 @@ use std::{ /// chain, and the same box summed from what its children asked for -- and has /// to decide whether the two are the same place. In floats they land a few /// bits apart, which is a defect wherever the answer changes what is drawn -/// rather than where. Here adding and subtracting are exact and only a -/// multiply or a conversion rounds, back onto the same steps, so two routes -/// that come within half a step land on one number and everything downstream -/// compares for equality instead of for nearness. +/// rather than where. Here adding and subtracting are exact, a multiply +/// drops to the step below, and a conversion between grids takes the nearest +/// one, so two routes to one place land on one number and everything +/// downstream compares for equality instead of for nearness. /// /// `SHIFT` is the number of fractional bits, which is what makes the steps /// divide a whole number: a power of two also converts to `f32` without @@ -31,9 +31,9 @@ use std::{ )] pub struct Fixed(i32); -/// A length or a coordinate in pixels, to a sixty-fourth. Finer than anything -/// a display can show, and exact in `f32` up to 262,144 px, which is what lets -/// the same number reach the GPU. +/// A length or a coordinate in pixels, in steps of `1/1024`. Finer than +/// anything a display can show, and exact in `f32` up to 16,384 px, which is +/// what lets the same number reach the GPU. pub type Px = Fixed; /// How many bits of a pixel a [`Px`] keeps. One place, because [`PxVec2`] @@ -142,18 +142,16 @@ impl Fixed { /// Scaled by a number on any grid, which is how a length takes a fraction /// of itself and keeps being a length: the product is measured in the /// receiver's steps. + /// + /// Dropped to the step below rather than taken to the nearest one + /// (Bryan, 2026-09-16), which costs a share a thousandth of a pixel of + /// its row -- less than an even number of pixels draws. Toward negative + /// infinity on both sides of zero, since that is a shift and nothing + /// else: a value and its negation therefore land different distances + /// from where they came, so a flipped span can sit a step from its + /// mirror image. pub const fn mul(self, by: Fixed) -> Self { - Self(shift_round(self.0 as i64 * by.0 as i64, BY) as i32) - } - - /// A part of a span that is often nothing: no part of nothing is - /// nothing, for the cost of a comparison rather than a widening - /// multiply and a rounding. - pub const fn scaled(self, by: Fixed) -> Self { - match self.0 == 0 { - true => self, - false => self.mul(by), - } + Self(((self.0 as i64 * by.0 as i64) >> BY) as i32) } /// Repeated a whole number of times, which no grid rounds. @@ -198,7 +196,7 @@ impl Fixed { /// `from` and `to` a fraction of the way apart, the fraction being the /// receiver -- the argument order [`crate::util::LerpUtil`] already uses. pub const fn lerp(self, from: Fixed, to: Fixed) -> Fixed { - from.add(to.sub(from).scaled(self)) + from.add(to.sub(from).mul(self)) } pub const fn min(self, other: Self) -> Self { @@ -465,19 +463,28 @@ mod tests { assert_eq!(Px::from_int(100) * Rel::ZERO, Px::ZERO); } + /// Toward negative infinity on both sides of zero, which is what makes + /// it a shift rather than a shift and a sign branch -- and what makes a + /// value and its negation land different distances from where they came, + /// so a flipped span can sit a step from its mirror image. #[test] - fn halves_round_away_from_zero_either_side() { + fn a_multiply_drops_to_the_step_below_on_both_sides_of_zero() { // A step and a half of one, which has no step of its own. let step_and_a_half = Rel::from_f32(1.5).div_int(Px::ONE.raw()); - assert_eq!(Px::ONE * step_and_a_half, Px::from_raw(2)); + assert_eq!(Px::ONE * step_and_a_half, Px::from_raw(1)); assert_eq!(Px::ONE.neg() * step_and_a_half, Px::from_raw(-2)); } + /// A division rounds to the nearest step, so it cannot put back the + /// steps a truncating multiply dropped: a round trip comes back short, + /// never long, and by the few steps the two operations gave up. #[test] - fn dividing_by_a_fraction_undoes_multiplying_by_it() { + fn dividing_by_a_fraction_cannot_undo_a_truncating_multiply() { let third = Rel::ONE / Rel::from_int(3); let len = Px::from_int(300); - assert_eq!(len * third / third, len); + let back = len * third / third; + assert!(back <= len, "{back:?} is longer than {len:?}"); + assert!(len - back <= Px::from_raw(3), "{back:?} against {len:?}"); assert_eq!(Px::from_int(100) / Rel::from_f32(0.5), Px::from_int(200)); } diff --git a/core/src/orientation/pos.rs b/core/src/orientation/pos.rs index 495575b..6b7dd13 100644 --- a/core/src/orientation/pos.rs +++ b/core/src/orientation/pos.rs @@ -282,26 +282,12 @@ impl UiSpan { self.end += offset; } - /// The whole of the box it sits in: a span that composes to nothing and - /// a parent that changes nothing. - pub const fn is_full(&self) -> bool { - self.start.rel.raw() == Rel::ZERO.raw() - && self.start.px.raw() == Px::ZERO.raw() - && self.end.rel.raw() == Rel::ONE.raw() - && self.end.px.raw() == Px::ZERO.raw() - } - + /// Composing a box through the one it sits in, and the hottest line in + /// layout. It used to skip the multiplies where a span was the whole of + /// its parent or the parent the whole of its own; both come out of the + /// multiply unchanged anyway, and the body those comparisons cost was + /// what kept the inliner from taking this at all. pub const fn within(&self, parent: &Self) -> Self { - // A part that is the whole box is the box, and a box composed through - // the whole of its parent is itself. Both are exact -- multiplying by - // one rounds to what it started as -- and both are common enough to - // be worth four comparisons rather than four multiplies to find out. - if self.is_full() { - return *parent; - } - if parent.is_full() { - return *self; - } Self { start: self.start.within(parent), end: self.end.within(parent), diff --git a/core/src/ui/holds.rs b/core/src/ui/holds.rs index b30ab6e..080352c 100644 --- a/core/src/ui/holds.rs +++ b/core/src/ui/holds.rs @@ -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); diff --git a/core/src/ui/render_state.rs b/core/src/ui/render_state.rs index 06f231c..051f21b 100644 --- a/core/src/ui/render_state.rs +++ b/core/src/ui/render_state.rs @@ -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) } } diff --git a/tests/cases/layout.rs b/tests/cases/layout.rs index 51b69b4..5b972cf 100644 --- a/tests/cases/layout.rs +++ b/tests/cases/layout.rs @@ -266,6 +266,11 @@ fn nested_spans_divide_the_space_once_however_deep_the_nesting_is() { /// The same space, unevenly nested: weights carried up mean a share is a /// share of the whole, not of whatever branch a widget happens to sit in. +/// +/// Each edge lands on the even division or one step below it, since a share +/// is a fraction of the room and a truncating multiply gives up what that +/// fraction does not divide. What stays exact is that each share starts +/// where the last one ended and the row ends at its own edge. #[test] fn an_uneven_nesting_still_gives_every_share_the_same_length() { let mut h = Harness::new((400, 200)); @@ -279,10 +284,24 @@ fn an_uneven_nesting_still_gives_every_share_the_same_length() { let three = (b, c, d).span(Dir::RIGHT).add(&mut h.rsc); h.set_root((one, three).span(Dir::RIGHT)); + let mut start = Px::ZERO; for (i, id) in [a, b, c, d].into_iter().enumerate() { - let x = i as f32 * 100.0; - assert_corners!(h, id, (x, 0), (x + 100.0, 200)); + let got = h.region(&id).expect("widget drew nothing"); + let even = Px::from_int((i as i32 + 1) * 100); + assert_eq!(got.top_left, PxVec2::new(start, Px::ZERO), "share {i}"); + assert_eq!(got.bot_right.y, Px::from_int(200), "share {i}"); + assert!( + got.bot_right.x == even || got.bot_right.x == even.next_down(), + "share {i} ends at {:?}, not {even:?}", + got.bot_right.x + ); + start = got.bot_right.x; } + assert_eq!( + start, + Px::from_int(400), + "the row stopped short of its edge" + ); } /// However many ways a row is divided, the shares add up to the row: each