Merge pull request #2263 from hannobraun/surface

Fully move surface geometry to geometry layer

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

@@ -90,13 +90,18 @@ impl Approx for &Face {
         // would need to provide its own approximation, as the edges that bound
         // it have nothing to do with its curvature.
 
-        let exterior =
-            (self.region().exterior().deref(), &self.surface().geometry())
-                .approx_with_cache(tolerance, cache, core);
+        let exterior = (
+            self.region().exterior().deref(),
+            &core.layers.geometry.of_surface(self.surface()),
+        )
+            .approx_with_cache(tolerance, cache, core);
 
         let mut interiors = BTreeSet::new();
         for cycle in self.region().interiors() {
-            let cycle = (cycle.deref(), &self.surface().geometry())
+            let cycle = (
+                cycle.deref(),
+                &core.layers.geometry.of_surface(self.surface()),
+            )
                 .approx_with_cache(tolerance, cache, core);
             interiors.insert(cycle);
         }

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

@@ -1,13 +1,14 @@
 use fj_math::Aabb;
 
-use crate::objects::Cycle;
+use crate::{geometry::Geometry, objects::Cycle};
 
 impl super::BoundingVolume<2> for Cycle {
-    fn aabb(&self) -> Option<Aabb<2>> {
+    fn aabb(&self, geometry: &Geometry) -> Option<Aabb<2>> {
         let mut aabb: Option<Aabb<2>> = None;
 
         for edge in self.half_edges() {
-            let new_aabb = edge.aabb().expect("`Edge` can always compute AABB");
+            let new_aabb =
+                edge.aabb(geometry).expect("`Edge` can always compute AABB");
             aabb = Some(aabb.map_or(new_aabb, |aabb| aabb.merged(&new_aabb)));
         }
 

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

@@ -1,9 +1,12 @@
 use fj_math::{Aabb, Vector};
 
-use crate::{geometry::SurfacePath, objects::HalfEdge};
+use crate::{
+    geometry::{Geometry, SurfacePath},
+    objects::HalfEdge,
+};
 
 impl super::BoundingVolume<2> for HalfEdge {
-    fn aabb(&self) -> Option<Aabb<2>> {
+    fn aabb(&self, _: &Geometry) -> Option<Aabb<2>> {
         match self.path() {
             SurfacePath::Circle(circle) => {
                 // Just calculate the AABB of the whole circle. This is not the

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

@@ -1,11 +1,14 @@
 use fj_math::Aabb;
 
-use crate::{geometry::GlobalPath, objects::Face};
+use crate::{
+    geometry::{Geometry, GlobalPath},
+    objects::Face,
+};
 
 impl super::BoundingVolume<3> for Face {
-    fn aabb(&self) -> Option<Aabb<3>> {
-        self.region().exterior().aabb().map(|aabb2| {
-            let surface = self.surface().geometry();
+    fn aabb(&self, geometry: &Geometry) -> Option<Aabb<3>> {
+        self.region().exterior().aabb(geometry).map(|aabb2| {
+            let surface = geometry.of_surface(self.surface());
 
             match surface.u {
                 GlobalPath::Circle(circle) => {

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

@@ -8,10 +8,12 @@ mod solid;
 
 use fj_math::Aabb;
 
+use crate::geometry::Geometry;
+
 /// Compute a bounding volume for an object
 pub trait BoundingVolume<const D: usize> {
     /// Compute an axis-aligned bounding box (AABB)
     ///
     /// Return `None`, if no AABB can be computed (if the object is empty).
-    fn aabb(&self) -> Option<Aabb<D>>;
+    fn aabb(&self, geometry: &Geometry) -> Option<Aabb<D>>;
 }

crates/fj-core/src/algorithms/bounding_volume/shell.rs

@@ -1,13 +1,13 @@
 use fj_math::Aabb;
 
-use crate::objects::Shell;
+use crate::{geometry::Geometry, objects::Shell};
 
 impl super::BoundingVolume<3> for Shell {
-    fn aabb(&self) -> Option<Aabb<3>> {
+    fn aabb(&self, geometry: &Geometry) -> Option<Aabb<3>> {
         let mut aabb: Option<Aabb<3>> = None;
 
         for face in self.faces() {
-            let new_aabb = face.aabb();
+            let new_aabb = face.aabb(geometry);
             aabb = aabb.map_or(new_aabb, |aabb| match new_aabb {
                 Some(new_aabb) => Some(aabb.merged(&new_aabb)),
                 None => Some(aabb),

crates/fj-core/src/algorithms/bounding_volume/solid.rs

@@ -1,13 +1,13 @@
 use fj_math::Aabb;
 
-use crate::objects::Solid;
+use crate::{geometry::Geometry, objects::Solid};
 
 impl super::BoundingVolume<3> for Solid {
-    fn aabb(&self) -> Option<Aabb<3>> {
+    fn aabb(&self, geometry: &Geometry) -> Option<Aabb<3>> {
         let mut aabb: Option<Aabb<3>> = None;
 
         for shell in self.shells() {
-            let new_aabb = shell.aabb();
+            let new_aabb = shell.aabb(geometry);
             aabb = aabb.map_or(new_aabb, |aabb| match new_aabb {
                 Some(new_aabb) => Some(aabb.merged(&new_aabb)),
                 None => Some(aabb),

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

@@ -161,12 +161,36 @@ mod tests {
 
         let triangles = triangulate(face, &mut core)?;
 
-        let a = surface.geometry().point_from_surface_coords(a);
-        let b = surface.geometry().point_from_surface_coords(b);
-        let e = surface.geometry().point_from_surface_coords(e);
-        let f = surface.geometry().point_from_surface_coords(f);
-        let g = surface.geometry().point_from_surface_coords(g);
-        let h = surface.geometry().point_from_surface_coords(h);
+        let a = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(a);
+        let b = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(b);
+        let e = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(e);
+        let f = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(f);
+        let g = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(g);
+        let h = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(h);
 
         // Let's test that some correct triangles are present. We don't need to
         // test them all.
@@ -224,11 +248,31 @@ mod tests {
 
         let triangles = triangulate(face, &mut core)?;
 
-        let a = surface.geometry().point_from_surface_coords(a);
-        let b = surface.geometry().point_from_surface_coords(b);
-        let c = surface.geometry().point_from_surface_coords(c);
-        let d = surface.geometry().point_from_surface_coords(d);
-        let e = surface.geometry().point_from_surface_coords(e);
+        let a = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(a);
+        let b = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(b);
+        let c = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(c);
+        let d = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(d);
+        let e = core
+            .layers
+            .geometry
+            .of_surface(&surface)
+            .point_from_surface_coords(e);
 
         assert!(triangles.contains_triangle([a, b, d]));
         assert!(triangles.contains_triangle([a, d, e]));

crates/fj-core/src/layers/objects.rs

@@ -1,6 +1,7 @@
 //! Layer infrastructure for [`Objects`]
 
 use crate::{
+    geometry::Geometry,
     objects::{AboutToBeStored, AnyObject, Objects},
     validation::Validation,
 };
@@ -14,6 +15,7 @@ impl Layer<Objects> {
     pub fn insert(
         &mut self,
         object: AnyObject<AboutToBeStored>,
+        geometry: &Geometry,
         validation: &mut Layer<Validation>,
     ) {
         let mut events = Vec::new();
@@ -22,6 +24,7 @@ impl Layer<Objects> {
         for event in events {
             let event = ValidateObject {
                 object: event.object.into(),
+                geometry,
             };
             validation.process(event, &mut Vec::new());
         }

crates/fj-core/src/layers/validation.rs

@@ -1,6 +1,7 @@
 //! Layer infrastructure for [`Validation`]
 
 use crate::{
+    geometry::Geometry,
     objects::{AnyObject, Stored},
     validation::{Validation, ValidationError, ValidationErrors},
 };
@@ -15,18 +16,22 @@ impl Layer<Validation> {
 }
 
 /// Validate an object
-pub struct ValidateObject {
+pub struct ValidateObject<'r> {
     /// The object to validate
     pub object: AnyObject<Stored>,
+
+    /// Reference to `Geometry`, which is required for validation
+    pub geometry: &'r Geometry,
 }
 
-impl Command<Validation> for ValidateObject {
+impl Command<Validation> for ValidateObject<'_> {
     type Result = ();
     type Event = ValidationFailed;
 
     fn decide(self, state: &Validation, events: &mut Vec<Self::Event>) {
         let mut errors = Vec::new();
-        self.object.validate(&state.config, &mut errors);
+        self.object
+            .validate(&state.config, &mut errors, self.geometry);
 
         for err in errors {
             events.push(ValidationFailed {

crates/fj-core/src/objects/any_object.rs

@@ -1,4 +1,5 @@
 use crate::{
+    geometry::Geometry,
     objects::{
         Curve, Cycle, Face, HalfEdge, Objects, Region, Shell, Sketch, Solid,
         Surface, Vertex,
@@ -38,10 +39,15 @@ macro_rules! any_object {
             pub fn validate(&self,
                 config: &ValidationConfig,
                 errors: &mut Vec<ValidationError>,
+                geometry: &Geometry,
             ) {
                 match self {
                     $(
-                        Self::$ty(object) => object.validate(config, errors),
+                        Self::$ty(object) => object.validate(
+                            config,
+                            errors,
+                            geometry,
+                        ),
                     )*
                 }
             }

crates/fj-core/src/objects/kinds/face.rs

@@ -2,7 +2,7 @@ use fj_math::Winding;
 
 use crate::{
     objects::{Region, Surface},
-    storage::Handle,
+    storage::{Handle, HandleWrapper},
 };
 
 /// A face of a shape
@@ -31,14 +31,17 @@ use crate::{
 /// [`Shell`]: crate::objects::Shell
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct Face {
-    surface: Handle<Surface>,
+    surface: HandleWrapper<Surface>,
     region: Handle<Region>,
 }
 
 impl Face {
     /// Construct an instance of `Face`
     pub fn new(surface: Handle<Surface>, region: Handle<Region>) -> Self {
-        Self { surface, region }
+        Self {
+            surface: surface.into(),
+            region,
+        }
     }
 
     /// Access the surface of the face

crates/fj-core/src/objects/kinds/surface.rs

@@ -1,19 +1,25 @@
-use crate::geometry::SurfaceGeometry;
-
 /// A two-dimensional shape
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
-pub struct Surface {
-    geometry: SurfaceGeometry,
-}
+///
+///
+/// ## Equality
+///
+/// `Surface` contains no data and exists purely to be referenced via a
+/// `Handle`, where `Handle::id` can be used to compare different instances of
+/// it.
+///
+/// If `Surface` had `Eq`/`PartialEq` implementations, it containing no data
+/// would mean that all instances of `Surface` would be considered equal. This
+/// would be very error-prone.
+///
+/// If you need to reference a `Surface` from a struct that needs to derive
+/// `Eq`/`Ord`/..., you can use `HandleWrapper<Vertex>` to do that. It will
+/// use `Handle::id` to provide those `Eq`/`Ord`/... implementations.
+#[derive(Clone, Copy, Debug, Default, Hash)]
+pub struct Surface {}
 
 impl Surface {
     /// Construct an instance of `Surface`
-    pub fn new(geometry: SurfaceGeometry) -> Self {
-        Self { geometry }
-    }
-
-    /// Access the surface's geometry
-    pub fn geometry(&self) -> SurfaceGeometry {
-        self.geometry
+    pub fn new() -> Self {
+        Self::default()
     }
 }

crates/fj-core/src/objects/kinds/vertex.rs

@@ -68,7 +68,7 @@
 /// ## Equality
 ///
 /// `Vertex` contains no data and exists purely to be referenced via a `Handle`,
-/// where `Handle::id` can be used to compare different instances of `Vertex`.
+/// where `Handle::id` can be used to compare different instances of it.
 ///
 /// If `Vertex` had `Eq`/`PartialEq` implementations, it containing no data
 /// would mean that all instances of `Vertex` would be considered equal. This