From f088555fed55dc5a050a53d0f298b3bd5e3df58e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:35:48 +0100 Subject: [PATCH 1/8] Create top face in region sweeping code --- crates/fj-core/src/operations/sweep/face.rs | 10 ---------- crates/fj-core/src/operations/sweep/region.rs | 9 +++++++++ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index f75004188..9932a5b6a 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -1,7 +1,6 @@ use fj_math::Vector; use crate::{ - algorithms::transform::TransformObject, objects::{Face, Shell}, operations::insert::Insert, services::Services, @@ -65,15 +64,6 @@ impl SweepFace for Handle { .map(|side_face| side_face.insert(services)); faces.extend(side_faces); - 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) - }; - faces.push(top_face); - Shell::new(faces) } } diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index 0336ecacc..bab4b1b47 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -2,6 +2,7 @@ use fj_interop::mesh::Color; use fj_math::Vector; use crate::{ + algorithms::transform::TransformObject, objects::{Cycle, Face, Region, Surface}, operations::{insert::Insert, reverse::Reverse}, services::Services, @@ -85,6 +86,14 @@ impl SweepRegion for Region { let top_region = Region::new(top_exterior, top_interiors, self.color()); + let top_face = { + let top_surface = + surface.translate(path, services).insert(services); + + Face::new(top_surface, top_region.clone().insert(services)) + }; + faces.push(top_face); + SweptRegion { faces, top_region } } } From 37a27e96cb4fbc723d29dbe88b28ebad6e044e2b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:36:23 +0100 Subject: [PATCH 2/8] Remove unused struct field --- crates/fj-core/src/operations/sweep/region.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index bab4b1b47..336f19b70 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -90,11 +90,11 @@ impl SweepRegion for Region { let top_surface = surface.translate(path, services).insert(services); - Face::new(top_surface, top_region.clone().insert(services)) + Face::new(top_surface, top_region.insert(services)) }; faces.push(top_face); - SweptRegion { faces, top_region } + SweptRegion { faces } } } @@ -104,12 +104,6 @@ impl SweepRegion for Region { 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( From 7a73617ca11cd0d8d85baa027944fdb1a6cef3d6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:38:26 +0100 Subject: [PATCH 3/8] Remove redundant struct --- crates/fj-core/src/operations/sweep/face.rs | 1 - crates/fj-core/src/operations/sweep/mod.rs | 2 +- crates/fj-core/src/operations/sweep/region.rs | 14 +++----------- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 9932a5b6a..21ae02e5b 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -59,7 +59,6 @@ impl SweepFace for Handle { ); let side_faces = swept_region - .faces .into_iter() .map(|side_face| side_face.insert(services)); faces.extend(side_faces); diff --git a/crates/fj-core/src/operations/sweep/mod.rs b/crates/fj-core/src/operations/sweep/mod.rs index 54863cc0f..17d80e13b 100644 --- a/crates/fj-core/src/operations/sweep/mod.rs +++ b/crates/fj-core/src/operations/sweep/mod.rs @@ -16,7 +16,7 @@ pub use self::{ face::SweepFace, half_edge::SweepHalfEdge, path::SweepSurfacePath, - region::{SweepRegion, SweptRegion}, + region::SweepRegion, sketch::SweepSketch, vertex::SweepVertex, }; diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index 336f19b70..978c7efb1 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -43,7 +43,7 @@ pub trait SweepRegion { path: impl Into>, cache: &mut SweepCache, services: &mut Services, - ) -> SweptRegion; + ) -> Vec; } impl SweepRegion for Region { @@ -53,7 +53,7 @@ impl SweepRegion for Region { path: impl Into>, cache: &mut SweepCache, services: &mut Services, - ) -> SweptRegion { + ) -> Vec { let path = path.into(); let mut faces = Vec::new(); @@ -94,18 +94,10 @@ impl SweepRegion for Region { }; faces.push(top_face); - SweptRegion { faces } + faces } } -/// The result of sweeping a [`Region`] -/// -/// See [`SweepRegion`]. -pub struct SweptRegion { - /// The faces created by sweeping each cycle of the region - pub faces: Vec, -} - fn sweep_cycle( bottom_cycle: &Cycle, bottom_surface: &Surface, From 1ce3cef88ee537e09149f7fc87a4a14077f7e283 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:38:40 +0100 Subject: [PATCH 4/8] Update variable name --- crates/fj-core/src/operations/sweep/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index 21ae02e5b..fd39b473c 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -51,14 +51,14 @@ impl SweepFace for Handle { let bottom_face = self.clone(); faces.push(bottom_face.clone()); - let swept_region = bottom_face.region().sweep_region( + let side_faces = bottom_face.region().sweep_region( bottom_face.surface(), path, cache, services, ); - let side_faces = swept_region + let side_faces = side_faces .into_iter() .map(|side_face| side_face.insert(services)); faces.extend(side_faces); From 2aa4e46cb53ff331c35b86df0de4c2fad592af34 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:38:53 +0100 Subject: [PATCH 5/8] Inline redundant variable --- crates/fj-core/src/operations/sweep/face.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/face.rs b/crates/fj-core/src/operations/sweep/face.rs index fd39b473c..5e4ce5c2c 100644 --- a/crates/fj-core/src/operations/sweep/face.rs +++ b/crates/fj-core/src/operations/sweep/face.rs @@ -51,14 +51,9 @@ impl SweepFace for Handle { let bottom_face = self.clone(); faces.push(bottom_face.clone()); - let side_faces = bottom_face.region().sweep_region( - bottom_face.surface(), - path, - cache, - services, - ); - - let side_faces = side_faces + let side_faces = bottom_face + .region() + .sweep_region(bottom_face.surface(), path, cache, services) .into_iter() .map(|side_face| side_face.insert(services)); faces.extend(side_faces); From 4c1e050d61e421dbdd54beeef5abf74a331d1ed4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:39:52 +0100 Subject: [PATCH 6/8] Refactor to improve clarity --- crates/fj-core/src/operations/sweep/region.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index 978c7efb1..426e14d99 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -84,11 +84,11 @@ impl SweepRegion for Region { top_interiors.push(top_cycle); } - let top_region = Region::new(top_exterior, top_interiors, self.color()); - let top_face = { let top_surface = surface.translate(path, services).insert(services); + let top_region = + Region::new(top_exterior, top_interiors, self.color()); Face::new(top_surface, top_region.insert(services)) }; From 79fbbaed92d1f5269e5f917c4f81dfeabf8a5714 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:40:08 +0100 Subject: [PATCH 7/8] Refactor --- crates/fj-core/src/operations/sweep/region.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index 426e14d99..d9a2ef975 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -88,9 +88,10 @@ impl SweepRegion for Region { let top_surface = surface.translate(path, services).insert(services); let top_region = - Region::new(top_exterior, top_interiors, self.color()); + Region::new(top_exterior, top_interiors, self.color()) + .insert(services); - Face::new(top_surface, top_region.insert(services)) + Face::new(top_surface, top_region) }; faces.push(top_face); From a6844ba1f1267a9c020786e9d564d2c9c2e3c972 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 29 Nov 2023 16:41:48 +0100 Subject: [PATCH 8/8] Update documentation of `SweepRegion` --- crates/fj-core/src/operations/sweep/region.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index d9a2ef975..f61a548ff 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -20,23 +20,14 @@ 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 + /// formed by sweeping one of the region's cycles, then adding a top face. /// /// 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. + /// There no "bottom" face. Whether having one is desirable depends on the + /// context of the caller of this operation, and falls outside of this + /// operation's scope. fn sweep_region( &self, surface: &Surface,