Merge pull request #2237 from hannobraun/surface

Don't expect `Surface`/`Handle<Surface>` where `SurfaceGeometry` will do

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

@@ -5,8 +5,8 @@ use std::collections::BTreeMap;
 use fj_math::Point;
 
 use crate::{
-    geometry::{CurveBoundary, GlobalPath, SurfacePath},
-    objects::{Curve, Surface},
+    geometry::{CurveBoundary, GlobalPath, SurfaceGeometry, SurfacePath},
+    objects::Curve,
     storage::{Handle, HandleWrapper},
     Core,
 };
@@ -17,7 +17,7 @@ impl Approx
     for (
         &Handle<Curve>,
         SurfacePath,
-        &Surface,
+        &SurfaceGeometry,
         CurveBoundary<Point<1>>,
     )
 {
@@ -51,7 +51,7 @@ impl Approx
 
 fn approx_curve(
     path: &SurfacePath,
-    surface: &Surface,
+    surface: &SurfaceGeometry,
     boundary: CurveBoundary<Point<1>>,
     tolerance: impl Into<Tolerance>,
     core: &mut Core,
@@ -62,63 +62,63 @@ fn approx_curve(
     // This will probably all be unified eventually, as `SurfacePath` and
     // `GlobalPath` grow APIs that are better suited to implementing this code
     // in a more abstract way.
-    let points =
-        match (path, surface.geometry().u) {
-            (SurfacePath::Circle(_), GlobalPath::Circle(_)) => {
-                todo!(
+    let points = match (path, surface.u) {
+        (SurfacePath::Circle(_), GlobalPath::Circle(_)) => {
+            todo!(
                 "Approximating a circle on a curved surface not supported yet."
             )
+        }
+        (SurfacePath::Circle(_), GlobalPath::Line(_)) => {
+            (path, boundary)
+                .approx_with_cache(tolerance, &mut (), core)
+                .into_iter()
+                .map(|(point_curve, point_surface)| {
+                    // We're throwing away `point_surface` here, which is a
+                    // bit weird, as we're recomputing it later (outside of
+                    // this function).
+                    //
+                    // It should be fine though:
+                    //
+                    // 1. We're throwing this version away, so there's no
+                    //    danger of inconsistency between this and the later
+                    //    version.
+                    // 2. This version should have been computed using the
+                    //    same path and parameters and the later version
+                    //    will be, so they should be the same anyway.
+                    // 3. Not all other cases handled in this function have
+                    //    a surface point available, so it needs to be
+                    //    computed later anyway, in the general case.
+
+                    let point_global =
+                        surface.point_from_surface_coords(point_surface);
+                    (point_curve, point_global)
+                })
+                .collect()
+        }
+        (SurfacePath::Line(line), _) => {
+            let range_u =
+                CurveBoundary::from(boundary.inner.map(|point_curve| {
+                    [path.point_from_path_coords(point_curve).u]
+                }));
+
+            let approx_u = (surface.u, range_u).approx_with_cache(
+                tolerance,
+                &mut (),
+                core,
+            );
+
+            let mut points = Vec::new();
+            for (u, _) in approx_u {
+                let t = (u.t - line.origin().u) / line.direction().u;
+                let point_surface = path.point_from_path_coords([t]);
+                let point_global =
+                    surface.point_from_surface_coords(point_surface);
+                points.push((u, point_global));
             }
-            (SurfacePath::Circle(_), GlobalPath::Line(_)) => {
-                (path, boundary)
-                    .approx_with_cache(tolerance, &mut (), core)
-                    .into_iter()
-                    .map(|(point_curve, point_surface)| {
-                        // We're throwing away `point_surface` here, which is a
-                        // bit weird, as we're recomputing it later (outside of
-                        // this function).
-                        //
-                        // It should be fine though:
-                        //
-                        // 1. We're throwing this version away, so there's no
-                        //    danger of inconsistency between this and the later
-                        //    version.
-                        // 2. This version should have been computed using the
-                        //    same path and parameters and the later version
-                        //    will be, so they should be the same anyway.
-                        // 3. Not all other cases handled in this function have
-                        //    a surface point available, so it needs to be
-                        //    computed later anyway, in the general case.
-
-                        let point_global = surface
-                            .geometry()
-                            .point_from_surface_coords(point_surface);
-                        (point_curve, point_global)
-                    })
-                    .collect()
-            }
-            (SurfacePath::Line(line), _) => {
-                let range_u =
-                    CurveBoundary::from(boundary.inner.map(|point_curve| {
-                        [path.point_from_path_coords(point_curve).u]
-                    }));
-
-                let approx_u = (surface.geometry().u, range_u)
-                    .approx_with_cache(tolerance, &mut (), core);
-
-                let mut points = Vec::new();
-                for (u, _) in approx_u {
-                    let t = (u.t - line.origin().u) / line.direction().u;
-                    let point_surface = path.point_from_path_coords([t]);
-                    let point_global = surface
-                        .geometry()
-                        .point_from_surface_coords(point_surface);
-                    points.push((u, point_global));
-                }
-
-                points
-            }
-        };
+
+            points
+        }
+    };
 
     let points = points
         .into_iter()
@@ -183,14 +183,14 @@ impl CurveApproxCache {
 
 #[cfg(test)]
 mod tests {
-    use std::{f64::consts::TAU, ops::Deref};
+    use std::f64::consts::TAU;
 
     use pretty_assertions::assert_eq;
 
     use crate::{
         algorithms::approx::{Approx, ApproxPoint},
         geometry::{CurveBoundary, GlobalPath, SurfaceGeometry, SurfacePath},
-        objects::{Curve, Surface},
+        objects::Curve,
         operations::insert::Insert,
         Core,
     };
@@ -203,10 +203,10 @@ mod tests {
         let (surface_path, boundary) =
             SurfacePath::line_from_points([[1., 1.], [2., 1.]]);
         let boundary = CurveBoundary::from(boundary);
-        let surface = core.layers.objects.surfaces.xz_plane();
+        let surface = core.layers.objects.surfaces.xz_plane().geometry();
 
         let tolerance = 1.;
-        let approx = (&curve, surface_path, surface.deref(), boundary)
+        let approx = (&curve, surface_path, &surface, boundary)
             .approx(tolerance, &mut core);
 
         assert_eq!(approx.points, vec![]);
@@ -220,10 +220,10 @@ mod tests {
         let (surface_path, boundary) =
             SurfacePath::line_from_points([[1., 1.], [2., 1.]]);
         let boundary = CurveBoundary::from(boundary);
-        let surface = Surface::new(SurfaceGeometry {
+        let surface = SurfaceGeometry {
             u: GlobalPath::circle_from_radius(1.),
             v: [0., 0., 1.].into(),
-        });
+        };
 
         let tolerance = 1.;
         let approx = (&curve, surface_path, &surface, boundary)
@@ -243,10 +243,10 @@ mod tests {
             ([TAU], [TAU, 1.]),
         ]);
         let boundary = CurveBoundary::from([[0.], [TAU]]);
-        let surface = Surface::new(SurfaceGeometry {
+        let surface = SurfaceGeometry {
             u: global_path,
             v: [0., 0., 1.].into(),
-        });
+        };
 
         let tolerance = 1.;
         let approx = (&curve, surface_path, &surface, boundary)
@@ -259,7 +259,7 @@ mod tests {
                 let point_surface =
                     surface_path.point_from_path_coords(point_local);
                 let point_global =
-                    surface.geometry().point_from_surface_coords(point_surface);
+                    surface.point_from_surface_coords(point_surface);
                 ApproxPoint::new(point_local, point_global)
             })
             .collect::<Vec<_>>();
@@ -274,10 +274,10 @@ mod tests {
         let surface_path =
             SurfacePath::circle_from_center_and_radius([0., 0.], 1.);
         let boundary = CurveBoundary::from([[0.], [TAU]]);
-        let surface = core.layers.objects.surfaces.xz_plane();
+        let surface = core.layers.objects.surfaces.xz_plane().geometry();
 
         let tolerance = 1.;
-        let approx = (&curve, surface_path, surface.deref(), boundary)
+        let approx = (&curve, surface_path, &surface, boundary)
             .approx(tolerance, &mut core);
 
         let expected_approx = (&surface_path, boundary)
@@ -287,7 +287,7 @@ mod tests {
                 let point_surface =
                     surface_path.point_from_path_coords(point_local);
                 let point_global =
-                    surface.geometry().point_from_surface_coords(point_surface);
+                    surface.point_from_surface_coords(point_surface);
                 ApproxPoint::new(point_local, point_global)
             })
             .collect::<Vec<_>>();

crates/fj-core/src/algorithms/approx/cycle.rs

@@ -6,17 +6,14 @@ use std::ops::Deref;
 
 use fj_math::Segment;
 
-use crate::{
-    objects::{Cycle, Surface},
-    Core,
-};
+use crate::{geometry::SurfaceGeometry, objects::Cycle, Core};
 
 use super::{
     edge::{HalfEdgeApprox, HalfEdgeApproxCache},
     Approx, ApproxPoint, Tolerance,
 };
 
-impl Approx for (&Cycle, &Surface) {
+impl Approx for (&Cycle, &SurfaceGeometry) {
     type Approximation = CycleApprox;
     type Cache = HalfEdgeApproxCache;
 

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

@@ -5,17 +5,14 @@
 //! approximations are usually used to build cycle approximations, and this way,
 //! the caller doesn't have to deal with duplicate vertices.
 
-use crate::{
-    objects::{HalfEdge, Surface},
-    Core,
-};
+use crate::{geometry::SurfaceGeometry, objects::HalfEdge, Core};
 
 use super::{
     curve::CurveApproxCache, vertex::VertexApproxCache, Approx, ApproxPoint,
     Tolerance,
 };
 
-impl Approx for (&HalfEdge, &Surface) {
+impl Approx for (&HalfEdge, &SurfaceGeometry) {
     type Approximation = HalfEdgeApprox;
     type Cache = HalfEdgeApproxCache;
 
@@ -33,9 +30,8 @@ impl Approx for (&HalfEdge, &Surface) {
         {
             Some(position) => position,
             None => {
-                let position_global = surface
-                    .geometry()
-                    .point_from_surface_coords(start_position_surface);
+                let position_global =
+                    surface.point_from_surface_coords(start_position_surface);
                 cache
                     .start_position
                     .insert(edge.start_vertex().clone(), position_global)

crates/fj-core/src/algorithms/approx/face.rs

@@ -91,12 +91,12 @@ impl Approx for &Face {
         // it have nothing to do with its curvature.
 
         let exterior =
-            (self.region().exterior().deref(), self.surface().deref())
+            (self.region().exterior().deref(), &self.surface().geometry())
                 .approx_with_cache(tolerance, cache, core);
 
         let mut interiors = BTreeSet::new();
         for cycle in self.region().interiors() {
-            let cycle = (cycle.deref(), self.surface().deref())
+            let cycle = (cycle.deref(), &self.surface().geometry())
                 .approx_with_cache(tolerance, cache, core);
             interiors.insert(cycle);
         }

crates/fj-core/src/algorithms/approx/mod.rs

@@ -19,7 +19,7 @@ use std::{
 
 use fj_math::Point;
 
-use crate::{objects::Surface, Core};
+use crate::Core;
 
 pub use self::tolerance::{InvalidTolerance, Tolerance};
 
@@ -79,19 +79,6 @@ impl<const D: usize> ApproxPoint<D> {
     }
 }
 
-impl ApproxPoint<2> {
-    /// Create an instance of `ApproxPoint` from a surface point
-    pub fn from_surface_point(
-        point_surface: impl Into<Point<2>>,
-        surface: &Surface,
-    ) -> Self {
-        let point_surface = point_surface.into();
-        let point_global =
-            surface.geometry().point_from_surface_coords(point_surface);
-        ApproxPoint::new(point_surface, point_global)
-    }
-}
-
 impl<const D: usize> Eq for ApproxPoint<D> {}
 
 impl<const D: usize> PartialEq for ApproxPoint<D> {

crates/fj-core/src/operations/sweep/cycle.rs

@@ -2,7 +2,8 @@ use fj_interop::Color;
 use fj_math::Vector;
 
 use crate::{
-    objects::{Cycle, Face, Surface},
+    geometry::SurfaceGeometry,
+    objects::{Cycle, Face},
     operations::{
         build::BuildCycle, join::JoinCycle, sweep::half_edge::SweepHalfEdge,
     },
@@ -37,7 +38,7 @@ pub trait SweepCycle {
     /// operation is called in, and therefore falls outside of its scope.
     fn sweep_cycle(
         &self,
-        surface: &Surface,
+        surface: &SurfaceGeometry,
         color: Option<Color>,
         path: impl Into<Vector<3>>,
         cache: &mut SweepCache,
@@ -48,7 +49,7 @@ pub trait SweepCycle {
 impl SweepCycle for Cycle {
     fn sweep_cycle(
         &self,
-        surface: &Surface,
+        surface: &SurfaceGeometry,
         color: Option<Color>,
         path: impl Into<Vector<3>>,
         cache: &mut SweepCache,

crates/fj-core/src/operations/sweep/half_edge.rs

@@ -2,7 +2,8 @@ use fj_interop::{ext::ArrayExt, Color};
 use fj_math::{Point, Scalar, Vector};
 
 use crate::{
-    objects::{Cycle, Face, HalfEdge, Region, Surface, Vertex},
+    geometry::SurfaceGeometry,
+    objects::{Cycle, Face, HalfEdge, Region, Vertex},
     operations::{
         build::{BuildCycle, BuildHalfEdge},
         insert::Insert,
@@ -37,7 +38,7 @@ pub trait SweepHalfEdge {
     fn sweep_half_edge(
         &self,
         end_vertex: Handle<Vertex>,
-        surface: &Surface,
+        surface: &SurfaceGeometry,
         color: Option<Color>,
         path: impl Into<Vector<3>>,
         cache: &mut SweepCache,
@@ -49,7 +50,7 @@ impl SweepHalfEdge for HalfEdge {
     fn sweep_half_edge(
         &self,
         end_vertex: Handle<Vertex>,
-        surface: &Surface,
+        surface: &SurfaceGeometry,
         color: Option<Color>,
         path: impl Into<Vector<3>>,
         cache: &mut SweepCache,