Merge pull request #1153 from hannobraun/ready/curve

Remove geometry from `GlobalVertex`

crates/fj-kernel/src/algorithms/intersect/surface_surface.rs

@@ -53,11 +53,7 @@ impl SurfaceSurfaceIntersection {
 
         let curves = planes_parametric.map(|(surface, plane)| {
             let path = project_line_into_plane(&line, &plane);
-            let global_form = stores.global_curves.insert(
-                GlobalCurve::from_path(GlobalPath::Line(
-                    Line::from_origin_and_direction(origin, direction),
-                )),
-            );
+            let global_form = GlobalCurve::new(stores);
 
             Curve::new(surface, path, global_form)
         });

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

@@ -5,9 +5,8 @@ use crate::{
         Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface,
         SurfaceVertex, Vertex,
     },
-    partial::HasPartial,
     path::SurfacePath,
-    stores::{Handle, Stores},
+    stores::Stores,
 };
 
 use super::Sweep;
@@ -41,19 +40,15 @@ impl Sweep for (Vertex, Surface) {
 
         // So, we're supposed to create the `Edge` by sweeping a `Vertex` using
         // `path`. Unless `path` is identical to the path that created the
-        // `Surface`, this doesn't make any sense.
+        // `Surface`, this doesn't make any sense. Let's make sure this
+        // requirement is met.
         //
         // Further, the `Curve` that was swept to create the `Surface` needs to
         // be the same `Curve` that the input `Vertex` is defined on. If it's
         // not, we have no way of knowing the surface coordinates of the input
         // `Vertex` on the `Surface`, and we're going to need to do that further
-        // down.
-        //
-        // Let's make sure that these requirements are met.
-        {
-            assert_eq!(vertex.curve().global_form().path(), surface.u());
-            assert_eq!(path, surface.v());
-        }
+        // down. There's no way to check for that, unfortunately.
+        assert_eq!(path, surface.v());
 
         // With that out of the way, let's start by creating the `GlobalEdge`,
         // as that is the most straight-forward part of this operations, and
@@ -135,13 +130,11 @@ impl Sweep for GlobalVertex {
     type Swept = GlobalEdge;
 
     fn sweep(self, path: impl Into<Vector<3>>, stores: &Stores) -> Self::Swept {
+        let curve = GlobalCurve::new(stores);
+
         let a = self;
         let b = GlobalVertex::from_position(self.position() + path.into());
 
-        let curve = Handle::<GlobalCurve>::partial()
-            .as_line_from_points([a.position(), b.position()])
-            .build(stores);
-
         GlobalEdge::new(curve, [a, b])
     }
 }
@@ -152,12 +145,9 @@ mod tests {
 
     use crate::{
         algorithms::sweep::Sweep,
-        objects::{
-            Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface,
-            Vertex,
-        },
+        objects::{Curve, HalfEdge, Surface, Vertex},
         partial::HasPartial,
-        stores::{Handle, Stores},
+        stores::Stores,
     };
 
     #[test]
@@ -181,18 +171,4 @@ mod tests {
             .build(&stores);
         assert_eq!(half_edge, expected_half_edge);
     }
-
-    #[test]
-    fn global_vertex() {
-        let stores = Stores::new();
-
-        let edge = GlobalVertex::from_position([0., 0., 0.])
-            .sweep([0., 0., 1.], &stores);
-
-        let expected_edge = GlobalEdge::new(
-            Handle::<GlobalCurve>::partial().as_z_axis().build(&stores),
-            [[0., 0., 0.], [0., 0., 1.]].map(GlobalVertex::from_position),
-        );
-        assert_eq!(edge, expected_edge);
-    }
 }

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

@@ -2,7 +2,7 @@ use fj_math::Transform;
 
 use crate::{
     objects::{Curve, GlobalCurve},
-    partial::{PartialCurve, PartialGlobalCurve},
+    partial::PartialCurve,
     stores::{Handle, Stores},
 };
 
@@ -21,10 +21,16 @@ impl TransformObject for Curve {
 }
 
 impl TransformObject for Handle<GlobalCurve> {
-    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        stores.global_curves.insert(GlobalCurve::from_path(
-            self.path().transform(transform, stores),
-        ))
+    fn transform(self, _: &Transform, stores: &Stores) -> Self {
+        // `GlobalCurve` doesn't contain any internal geometry. If it did, that
+        // would just be redundant with the geometry of other objects, and this
+        // other geometry is already being transformed by other implementations
+        // of this trait.
+        //
+        // All we need to do here is create a new `GlobalCurve` instance, to
+        // make sure the transformed `GlobalCurve` has a different identity than
+        // the original one.
+        GlobalCurve::new(stores)
     }
 }
 
@@ -35,21 +41,14 @@ impl TransformObject for PartialCurve {
             .map(|surface| surface.transform(transform, stores));
         let global_form = self
             .global_form
-            .map(|global_form| global_form.transform(transform, stores));
+            .map(|global_form| global_form.0.transform(transform, stores));
 
         // Don't need to transform `self.path`, as that's defined in surface
         // coordinates, and thus transforming `surface` takes care of it.
         Self {
             surface,
             path: self.path,
-            global_form,
+            global_form: global_form.map(Into::into),
         }
     }
 }
-
-impl TransformObject for PartialGlobalCurve {
-    fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let path = self.path.map(|path| path.transform(transform, stores));
-        Self { path }
-    }
-}

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

@@ -47,11 +47,15 @@ impl TransformObject for GlobalEdge {
 
 impl TransformObject for PartialGlobalEdge {
     fn transform(self, transform: &Transform, stores: &Stores) -> Self {
-        let curve = self.curve.map(|curve| curve.transform(transform, stores));
+        let curve =
+            self.curve.map(|curve| curve.0.transform(transform, stores));
         let vertices = self.vertices.map(|vertices| {
             vertices.map(|vertex| vertex.transform(transform, stores))
         });
 
-        Self { curve, vertices }
+        Self {
+            curve: curve.map(Into::into),
+            vertices,
+        }
     }
 }

crates/fj-kernel/src/algorithms/validate/coherence.rs

@@ -4,39 +4,6 @@ use fj_math::{Point, Scalar};
 
 use crate::objects::{Curve, Vertex};
 
-pub fn validate_curve(
-    curve: &Curve,
-    max_distance: impl Into<Scalar>,
-) -> Result<(), CoherenceIssues> {
-    let max_distance = max_distance.into();
-
-    let points_curve = [-2., -1., 0., 1., 2.].map(|point| Point::from([point]));
-
-    for point_curve in points_curve {
-        let point_surface = curve.path().point_from_path_coords(point_curve);
-        let point_surface_as_global =
-            curve.surface().point_from_surface_coords(point_surface);
-        let point_global = curve
-            .global_form()
-            .path()
-            .point_from_path_coords(point_curve);
-
-        let distance = (point_surface_as_global - point_global).magnitude();
-
-        if distance > max_distance {
-            Err(CurveCoherenceMismatch {
-                point_curve,
-                point_surface,
-                point_surface_as_global,
-                point_global,
-                curve: curve.clone(),
-            })?
-        }
-    }
-
-    Ok(())
-}
-
 pub fn validate_vertex(
     vertex: &Vertex,
     max_distance: impl Into<Scalar>,

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

@@ -63,10 +63,6 @@ where
     ) -> Result<Validated<Self>, ValidationError> {
         let mut global_vertices = HashSet::new();
 
-        for curve in self.curve_iter() {
-            coherence::validate_curve(curve, config.identical_max_distance)?;
-        }
-
         for global_vertex in self.global_vertex_iter() {
             uniqueness::validate_vertex(
                 global_vertex,
@@ -156,7 +152,7 @@ pub enum ValidationError {
 
 #[cfg(test)]
 mod tests {
-    use fj_math::{Line, Point, Scalar};
+    use fj_math::{Point, Scalar};
 
     use crate::{
         algorithms::validate::{Validate, ValidationConfig, ValidationError},
@@ -165,30 +161,10 @@ mod tests {
             SurfaceVertex, Vertex,
         },
         partial::HasPartial,
-        path::{GlobalPath, SurfacePath},
+        path::SurfacePath,
         stores::Stores,
     };
 
-    #[test]
-    fn coherence_curve() {
-        let stores = Stores::new();
-
-        let line_global = Line::from_points([[0., 0., 0.], [1., 0., 0.]]);
-        let global_curve = stores
-            .global_curves
-            .insert(GlobalCurve::from_path(GlobalPath::Line(line_global)));
-
-        let line_surface = Line::from_points([[0., 0.], [2., 0.]]);
-        let curve = Curve::new(
-            Surface::xy_plane(),
-            SurfacePath::Line(line_surface),
-            global_curve,
-        );
-
-        let result = curve.validate();
-        assert!(result.is_err());
-    }
-
     #[test]
     fn coherence_edge() {
         let stores = Stores::new();
@@ -200,10 +176,7 @@ mod tests {
 
         let curve = {
             let path = SurfacePath::line_from_points(points_surface);
-            let curve_global =
-                stores.global_curves.insert(GlobalCurve::from_path(
-                    GlobalPath::line_from_points(points_global),
-                ));
+            let curve_global = GlobalCurve::new(&stores);
             Curve::new(surface, path, curve_global)
         };
 

crates/fj-kernel/src/iter.rs

@@ -365,7 +365,7 @@ mod tests {
             Sketch, Solid, Surface, SurfaceVertex, Vertex,
         },
         partial::HasPartial,
-        stores::{Handle, Stores},
+        stores::Stores,
     };
 
     use super::ObjectIters as _;
@@ -441,8 +441,7 @@ mod tests {
     fn global_curve() {
         let stores = Stores::new();
 
-        let object =
-            Handle::<GlobalCurve>::partial().as_x_axis().build(&stores);
+        let object = GlobalCurve::new(&stores);
 
         assert_eq!(0, object.curve_iter().count());
         assert_eq!(0, object.cycle_iter().count());

crates/fj-kernel/src/objects/curve.rs

@@ -1,6 +1,6 @@
 use crate::{
-    path::{GlobalPath, SurfacePath},
-    stores::Handle,
+    path::SurfacePath,
+    stores::{Handle, HandleWrapper, Stores},
 };
 
 use super::Surface;
@@ -10,16 +10,18 @@ use super::Surface;
 pub struct Curve {
     path: SurfacePath,
     surface: Surface,
-    global_form: Handle<GlobalCurve>,
+    global_form: HandleWrapper<GlobalCurve>,
 }
 
 impl Curve {
     /// Construct a new instance of `Curve`
     pub fn new(
         surface: Surface,
         path: SurfacePath,
-        global_form: Handle<GlobalCurve>,
+        global_form: impl Into<HandleWrapper<GlobalCurve>>,
     ) -> Self {
+        let global_form = global_form.into();
+
         Self {
             surface,
             path,
@@ -44,19 +46,12 @@ impl Curve {
 }
 
 /// A curve, defined in global (3D) coordinates
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct GlobalCurve {
-    path: GlobalPath,
-}
+#[derive(Clone, Copy, Debug)]
+pub struct GlobalCurve;
 
 impl GlobalCurve {
-    /// Construct a `GlobalCurve` from the path that defines it
-    pub fn from_path(path: GlobalPath) -> Self {
-        Self { path }
-    }
-
-    /// Access the path that defines this curve
-    pub fn path(&self) -> GlobalPath {
-        self.path
+    /// Construct a new instance of `Handle` and add it to the store
+    pub fn new(stores: &Stores) -> Handle<Self> {
+        stores.global_curves.insert(GlobalCurve)
     }
 }

crates/fj-kernel/src/objects/edge.rs

@@ -2,7 +2,7 @@ use std::fmt;
 
 use pretty_assertions::{assert_eq, assert_ne};
 
-use crate::stores::Handle;
+use crate::stores::{Handle, HandleWrapper};
 
 use super::{Curve, GlobalCurve, GlobalVertex, Vertex};
 
@@ -46,8 +46,8 @@ impl HalfEdge {
 
         // Make sure `curve` and `vertices` match `global_form`.
         assert_eq!(
-            curve.global_form(),
-            global_form.curve(),
+            curve.global_form().id(),
+            global_form.curve().id(),
             "The global form of a half-edge's curve must match the curve of \
             the half-edge's global form"
         );
@@ -110,16 +110,17 @@ impl fmt::Display for HalfEdge {
 /// An edge, defined in global (3D) coordinates
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct GlobalEdge {
-    curve: Handle<GlobalCurve>,
+    curve: HandleWrapper<GlobalCurve>,
     vertices: [GlobalVertex; 2],
 }
 
 impl GlobalEdge {
     /// Create a new instance
     pub fn new(
-        curve: Handle<GlobalCurve>,
+        curve: impl Into<HandleWrapper<GlobalCurve>>,
         vertices: [GlobalVertex; 2],
     ) -> Self {
+        let curve = curve.into();
         Self { curve, vertices }
     }