From 54917b58b92cf844fab14897e4d038bdd710c0b0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 17 May 2022 17:59:56 +0200 Subject: [PATCH 1/2] Add `FaceBRep` to ease dealing with `Face` --- crates/fj-kernel/src/algorithms/sweep.rs | 4 +- .../src/algorithms/triangulation/mod.rs | 6 +- crates/fj-kernel/src/shape/object.rs | 18 ++- crates/fj-kernel/src/shape/validate.rs | 16 +-- crates/fj-kernel/src/topology/face.rs | 114 ++++++++++++------ 5 files changed, 96 insertions(+), 62 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index ab8839bb5..da0cf1d04 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -257,7 +257,7 @@ impl Relation { fn exteriors_for_face(&self, face: &Face) -> Vec>> { let exteriors = match face { - Face::Face { exteriors, .. } => exteriors, + Face::Face(face) => &face.exteriors, _ => { // Sketches are created using boundary representation, so this // case can't happen. @@ -273,7 +273,7 @@ impl Relation { fn interiors_for_face(&self, face: &Face) -> Vec>> { let interiors = match face { - Face::Face { interiors, .. } => interiors, + Face::Face(face) => &face.interiors, _ => { // Sketches are created using boundary representation, so this // case can't happen. diff --git a/crates/fj-kernel/src/algorithms/triangulation/mod.rs b/crates/fj-kernel/src/algorithms/triangulation/mod.rs index fa86746f0..5ab142438 100644 --- a/crates/fj-kernel/src/algorithms/triangulation/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulation/mod.rs @@ -22,8 +22,8 @@ pub fn triangulate( for face in shape.faces() { let face = face.get(); match &face { - Face::Face { surface, color, .. } => { - let surface = surface.get(); + Face::Face(brep) => { + let surface = brep.surface.get(); let approx = FaceApprox::new(&face, tolerance); let points: Vec<_> = approx @@ -68,7 +68,7 @@ pub fn triangulate( for triangle in triangles { let points = triangle.map(|point| point.canonical()); - mesh.push_triangle(points, *color); + mesh.push_triangle(points, brep.color); } } Face::Triangles(triangles) => { diff --git a/crates/fj-kernel/src/shape/object.rs b/crates/fj-kernel/src/shape/object.rs index a0d568701..31d1f4a22 100644 --- a/crates/fj-kernel/src/shape/object.rs +++ b/crates/fj-kernel/src/shape/object.rs @@ -84,28 +84,24 @@ impl Object for Cycle<3> { impl Object for Face { fn merge_into(self, shape: &mut Shape) -> ValidationResult { match self { - Face::Face { - surface, - exteriors, - interiors, - color, - } => { - let surface = surface.get().merge_into(shape)?; + Face::Face(face) => { + let surface = face.surface.get().merge_into(shape)?; let mut exts = Vec::new(); - for cycle in exteriors.as_canonical() { + for cycle in face.exteriors.as_canonical() { let cycle = cycle.merge_into(shape)?; exts.push(cycle); } let mut ints = Vec::new(); - for cycle in interiors.as_canonical() { + for cycle in face.interiors.as_canonical() { let cycle = cycle.merge_into(shape)?; ints.push(cycle); } - shape - .get_handle_or_insert(Face::new(surface, exts, ints, color)) + shape.get_handle_or_insert(Face::new( + surface, exts, ints, face.color, + )) } Face::Triangles(_) => shape.get_handle_or_insert(self), } diff --git a/crates/fj-kernel/src/shape/validate.rs b/crates/fj-kernel/src/shape/validate.rs index 823d335a9..b47d37e4d 100644 --- a/crates/fj-kernel/src/shape/validate.rs +++ b/crates/fj-kernel/src/shape/validate.rs @@ -136,20 +136,16 @@ impl Validate for Face { _: Scalar, stores: &Stores, ) -> Result<(), ValidationError> { - if let Face::Face { - surface, - exteriors, - interiors, - .. - } = self - { + if let Face::Face(face) = self { let mut missing_surface = None; let mut missing_cycles = HashSet::new(); - if !stores.surfaces.contains(surface) { - missing_surface = Some(surface.clone()); + if !stores.surfaces.contains(&face.surface) { + missing_surface = Some(face.surface.clone()); } - for cycle in exteriors.as_handle().chain(interiors.as_handle()) { + for cycle in + face.exteriors.as_handle().chain(face.interiors.as_handle()) + { if !stores.cycles.contains(cycle) { missing_cycles.insert(cycle.clone()); } diff --git a/crates/fj-kernel/src/topology/face.rs b/crates/fj-kernel/src/topology/face.rs index 69a854892..a6d78c238 100644 --- a/crates/fj-kernel/src/topology/face.rs +++ b/crates/fj-kernel/src/topology/face.rs @@ -21,40 +21,13 @@ use super::{Cycle, FaceBuilder}; /// /// A face that is part of a [`Shape`] must be structurally sound. That means /// the surface and any cycles it refers to, must be part of the same shape. -#[derive(Clone, Debug, Eq, Ord, PartialOrd)] +#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub enum Face { /// A face of a shape /// /// A face is defined by a surface, and is bounded by edges that lie in that /// surface. - Face { - /// The surface that defines this face - surface: Handle, - - /// The cycles that bound the face on the outside - /// - /// # Implementation Note - /// - /// Since these cycles bound the face, the edges they consist of must - /// lie in the surface. The data we're using here is 3-dimensional - /// though, so no such limitation is enforced. - /// - /// It might be less error-prone to specify the cycles in surface - /// coordinates. - exteriors: CyclesInFace, - - /// The cycles that bound the face on the inside - /// - /// Each of these cycles defines a hole in the face. - /// - /// # Implementation note - /// - /// See note on `exterior` field. - interiors: CyclesInFace, - - /// The color of the face - color: [u8; 4], - }, + Face(FaceBRep), /// The triangles of the face /// @@ -76,12 +49,12 @@ impl Face { let exteriors = CyclesInFace::from_canonical(exteriors); let interiors = CyclesInFace::from_canonical(interiors); - Self::Face { + Self::Face(FaceBRep { surface, exteriors, interiors, color, - } + }) } /// Build a face using the [`FaceBuilder`] API pub fn builder(surface: Surface, shape: &mut Shape) -> FaceBuilder { @@ -94,7 +67,7 @@ impl Face { /// [`Handle`]. pub fn surface(&self) -> Surface { match self { - Self::Face { surface, .. } => surface.get(), + Self::Face(face) => face.surface(), _ => { // No code that still uses triangle representation is calling // this method. @@ -109,7 +82,7 @@ impl Face { /// [`Handle`]s. pub fn exteriors(&self) -> impl Iterator> + '_ { match self { - Self::Face { exteriors, .. } => exteriors.as_canonical(), + Self::Face(face) => face.exteriors(), _ => { // No code that still uses triangle representation is calling // this method. @@ -124,7 +97,7 @@ impl Face { /// [`Handle`]s. pub fn interiors(&self) -> impl Iterator> + '_ { match self { - Self::Face { interiors, .. } => interiors.as_canonical(), + Self::Face(face) => face.interiors(), _ => { // No code that still uses triangle representation is calling // this method. @@ -142,7 +115,76 @@ impl Face { } } -impl PartialEq for Face { +/// The boundary representation of a 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)] +pub struct FaceBRep { + /// The surface that defines this face + pub surface: Handle, + + /// The cycles that bound the face on the outside + /// + /// # Implementation Note + /// + /// Since these cycles bound the face, the edges they consist of must + /// lie in the surface. The data we're using here is 3-dimensional + /// though, so no such limitation is enforced. + /// + /// It might be less error-prone to specify the cycles in surface + /// coordinates. + pub exteriors: CyclesInFace, + + /// The cycles that bound the face on the inside + /// + /// Each of these cycles defines a hole in the face. + /// + /// # Implementation note + /// + /// See note on `exterior` field. + pub interiors: CyclesInFace, + + /// The color of the face + pub color: [u8; 4], +} + +impl FaceBRep { + /// Access the surface that the face refers to + /// + /// This is a convenience method that saves the caller from dealing with the + /// [`Handle`]. + pub fn surface(&self) -> Surface { + self.surface.get() + } + + /// Access the exterior cycles that the face refers to + /// + /// This is a convenience method that saves the caller from dealing with the + /// [`Handle`]s. + pub fn exteriors(&self) -> impl Iterator> + '_ { + self.exteriors.as_canonical() + } + + /// Access the interior cycles that the face refers to + /// + /// This is a convenience method that saves the caller from dealing with the + /// [`Handle`]s. + pub fn interiors(&self) -> impl Iterator> + '_ { + self.interiors.as_canonical() + } + + /// Access all cycles that the face refers to + /// + /// This is equivalent to chaining the iterators returned by + /// [`Face::exteriors`] and [`Face::interiors`]. + pub fn all_cycles(&self) -> impl Iterator> + '_ { + self.exteriors().chain(self.interiors()) + } +} + +impl PartialEq for FaceBRep { fn eq(&self, other: &Self) -> bool { self.surface() == other.surface() && self.exteriors().eq(other.exteriors()) @@ -150,7 +192,7 @@ impl PartialEq for Face { } } -impl Hash for Face { +impl Hash for FaceBRep { fn hash(&self, state: &mut H) { self.surface().hash(state); for cycle in self.all_cycles() { From a7a176e7e6f9b6335906c811e95432cf4c579d98 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 17 May 2022 18:05:25 +0200 Subject: [PATCH 2/2] Add `Face::brep` --- crates/fj-kernel/src/algorithms/sweep.rs | 24 +++------------- crates/fj-kernel/src/topology/face.rs | 35 +++++++++--------------- 2 files changed, 17 insertions(+), 42 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index da0cf1d04..141d939be 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -256,32 +256,16 @@ impl Relation { } fn exteriors_for_face(&self, face: &Face) -> Vec>> { - let exteriors = match face { - Face::Face(face) => &face.exteriors, - _ => { - // Sketches are created using boundary representation, so this - // case can't happen. - unreachable!() - } - }; - - exteriors + face.brep() + .exteriors .as_handle() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() } fn interiors_for_face(&self, face: &Face) -> Vec>> { - let interiors = match face { - Face::Face(face) => &face.interiors, - _ => { - // Sketches are created using boundary representation, so this - // case can't happen. - unreachable!() - } - }; - - interiors + face.brep() + .interiors .as_handle() .map(|cycle| self.cycles.get(cycle).unwrap().clone()) .collect() diff --git a/crates/fj-kernel/src/topology/face.rs b/crates/fj-kernel/src/topology/face.rs index a6d78c238..2f5f67627 100644 --- a/crates/fj-kernel/src/topology/face.rs +++ b/crates/fj-kernel/src/topology/face.rs @@ -61,13 +61,10 @@ impl Face { FaceBuilder::new(surface, shape) } - /// Access the surface that the face refers to - /// - /// This is a convenience method that saves the caller from dealing with the - /// [`Handle`]. - pub fn surface(&self) -> Surface { + /// Access the boundary representation of the face + pub fn brep(&self) -> &FaceBRep { match self { - Self::Face(face) => face.surface(), + Self::Face(face) => face, _ => { // No code that still uses triangle representation is calling // this method. @@ -76,19 +73,20 @@ impl Face { } } + /// Access the surface that the face refers to + /// + /// This is a convenience method that saves the caller from dealing with the + /// [`Handle`]. + pub fn surface(&self) -> Surface { + self.brep().surface() + } + /// Access the exterior cycles that the face refers to /// /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. pub fn exteriors(&self) -> impl Iterator> + '_ { - match self { - Self::Face(face) => face.exteriors(), - _ => { - // No code that still uses triangle representation is calling - // this method. - unreachable!() - } - } + self.brep().exteriors() } /// Access the interior cycles that the face refers to @@ -96,14 +94,7 @@ impl Face { /// This is a convenience method that saves the caller from dealing with the /// [`Handle`]s. pub fn interiors(&self) -> impl Iterator> + '_ { - match self { - Self::Face(face) => face.interiors(), - _ => { - // No code that still uses triangle representation is calling - // this method. - unreachable!() - } - } + self.brep().interiors() } /// Access all cycles that the face refers to