Merge pull request #1063 from hannobraun/sweep

Fix sweep algorithm assuming xy-plane

crates/fj-kernel/src/algorithms/sweep/curve.rs

@@ -1,22 +1,24 @@
+use fj_math::Vector;
+
 use crate::objects::{Curve, GlobalCurve, Surface, SweptCurve};
 
 use super::Sweep;
 
 impl Sweep for Curve {
     type Swept = Surface;
 
-    fn sweep(self, path: impl Into<super::Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         self.global_form().sweep(path)
     }
 }
 
 impl Sweep for GlobalCurve {
     type Swept = Surface;
 
-    fn sweep(self, path: impl Into<super::Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         Surface::SweptCurve(SweptCurve {
             curve: *self.kind(),
-            path: path.into().inner(),
+            path: path.into(),
         })
     }
 }

crates/fj-kernel/src/algorithms/sweep/edge.rs

@@ -1,5 +1,5 @@
 use fj_interop::mesh::Color;
-use fj_math::{Line, Scalar};
+use fj_math::{Line, Scalar, Vector};
 
 use crate::{
     algorithms::{reverse::Reverse, transform::TransformObject},
@@ -8,21 +8,15 @@ use crate::{
     },
 };
 
-use super::{Path, Sweep};
+use super::Sweep;
 
 impl Sweep for (Edge, Color) {
     type Swept = Face;
 
-    fn sweep(self, path: impl Into<Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         let (edge, color) = self;
         let path = path.into();
 
-        let edge = if path.is_negative_direction() {
-            edge.reverse_including_curve()
-        } else {
-            edge
-        };
-
         let surface = edge.curve().sweep(path);
 
         // We can't use the edge we're sweeping from as the bottom edge, as that
@@ -93,8 +87,7 @@ impl Sweep for (Edge, Color) {
             });
 
             let curve = {
-                let global =
-                    bottom_edge.curve().global_form().translate(path.inner());
+                let global = bottom_edge.curve().global_form().translate(path);
 
                 // Please note that creating a line here is correct, even if the
                 // global curve is a circle. Projected into the side surface, it

crates/fj-kernel/src/algorithms/sweep/face.rs

@@ -1,27 +1,50 @@
+use fj_math::{Scalar, Vector};
+
 use crate::{
     algorithms::{reverse::Reverse, transform::TransformObject},
-    objects::{Face, Shell},
+    objects::{CurveKind, Face, Shell, Surface},
 };
 
-use super::{Path, Sweep};
+use super::Sweep;
 
 impl Sweep for Face {
     type Swept = Shell;
 
-    fn sweep(self, path: impl Into<Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         let path = path.into();
 
         let mut faces = Vec::new();
 
-        let bottom_face =
-            create_bottom_face(&self, path.is_negative_direction());
+        let is_negative_sweep = {
+            let Surface::SweptCurve(surface) = self.surface();
+
+            let a = match surface.curve {
+                CurveKind::Circle(_) => todo!(
+                    "Sweeping from faces defined in round surfaces is not \
+                    supported"
+                ),
+                CurveKind::Line(line) => line.direction(),
+            };
+            let b = surface.path;
+
+            let normal = a.cross(&b);
+
+            normal.dot(&path) < Scalar::ZERO
+        };
+
+        let bottom_face = create_bottom_face(&self, is_negative_sweep);
         faces.push(bottom_face);
 
-        let top_face = create_top_face(self.clone(), path);
+        let top_face = create_top_face(self.clone(), path, is_negative_sweep);
         faces.push(top_face);
 
         for cycle in self.all_cycles() {
             for &edge in cycle.edges() {
+                let edge = if is_negative_sweep {
+                    edge.reverse_including_curve()
+                } else {
+                    edge
+                };
                 let face = (edge, self.color()).sweep(path);
                 faces.push(face);
             }
@@ -31,21 +54,22 @@ impl Sweep for Face {
     }
 }
 
-fn create_bottom_face(
-    face: &Face,
-    is_sweep_along_negative_direction: bool,
-) -> Face {
-    if is_sweep_along_negative_direction {
+fn create_bottom_face(face: &Face, is_negative_sweep: bool) -> Face {
+    if is_negative_sweep {
         face.clone()
     } else {
         face.clone().reverse()
     }
 }
 
-fn create_top_face(face: Face, path: Path) -> Face {
-    let mut face = face.translate(path.inner());
+fn create_top_face(
+    face: Face,
+    path: Vector<3>,
+    is_negative_sweep: bool,
+) -> Face {
+    let mut face = face.translate(path);
 
-    if path.is_negative_direction() {
+    if is_negative_sweep {
         face = face.reverse();
     };
 

crates/fj-kernel/src/algorithms/sweep/mod.rs

@@ -6,38 +6,13 @@ mod face;
 mod sketch;
 mod vertex;
 
-use fj_math::{Scalar, Vector};
+use fj_math::Vector;
 
 /// Sweep an object along a path to create another object
 pub trait Sweep {
     /// The object that is created by sweeping the implementing object
     type Swept;
 
     /// Sweep the object along the given path
-    fn sweep(self, path: impl Into<Path>) -> Self::Swept;
-}
-
-/// A path to be used with [`Sweep`]
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct Path(Vector<3>);
-
-impl Path {
-    /// Return the vector that defines this path
-    pub fn inner(&self) -> Vector<3> {
-        self.0
-    }
-
-    /// Indicate whether the path is in the negative direction
-    pub fn is_negative_direction(&self) -> bool {
-        self.0.dot(&Vector::from([0., 0., 1.])) < Scalar::ZERO
-    }
-}
-
-impl<T> From<T> for Path
-where
-    T: Into<Vector<3>>,
-{
-    fn from(value: T) -> Self {
-        Self(value.into())
-    }
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept;
 }

crates/fj-kernel/src/algorithms/sweep/sketch.rs

@@ -1,11 +1,13 @@
+use fj_math::Vector;
+
 use crate::objects::{Sketch, Solid};
 
-use super::{Path, Sweep};
+use super::Sweep;
 
 impl Sweep for Sketch {
     type Swept = Solid;
 
-    fn sweep(self, path: impl Into<Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         let path = path.into();
 
         let mut shells = Vec::new();

crates/fj-kernel/src/algorithms/sweep/vertex.rs

@@ -1,16 +1,16 @@
-use fj_math::{Line, Point, Scalar};
+use fj_math::{Line, Point, Scalar, Vector};
 
 use crate::objects::{
     Curve, CurveKind, Edge, GlobalCurve, GlobalEdge, GlobalVertex, Surface,
     SurfaceVertex, SweptCurve, Vertex,
 };
 
-use super::{Path, Sweep};
+use super::Sweep;
 
 impl Sweep for (Vertex, Surface) {
     type Swept = Edge;
 
-    fn sweep(self, path: impl Into<Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         let (vertex, surface) = self;
         let path = path.into();
 
@@ -53,7 +53,7 @@ impl Sweep for (Vertex, Surface) {
             }) = surface;
 
             assert_eq!(vertex.curve().global_form().kind(), &surface_curve);
-            assert_eq!(path.inner(), surface_path);
+            assert_eq!(path, surface_path);
         }
 
         // With that out of the way, let's start by creating the `GlobalEdge`,
@@ -132,10 +132,9 @@ impl Sweep for (Vertex, Surface) {
 impl Sweep for GlobalVertex {
     type Swept = GlobalEdge;
 
-    fn sweep(self, path: impl Into<Path>) -> Self::Swept {
+    fn sweep(self, path: impl Into<Vector<3>>) -> Self::Swept {
         let a = self;
-        let b =
-            GlobalVertex::from_position(self.position() + path.into().inner());
+        let b = GlobalVertex::from_position(self.position() + path.into());
 
         let curve =
             GlobalCurve::build().line_from_points([a.position(), b.position()]);