diff --git a/crates/fj-kernel/src/algorithms/approx/cycles.rs b/crates/fj-kernel/src/algorithms/approx/cycles.rs index cafed62d0..9cba6c1cf 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycles.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycles.rs @@ -16,7 +16,7 @@ impl CycleApprox { /// /// `tolerance` defines how far the approximation is allowed to deviate from /// the actual face. - pub fn new(cycle: &Cycle, tolerance: Tolerance) -> Self { + pub fn new(cycle: &Cycle<3>, tolerance: Tolerance) -> Self { let mut points = Vec::new(); for edge in cycle.edges() { diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 290b81e09..7699a160a 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -65,12 +65,8 @@ pub fn sweep_shape( let edges_bottom = source_to_bottom.edges_for_cycle(&cycle_source); let edges_top = source_to_top.edges_for_cycle(&cycle_source); - let cycle_bottom = target - .insert(Cycle { - edges: edges_bottom, - }) - .unwrap(); - let cycle_top = target.insert(Cycle { edges: edges_top }).unwrap(); + let cycle_bottom = target.insert(Cycle::new(edges_bottom)).unwrap(); + let cycle_top = target.insert(Cycle::new(edges_top)).unwrap(); source_to_bottom .cycles @@ -152,6 +148,8 @@ pub fn sweep_shape( let mut vertex_bottom_to_edge = HashMap::new(); for edge_source in &cycle_source.get().edges { + let edge_source = edge_source.canonical(); + // Can't panic. We already ruled out the continuous edge case // above, so this edge must have vertices. let vertices_source = @@ -206,14 +204,12 @@ pub fn sweep_shape( .unwrap(); let cycle = target - .insert(Cycle { - edges: vec![ - bottom_edge, - top_edge, - side_edge_a, - side_edge_b, - ], - }) + .insert(Cycle::new(vec![ + bottom_edge, + top_edge, + side_edge_a, + side_edge_b, + ])) .unwrap(); target @@ -234,7 +230,7 @@ pub fn sweep_shape( struct Relation { vertices: HashMap>, Handle>>, edges: HashMap>, Handle>>, - cycles: HashMap, Handle>, + cycles: HashMap>, Handle>>, } impl Relation { @@ -257,16 +253,19 @@ impl Relation { }) } - fn edges_for_cycle(&self, cycle: &Handle) -> Vec>> { + fn edges_for_cycle( + &self, + cycle: &Handle>, + ) -> Vec>> { cycle .get() .edges .iter() - .map(|edge| self.edges.get(edge).unwrap().clone()) + .map(|edge| self.edges.get(edge.canonical()).unwrap().clone()) .collect() } - fn exteriors_for_face(&self, face: &Face) -> Vec> { + fn exteriors_for_face(&self, face: &Face) -> Vec>> { let exteriors = match face { Face::Face { exteriors, .. } => exteriors, _ => { @@ -282,7 +281,7 @@ impl Relation { .collect() } - fn interiors_for_face(&self, face: &Face) -> Vec> { + fn interiors_for_face(&self, face: &Face) -> Vec>> { let interiors = match face { Face::Face { interiors, .. } => interiors, _ => { @@ -379,9 +378,7 @@ mod tests { let ca = Edge::builder(&mut shape) .build_line_segment_from_points([c, a])?; - let cycles = shape.insert(Cycle { - edges: vec![ab, bc, ca], - })?; + let cycles = shape.insert(Cycle::new(vec![ab, bc, ca]))?; let surface = Surface::SweptCurve(SweptCurve::plane_from_points([a, b, c])); diff --git a/crates/fj-kernel/src/shape/api.rs b/crates/fj-kernel/src/shape/api.rs index 1e94c99f8..9ef11e54c 100644 --- a/crates/fj-kernel/src/shape/api.rs +++ b/crates/fj-kernel/src/shape/api.rs @@ -175,7 +175,7 @@ impl Shape { /// Access an iterator over all cycles /// /// The caller must not make any assumptions about the order of cycles. - pub fn cycles(&self) -> Iter { + pub fn cycles(&self) -> Iter> { self.stores.cycles.iter() } @@ -240,7 +240,7 @@ mod tests { assert!(shape.get_handle(&vertex.get()).as_ref() == Some(&vertex)); assert!(shape.get_handle(&edge.get()).as_ref() == Some(&edge)); - let cycle = Cycle { edges: vec![edge] }; + let cycle = Cycle::new(vec![edge]); assert!(shape.get_handle(&cycle).is_none()); let cycle = shape.insert(cycle)?; @@ -321,16 +321,12 @@ mod tests { // Trying to refer to edge that is not from the same shape. Should fail. let edge = other.add_edge()?; - let err = shape - .insert(Cycle { - edges: vec![edge.clone()], - }) - .unwrap_err(); + let err = shape.insert(Cycle::new(vec![edge.clone()])).unwrap_err(); assert!(err.missing_edge(&edge)); // Referring to edge that *is* from the same shape. Should work. let edge = shape.add_edge()?; - shape.insert(Cycle { edges: vec![edge] })?; + shape.insert(Cycle::new(vec![edge]))?; Ok(()) } @@ -402,9 +398,9 @@ mod tests { Ok(edge) } - fn add_cycle(&mut self) -> anyhow::Result> { + fn add_cycle(&mut self) -> anyhow::Result>> { let edge = self.add_edge()?; - let cycle = self.insert(Cycle { edges: vec![edge] })?; + let cycle = self.insert(Cycle::new(vec![edge]))?; Ok(cycle) } } diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index 4ac2c4462..7418ad129 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -19,7 +19,7 @@ impl private::Sealed for Surface {} impl private::Sealed for Vertex<3> {} impl private::Sealed for Edge<3> {} -impl private::Sealed for Cycle {} +impl private::Sealed for Cycle<3> {} impl private::Sealed for Face {} impl Object for Point<3> {} @@ -28,7 +28,7 @@ impl Object for Surface {} impl Object for Vertex<3> {} impl Object for Edge<3> {} -impl Object for Cycle {} +impl Object for Cycle<3> {} impl Object for Face {} mod private { diff --git a/crates/fj-kernel/src/shape/stores.rs b/crates/fj-kernel/src/shape/stores.rs index ef7d450e5..be7b2a513 100644 --- a/crates/fj-kernel/src/shape/stores.rs +++ b/crates/fj-kernel/src/shape/stores.rs @@ -57,7 +57,7 @@ pub type Surfaces = Store; pub type Vertices = Store>; pub type Edges = Store>; -pub type Cycles = Store; +pub type Cycles = Store>; pub type Faces = Store; #[derive(Debug)] diff --git a/crates/fj-kernel/src/shape/validate.rs b/crates/fj-kernel/src/shape/validate.rs index 918ad9820..16a10b69b 100644 --- a/crates/fj-kernel/src/shape/validate.rs +++ b/crates/fj-kernel/src/shape/validate.rs @@ -95,7 +95,7 @@ impl Validate for Edge<3> { } } -impl Validate for Cycle { +impl Validate for Cycle<3> { /// Validate the cycle /// /// # Implementation note @@ -111,6 +111,8 @@ impl Validate for Cycle { ) -> Result<(), ValidationError> { let mut missing_edges = HashSet::new(); for edge in &self.edges { + let edge = edge.canonical(); + if !stores.edges.contains(edge) { missing_edges.insert(edge.clone()); } @@ -248,7 +250,7 @@ impl ValidationError { /// Indicate whether validation found a missing cycle #[cfg(test)] - pub fn missing_cycle(&self, cycle: &Handle) -> bool { + pub fn missing_cycle(&self, cycle: &Handle>) -> bool { if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self { return missing_cycles.contains(cycle); @@ -282,5 +284,5 @@ pub struct StructuralIssues { pub missing_surface: Option>, /// Missing cycles found in face validation - pub missing_cycles: HashSet>, + pub missing_cycles: HashSet>>, } diff --git a/crates/fj-kernel/src/topology/builder.rs b/crates/fj-kernel/src/topology/builder.rs index e46d4f6d1..2b1fc1b5a 100644 --- a/crates/fj-kernel/src/topology/builder.rs +++ b/crates/fj-kernel/src/topology/builder.rs @@ -107,7 +107,7 @@ impl<'r> CycleBuilder<'r> { pub fn build_polygon( self, points: impl IntoIterator>>, - ) -> ValidationResult { + ) -> ValidationResult> { // A polygon is closed, so we need to add the first point at the end // again, for the next step. let mut points: Vec<_> = points.into_iter().map(Into::into).collect(); @@ -127,7 +127,7 @@ impl<'r> CycleBuilder<'r> { edges.push(edge); } - self.shape.insert(Cycle { edges }) + self.shape.insert(Cycle::new(edges)) } } diff --git a/crates/fj-kernel/src/topology/cycles.rs b/crates/fj-kernel/src/topology/cycles.rs index db9ae2075..8f18b9d94 100644 --- a/crates/fj-kernel/src/topology/cycles.rs +++ b/crates/fj-kernel/src/topology/cycles.rs @@ -1,6 +1,4 @@ -use std::hash::{Hash, Hasher}; - -use crate::shape::{Handle, Shape}; +use crate::shape::{Handle, LocalForm, Shape}; use super::{CycleBuilder, Edge}; @@ -19,13 +17,20 @@ use super::{CycleBuilder, Edge}; /// /// A cycle that is part of a [`Shape`] must be structurally sound. That means /// the edges it refers to, must be part of the same shape. -#[derive(Clone, Debug, Eq, Ord, PartialOrd)] -pub struct Cycle { +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct Cycle { /// The edges that make up the cycle - pub edges: Vec>>, + pub edges: Vec, Edge<3>>>, } -impl Cycle { +impl Cycle<3> { + /// Construct a `Cycle` + pub fn new(edges: impl IntoIterator>>) -> Self { + let edges = edges.into_iter().map(LocalForm::canonical_only).collect(); + + Self { edges } + } + /// Build a cycle using the [`CycleBuilder`] API pub fn builder(shape: &mut Shape) -> CycleBuilder { CycleBuilder::new(shape) @@ -36,20 +41,6 @@ impl Cycle { /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. pub fn edges(&self) -> impl Iterator> + '_ { - self.edges.iter().map(|handle| handle.get()) - } -} - -impl PartialEq for Cycle { - fn eq(&self, other: &Self) -> bool { - self.edges().eq(other.edges()) - } -} - -impl Hash for Cycle { - fn hash(&self, state: &mut H) { - for edge in self.edges() { - edge.hash(state); - } + self.edges.iter().map(|handle| handle.canonical().get()) } } diff --git a/crates/fj-kernel/src/topology/faces.rs b/crates/fj-kernel/src/topology/faces.rs index 74aceaf03..34461bedd 100644 --- a/crates/fj-kernel/src/topology/faces.rs +++ b/crates/fj-kernel/src/topology/faces.rs @@ -41,7 +41,7 @@ pub enum Face { /// /// It might be less error-prone to specify the cycles in surface /// coordinates. - exteriors: Vec>, + exteriors: Vec>>, /// The cycles that bound the face on the inside /// @@ -50,7 +50,7 @@ pub enum Face { /// # Implementation note /// /// See note on `exterior` field. - interiors: Vec>, + interiors: Vec>>, /// The color of the face color: [u8; 4], @@ -90,7 +90,7 @@ impl Face { /// /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. - pub fn exteriors(&self) -> impl Iterator + '_ { + pub fn exteriors(&self) -> impl Iterator> + '_ { match self { Self::Face { exteriors, .. } => { exteriors.iter().map(|handle| handle.get()) @@ -107,7 +107,7 @@ impl Face { /// /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. - pub fn interiors(&self) -> impl Iterator + '_ { + pub fn interiors(&self) -> impl Iterator> + '_ { match self { Self::Face { interiors, .. } => { interiors.iter().map(|handle| handle.get()) @@ -124,7 +124,7 @@ impl Face { /// /// This is equivalent to chaining the iterators returned by /// [`Face::exteriors`] and [`Face::interiors`]. - pub fn all_cycles(&self) -> impl Iterator + '_ { + pub fn all_cycles(&self) -> impl Iterator> + '_ { self.exteriors().chain(self.interiors()) } } diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index a0dcff129..3c86e85fc 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -19,7 +19,7 @@ impl ToShape for fj::Circle { let edge = Edge::builder(&mut shape) .build_circle(Scalar::from_f64(self.radius())) .unwrap(); - shape.insert(Cycle { edges: vec![edge] }).unwrap(); + shape.insert(Cycle::new(vec![edge])).unwrap(); let cycles = shape.cycles().collect(); let surface = shape.insert(Surface::xy_plane()).unwrap(); diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index dd97c9744..7011bac74 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -92,11 +92,11 @@ impl ToShape for fj::Difference2d { } fn add_cycle( - cycle: Handle, + cycle: Handle>, vertices: &mut HashMap, Handle>>, shape: &mut Shape, reverse: bool, -) -> Handle { +) -> Handle> { let mut edges = Vec::new(); for edge in cycle.get().edges() { let curve = edge.curve(); @@ -129,5 +129,5 @@ fn add_cycle( edges.reverse(); } - shape.insert(Cycle { edges }).unwrap() + shape.insert(Cycle::new(edges)).unwrap() } diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index 584dbd0be..1e88d1789 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -74,14 +74,13 @@ fn copy_shape(orig: Shape, target: &mut Shape) { } for cycle_orig in orig.cycles() { let cycle = target - .insert(Cycle { - edges: cycle_orig + .insert(Cycle::new( + cycle_orig .get() .edges .iter() - .map(|edge| edges[edge].clone()) - .collect(), - }) + .map(|edge| edges[edge.canonical()].clone()), + )) .unwrap(); cycles.insert(cycle_orig, cycle); }