Allow for geo-3d integration – Part 1.1: Code quality

src/bsp.rs

@@ -298,7 +298,7 @@ impl<S: Clone + Send + Sync + Debug> Node<S> {
         let plane = self.plane.clone().unwrap();
 
         // Split polygons in parallel
-        let (mut coplanar_front, mut coplanar_back, mut front, mut back) = polygons
+        let (mut coplanar_front, mut coplanar_back, front, back) = polygons
             .par_iter()
             .map(|p| plane.split_polygon(p)) // <-- just pass p
             .reduce(

src/csg.rs

@@ -16,7 +16,6 @@ use geo::{
 };
 use nalgebra::{
     Isometry3, Matrix3, Matrix4, Point3, Quaternion, Rotation3, Translation3, Unit, Vector3,
-    partial_max, partial_min,
 };
 use std::fmt::Debug;
 use std::sync::OnceLock;
@@ -26,7 +25,7 @@ use rayon::prelude::*;
 
 /// The main CSG solid structure. Contains a list of 3D polygons, 2D polylines, and some metadata.
 #[derive(Debug, Clone)]
-pub struct CSG<S: Clone> {
+pub struct CSG<S: Clone = ()> {
     /// 3D polygons for volumetric shapes
     pub polygons: Vec<Polygon<S>>,
 
@@ -40,6 +39,12 @@ pub struct CSG<S: Clone> {
     pub metadata: Option<S>,
 }
 
+impl<S: Clone + Debug + Send + Sync> Default for CSG<S> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
 impl<S: Clone + Debug + Send + Sync> CSG<S> {
     /// Create an empty CSG
     pub fn new() -> Self {
@@ -968,13 +973,13 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
             // 1) Gather from the 3D polygons
             for poly in &self.polygons {
                 for v in &poly.vertices {
-                    min_x = *partial_min(&min_x, &v.pos.x).unwrap();
-                    min_y = *partial_min(&min_y, &v.pos.y).unwrap();
-                    min_z = *partial_min(&min_z, &v.pos.z).unwrap();
+                    min_x = min_x.min(v.pos.x);
+                    min_y = min_y.min(v.pos.y);
+                    min_z = min_z.min(v.pos.z);
 
-                    max_x = *partial_max(&max_x, &v.pos.x).unwrap();
-                    max_y = *partial_max(&max_y, &v.pos.y).unwrap();
-                    max_z = *partial_max(&max_z, &v.pos.z).unwrap();
+                    max_x = max_x.max(v.pos.x);
+                    max_y = max_y.max(v.pos.y);
+                    max_z = max_z.max(v.pos.z);
                 }
             }
 
@@ -988,13 +993,13 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
                 let max_pt = rect.max();
 
                 // Merge the 2D bounds into our existing min/max, forcing z=0 for 2D geometry.
-                min_x = *partial_min(&min_x, &min_pt.x).unwrap();
-                min_y = *partial_min(&min_y, &min_pt.y).unwrap();
-                min_z = *partial_min(&min_z, &0.0).unwrap();
+                min_x = min_x.min(min_pt.x);
+                min_y = min_y.min(min_pt.y);
+                min_z = min_z.min(0.0);
 
-                max_x = *partial_max(&max_x, &max_pt.x).unwrap();
-                max_y = *partial_max(&max_y, &max_pt.y).unwrap();
-                max_z = *partial_max(&max_z, &0.0).unwrap();
+                max_x = max_x.max(max_pt.x);
+                max_y = max_y.max(max_pt.y);
+                max_z = max_z.max(0.0);
             }
 
             // If still uninitialized (e.g., no polygons or geometry), return a trivial AABB at origin

src/extrudes.rs

@@ -593,8 +593,7 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
             }
 
             // Build the polygon
-            let poly = Polygon::new(verts, metadata.clone());
-            Some(poly)
+            Some(Polygon::new(verts, metadata.clone()))
         }
 
         //----------------------------------------------------------------------

src/io/svg.rs

@@ -377,6 +377,7 @@ impl<F: CoordNum> PathBuilder<F> {
 }
 
 pub trait FromSVG: Sized {
+    #[allow(unused)]
     fn from_svg(doc: &str) -> Result<Self, IoError>;
 }
 
@@ -532,6 +533,7 @@ impl FromSVG for CSG<()> {
 }
 
 pub trait ToSVG {
+    #[allow(unused)]
     fn to_svg(&self) -> String;
 }
 
@@ -884,9 +886,9 @@ fn svg_points_to_line_string<F: CoordNum>(points: &str) -> Result<LineString<F>,
     use nom::branch::alt;
     use nom::character::complete::{char, multispace0, multispace1};
     use nom::combinator::opt;
-    use nom::multi::separated_list1;
     use nom::number::complete::float;
-    use nom::sequence::{delimited, pair, preceded, separated_pair, terminated, tuple};
+    use nom::sequence::{pair, tuple, delimited, separated_pair};
+    use nom::multi::separated_list1;
 
     fn comma_wsp(i: &str) -> IResult<&str, ()> {
         let (i, _) = alt((

src/lib.rs

@@ -6,7 +6,7 @@
 //!
 //! # Features
 //! #### Default
-//! - **f64**: use f64 as Real
+//! - **f64**: use f64 as `Real`
 //! - [**stl-io**](https://en.wikipedia.org/wiki/STL_(file_format)): `.stl` import/export
 //! - [**dxf-io**](https://en.wikipedia.org/wiki/AutoCAD_DXF): `.dxf` import/export
 //! - **chull-io**: convex hull and minkowski sum
@@ -17,7 +17,7 @@
 //! - **delaunay**: use `geo`s `spade` feature for triangulation
 //!
 //! #### Optional
-//! - **f32**: use f32 as Real, this conflicts with f64
+//! - **f32**: use f32 as `Real`, this conflicts with f64
 //! - **parallel**: use rayon for multithreading
 //! - **svg-io**: create `CSG`s from and convert `CSG`s to SVG's
 //! - **truetype-text**: create `CSG`s using TrueType fonts `.ttf`

src/main.rs

@@ -4,9 +4,9 @@
 // Here, we do not use any shared data, so we'll bind the generic S to ().
 
 use csgrs::float_types::Real;
-use csgrs::plane::Plane;
-use nalgebra::{Point3, Vector3};
 use std::fs;
+use nalgebra::{Vector3, Point3, Point2};
+use csgrs::plane::Plane;
 
 #[cfg(feature = "image")]
 use image::{GrayImage, ImageBuffer};
@@ -189,11 +189,11 @@ fn main() {
         [0.5, 1.0, 0.0],
         [0.5, 0.5, 1.0],
     ];
-    let faces = vec![
-        vec![0, 2, 1], // base triangle
-        vec![0, 1, 3], // side
-        vec![1, 2, 3],
-        vec![2, 0, 3],
+    let faces: Vec<&[usize]> = vec![
+        &[0, 2, 1], // base triangle
+        &[0, 1, 3], // side
+        &[1, 2, 3],
+        &[2, 0, 3],
     ];
     let poly = CSG::polyhedron(&points, &faces, None);
     #[cfg(feature = "stl-io")]
@@ -224,7 +224,7 @@ fn main() {
 
     // 14) Mass properties (just printing them)
     let (mass, com, principal_frame) = cube.mass_properties(1.0);
-    println!("Cube mass = {}", mass);
+    println!("Cube mass = {mass}");
     println!("Cube center of mass = {:?}", com);
     println!("Cube principal inertia local frame = {:?}", principal_frame);
 
@@ -330,7 +330,6 @@ fn main() {
     let _ = fs::write("stl/pie_slice.stl", wedge.to_stl_ascii("pie_slice"));
 
     // Create a 2D "metaball" shape from 3 circles
-    use nalgebra::Point2;
     let balls_2d = vec![
         (Point2::new(0.0, 0.0), 1.0),
         (Point2::new(1.5, 0.0), 1.0),

src/metaballs.rs

@@ -99,7 +99,7 @@ fn stitch(contours: &[LineString<Real>]) -> Vec<LineString<Real>> {
         }
 
         // close if ends coincide
-        if chain.len() >= 3 && (chain[0] == *chain.last().unwrap()) == false {
+        if chain.len() >= 3 && chain[0] != *chain.last().unwrap() {
             chain.push(chain[0]);
         }
         chains.push(LineString::new(chain));

src/offset.rs

@@ -11,6 +11,8 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
     ///   - If it's a Polygon, buffer it and store the result as a MultiPolygon
     ///   - If it's a MultiPolygon, buffer it directly
     ///   - Otherwise, ignore (exclude) it from the new collection
+    ///
+    /// Note: this does not affect the 3d polygons of the `CSG`
     pub fn offset(&self, distance: Real) -> CSG<S> {
         let offset_geoms = self
             .geometry

src/plane.rs

@@ -35,7 +35,16 @@ pub struct Plane {
 }
 
 impl Plane {
-    /// Tries to pick three vertices that span the largest area triangle
+    /// Create a plane from three points
+    pub const fn from_points(a: &Point3<Real>, b: &Point3<Real>, c: &Point3<Real>) -> Plane {
+        Plane {
+            point_a: *a,
+            point_b: *b,
+            point_c: *c,
+        }
+    }
+
+    /// Tries to pick three vertices that span the largest area triangle 
     /// (maximally well-spaced) and returns a plane defined by them.
     /// Care is taken to preserve the original winding of the vertices.
     ///
@@ -212,7 +221,7 @@ impl Plane {
         self.normal().dot(&self.point_a.coords)
     }
 
-    pub fn flip(&mut self) {
+    pub const fn flip(&mut self) {
         std::mem::swap(&mut self.point_a, &mut self.point_b);
     }
 

src/polygon.rs

@@ -38,6 +38,43 @@ impl<S: Clone + Send + Sync> Polygon<S> {
         }
     }
 
+    /// Create a polygon from vertices
+    pub fn new_with_plane(vertices: Vec<Vertex>, metadata: Option<S>, plane: Plane) -> Self {
+        assert!(vertices.len() >= 3, "degenerate polygon");
+
+        Polygon {
+            vertices,
+            plane,
+            bounding_box: OnceLock::new(),
+            metadata,
+        }
+    }
+
+    /// Create a polygon from three vertices
+    pub fn from_tri(vertices: &[Vertex; 3], metadata: Option<S>) -> Self {
+        Polygon {
+            vertices: vertices.to_vec(),
+            plane: Plane::from_points(&vertices[0].pos, &vertices[1].pos, &vertices[2].pos),
+            bounding_box: OnceLock::new(),
+            metadata,
+        }
+    }
+
+    /// Returns the [`Plane`] of a [`Polygon`]
+    ///
+    // I think a worst-case can be constructed for any heuristic here the
+    // only optimal solution may be to calculate which vertices are far apart
+    // which we would need a fast solution for.
+    //
+    // Finding the best solution is likely to be too intensive,
+    // but finding a "good enough" solution may still be quick.
+    // It may be useful to retain this version and implement a slower higher
+    // quality solution as a second function.
+    #[inline]
+    pub fn plane(&self) -> &Plane {
+        &self.plane
+    }
+
     /// Axis aligned bounding box of this Polygon (cached after first call)
     pub fn bounding_box(&self) -> Aabb {
         *self.bounding_box.get_or_init(|| {
@@ -179,10 +216,16 @@ impl<S: Clone + Send + Sync> Polygon<S> {
             // We'll keep a queue of triangles to process
             let mut queue = vec![tri];
             for _ in 0..subdivisions.get() {
-                let mut next_level = Vec::new();
+                let mut next_level = Vec::with_capacity(queue.len() * 4);
                 for t in queue {
                     let subs = subdivide_triangle(t);
-                    next_level.extend(subs);
+
+                    // only reserves if needed, this is unneeded as it's done ahead of time
+                    // next_level.reserve(4);
+                    // add to vec without copy
+                    for sub in subs.into_iter() {
+                        next_level.push(sub);
+                    }
                 }
                 queue = next_level;
             }
@@ -235,12 +278,12 @@ impl<S: Clone + Send + Sync> Polygon<S> {
     }
 
     /// Returns a reference to the metadata, if any.
-    pub fn metadata(&self) -> Option<&S> {
+    pub const fn metadata(&self) -> Option<&S> {
         self.metadata.as_ref()
     }
 
     /// Returns a mutable reference to the metadata, if any.
-    pub fn metadata_mut(&mut self) -> Option<&mut S> {
+    pub const fn metadata_mut(&mut self) -> Option<&mut S> {
         self.metadata.as_mut()
     }
 
@@ -275,13 +318,13 @@ pub fn build_orthonormal_basis(n: Vector3<Real>) -> (Vector3<Real>, Vector3<Real
     (u, v)
 }
 
-// Helper function to subdivide a triangle
-pub fn subdivide_triangle(tri: [Vertex; 3]) -> Vec<[Vertex; 3]> {
+/// Helper function to subdivide a triangle into 4
+pub fn subdivide_triangle(tri: [Vertex; 3]) -> [[Vertex; 3]; 4] {
     let v01 = tri[0].interpolate(&tri[1], 0.5);
     let v12 = tri[1].interpolate(&tri[2], 0.5);
     let v20 = tri[2].interpolate(&tri[0], 0.5);
 
-    vec![
+    [
         [tri[0].clone(), v01.clone(), v20.clone()],
         [v01.clone(), tri[1].clone(), v12.clone()],
         [v20.clone(), v12.clone(), tri[2].clone()],

src/shapes2d.rs

@@ -267,6 +267,7 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
     /// Teardrop shape.  A simple approach:
     /// - a circle arc for the "round" top
     /// - it tapers down to a cusp at bottom.
+    ///
     /// This is just one of many possible "teardrop" definitions.
     // todo: center on focus of the arc
     pub fn teardrop_outline(

src/shapes3d.rs

@@ -330,7 +330,7 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
         CSG::frustum_ptp(
             Point3::origin(),
             Point3::new(0.0, 0.0, height),
-            radius.clone(),
+            radius,
             radius,
             segments,
             metadata,
@@ -373,15 +373,15 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
     ) -> CSG<S> {
         let mut polygons = Vec::new();
 
-        for face in faces {
+        for face in faces.iter() {
             // Skip degenerate faces
             if face.len() < 3 {
                 continue;
             }
 
             // Gather the vertices for this face
             let mut face_vertices = Vec::with_capacity(face.len());
-            for &idx in face {
+            for &idx in face.iter() {
                 // Ensure the index is valid
                 if idx >= points.len() {
                     panic!(
@@ -529,7 +529,7 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
     /// - `direction`: the vector defining arrow length and intended pointing direction
     /// - `segments`: number of segments for approximating the cylinder and frustum
     /// - `orientation`: when false (default) the arrow points away from start (its base is at start);
-    ///                        when true the arrow points toward start (its tip is at start).
+    ///    when true the arrow points toward start (its tip is at start).
     /// - `metadata`: optional metadata for the generated polygons.
     pub fn arrow(
         start: Point3<Real>,
@@ -682,9 +682,7 @@ impl<S: Clone + Debug + Send + Sync> CSG<S> {
             [9, 8, 1],
         ];
 
-        let faces_vec: Vec<Vec<usize>> = faces.iter().map(|f| f.to_vec()).collect();
-
-        Self::polyhedron(&pts, &faces_vec, metadata).scale(factor, factor, factor)
+        Self::polyhedron(&pts, &faces, metadata).scale(factor, factor, factor)
     }
 
     /// Torus centred at the origin in the *XY* plane.