Merge pull request #1607 from hannobraun/curve

Remove reference to `GlobalCurve` from `Curve`

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

@@ -19,7 +19,7 @@ use crate::{
 
 use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance};
 
-impl Approx for (&Handle<Curve>, &Surface, RangeOnPath) {
+impl Approx for (&Handle<Curve>, &Surface, Handle<GlobalCurve>, RangeOnPath) {
     type Approximation = CurveApprox;
     type Cache = CurveCache;
 
@@ -28,9 +28,8 @@ impl Approx for (&Handle<Curve>, &Surface, RangeOnPath) {
         tolerance: impl Into<Tolerance>,
         cache: &mut Self::Cache,
     ) -> Self::Approximation {
-        let (curve, surface, range) = self;
+        let (curve, surface, global_curve, range) = self;
 
-        let global_curve = curve.global_form().clone();
         let global_curve_approx = match cache.get(global_curve.clone(), range) {
             Some(approx) => approx,
             None => {
@@ -219,6 +218,7 @@ mod tests {
         builder::{CurveBuilder, SurfaceBuilder},
         geometry::path::GlobalPath,
         insert::Insert,
+        objects::GlobalCurve,
         partial::{PartialCurve, PartialObject, PartialSurface},
         services::Services,
     };
@@ -235,9 +235,10 @@ mod tests {
         let curve = curve
             .build(&mut services.objects)
             .insert(&mut services.objects);
+        let global_curve = GlobalCurve.insert(&mut services.objects);
         let range = RangeOnPath::from([[0.], [1.]]);
 
-        let approx = (&curve, surface.deref(), range).approx(1.);
+        let approx = (&curve, surface.deref(), global_curve, range).approx(1.);
 
         assert_eq!(approx, CurveApprox::empty());
     }
@@ -257,9 +258,10 @@ mod tests {
         let curve = curve
             .build(&mut services.objects)
             .insert(&mut services.objects);
+        let global_curve = GlobalCurve.insert(&mut services.objects);
         let range = RangeOnPath::from([[0.], [1.]]);
 
-        let approx = (&curve, surface.deref(), range).approx(1.);
+        let approx = (&curve, surface.deref(), global_curve, range).approx(1.);
 
         assert_eq!(approx, CurveApprox::empty());
     }
@@ -277,11 +279,13 @@ mod tests {
         let curve = curve
             .build(&mut services.objects)
             .insert(&mut services.objects);
+        let global_curve = GlobalCurve.insert(&mut services.objects);
 
         let range = RangeOnPath::from([[0.], [TAU]]);
         let tolerance = 1.;
 
-        let approx = (&curve, surface.deref(), range).approx(tolerance);
+        let approx =
+            (&curve, surface.deref(), global_curve, range).approx(tolerance);
 
         let expected_approx = (path, range)
             .approx(tolerance)
@@ -307,10 +311,12 @@ mod tests {
         let curve = curve
             .build(&mut services.objects)
             .insert(&mut services.objects);
+        let global_curve = GlobalCurve.insert(&mut services.objects);
 
         let range = RangeOnPath::from([[0.], [TAU]]);
         let tolerance = 1.;
-        let approx = (&curve, surface.deref(), range).approx(tolerance);
+        let approx =
+            (&curve, surface.deref(), global_curve, range).approx(tolerance);
 
         let expected_approx = (curve.path(), range)
             .approx(tolerance)

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

@@ -35,7 +35,12 @@ impl Approx for (&Handle<HalfEdge>, &Surface) {
             half_edge.start_vertex().global_form().position(),
         )
         .with_source((half_edge.clone(), half_edge.boundary()[0]));
-        let curve_approx = (half_edge.curve(), surface, range)
+        let curve_approx = (
+            half_edge.curve(),
+            surface,
+            half_edge.global_form().curve().clone(),
+            range,
+        )
             .approx_with_cache(tolerance, cache);
 
         HalfEdgeApprox {

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

@@ -3,7 +3,7 @@ use fj_math::{Line, Plane, Point, Scalar};
 use crate::{
     geometry::path::{GlobalPath, SurfacePath},
     insert::Insert,
-    objects::{Curve, GlobalCurve, Objects, Surface},
+    objects::{Curve, Objects, Surface},
     services::Service,
     storage::Handle,
 };
@@ -55,9 +55,7 @@ impl SurfaceSurfaceIntersection {
 
         let curves = planes.map(|plane| {
             let path = SurfacePath::Line(plane.project_line(&line));
-            let global_form = GlobalCurve.insert(objects);
+            Curve::new(path).insert(objects)
-
-            Curve::new(path, global_form).insert(objects)
         });
 
         Some(Self {

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

@@ -134,11 +134,7 @@ impl Sweep for (Handle<HalfEdge>, &Surface, Color) {
         [edge_bottom.write(), edge_up.write(), edge_down.write()]
             .zip_ext(global_edges)
             .map(|(mut half_edge, global_edge)| {
-                let global_edge = Partial::from(global_edge);
+                half_edge.global_form = Partial::from(global_edge);
-
-                half_edge.curve.write().global_form =
-                    global_edge.read().curve.clone();
-                half_edge.global_form = global_edge;
             });
 
         // And we're done creating the face! All that's left to do is build our

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

@@ -10,20 +10,15 @@ use super::{TransformCache, TransformObject};
 impl TransformObject for Curve {
     fn transform_with_cache(
         self,
-        transform: &Transform,
+        _: &Transform,
-        objects: &mut Service<Objects>,
+        _: &mut Service<Objects>,
-        cache: &mut TransformCache,
+        _: &mut TransformCache,
     ) -> Self {
         // Don't need to transform path, as that's defined in surface
         // coordinates, and thus transforming `surface` takes care of it.
         let path = self.path();
 
-        let global_form = self
+        Self::new(path)
-            .global_form()
-            .clone()
-            .transform_with_cache(transform, objects, cache);
-
-        Self::new(path, global_form)
     }
 }
 

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

@@ -166,7 +166,6 @@ impl HalfEdgeBuilder for PartialHalfEdge {
     }
 
     fn infer_global_form(&mut self) -> Partial<GlobalEdge> {
-        self.global_form.write().curve = self.curve.read().global_form.clone();
         self.global_form.write().vertices = self
             .vertices
             .each_ref_ext()
@@ -224,8 +223,7 @@ impl HalfEdgeBuilder for PartialHalfEdge {
         other: &Partial<HalfEdge>,
         surface: &SurfaceGeometry,
     ) {
-        let global_curve = other.read().curve.read().global_form.clone();
+        let global_curve = other.read().global_form.read().curve.clone();
-        self.curve.write().global_form = global_curve.clone();
         self.global_form.write().curve = global_curve;
 
         self.curve.write().path =

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

@@ -1,36 +1,21 @@
-use crate::{
+use crate::geometry::path::SurfacePath;
-    geometry::path::SurfacePath,
-    storage::{Handle, HandleWrapper},
-};
 
 /// A curve, defined in local surface coordinates
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Curve {
     path: SurfacePath,
-    global_form: HandleWrapper<GlobalCurve>,
 }
 
 impl Curve {
     /// Construct a new instance of `Curve`
-    pub fn new(
+    pub fn new(path: SurfacePath) -> Self {
-        path: SurfacePath,
+        Self { path }
-        global_form: impl Into<HandleWrapper<GlobalCurve>>,
-    ) -> Self {
-        Self {
-            path,
-            global_form: global_form.into(),
-        }
     }
 
     /// Access the path that defines the curve
     pub fn path(&self) -> SurfacePath {
         self.path
     }
-
-    /// Access the global form of the curve
-    pub fn global_form(&self) -> &Handle<GlobalCurve> {
-        &self.global_form
-    }
 }
 
 /// A curve, defined in global (3D) coordinates

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

@@ -64,7 +64,7 @@ impl fmt::Display for HalfEdge {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let [a, b] = self.boundary();
         write!(f, "edge from {a:?} to {b:?}")?;
-        write!(f, " on {:?}", self.curve().global_form())?;
+        write!(f, " on {:?}", self.global_form().curve())?;
 
         Ok(())
     }

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

@@ -11,13 +11,6 @@ use super::{
 };
 
 /// The available object stores
-///
-/// # Implementation Note
-///
-/// The intention is to eventually manage all objects in here. Making this
-/// happen is simply a case of putting in the required work. See [#1021].
-///
-/// [#1021]: https://github.com/hannobraun/Fornjot/issues/1021
 #[derive(Debug, Default)]
 pub struct Objects {
     /// Store for [`Curve`]s
@@ -80,7 +73,7 @@ impl Surfaces {
         self.store.reserve()
     }
 
-    /// Insert a [`Surface`] into the store
+    /// Insert an object into the store
     pub fn insert(&mut self, handle: Handle<Surface>, surface: Surface) {
         self.store.insert(handle, surface);
     }

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

@@ -3,7 +3,7 @@ use fj_math::Scalar;
 use crate::{
     geometry::path::SurfacePath,
     objects::{Curve, GlobalCurve, Objects},
-    partial::{FullToPartialCache, Partial, PartialObject},
+    partial::{FullToPartialCache, PartialObject},
     services::Service,
 };
 
@@ -12,22 +12,18 @@ use crate::{
 pub struct PartialCurve {
     /// The path that defines the curve
     pub path: Option<MaybeSurfacePath>,
-
-    /// The global form of the curve
-    pub global_form: Partial<GlobalCurve>,
 }
 
 impl PartialObject for PartialCurve {
     type Full = Curve;
 
-    fn from_full(curve: &Self::Full, cache: &mut FullToPartialCache) -> Self {
+    fn from_full(curve: &Self::Full, _: &mut FullToPartialCache) -> Self {
         Self {
             path: Some(curve.path().into()),
-            global_form: Partial::from_full(curve.global_form().clone(), cache),
         }
     }
 
-    fn build(self, objects: &mut Service<Objects>) -> Self::Full {
+    fn build(self, _: &mut Service<Objects>) -> Self::Full {
         let path = match self.path.expect("Need path to build curve") {
             MaybeSurfacePath::Defined(path) => path,
             undefined => {
@@ -36,9 +32,8 @@ impl PartialObject for PartialCurve {
                 )
             }
         };
-        let global_form = self.global_form.build(objects);
 
-        Curve::new(path, global_form)
+        Curve::new(path)
     }
 }
 

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

@@ -74,7 +74,6 @@ impl Default for PartialHalfEdge {
             (None, surface_form)
         });
 
-        let global_curve = curve.read().global_form.clone();
         let global_vertices = vertices.each_ref_ext().map(
             |vertex: &(Option<Point<1>>, Partial<SurfaceVertex>)| {
                 let surface_vertex = vertex.1.clone();
@@ -84,8 +83,8 @@ impl Default for PartialHalfEdge {
         );
 
         let global_form = Partial::from_partial(PartialGlobalEdge {
-            curve: global_curve,
             vertices: global_vertices,
+            ..Default::default()
         });
 
         Self {

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

@@ -13,7 +13,6 @@ impl Validate for HalfEdge {
         config: &ValidationConfig,
         errors: &mut Vec<ValidationError>,
     ) {
-        HalfEdgeValidationError::check_global_curve_identity(self, errors);
         HalfEdgeValidationError::check_global_vertex_identity(self, errors);
         HalfEdgeValidationError::check_vertex_coincidence(self, config, errors);
     }
@@ -107,26 +106,6 @@ pub enum HalfEdgeValidationError {
 }
 
 impl HalfEdgeValidationError {
-    fn check_global_curve_identity(
-        half_edge: &HalfEdge,
-        errors: &mut Vec<ValidationError>,
-    ) {
-        let global_curve_from_curve = half_edge.curve().global_form();
-        let global_curve_from_global_form = half_edge.global_form().curve();
-
-        if global_curve_from_curve.id() != global_curve_from_global_form.id() {
-            errors.push(
-                Box::new(Self::GlobalCurveMismatch {
-                    global_curve_from_curve: global_curve_from_curve.clone(),
-                    global_curve_from_global_form:
-                        global_curve_from_global_form.clone(),
-                    half_edge: half_edge.clone(),
-                })
-                .into(),
-            );
-        }
-    }
-
     fn check_global_vertex_identity(
         half_edge: &HalfEdge,
         errors: &mut Vec<ValidationError>,
@@ -185,47 +164,12 @@ mod tests {
 
     use crate::{
         builder::HalfEdgeBuilder,
-        insert::Insert,
+        objects::HalfEdge,
-        objects::{GlobalCurve, HalfEdge},
         partial::{Partial, PartialHalfEdge, PartialObject},
         services::Services,
         validate::Validate,
     };
 
-    #[test]
-    fn half_edge_global_curve_mismatch() -> anyhow::Result<()> {
-        let mut services = Services::new();
-
-        let valid = {
-            let surface = services.objects.surfaces.xy_plane();
-
-            let mut half_edge = PartialHalfEdge::default();
-            half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]);
-            half_edge.infer_vertex_positions_if_necessary(&surface.geometry());
-
-            half_edge.build(&mut services.objects)
-        };
-        let invalid = {
-            let global_form = {
-                let mut global_edge =
-                    Partial::from(valid.global_form().clone());
-                global_edge.write().curve =
-                    Partial::from(GlobalCurve.insert(&mut services.objects));
-                global_edge.build(&mut services.objects)
-            };
-            let vertices = valid
-                .boundary()
-                .zip_ext(valid.surface_vertices().map(Clone::clone));
-
-            HalfEdge::new(valid.curve().clone(), vertices, global_form)
-        };
-
-        valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
-
-        Ok(())
-    }
-
     #[test]
     fn half_edge_global_vertex_mismatch() -> anyhow::Result<()> {
         let mut services = Services::new();