From c326a06e16004ac9d3537c03fa6e0877d3e1bbe5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:08:42 +0100 Subject: [PATCH 1/7] Refactor to prepare for follow-on change --- crates/fj-core/src/operations/sweep/face.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index f0d6db794..6d77b4d34 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -95,14 +95,13 @@ impl SweepFace for Face { )); } - let top_cycle = Cycle::empty() - .add_joined_edges(top_edges, services) - .insert(services); + let top_cycle = + Cycle::empty().add_joined_edges(top_edges, services); if i == 0 { - top_exterior = Some(top_cycle); + top_exterior = Some(top_cycle.insert(services)); } else { - top_interiors.push(top_cycle); + top_interiors.push(top_cycle.insert(services)); }; } From 5a16526ed785f35544c27f39939c80fa779a62b2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:10:21 +0100 Subject: [PATCH 2/7] Inline redundant variable --- crates/fj-core/src/operations/sweep/face.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 6d77b4d34..71cdce957 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -84,9 +84,7 @@ impl SweepFace for Face { services, ); - let side_face = side_face.insert(services); - - faces.push(side_face); + faces.push(side_face.insert(services)); top_edges.push(( top_edge, From f88ec3fb4c0264a6d80c9ca7124c5067c0dec5c4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:13:09 +0100 Subject: [PATCH 3/7] Add `SweepCycle` --- crates/fj-core/src/operations/sweep/cycle.rs | 105 +++++++++++++++++++ crates/fj-core/src/operations/sweep/face.rs | 53 ++++------ crates/fj-core/src/operations/sweep/mod.rs | 9 +- 3 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 crates/fj-core/src/operations/sweep/cycle.rs diff --git a/crates/fj-core/src/operations/sweep/cycle.rs b/crates/fj-core/src/operations/sweep/cycle.rs new file mode 100644 index 000000000..435f3acfa --- /dev/null +++ b/crates/fj-core/src/operations/sweep/cycle.rs @@ -0,0 +1,105 @@ +use fj_interop::mesh::Color; +use fj_math::Vector; + +use crate::{ + objects::{Cycle, Face, Surface}, + operations::{ + build::BuildCycle, join::JoinCycle, sweep::half_edge::SweepHalfEdge, + }, + services::Services, +}; + +use super::SweepCache; + +/// # Sweep a [`Cycle`] +/// +/// See [module documentation] for more information. +/// +/// [module documentation]: super +pub trait SweepCycle { + /// # Sweep the [`Cycle`] + /// + /// Sweep the cycle into a set of connected faces. Each half-edge in the + /// cycle is swept into a face, meaning all resulting faces form a connected + /// set of side walls. + /// + /// Requires the surface that the half-edges of the cycle are defined in, + /// and optionally the color of the created faces. + /// + /// There is no face at the "top" (the end of the sweep path), as we don't + /// have enough information here to create that. We just have a single + /// cycle, and don't know whether that is supposed to be the only cycle + /// within a potential top face, or whether it's an exterior or interior + /// one. + /// + /// For the same reason, there also is no "bottom" face. Additionally, + /// whether a bottom face is even desirable depends on the context this + /// operation is called in, and therefore falls outside of its scope. + fn sweep_cycle( + &self, + surface: &Surface, + color: Option, + path: impl Into>, + cache: &mut SweepCache, + services: &mut Services, + ) -> SweptCycle; +} + +impl SweepCycle for Cycle { + fn sweep_cycle( + &self, + surface: &Surface, + color: Option, + path: impl Into>, + cache: &mut SweepCache, + services: &mut Services, + ) -> SweptCycle { + let path = path.into(); + + let mut faces = Vec::new(); + let mut top_edges = Vec::new(); + + for bottom_half_edge_pair in self.half_edges().pairs() { + let (bottom_half_edge, bottom_half_edge_next) = + bottom_half_edge_pair; + + let (side_face, top_edge) = bottom_half_edge.sweep_half_edge( + bottom_half_edge_next.start_vertex().clone(), + surface, + color, + path, + cache, + services, + ); + + faces.push(side_face); + + top_edges.push(( + top_edge, + bottom_half_edge.path(), + bottom_half_edge.boundary(), + )); + } + + let top_cycle = Cycle::empty().add_joined_edges(top_edges, services); + + SweptCycle { faces, top_cycle } + } +} + +/// The result of sweeping a cycle +/// +/// See [`SweepCycle`]. +pub struct SweptCycle { + /// The faces created by sweeping each half-edge of the cycle + /// + /// See [`SweepCycle::sweep_cycle`] for more information. + pub faces: Vec, + + /// A cycle mad up of the "top" half-edges of the resulting faces + /// + /// "Top" here refers to the place that the sweep path points to from the + /// original cycle. Essentially, this is a translated (along the sweep path) + /// and reversed version of the original cycle. + pub top_cycle: Cycle, +} diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 71cdce957..fee45858b 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -1,18 +1,14 @@ -use std::ops::Deref; - use fj_math::{Scalar, Vector}; use crate::{ algorithms::transform::TransformObject, geometry::GlobalPath, - objects::{Cycle, Face, Region, Shell}, - operations::{ - build::BuildCycle, insert::Insert, join::JoinCycle, reverse::Reverse, - }, + objects::{Face, Region, Shell}, + operations::{insert::Insert, reverse::Reverse}, services::Services, }; -use super::{SweepCache, SweepHalfEdge}; +use super::{SweepCache, SweepCycle}; /// # Sweep a [`Face`] /// @@ -70,36 +66,25 @@ impl SweepFace for Face { for (i, bottom_cycle) in bottom_face.region().all_cycles().enumerate() { let bottom_cycle = bottom_cycle.reverse(services); - let mut top_edges = Vec::new(); - for bottom_half_edge_pair in bottom_cycle.half_edges().pairs() { - let (bottom_half_edge, bottom_half_edge_next) = - bottom_half_edge_pair; - - let (side_face, top_edge) = bottom_half_edge.sweep_half_edge( - bottom_half_edge_next.start_vertex().clone(), - bottom_face.surface().deref(), - bottom_face.region().color(), - path, - cache, - services, - ); - - faces.push(side_face.insert(services)); - - top_edges.push(( - top_edge, - bottom_half_edge.path(), - bottom_half_edge.boundary(), - )); - } - - let top_cycle = - Cycle::empty().add_joined_edges(top_edges, services); + let swept_cycle = bottom_cycle.sweep_cycle( + bottom_face.surface(), + bottom_face.region().color(), + path, + cache, + services, + ); + + faces.extend( + swept_cycle + .faces + .into_iter() + .map(|side_face| side_face.insert(services)), + ); if i == 0 { - top_exterior = Some(top_cycle.insert(services)); + top_exterior = Some(swept_cycle.top_cycle.insert(services)); } else { - top_interiors.push(top_cycle.insert(services)); + top_interiors.push(swept_cycle.top_cycle.insert(services)); }; } diff --git a/crates/fj-core/src/operations/sweep/mod.rs b/crates/fj-core/src/operations/sweep/mod.rs index 6c92b59ef..1fc0934e8 100644 --- a/crates/fj-core/src/operations/sweep/mod.rs +++ b/crates/fj-core/src/operations/sweep/mod.rs @@ -3,6 +3,7 @@ //! Sweeps 1D or 2D objects along a straight path, creating a 2D or 3D object, //! respectively. +mod cycle; mod face; mod half_edge; mod path; @@ -10,8 +11,12 @@ mod sketch; mod vertex; pub use self::{ - face::SweepFace, half_edge::SweepHalfEdge, path::SweepSurfacePath, - sketch::SweepSketch, vertex::SweepVertex, + cycle::{SweepCycle, SweptCycle}, + face::SweepFace, + half_edge::SweepHalfEdge, + path::SweepSurfacePath, + sketch::SweepSketch, + vertex::SweepVertex, }; use std::collections::BTreeMap; From 2c4fd4829f9a86b0573b47f811d1909da8cddce9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:13:23 +0100 Subject: [PATCH 4/7] Remove outdated comment --- crates/fj-core/src/operations/sweep/face.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index fee45858b..3bb344803 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -58,11 +58,6 @@ impl SweepFace for Face { let mut top_exterior = None; let mut top_interiors = Vec::new(); - // This might not be the cleanest way to do it, but here we're creating - // the side faces, and all the ingredients for the top face, in one big - // loop. Reason is, the side faces need to be connected to the top face, - // and this seems like the most straight-forward way to make sure of - // that. for (i, bottom_cycle) in bottom_face.region().all_cycles().enumerate() { let bottom_cycle = bottom_cycle.reverse(services); From 417d17de7c7534acfbd6bf848d52b138fa975e80 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:14:42 +0100 Subject: [PATCH 5/7] Inline redundant variable --- crates/fj-core/src/operations/sweep/face.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 3bb344803..0dcdd157c 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -59,9 +59,7 @@ impl SweepFace for Face { let mut top_interiors = Vec::new(); for (i, bottom_cycle) in bottom_face.region().all_cycles().enumerate() { - let bottom_cycle = bottom_cycle.reverse(services); - - let swept_cycle = bottom_cycle.sweep_cycle( + let swept_cycle = bottom_cycle.reverse(services).sweep_cycle( bottom_face.surface(), bottom_face.region().color(), path, From 9da808e8f79e888896e9e8523e3c347178a5497b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:28:39 +0100 Subject: [PATCH 6/7] Refactor to prepare for follow-on change --- crates/fj-core/src/operations/sweep/face.rs | 47 +++++++++++++++------ 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 0dcdd157c..9c73a52bc 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -3,9 +3,10 @@ use fj_math::{Scalar, Vector}; use crate::{ algorithms::transform::TransformObject, geometry::GlobalPath, - objects::{Face, Region, Shell}, + objects::{Cycle, Face, Region, Shell}, operations::{insert::Insert, reverse::Reverse}, services::Services, + storage::Handle, }; use super::{SweepCache, SweepCycle}; @@ -59,25 +60,19 @@ impl SweepFace for Face { let mut top_interiors = Vec::new(); for (i, bottom_cycle) in bottom_face.region().all_cycles().enumerate() { - let swept_cycle = bottom_cycle.reverse(services).sweep_cycle( - bottom_face.surface(), - bottom_face.region().color(), + let top_cycle = sweep_cycle( + bottom_cycle, + &bottom_face, + &mut faces, path, cache, services, ); - faces.extend( - swept_cycle - .faces - .into_iter() - .map(|side_face| side_face.insert(services)), - ); - if i == 0 { - top_exterior = Some(swept_cycle.top_cycle.insert(services)); + top_exterior = Some(top_cycle); } else { - top_interiors.push(swept_cycle.top_cycle.insert(services)); + top_interiors.push(top_cycle); }; } @@ -117,3 +112,29 @@ fn bottom_face(face: &Face, path: Vector<3>, services: &mut Services) -> Face { face.reverse(services) } } + +fn sweep_cycle( + bottom_cycle: &Cycle, + bottom_face: &Face, + faces: &mut Vec>, + path: Vector<3>, + cache: &mut SweepCache, + services: &mut Services, +) -> Handle { + let swept_cycle = bottom_cycle.reverse(services).sweep_cycle( + bottom_face.surface(), + bottom_face.region().color(), + path, + cache, + services, + ); + + faces.extend( + swept_cycle + .faces + .into_iter() + .map(|side_face| side_face.insert(services)), + ); + + swept_cycle.top_cycle.insert(services) +} From df1af1b1a65df5d2f2f71c4841342067ca759501 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:30:27 +0100 Subject: [PATCH 7/7] Refactor to remove `unwrap` --- crates/fj-core/src/operations/sweep/face.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 9c73a52bc..db319ba0d 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -56,10 +56,18 @@ impl SweepFace for Face { let top_surface = bottom_face.surface().clone().translate(path, services); - let mut top_exterior = None; + let top_exterior = sweep_cycle( + bottom_face.region().exterior(), + &bottom_face, + &mut faces, + path, + cache, + services, + ); + let mut top_interiors = Vec::new(); - for (i, bottom_cycle) in bottom_face.region().all_cycles().enumerate() { + for bottom_cycle in bottom_face.region().interiors() { let top_cycle = sweep_cycle( bottom_cycle, &bottom_face, @@ -69,15 +77,11 @@ impl SweepFace for Face { services, ); - if i == 0 { - top_exterior = Some(top_cycle); - } else { - top_interiors.push(top_cycle); - }; + top_interiors.push(top_cycle); } let top_region = Region::new( - top_exterior.unwrap(), + top_exterior, top_interiors, bottom_face.region().color(), )