Merge pull request #754 from hannobraun/face

Fix face equality

crates/fj-kernel/src/algorithms/reverse.rs

@@ -1,5 +1,7 @@
+use fj_math::{Circle, Line, Point, Vector};
+
 use crate::{
-    objects::{Cycle, CyclesInFace, Edge, Face},
+    objects::{Curve, Cycle, CyclesInFace, Edge, Face},
     shape::LocalForm,
 };
 
@@ -32,19 +34,28 @@ fn reverse_local_coordinates_in_cycle(cycles: &CyclesInFace) -> CyclesInFace {
             .edges
             .iter()
             .map(|edge| {
-                let curve = LocalForm::new(
-                    // This is wrong. We have reversed the direction of the
-                    // surface, thereby modifying its coordinate system. So we
-                    // can't just use the local form of the curve, which is
-                    // expressed in surface coordinates, as-is.
-                    //
-                    // This is a coherence issue, but since coherence validation
-                    // is not complete, and the whole local form stuff is still
-                    // a work in progress, this doesn't lead to any observable
-                    // bugs.
-                    *edge.local().curve.local(),
-                    *edge.local().curve.canonical(),
-                );
+                let curve = {
+                    let local = match *edge.local().curve.local() {
+                        Curve::Circle(Circle { center, a, b }) => {
+                            let center = Point::from([center.u, -center.v]);
+
+                            let a = Vector::from([a.u, -a.v]);
+                            let b = Vector::from([b.u, -b.v]);
+
+                            Curve::Circle(Circle { center, a, b })
+                        }
+                        Curve::Line(Line { origin, direction }) => {
+                            let origin = Point::from([origin.u, -origin.v]);
+                            let direction =
+                                Vector::from([direction.u, -direction.v]);
+
+                            Curve::Line(Line { origin, direction })
+                        }
+                    };
+
+                    let canonical = *edge.local().curve.canonical();
+                    LocalForm::new(local, canonical)
+                };
                 let vertices = edge.local().vertices.clone();
                 let local = Edge { curve, vertices };
                 LocalForm::new(local, edge.canonical().clone())

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

@@ -1,5 +1,3 @@
-use std::hash::{Hash, Hasher};
-
 use fj_interop::mesh::Color;
 use fj_math::Triangle;
 
@@ -103,7 +101,7 @@ impl Face {
 /// This type exists to ease the handling of faces that use boundary
 /// representation. It will eventually be merged into `Face`, once
 /// `Face::Triangles` can finally be removed.
-#[derive(Clone, Debug, Eq, Ord, PartialOrd)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct FaceBRep {
     /// The surface that defines this face
     pub surface: Surface,
@@ -167,23 +165,6 @@ impl FaceBRep {
     }
 }
 
-impl PartialEq for FaceBRep {
-    fn eq(&self, other: &Self) -> bool {
-        self.surface() == other.surface()
-            && self.exteriors().eq(other.exteriors())
-            && self.interiors().eq(other.interiors())
-    }
-}
-
-impl Hash for FaceBRep {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        self.surface().hash(state);
-        for cycle in self.all_cycles() {
-            cycle.hash(state);
-        }
-    }
-}
-
 /// A list of cycles, as they are stored in `Face`
 #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
 pub struct CyclesInFace(Vec<LocalForm<Cycle<2>, Cycle<3>>>);