Final cleanups for GridMatrix to Gridgid migration.

* Remove `GridMatrix` support from `GridAab::transform()`.
* Remove `GridRotation::to_positive_octant_matrix()`.

CHANGELOG.md

@@ -16,14 +16,18 @@
 
 - `all-is-cubes` library:
     - The following functions have changed signature to use the new type `math::Gridgid`:
-        - `math::GridMatrix::decompose`
+        - `math::GridAab::transform()`
+        - `math::GridMatrix::decompose()`
+        - `space::Space::draw_target()`
+        - `space::SpaceTransaction::draw_target()`
 
 ### Removed
 
 - `all-is-cubes` library:
     - `math::Face7::matrix()` no longer exists. Use `Face6::face_transform()` instead.
     - `math::GridAab::index()` and `contains_cube()` no longer accept `impl Into<GridPoint>`.
       Call sites should be changed to pass only `GridPoint`.
+    - `math::GridRotation::to_positive_octant_matrix()` no longer exists. Use `to_positive_octant_transform()` instead.
 
 
 ## 0.6.0 (2023-07-29)

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

@@ -548,10 +548,7 @@ impl CityPlanner {
             [Self::LAMP_POSITION_RADIUS + 1, 2, city_radius + 1],
         );
         occupied_plots.push(road);
-        occupied_plots.push(
-            road.transform(GridRotation::CLOCKWISE.to_rotation_matrix())
-                .unwrap(),
-        );
+        occupied_plots.push(road.transform(GridRotation::CLOCKWISE.into()).unwrap());
         Self {
             space_bounds,
             city_radius,

all-is-cubes/src/drawing.rs

@@ -46,7 +46,7 @@ use crate::universe::Universe;
 /// Please note that coordinate behavior may be surprising. [`embedded_graphics`]
 /// considers coordinates to refer to pixel centers, which is similar but not identical
 /// to our use of [`GridPoint`] to identify a cube by its low corner. The `transform` is
-/// then applied to those coordinates. So, for example, applying [`GridMatrix::FLIP_Y`]
+/// then applied to those coordinates. So, for example, applying [`Gridgid::FLIP_Y`]
 /// to a [`Rectangle`] whose top-left corner is `[0, 0]` will result in a [`GridAab`]
 /// which *includes* the <var>y</var> = 0 row — not one which abuts it and is strictly in
 /// the negative y range.

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

@@ -5,12 +5,11 @@ use std::fmt;
 use std::iter::FusedIterator;
 use std::ops::Range;
 
-use cgmath::{EuclideanSpace, Point3, Transform, Vector3};
+use cgmath::{EuclideanSpace as _, Point3, Vector3};
 
 use crate::block::Resolution;
 use crate::math::{
-    sort_two, Aab, Face6, FaceMap, FreeCoordinate, GridCoordinate, GridMatrix, GridPoint,
-    GridVector,
+    sort_two, Aab, Face6, FaceMap, FreeCoordinate, GridCoordinate, GridPoint, GridVector, Gridgid,
 };
 
 /// An axis-aligned box with integer coordinates, whose volume is no larger than [`usize::MAX`].
@@ -546,20 +545,12 @@ impl GridAab {
         Self::from_lower_upper(new_lb, new_ub)
     }
 
-    /// Transforms the box.
-    ///
-    /// Caution: The results are undefined if the matrix mixes axes
-    /// rather than only swapping and scaling them.
-    /// TODO: Find the proper mathematical concept to explain that.
-    /// TODO: Check and error in that case.
+    /// Translate and rotate the box according to the given transform.
     ///
     /// TODO: Fail nicely on numeric overflow.
     /// The `Option` return is not currently used.
-    ///
-    /// TODO: the `Into<GridMatrix>` is temporary while we migrate to `Gridgid`
     #[must_use]
-    pub fn transform(self, transform: impl Into<GridMatrix>) -> Option<Self> {
-        let transform = transform.into();
+    pub fn transform(self, transform: Gridgid) -> Option<Self> {
         let mut p1 = transform.transform_point(self.lower_bounds());
         let mut p2 = transform.transform_point(self.upper_bounds());
 
@@ -1135,6 +1126,7 @@ impl fmt::Debug for RangeWithLength {
 mod tests {
     use super::*;
     use crate::block::Resolution::*;
+    use crate::math::{GridRotation, Gridgid};
     use cgmath::point3;
     use indoc::indoc;
 
@@ -1220,30 +1212,15 @@ mod tests {
     #[test]
     fn transform_general() {
         assert_eq!(
-            GridAab::from_lower_size([1, 2, 3], [10, 20, 30]).transform(GridMatrix::new(
-                0, 1, 0, //
-                2, 0, 0, //
-                0, 0, -1, //
-                100, 100, 100,
-            )),
-            Some(GridAab::from_lower_size([104, 101, 67], [40, 10, 30]))
-        );
-    }
-
-    #[test]
-    fn transform_degenerate() {
-        assert_eq!(
-            GridAab::from_lower_size([1, 2, 3], [10, 20, 30]).transform(GridMatrix::new(
-                1, 0, 0, //
-                0, 0, 0, //
-                0, 0, 1, //
-                3, 4, 5
-            )),
-            Some(GridAab::from_lower_size([4, 4, 8], [10, 0, 30]))
+            GridAab::from_lower_size([1, 2, 3], [10, 20, 30]).transform(Gridgid {
+                rotation: GridRotation::RYXz,
+                translation: GridVector::new(100, 100, 100),
+            }),
+            Some(GridAab::from_lower_size([102, 101, 67], [20, 10, 30]))
         );
     }
 
-    // TODO: test and improve transform() on matrices with skew / other non-axis-swaps
+    // TODO: test transform() on more cases
 
     /// Translation overflowing to partially outside of the numeric range
     /// clips the box's size to the range.

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

@@ -216,7 +216,9 @@ impl GridRotation {
         }
     }
 
-    // TODO: public? do we want this to be our API? should this also be a From impl?
+    /// Returns the basis vectors for the unrotated coordinate system in
+    /// the rotated coordinate system.
+    // TODO: public? do we want this to be our API?
     #[doc(hidden)]
     #[inline]
     #[rustfmt::skip] // dense data layout
@@ -324,51 +326,6 @@ impl GridRotation {
         }
     }
 
-    /// Expresses this rotation as a matrix which rotates “in place” the
-    /// points within the volume defined by coordinates in the range [0, size].
-    ///
-    /// That is, a [`GridAab`] of that volume will be unchanged by rotation:
-    ///
-    /// ```
-    /// use all_is_cubes::block::Resolution;
-    /// use all_is_cubes::math::{GridAab, GridRotation};
-    ///
-    /// let b = GridAab::for_block(Resolution::R8);
-    /// let rotation = GridRotation::CLOCKWISE.to_positive_octant_matrix(8);
-    /// assert_eq!(b.transform(rotation), Some(b));
-    /// ```
-    ///
-    /// Such matrices are suitable for rotating the voxels of a block, provided
-    /// that the coordinates are then transformed with [`GridMatrix::transform_cube`],
-    /// *not* [`GridMatrix::transform_point`](cgmath::Transform::transform_point)
-    /// (due to the lower-corner format of cube coordinates).
-    /// ```
-    /// # use all_is_cubes::math::{GridAab, GridPoint, GridRotation};
-    /// let rotation = GridRotation::CLOCKWISE.to_positive_octant_matrix(4);
-    /// assert_eq!(rotation.transform_cube(GridPoint::new(0, 0, 0)), GridPoint::new(3, 0, 0));
-    /// assert_eq!(rotation.transform_cube(GridPoint::new(3, 0, 0)), GridPoint::new(3, 0, 3));
-    /// assert_eq!(rotation.transform_cube(GridPoint::new(3, 0, 3)), GridPoint::new(0, 0, 3));
-    /// assert_eq!(rotation.transform_cube(GridPoint::new(0, 0, 3)), GridPoint::new(0, 0, 0));
-    /// ```
-    ///
-    // TODO: add tests
-    pub fn to_positive_octant_matrix(self, size: GridCoordinate) -> GridMatrix {
-        fn offset(face: Face6, size: GridCoordinate) -> GridVector {
-            if face.is_positive() {
-                GridVector::zero()
-            } else {
-                face.normal_vector() * -size
-            }
-        }
-        let basis = self.to_basis();
-        GridMatrix {
-            x: basis.x.normal_vector(),
-            y: basis.y.normal_vector(),
-            z: basis.z.normal_vector(),
-            w: offset(basis.x, size) + offset(basis.y, size) + offset(basis.z, size),
-        }
-    }
-
     /// Expresses this rotation as a matrix without any translation.
     // TODO: add tests
     pub fn to_rotation_matrix(self) -> GridMatrix {