From eb59bdf4569c83b7a265068b1395c6ac1dc0fb1e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:42:52 +0100 Subject: [PATCH 1/7] Refactor to prepare for follow-on change --- crates/fj-core/src/operations/sweep/face.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index db319ba0d..f75235c69 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -1,9 +1,10 @@ +use fj_interop::mesh::Color; use fj_math::{Scalar, Vector}; use crate::{ algorithms::transform::TransformObject, geometry::GlobalPath, - objects::{Cycle, Face, Region, Shell}, + objects::{Cycle, Face, Region, Shell, Surface}, operations::{insert::Insert, reverse::Reverse}, services::Services, storage::Handle, @@ -58,7 +59,8 @@ impl SweepFace for Face { let top_exterior = sweep_cycle( bottom_face.region().exterior(), - &bottom_face, + bottom_face.surface(), + bottom_face.region().color(), &mut faces, path, cache, @@ -70,7 +72,8 @@ impl SweepFace for Face { for bottom_cycle in bottom_face.region().interiors() { let top_cycle = sweep_cycle( bottom_cycle, - &bottom_face, + bottom_face.surface(), + bottom_face.region().color(), &mut faces, path, cache, @@ -119,15 +122,16 @@ fn bottom_face(face: &Face, path: Vector<3>, services: &mut Services) -> Face { fn sweep_cycle( bottom_cycle: &Cycle, - bottom_face: &Face, + bottom_surface: &Surface, + color: Option, 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(), + bottom_surface, + color, path, cache, services, From c735b7646609f12b0868fe9c7a17e33b3181d005 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 16:54:11 +0100 Subject: [PATCH 2/7] Improve doc comment --- crates/fj-core/src/operations/sweep/cycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/operations/sweep/cycle.rs b/crates/fj-core/src/operations/sweep/cycle.rs index 435f3acfa..78665267c 100644 --- a/crates/fj-core/src/operations/sweep/cycle.rs +++ b/crates/fj-core/src/operations/sweep/cycle.rs @@ -87,7 +87,7 @@ impl SweepCycle for Cycle { } } -/// The result of sweeping a cycle +/// The result of sweeping a [`Cycle`] /// /// See [`SweepCycle`]. pub struct SweptCycle { From a203fc492613773db10ade64480b8767e9a660cd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 17:08:28 +0100 Subject: [PATCH 3/7] Fix typo in doc comment --- crates/fj-core/src/operations/sweep/cycle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/operations/sweep/cycle.rs b/crates/fj-core/src/operations/sweep/cycle.rs index 78665267c..58f4b5ff0 100644 --- a/crates/fj-core/src/operations/sweep/cycle.rs +++ b/crates/fj-core/src/operations/sweep/cycle.rs @@ -96,7 +96,7 @@ pub struct SweptCycle { /// See [`SweepCycle::sweep_cycle`] for more information. pub faces: Vec, - /// A cycle mad up of the "top" half-edges of the resulting faces + /// A cycle made 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) From cd6e7edf7166254dcbeaf7b56d97c6f8597c8492 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 17:08:45 +0100 Subject: [PATCH 4/7] Extract `SweepRegion` from `SweepFace` --- crates/fj-core/src/operations/sweep/face.rs | 67 ++-------- crates/fj-core/src/operations/sweep/mod.rs | 2 + crates/fj-core/src/operations/sweep/region.rs | 126 ++++++++++++++++++ 3 files changed, 138 insertions(+), 57 deletions(-) create mode 100644 crates/fj-core/src/operations/sweep/region.rs diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index f75235c69..51de3266b 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -1,16 +1,14 @@ -use fj_interop::mesh::Color; use fj_math::{Scalar, Vector}; use crate::{ algorithms::transform::TransformObject, geometry::GlobalPath, - objects::{Cycle, Face, Region, Shell, Surface}, + objects::{Face, Shell}, operations::{insert::Insert, reverse::Reverse}, services::Services, - storage::Handle, }; -use super::{SweepCache, SweepCycle}; +use super::{SweepCache, SweepRegion}; /// # Sweep a [`Face`] /// @@ -57,38 +55,20 @@ impl SweepFace for Face { let top_surface = bottom_face.surface().clone().translate(path, services); - let top_exterior = sweep_cycle( - bottom_face.region().exterior(), + let swept_region = bottom_face.region().sweep_region( bottom_face.surface(), - bottom_face.region().color(), - &mut faces, path, cache, services, ); - let mut top_interiors = Vec::new(); - - for bottom_cycle in bottom_face.region().interiors() { - let top_cycle = sweep_cycle( - bottom_cycle, - bottom_face.surface(), - bottom_face.region().color(), - &mut faces, - path, - cache, - services, - ); - - top_interiors.push(top_cycle); - } - - let top_region = Region::new( - top_exterior, - top_interiors, - bottom_face.region().color(), - ) - .insert(services); + faces.extend( + swept_region + .faces + .into_iter() + .map(|side_face| side_face.insert(services)), + ); + let top_region = swept_region.top_region.insert(services); let top_face = Face::new(top_surface, top_region).insert(services); faces.push(top_face); @@ -119,30 +99,3 @@ fn bottom_face(face: &Face, path: Vector<3>, services: &mut Services) -> Face { face.reverse(services) } } - -fn sweep_cycle( - bottom_cycle: &Cycle, - bottom_surface: &Surface, - color: Option, - faces: &mut Vec>, - path: Vector<3>, - cache: &mut SweepCache, - services: &mut Services, -) -> Handle { - let swept_cycle = bottom_cycle.reverse(services).sweep_cycle( - bottom_surface, - color, - path, - cache, - services, - ); - - faces.extend( - swept_cycle - .faces - .into_iter() - .map(|side_face| side_face.insert(services)), - ); - - 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 1fc0934e8..54863cc0f 100644 --- a/crates/fj-core/src/operations/sweep/mod.rs +++ b/crates/fj-core/src/operations/sweep/mod.rs @@ -7,6 +7,7 @@ mod cycle; mod face; mod half_edge; mod path; +mod region; mod sketch; mod vertex; @@ -15,6 +16,7 @@ pub use self::{ face::SweepFace, half_edge::SweepHalfEdge, path::SweepSurfacePath, + region::{SweepRegion, SweptRegion}, sketch::SweepSketch, vertex::SweepVertex, }; diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs new file mode 100644 index 000000000..0336ecacc --- /dev/null +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -0,0 +1,126 @@ +use fj_interop::mesh::Color; +use fj_math::Vector; + +use crate::{ + objects::{Cycle, Face, Region, Surface}, + operations::{insert::Insert, reverse::Reverse}, + services::Services, + storage::Handle, +}; + +use super::{SweepCache, SweepCycle}; + +/// # Sweep a [`Region`] +/// +/// See [module documentation] for more information. +/// +/// [module documentation]: super +pub trait SweepRegion { + /// # Sweep the [`Region`] + /// + /// Sweep the region into multiple sets of faces. Each set of faces is + /// formed by sweeping one of the region's cycles + /// + /// Requires the surface that the face that the region belongs to is defined + /// in. + /// + /// There are no faces at the "top" (the end of the sweep path) or "bottom". + /// + /// There is no face at the "top" (the end of the sweep path). We *would* + /// have enough information to create that, as we have access to the surface + /// too and could translate that here. However, that we have access to that + /// surface is a bit incidental, and a weird artifact of how the object + /// graph currently works. For this reason, the creating the top face is + /// considered out of scope for this operation, and left to the caller. + /// + /// There also is no "bottom" face. Whether having one is desirable, depends + /// on the context of the caller of this operation, and there also falls + /// outside of its scope. + fn sweep_region( + &self, + surface: &Surface, + path: impl Into>, + cache: &mut SweepCache, + services: &mut Services, + ) -> SweptRegion; +} + +impl SweepRegion for Region { + fn sweep_region( + &self, + surface: &Surface, + path: impl Into>, + cache: &mut SweepCache, + services: &mut Services, + ) -> SweptRegion { + let path = path.into(); + + let mut faces = Vec::new(); + + let top_exterior = sweep_cycle( + self.exterior(), + surface, + self.color(), + &mut faces, + path, + cache, + services, + ); + + let mut top_interiors = Vec::new(); + + for bottom_cycle in self.interiors() { + let top_cycle = sweep_cycle( + bottom_cycle, + surface, + self.color(), + &mut faces, + path, + cache, + services, + ); + + top_interiors.push(top_cycle); + } + + let top_region = Region::new(top_exterior, top_interiors, self.color()); + + SweptRegion { faces, top_region } + } +} + +/// The result of sweeping a [`Region`] +/// +/// See [`SweepRegion`]. +pub struct SweptRegion { + /// The faces created by sweeping each cycle of the region + pub faces: Vec, + + /// A region made up of the "top" cycles + /// + /// This is essentially a version of the original region, translated by the + /// sweep path. + pub top_region: Region, +} + +fn sweep_cycle( + bottom_cycle: &Cycle, + bottom_surface: &Surface, + color: Option, + faces: &mut Vec, + path: Vector<3>, + cache: &mut SweepCache, + services: &mut Services, +) -> Handle { + let swept_cycle = bottom_cycle.reverse(services).sweep_cycle( + bottom_surface, + color, + path, + cache, + services, + ); + + faces.extend(swept_cycle.faces); + + swept_cycle.top_cycle.insert(services) +} From 35c2eb79f31681f7b3b552c930ce4579f3dbb421 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 17:10:51 +0100 Subject: [PATCH 5/7] Refactor to improve clarity --- crates/fj-core/src/operations/sweep/face.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 51de3266b..62d6b6aa3 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -52,9 +52,6 @@ impl SweepFace for Face { let bottom_face = bottom_face(self, path, services).insert(services); faces.push(bottom_face.clone()); - let top_surface = - bottom_face.surface().clone().translate(path, services); - let swept_region = bottom_face.region().sweep_region( bottom_face.surface(), path, @@ -70,7 +67,12 @@ impl SweepFace for Face { ); let top_region = swept_region.top_region.insert(services); - let top_face = Face::new(top_surface, top_region).insert(services); + let top_face = { + let top_surface = + bottom_face.surface().clone().translate(path, services); + + Face::new(top_surface, top_region).insert(services) + }; faces.push(top_face); Shell::new(faces) From a82df7297d1d981c77afc91c6c5e526c1764e493 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 17:12:04 +0100 Subject: [PATCH 6/7] Refactor to improve clarity --- crates/fj-core/src/operations/sweep/face.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 62d6b6aa3..0be86ad07 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -65,11 +65,11 @@ impl SweepFace for Face { .into_iter() .map(|side_face| side_face.insert(services)), ); - let top_region = swept_region.top_region.insert(services); let top_face = { let top_surface = bottom_face.surface().clone().translate(path, services); + let top_region = swept_region.top_region.insert(services); Face::new(top_surface, top_region).insert(services) }; From 787cd7f5183eb458594c1453c63395e2ae83b696 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Nov 2023 17:13:45 +0100 Subject: [PATCH 7/7] Refactor to improve clarity --- crates/fj-core/src/operations/sweep/face.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 0be86ad07..1ba7c5a86 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -59,12 +59,11 @@ impl SweepFace for Face { services, ); - faces.extend( - swept_region - .faces - .into_iter() - .map(|side_face| side_face.insert(services)), - ); + let side_faces = swept_region + .faces + .into_iter() + .map(|side_face| side_face.insert(services)); + faces.extend(side_faces); let top_face = { let top_surface =