math: Make GridSize unsigned.

This makes it so that `GridAab::size()` can simply return unsigned
integers and never panic. It comes at the cost of needing a bunch
more numeric casting. I might fix this by changing the
`impl Into<GridSize>` bound on `GridAab` construction, since
the constructor has to do range checks anyway, but right now I don’t
want to invent a new trait for that purpose. (In principle,
`size: impl Into<Size3D<T>> where T: NumCast` would do, but that
would probably be inconveniently ambiguous.)

CHANGELOG.md

@@ -21,7 +21,7 @@
     - `math::GridAab::union_cube()` is a convenience for expanding the box to include one cube.
     - `math::GridAab::to_free()` is another name for `Aab::from()`, useful for method chaining and not importing `Aab`.
     - `math::GridRotation::transform_size()` rotates `GridSize` values, like `transform_vector()` without negation.
-    - `math::GridSize` type alias for `euclid::Size3D<GridCoordinate, Cube>`.
+    - `math::GridSize` type alias for `euclid::Size3D<u32, Cube>`, the sizes of `GridAab`s.
     - `math::Octant` is a new type identifying an octant.
     - `math::OctantMap` is a new type storing eight values associated with octants (like `FaceMap` is to `Face6`).
     - `math::Vol::subdivide()` splits a `Vol`'s bounds and data into parts that can be processed in parallel.

all-is-cubes-base/src/math/coord.rs

@@ -7,14 +7,19 @@ use crate::math::Cube;
 /// Coordinates that are locked to the cube grid.
 pub type GridCoordinate = i32;
 
+/// Numeric type in a [`GridSize`].
+///
+/// TODO: This needs a cleaner name.
+pub type GridSizeCoord = u32;
+
 /// Positions that are locked to the cube grid.
 pub type GridPoint = Point3D<GridCoordinate, Cube>;
 
 /// Vectors that are locked to the cube grid.
 pub type GridVector = Vector3D<GridCoordinate, Cube>;
 
 /// Sizes of grid-aligned objects.
-pub type GridSize = Size3D<GridCoordinate, Cube>;
+pub type GridSize = Size3D<GridSizeCoord, Cube>;
 
 /// Coordinates that are not locked to the cube grid.
 ///

all-is-cubes-base/src/math/grid_aab.rs

@@ -4,12 +4,12 @@
 use core::fmt;
 use core::ops::Range;
 
-use euclid::{Size3D, Vector3D};
+use euclid::Vector3D;
 use manyfmt::Refmt;
 
 use crate::math::{
     sort_two, Aab, Axis, Cube, Face6, FaceMap, FreeCoordinate, FreePoint, GridCoordinate, GridIter,
-    GridPoint, GridSize, GridVector, Gridgid, Vol,
+    GridPoint, GridSize, GridSizeCoord, GridVector, Gridgid, Vol,
 };
 use crate::resolution::Resolution;
 use crate::util::ConciseDebug;
@@ -64,6 +64,10 @@ impl GridAab {
     ///
     /// Panics if the sizes are negative or the resulting range would cause
     /// numeric overflow. Use [`GridAab::checked_from_lower_upper`] to avoid panics.
+    //---
+    // TODO: It would be more convenient for callers if `sizes` accepted `Size3D<GridCoordinate>`
+    // and other such alternate numeric types. There would be no disadvantage since this is a
+    // range-checked operation anyway. However, we'd need a custom conversion trait to handle that.
     #[track_caller]
     #[allow(clippy::missing_inline_in_public_items)] // is generic already
     pub fn from_lower_size(lower_bounds: impl Into<GridPoint>, sizes: impl Into<GridSize>) -> Self {
@@ -154,9 +158,9 @@ impl GridAab {
         fn inner(lower_bounds: GridPoint, size: GridSize) -> Result<GridAab, GridOverflowError> {
             let upper_bounds = (|| {
                 Some(GridPoint::new(
-                    lower_bounds.x.checked_add(size.width)?,
-                    lower_bounds.y.checked_add(size.height)?,
-                    lower_bounds.z.checked_add(size.depth)?,
+                    lower_bounds.x.checked_add_unsigned(size.width)?,
+                    lower_bounds.y.checked_add_unsigned(size.height)?,
+                    lower_bounds.z.checked_add_unsigned(size.depth)?,
                 ))
             })()
             .ok_or(GridOverflowError(OverflowKind::OverflowedSize {
@@ -213,7 +217,7 @@ impl GridAab {
     // TODO: add doctest example of failure
     #[inline]
     pub fn volume(&self) -> Option<usize> {
-        let sizes = self.unsigned_size();
+        let sizes = self.size();
         let mut volume: usize = 1;
         for i in Axis::ALL {
             volume = volume.checked_mul(usize::try_from(sizes[i]).ok()?)?;
@@ -225,7 +229,7 @@ impl GridAab {
     /// converted to [`f64`].
     #[inline]
     pub fn volume_f64(&self) -> f64 {
-        self.unsigned_size().to_f64().volume()
+        self.size().to_f64().volume()
     }
 
     /// Computes the surface area of this box; 1 unit of area = 1 cube-face.
@@ -234,7 +238,7 @@ impl GridAab {
     /// want float anyway.
     #[inline]
     pub fn surface_area_f64(&self) -> f64 {
-        let size = self.unsigned_size().to_f64();
+        let size = self.size().to_f64();
         size.width * size.height * 2. + size.width * size.depth * 2. + size.height * size.depth * 2.
     }
 
@@ -243,7 +247,7 @@ impl GridAab {
     /// This does not necessarily mean that its size is zero on all axes.
     #[inline]
     pub fn is_empty(&self) -> bool {
-        self.unsigned_size().is_empty()
+        self.size().is_empty()
     }
 
     /// Inclusive upper bounds on cube coordinates, or the most negative corner of the
@@ -263,24 +267,10 @@ impl GridAab {
     }
 
     /// Size of the box in each axis; equivalent to
-    /// `self.upper_bounds() - self.lower_bounds()`.
-    ///
-    /// TODO: This calculation can overflow. Switch to unsigned arithmetic or `Option` or something.
+    /// `self.upper_bounds() - self.lower_bounds()`, except that the result is
+    /// unsigned (which is necessary so that it cannot overflow).
     #[inline]
     pub fn size(&self) -> GridSize {
-        self.upper_bounds
-            .zip(self.lower_bounds, GridCoordinate::wrapping_sub)
-            .into()
-    }
-
-    /// Size of the box in each axis; equivalent to
-    /// `self.upper_bounds() - self.lower_bounds()`, except that the result is an
-    /// unsigned integer.
-    ///
-    /// Compared to [`GridAab::size()`], this is a convenience so that callers needing
-    /// unsigned integers do not need to write a fallible-looking conversion.
-    #[inline]
-    pub fn unsigned_size(&self) -> Size3D<u32, Cube> {
         self.upper_bounds
             // Two’s complement math trick: If the subtraction overflows and wraps, the following
             // conversion to u32 will give us the right answer anyway.
@@ -563,7 +553,7 @@ impl GridAab {
         let lower = self.lower_bounds().min(other.lower_bounds());
         let upper = self.upper_bounds().max(other.upper_bounds());
         // Subtraction and construction should not fail.
-        Self::from_lower_size(lower, upper - lower)
+        Self::from_lower_size(lower, (upper - lower).to_u32())
     }
 
     /// Extend the bounds of `self` as needed to enclose `other`.
@@ -845,8 +835,18 @@ impl GridAab {
     pub fn abut(self, face: Face6, thickness: GridCoordinate) -> Result<Self, GridOverflowError> {
         let axis = face.axis();
 
+        // Apply change in size.
         let mut size = self.size();
-        size[axis] = thickness.max(-size[axis]).abs();
+        size[axis] = match GridSizeCoord::try_from(thickness) {
+            // If thickness is nonnegative, the new size is defined by it directly.
+            Ok(positive) => positive,
+            Err(_) => {
+                // If negative, the new size cannot be larger than the old size.
+                // The tricky part is handling GridCoordinate::MIN, which cannot be
+                // directly negated without overflow -- so we use unsigned_abs() to do it.
+                thickness.unsigned_abs().min(size[axis])
+            }
+        };
 
         // Coordinate on the axis that the two boxes share
         let abutting_coordinate = if face.is_positive() {
@@ -867,7 +867,7 @@ impl GridAab {
             abutting_coordinate
         } else {
             // Cannot overflow because we already min()ed it.
-            abutting_coordinate - size[axis]
+            abutting_coordinate.wrapping_sub_unsigned(size[axis])
         };
         lower_bounds[axis] = new_lower_bound;
 
@@ -1070,7 +1070,7 @@ mod tests {
 
     #[test]
     fn to_vol_error() {
-        let big = GridAab::from_lower_size([0, 0, 0], [i32::MAX, i32::MAX, i32::MAX]);
+        let big = GridAab::from_lower_size([0, 0, 0], GridSize::splat(i32::MAX as u32));
         assert_eq!(
             big.to_vol::<ZMaj>().unwrap_err().to_string(),
             "GridAab(0..2147483647, 0..2147483647, 0..2147483647) has a volume of \

all-is-cubes-base/src/math/rotation.rs

@@ -411,7 +411,7 @@ impl GridRotation {
     /// Rotate the size value by this rotation.
     ///
     /// This is similar to [`GridRotation::transform_vector()`] except that the components
-    /// are only swapped, not negated.
+    /// are only swapped, not negated, and there is no possibility of numeric overflow.
     #[inline]
     pub fn transform_size(self, size: GridSize) -> GridSize {
         let basis = self.to_basis();
@@ -687,8 +687,9 @@ mod tests {
         for rot in GridRotation::ALL {
             let vector = GridVector::new(1, 20, 300);
             assert_eq!(
-                rot.transform_vector(vector).abs(),
-                rot.transform_size(GridSize::from(vector)).to_vector()
+                rot.transform_vector(vector).abs().to_u32(),
+                rot.transform_size(GridSize::from(vector.to_u32()))
+                    .to_vector()
             )
         }
     }

all-is-cubes-base/src/math/vol.rs

@@ -207,7 +207,7 @@ where
         V: Clone,
     {
         Self::from_fn(
-            GridAab::from_lower_size([0, 0, 0], vec3(DX, DY, DZ).to_i32()),
+            GridAab::from_lower_size([0, 0, 0], vec3(DX, DY, DZ).to_u32()),
             |p| array[p.z as usize][(DY - 1) - (p.y as usize)][p.x as usize].clone(),
         )
     }
@@ -346,9 +346,9 @@ impl<C> Vol<C, ZMaj> {
             .to_point();
 
         // Bounds check, expressed as a single unsigned comparison.
-        if (deoffsetted.x as u32 >= sizes.width as u32)
-            | (deoffsetted.y as u32 >= sizes.height as u32)
-            | (deoffsetted.z as u32 >= sizes.depth as u32)
+        if (deoffsetted.x as u32 >= sizes.width)
+            | (deoffsetted.y as u32 >= sizes.height)
+            | (deoffsetted.z as u32 >= sizes.depth)
         {
             return None;
         }
@@ -658,7 +658,7 @@ pub(crate) mod vol_arb {
             volume: usize,
         ) -> arbitrary::Result<Self> {
             // Pick sizes within the volume constraint.
-            let mut limit: GridCoordinate = volume.try_into().unwrap_or(GridCoordinate::MAX);
+            let mut limit: u32 = volume.try_into().unwrap_or(u32::MAX);
             let size_1 = u.int_in_range(0..=limit)?;
             limit /= size_1.max(1);
             let size_2 = u.int_in_range(0..=limit)?;
@@ -667,24 +667,27 @@ pub(crate) mod vol_arb {
 
             // Shuffle the sizes to remove any bias.
             let sizes = *u.choose(&[
-                GridVector::new(size_1, size_2, size_3),
-                GridVector::new(size_1, size_3, size_2),
-                GridVector::new(size_2, size_1, size_3),
-                GridVector::new(size_2, size_3, size_1),
-                GridVector::new(size_3, size_1, size_2),
-                GridVector::new(size_3, size_2, size_1),
+                vec3(size_1, size_2, size_3),
+                vec3(size_1, size_3, size_2),
+                vec3(size_2, size_1, size_3),
+                vec3(size_2, size_3, size_1),
+                vec3(size_3, size_1, size_2),
+                vec3(size_3, size_2, size_1),
             ])?;
 
             // Compute lower bounds that are valid for the sizes.
+            let possible_lower_bounds = sizes.map(|coord| {
+                GridCoordinate::MIN..=GridCoordinate::MAX.saturating_sub_unsigned(coord)
+            });
             let lower_bounds = GridPoint::new(
-                u.int_in_range(GridCoordinate::MIN..=GridCoordinate::MAX - sizes.x)?,
-                u.int_in_range(GridCoordinate::MIN..=GridCoordinate::MAX - sizes.y)?,
-                u.int_in_range(GridCoordinate::MIN..=GridCoordinate::MAX - sizes.z)?,
+                u.int_in_range(possible_lower_bounds.x)?,
+                u.int_in_range(possible_lower_bounds.y)?,
+                u.int_in_range(possible_lower_bounds.z)?,
             );
 
             Ok(GridAab::from_lower_size(lower_bounds, sizes)
                 .to_vol()
-                .unwrap())
+                .expect("GridAab as Arbitrary failed to compute valid bounds"))
         }
     }
 
@@ -772,15 +775,17 @@ fn find_zmaj_subdivision(bounds: GridAab) -> Option<(GridAab, GridAab, usize)> {
     // for the result to be valid.
     for axis in [Axis::X, Axis::Y, Axis::Z] {
         let axis_range = bounds.axis_range(axis);
-        if axis_range.len() >= 2 {
-            let split_size = (axis_range.end - axis_range.start) / 2;
+        let size: u32 = bounds.size()[axis];
+        if size >= 2 {
+            #[allow(clippy::cast_possible_wrap)] // known to fit
+            let split_coordinate = axis_range.start + (size / 2) as i32;
 
-            let mut lower_half_size = bounds.size();
-            lower_half_size[axis] = split_size;
-            let lower_half = GridAab::from_lower_size(bounds.lower_bounds(), lower_half_size);
+            let mut lower_half_ub = bounds.upper_bounds();
+            lower_half_ub[axis] = split_coordinate;
+            let lower_half = GridAab::from_lower_upper(bounds.lower_bounds(), lower_half_ub);
 
             let mut upper_half_lb = bounds.lower_bounds();
-            upper_half_lb[axis] += split_size;
+            upper_half_lb[axis] = split_coordinate;
             let upper_half = GridAab::from_lower_upper(upper_half_lb, bounds.upper_bounds());
 
             let lower_half_volume = lower_half

all-is-cubes-content/src/alg.rs

@@ -105,7 +105,7 @@ where
     let interior = bounding_box.expand(FaceMap::symmetric([-1, 0, -1]));
     let low = bounding_box.lower_bounds();
     let high = bounding_box.upper_bounds() - GridVector::new(1, 1, 1);
-    let size = bounding_box.size();
+    let size = bounding_box.size().to_i32();
     f(
         low,
         Face6::PZ,

all-is-cubes-content/src/blocks.rs

@@ -21,7 +21,7 @@ use all_is_cubes::euclid::Vector3D;
 use all_is_cubes::linking::{BlockModule, BlockProvider, GenError, InGenError};
 use all_is_cubes::math::{
     rgb_const, rgba_const, Cube, Face6, FreeCoordinate, GridAab, GridCoordinate, GridRotation,
-    GridVector, Gridgid, Rgb, Rgba,
+    GridSizeCoord, GridVector, Gridgid, Rgb, Rgba,
 };
 use all_is_cubes::op::Operation;
 use all_is_cubes::space::{Space, SpacePhysics, SpaceTransaction};
@@ -287,7 +287,7 @@ pub async fn install_demo_blocks(
                 space.fill_uniform(
                     GridAab::from_lower_size(
                         [resolution_g / 2 - 1, 0, resolution_g / 2 - 1],
-                        [2, resolution_g / 2, 2],
+                        [2, GridSizeCoord::from(resolution) / 2, 2],
                     ),
                     &leg,
                 )?;

all-is-cubes-content/src/city.rs

@@ -16,7 +16,7 @@ use all_is_cubes::inv::{Slot, Tool};
 use all_is_cubes::linking::{BlockProvider, InGenError};
 use all_is_cubes::math::{
     rgba_const, Cube, Face6, FaceMap, FreeCoordinate, GridAab, GridCoordinate, GridRotation,
-    GridSize, GridVector, Gridgid,
+    GridSize, GridVector, Gridgid, VectorOps,
 };
 use all_is_cubes::op::Operation;
 use all_is_cubes::raycast::Raycaster;
@@ -77,7 +77,10 @@ pub(crate) async fn demo_city<I: Instant>(
     let lamp_spacing = 20;
     let sky_height = 30;
     let ground_depth = 30; // TODO: wavy_landscape is forcing us to have extra symmetry here
-    let space_size = params.size.unwrap_or(GridSize::new(160, 60, 160));
+    let space_size = params
+        .size
+        .unwrap_or(GridSize::new(160, 60, 160))
+        .map(|size| i32::try_from(size).unwrap_or(i32::MAX));
     let bounds = GridAab::from_lower_upper(
         [-space_size.width / 2, -ground_depth, -space_size.depth / 2],
         [space_size.width / 2, sky_height, space_size.depth / 2],
@@ -513,7 +516,7 @@ fn place_one_exhibit<I: Instant>(
                     .bounds()
                     .size()
                     .width
-                    .min(enclosure_footprint.size().width * i32::from(info_resolution)),
+                    .min(enclosure_footprint.size().width * u32::from(info_resolution)),
                 ..exhibit_info_space.bounds().size()
             },
         );
@@ -559,8 +562,11 @@ fn place_one_exhibit<I: Instant>(
             Placement::Underground => GridVector::new(
                 // centered horizontally
                 enclosure_footprint.lower_bounds().x
-                    + (enclosure_footprint.size().width - info_sign_space.bounds().size().width)
-                        / 2,
+                    + i32::try_from(
+                        (enclosure_footprint.size().width - info_sign_space.bounds().size().width)
+                            / 2,
+                    )
+                    .unwrap(),
                 // at above-the-entrance level
                 entranceway_height,
                 // on the surface of the corridor wall -- TODO: We don't actually know fundamentally that the corridor