From 1d7acae0d936c242a8e67dc9f90f72cdcaa21bd1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:41:32 +0100 Subject: [PATCH 01/21] Simplify `Reverse` impl for `Handle` --- .../fj-kernel/src/algorithms/reverse/face.rs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/face.rs b/crates/fj-kernel/src/algorithms/reverse/face.rs index caed07c55..2d569c3e3 100644 --- a/crates/fj-kernel/src/algorithms/reverse/face.rs +++ b/crates/fj-kernel/src/algorithms/reverse/face.rs @@ -1,7 +1,6 @@ use crate::{ insert::Insert, objects::{Face, Objects}, - partial::{FullToPartialCache, Partial, PartialFace, PartialObject}, services::Service, storage::Handle, }; @@ -10,24 +9,12 @@ use super::Reverse; impl Reverse for Handle { fn reverse(self, objects: &mut Service) -> Self { - let mut cache = FullToPartialCache::default(); - - let exterior = Partial::from_full( - self.exterior().clone().reverse(objects), - &mut cache, - ); + let exterior = self.exterior().clone().reverse(objects); let interiors = self .interiors() - .map(|cycle| { - Partial::from_full(cycle.clone().reverse(objects), &mut cache) - }) + .map(|cycle| cycle.clone().reverse(objects)) .collect::>(); - let face = PartialFace { - exterior, - interiors, - color: Some(self.color()), - }; - face.build(objects).insert(objects) + Face::new(exterior, interiors, self.color()).insert(objects) } } From 5914f2b11c92e0ce01b6a9115495ddeda1a74677 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:44:04 +0100 Subject: [PATCH 02/21] Add reference to `Surface` to `PartialFace` --- crates/fj-kernel/src/partial/objects/face.rs | 6 +++++- crates/fj-operations/src/difference_2d.rs | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/objects/face.rs b/crates/fj-kernel/src/partial/objects/face.rs index b39e04ff0..e13db7a7a 100644 --- a/crates/fj-kernel/src/partial/objects/face.rs +++ b/crates/fj-kernel/src/partial/objects/face.rs @@ -1,7 +1,7 @@ use fj_interop::mesh::Color; use crate::{ - objects::{Cycle, Face, Objects}, + objects::{Cycle, Face, Objects, Surface}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, }; @@ -9,6 +9,9 @@ use crate::{ /// A partial [`Face`] #[derive(Clone, Debug, Default)] pub struct PartialFace { + /// The surface that the face is defined in + pub surface: Partial, + /// The cycle that bounds the face on the outside pub exterior: Partial, @@ -26,6 +29,7 @@ impl PartialObject for PartialFace { fn from_full(face: &Self::Full, cache: &mut FullToPartialCache) -> Self { Self { + surface: Partial::from_full(face.surface().clone(), cache), exterior: Partial::from_full(face.exterior().clone(), cache), interiors: face .interiors() diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 898b07e21..6f9fa3737 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -82,6 +82,7 @@ impl Shape for fj::Difference2d { ); let face = PartialFace { + surface: Partial::from(surface.clone()), exterior: Partial::from(exterior), interiors, color: Some(Color(self.color())), From e0721fefa8e95dde48550196f9d84431d60509dc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:49:11 +0100 Subject: [PATCH 03/21] Prepare code for follow-on change --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 70a4c1cf1..86e18cbe0 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -33,10 +33,13 @@ impl Sweep for (Handle, &Surface, Color) { // A face (and everything in it) is defined on a surface. A surface can // be created by sweeping a curve, so let's sweep the curve of the edge // we're sweeping. - face.exterior.write().surface = Partial::from( - (edge.curve().clone(), surface) - .sweep_with_cache(path, cache, objects), - ); + { + let surface = Partial::from( + (edge.curve().clone(), surface) + .sweep_with_cache(path, cache, objects), + ); + face.exterior.write().surface = surface; + } // Now we're ready to create the edges. let mut edge_bottom = face.exterior.write().add_half_edge(); From 0e7cd4f08be528798c771fcb7fc53b834f00bca0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 15:57:46 +0100 Subject: [PATCH 04/21] Prepare code for follow-on change --- crates/fj-kernel/src/algorithms/sweep/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 34a7cfb07..e520bf42f 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -53,12 +53,12 @@ impl Sweep for Handle { }; faces.push(bottom_face.clone()); + let top_surface = + bottom_face.surface().clone().translate(path, objects); let mut top_face = PartialFace { color: Some(self.color()), ..PartialFace::default() }; - let top_surface = - bottom_face.surface().clone().translate(path, objects); for (i, cycle) in bottom_face.all_cycles().cloned().enumerate() { let cycle = cycle.reverse(objects); From d207515626b7e958d7e77bdd79cc7c2d22d3c326 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:00:00 +0100 Subject: [PATCH 05/21] Group related code in block --- crates/fj-kernel/src/algorithms/sweep/face.rs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index e520bf42f..a9d5fbd26 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -241,18 +241,22 @@ mod tests { .insert(&mut services.objects) .sweep(DOWN, &mut services.objects); - let mut bottom = PartialFace::default(); - bottom.exterior.write().surface = Partial::from( - surface.clone().translate(DOWN, &mut services.objects), - ); - bottom - .exterior - .write() - .update_as_polygon_from_points(TRIANGLE); - let bottom = bottom - .build(&mut services.objects) - .insert(&mut services.objects) - .reverse(&mut services.objects); + let bottom = { + let mut bottom = PartialFace::default(); + + bottom.exterior.write().surface = Partial::from( + surface.clone().translate(DOWN, &mut services.objects), + ); + bottom + .exterior + .write() + .update_as_polygon_from_points(TRIANGLE); + + bottom + .build(&mut services.objects) + .insert(&mut services.objects) + .reverse(&mut services.objects) + }; let mut top = PartialFace::default(); top.exterior.write().surface = Partial::from(surface.clone()); top.exterior.write().update_as_polygon_from_points(TRIANGLE); From 05c78fd89e9ee6f59564c6f5b479899b4ecfff7f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:01:36 +0100 Subject: [PATCH 06/21] Group related code into block --- crates/fj-kernel/src/algorithms/sweep/face.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index a9d5fbd26..06884ff08 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -257,12 +257,15 @@ mod tests { .insert(&mut services.objects) .reverse(&mut services.objects) }; - let mut top = PartialFace::default(); - top.exterior.write().surface = Partial::from(surface.clone()); - top.exterior.write().update_as_polygon_from_points(TRIANGLE); - let top = top - .build(&mut services.objects) - .insert(&mut services.objects); + let top = { + let mut top = PartialFace::default(); + + top.exterior.write().surface = Partial::from(surface.clone()); + top.exterior.write().update_as_polygon_from_points(TRIANGLE); + + top.build(&mut services.objects) + .insert(&mut services.objects) + }; assert!(solid.find_face(&bottom).is_some()); assert!(solid.find_face(&top).is_some()); From 4dd05dc74313f0c0d1d45c4663c4c836219a685c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:02:31 +0100 Subject: [PATCH 07/21] Extract value into dedicated variable --- crates/fj-kernel/src/algorithms/sweep/face.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 06884ff08..952660f9b 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -242,11 +242,12 @@ mod tests { .sweep(DOWN, &mut services.objects); let bottom = { + let surface = + surface.clone().translate(DOWN, &mut services.objects); + let mut bottom = PartialFace::default(); - bottom.exterior.write().surface = Partial::from( - surface.clone().translate(DOWN, &mut services.objects), - ); + bottom.exterior.write().surface = Partial::from(surface); bottom .exterior .write() From 178c01466f4262aabe2dc507798c1d36d7b7926b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:06:27 +0100 Subject: [PATCH 08/21] Group related code into block --- crates/fj-kernel/src/algorithms/sweep/face.rs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 952660f9b..a4a645075 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -175,16 +175,20 @@ mod tests { .insert(&mut services.objects) .sweep(UP, &mut services.objects); - let mut bottom = PartialFace::default(); - bottom.exterior.write().surface = Partial::from(surface.clone()); - bottom - .exterior - .write() - .update_as_polygon_from_points(TRIANGLE); - let bottom = bottom - .build(&mut services.objects) - .insert(&mut services.objects) - .reverse(&mut services.objects); + let bottom = { + let mut bottom = PartialFace::default(); + + bottom.exterior.write().surface = Partial::from(surface.clone()); + bottom + .exterior + .write() + .update_as_polygon_from_points(TRIANGLE); + + bottom + .build(&mut services.objects) + .insert(&mut services.objects) + .reverse(&mut services.objects) + }; let mut top = PartialFace::default(); top.exterior.write().surface = Partial::from(surface.clone().translate(UP, &mut services.objects)); From 01f2172f0afe1dd4c88c9ab75f0ef9522acb82fb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:07:08 +0100 Subject: [PATCH 09/21] Group related code into block --- crates/fj-kernel/src/algorithms/sweep/face.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index a4a645075..871119e5c 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -189,13 +189,17 @@ mod tests { .insert(&mut services.objects) .reverse(&mut services.objects) }; - let mut top = PartialFace::default(); - top.exterior.write().surface = - Partial::from(surface.clone().translate(UP, &mut services.objects)); - top.exterior.write().update_as_polygon_from_points(TRIANGLE); - let top = top - .build(&mut services.objects) - .insert(&mut services.objects); + let top = { + let mut top = PartialFace::default(); + + top.exterior.write().surface = Partial::from( + surface.clone().translate(UP, &mut services.objects), + ); + top.exterior.write().update_as_polygon_from_points(TRIANGLE); + + top.build(&mut services.objects) + .insert(&mut services.objects) + }; assert!(solid.find_face(&bottom).is_some()); assert!(solid.find_face(&top).is_some()); From 69c12b9fced2d2ff8131cafd6bbc1838068d1239 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:09:35 +0100 Subject: [PATCH 10/21] Extract value to dedicated variable --- crates/fj-kernel/src/algorithms/sweep/face.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 871119e5c..f81e9bd39 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -190,11 +190,11 @@ mod tests { .reverse(&mut services.objects) }; let top = { + let surface = surface.clone().translate(UP, &mut services.objects); + let mut top = PartialFace::default(); - top.exterior.write().surface = Partial::from( - surface.clone().translate(UP, &mut services.objects), - ); + top.exterior.write().surface = Partial::from(surface); top.exterior.write().update_as_polygon_from_points(TRIANGLE); top.build(&mut services.objects) From 32839b2dfdb62e9211140fb286ea0f59444b2443 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:11:14 +0100 Subject: [PATCH 11/21] Extract value to dedicated variable --- crates/fj-kernel/src/algorithms/triangulate/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index ec841dd1a..fc931d44e 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -96,9 +96,10 @@ mod tests { let c = [2., 2.]; let d = [0., 1.]; + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior .write() .update_as_polygon_from_points([a, b, c, d]); From d379cab85218fd3bf616880c12d5321a5fec9fd1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:12:34 +0100 Subject: [PATCH 12/21] Extract values into dedicated variables --- .../src/algorithms/intersect/ray_face.rs | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 336ffb5be..4cd0cccd1 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -163,9 +163,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.yz_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.yz_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -186,9 +187,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.yz_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.yz_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -212,9 +214,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.yz_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.yz_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -235,9 +238,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.yz_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.yz_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -269,9 +273,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.yz_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.yz_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -303,9 +308,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], @@ -328,9 +334,10 @@ mod tests { let ray = HorizontalRayToTheRight::from([0., 0., 0.]); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], [1., -1.], From 3038a75c8e54560478d8371e5b050a64ca7097b2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:20:18 +0100 Subject: [PATCH 13/21] Extract values into dedicated variables --- .../src/algorithms/intersect/face_point.rs | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index 5fcdcb72a..526a77b0e 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -148,9 +148,10 @@ mod tests { fn point_is_outside_face() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [1., 1.], @@ -169,9 +170,10 @@ mod tests { fn ray_hits_vertex_while_passing_outside() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -193,9 +195,10 @@ mod tests { fn ray_hits_vertex_at_cycle_seam() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [4., 2.], [0., 4.], @@ -217,9 +220,10 @@ mod tests { fn ray_hits_vertex_while_staying_inside() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -242,9 +246,10 @@ mod tests { fn ray_hits_parallel_edge_and_leaves_face_at_vertex() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -267,9 +272,10 @@ mod tests { fn ray_hits_parallel_edge_and_does_not_leave_face_there() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 1.], @@ -293,9 +299,10 @@ mod tests { fn point_is_coincident_with_edge() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [2., 0.], @@ -326,9 +333,10 @@ mod tests { fn point_is_coincident_with_vertex() { let mut services = Services::new(); + let surface = Partial::from(services.objects.surfaces.xy_plane()); + let mut face = PartialFace::default(); - face.exterior.write().surface = - Partial::from(services.objects.surfaces.xy_plane()); + face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], [1., 0.], From 1a2962d4c4007567ba0c1e245722f8fc452b2dba Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:28:34 +0100 Subject: [PATCH 14/21] Reference surface in `PartialFace` --- .../src/algorithms/intersect/curve_face.rs | 5 ++- .../src/algorithms/intersect/face_face.rs | 10 ++++- .../src/algorithms/intersect/face_point.rs | 40 +++++++++++++++---- .../src/algorithms/intersect/ray_face.rs | 35 ++++++++++++---- crates/fj-kernel/src/algorithms/sweep/edge.rs | 4 +- crates/fj-kernel/src/algorithms/sweep/face.rs | 23 +++++++++-- .../src/algorithms/triangulate/mod.rs | 15 +++++-- crates/fj-kernel/src/validate/face.rs | 10 ++++- crates/fj-operations/src/sketch.rs | 6 ++- 9 files changed, 118 insertions(+), 30 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs index 5e5048d89..de42b5e60 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_face.rs @@ -183,7 +183,10 @@ mod tests { ]; let face = { - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior .write() diff --git a/crates/fj-kernel/src/algorithms/intersect/face_face.rs b/crates/fj-kernel/src/algorithms/intersect/face_face.rs index fcf233da8..904f62630 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_face.rs @@ -93,7 +93,10 @@ mod tests { services.objects.surfaces.xz_plane(), ] .map(|surface| { - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points(points); @@ -122,7 +125,10 @@ mod tests { services.objects.surfaces.xz_plane(), ]; let [a, b] = surfaces.clone().map(|surface| { - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points(points); diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index 526a77b0e..a44ca8c07 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -150,7 +150,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -172,7 +175,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -197,7 +203,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [4., 2.], @@ -222,7 +231,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -248,7 +260,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -274,7 +289,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -301,7 +319,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -335,7 +356,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [0., 0.], diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 4cd0cccd1..44269fbdf 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -165,7 +165,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], @@ -189,7 +192,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], @@ -216,7 +222,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], @@ -240,7 +249,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], @@ -275,7 +287,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.yz_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], @@ -310,7 +325,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], @@ -336,7 +354,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior.write().update_as_polygon_from_points([ [-1., -1.], diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 86e18cbe0..ca25066c7 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -38,6 +38,7 @@ impl Sweep for (Handle, &Surface, Color) { (edge.curve().clone(), surface) .sweep_with_cache(path, cache, objects), ); + face.surface = surface.clone(); face.exterior.write().surface = surface; } @@ -269,7 +270,7 @@ mod tests { }; let mut cycle = PartialCycle { - surface: Partial::from(surface), + surface: Partial::from(surface.clone()), ..Default::default() }; cycle.half_edges.extend( @@ -277,6 +278,7 @@ mod tests { ); let face = PartialFace { + surface: Partial::from(surface), exterior: Partial::from_partial(cycle), ..Default::default() }; diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index f81e9bd39..e72442b77 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -56,6 +56,7 @@ impl Sweep for Handle { let top_surface = bottom_face.surface().clone().translate(path, objects); let mut top_face = PartialFace { + surface: Partial::from(top_surface.clone()), color: Some(self.color()), ..PartialFace::default() }; @@ -161,6 +162,7 @@ mod tests { let mut sketch = PartialSketch::default(); let mut face = sketch.add_face(); + face.write().surface = Partial::from(surface.clone()); face.write().exterior.write().surface = Partial::from(surface.clone()); face.write() @@ -176,7 +178,10 @@ mod tests { .sweep(UP, &mut services.objects); let bottom = { - let mut bottom = PartialFace::default(); + let mut bottom = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; bottom.exterior.write().surface = Partial::from(surface.clone()); bottom @@ -192,7 +197,10 @@ mod tests { let top = { let surface = surface.clone().translate(UP, &mut services.objects); - let mut top = PartialFace::default(); + let mut top = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; top.exterior.write().surface = Partial::from(surface); top.exterior.write().update_as_polygon_from_points(TRIANGLE); @@ -235,6 +243,7 @@ mod tests { let mut sketch = PartialSketch::default(); let mut face = sketch.add_face(); + face.write().surface = Partial::from(surface.clone()); face.write().exterior.write().surface = Partial::from(surface.clone()); face.write() @@ -253,7 +262,10 @@ mod tests { let surface = surface.clone().translate(DOWN, &mut services.objects); - let mut bottom = PartialFace::default(); + let mut bottom = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; bottom.exterior.write().surface = Partial::from(surface); bottom @@ -267,7 +279,10 @@ mod tests { .reverse(&mut services.objects) }; let top = { - let mut top = PartialFace::default(); + let mut top = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; top.exterior.write().surface = Partial::from(surface.clone()); top.exterior.write().update_as_polygon_from_points(TRIANGLE); diff --git a/crates/fj-kernel/src/algorithms/triangulate/mod.rs b/crates/fj-kernel/src/algorithms/triangulate/mod.rs index fc931d44e..87d29de38 100644 --- a/crates/fj-kernel/src/algorithms/triangulate/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulate/mod.rs @@ -98,7 +98,10 @@ mod tests { let surface = Partial::from(services.objects.surfaces.xy_plane()); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: surface.clone(), + ..Default::default() + }; face.exterior.write().surface = surface; face.exterior .write() @@ -137,7 +140,10 @@ mod tests { let h = [3., 1.]; let surface = services.objects.surfaces.xy_plane(); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; face.exterior.write().surface = Partial::from(surface.clone()); face.exterior .write() @@ -201,7 +207,10 @@ mod tests { let e = [0.0, 1.0]; let surface = services.objects.surfaces.xy_plane(); - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; face.exterior.write().surface = Partial::from(surface.clone()); face.exterior .write() diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 2fa6a0022..6bbea78c3 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -115,7 +115,10 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let valid = { - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points([ [0., 0.], @@ -157,7 +160,10 @@ mod tests { let surface = services.objects.surfaces.xy_plane(); let valid = { - let mut face = PartialFace::default(); + let mut face = PartialFace { + surface: Partial::from(surface.clone()), + ..Default::default() + }; face.exterior.write().surface = Partial::from(surface); face.exterior.write().update_as_polygon_from_points([ [0., 0.], diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index f9e402aeb..83dec0e28 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -37,7 +37,7 @@ impl Shape for fj::Sketch { }; let exterior = { let mut cycle = PartialCycle { - surface, + surface: surface.clone(), ..Default::default() }; cycle.half_edges.push(half_edge); @@ -45,6 +45,7 @@ impl Shape for fj::Sketch { }; PartialFace { + surface, exterior, color: Some(Color(self.color())), ..Default::default() @@ -59,7 +60,7 @@ impl Shape for fj::Sketch { let exterior = { let mut cycle = PartialCycle { - surface: Partial::from(surface), + surface: Partial::from(surface.clone()), ..Default::default() }; let mut line_segments = vec![]; @@ -98,6 +99,7 @@ impl Shape for fj::Sketch { }; PartialFace { + surface: Partial::from(surface), exterior, color: Some(Color(self.color())), ..Default::default() From 7b16289755b48c8b11b23bf32f979f9e1e5f7977 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:29:22 +0100 Subject: [PATCH 15/21] Reference surface in `Face` --- crates/fj-kernel/src/algorithms/reverse/face.rs | 3 ++- crates/fj-kernel/src/algorithms/transform/face.rs | 6 +++++- crates/fj-kernel/src/objects/full/face.rs | 3 +++ crates/fj-kernel/src/partial/objects/face.rs | 3 ++- crates/fj-kernel/src/validate/face.rs | 14 ++++++++++++-- 5 files changed, 24 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/face.rs b/crates/fj-kernel/src/algorithms/reverse/face.rs index 2d569c3e3..3330ee822 100644 --- a/crates/fj-kernel/src/algorithms/reverse/face.rs +++ b/crates/fj-kernel/src/algorithms/reverse/face.rs @@ -15,6 +15,7 @@ impl Reverse for Handle { .map(|cycle| cycle.clone().reverse(objects)) .collect::>(); - Face::new(exterior, interiors, self.color()).insert(objects) + Face::new(self.surface().clone(), exterior, interiors, self.color()) + .insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/transform/face.rs b/crates/fj-kernel/src/algorithms/transform/face.rs index 86a9f2b8e..fb4a58906 100644 --- a/crates/fj-kernel/src/algorithms/transform/face.rs +++ b/crates/fj-kernel/src/algorithms/transform/face.rs @@ -17,6 +17,10 @@ impl TransformObject for Face { // Color does not need to be transformed. let color = self.color(); + let surface = self + .surface() + .clone() + .transform_with_cache(transform, objects, cache); let exterior = self .exterior() .clone() @@ -25,7 +29,7 @@ impl TransformObject for Face { interior.transform_with_cache(transform, objects, cache) }); - Self::new(exterior, interiors, color) + Self::new(surface, exterior, interiors, color) } } diff --git a/crates/fj-kernel/src/objects/full/face.rs b/crates/fj-kernel/src/objects/full/face.rs index 95e9d6b87..fdf5b5647 100644 --- a/crates/fj-kernel/src/objects/full/face.rs +++ b/crates/fj-kernel/src/objects/full/face.rs @@ -34,6 +34,7 @@ use crate::{ /// [`Shell`]: crate::objects::Shell #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Face { + surface: Handle, exterior: Handle, interiors: Vec>, color: Color, @@ -42,6 +43,7 @@ pub struct Face { impl Face { /// Construct an instance of `Face` pub fn new( + surface: Handle, exterior: Handle, interiors: impl IntoIterator>, color: Color, @@ -49,6 +51,7 @@ impl Face { let interiors = interiors.into_iter().collect(); Self { + surface, exterior, interiors, color, diff --git a/crates/fj-kernel/src/partial/objects/face.rs b/crates/fj-kernel/src/partial/objects/face.rs index e13db7a7a..2d4d5d0c4 100644 --- a/crates/fj-kernel/src/partial/objects/face.rs +++ b/crates/fj-kernel/src/partial/objects/face.rs @@ -40,11 +40,12 @@ impl PartialObject for PartialFace { } fn build(self, objects: &mut Service) -> Self::Full { + let surface = self.surface.build(objects); let exterior = self.exterior.build(objects); let interiors = self.interiors.into_iter().map(|cycle| cycle.build(objects)); let color = self.color.unwrap_or_default(); - Face::new(exterior, interiors, color) + Face::new(surface, exterior, interiors, color) } } diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 6bbea78c3..ff500cffa 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -144,7 +144,12 @@ mod tests { .insert(&mut services.objects); let interiors = [cycle]; - Face::new(valid.exterior().clone(), interiors, valid.color()) + Face::new( + valid.surface().clone(), + valid.exterior().clone(), + interiors, + valid.color(), + ) }; valid.validate_and_return_first_error()?; @@ -184,7 +189,12 @@ mod tests { .map(|cycle| cycle.reverse(&mut services.objects)) .collect::>(); - Face::new(valid.exterior().clone(), interiors, valid.color()) + Face::new( + valid.surface().clone(), + valid.exterior().clone(), + interiors, + valid.color(), + ) }; valid.validate_and_return_first_error()?; From 9a75180d5b0bb3fecd54d5d7e140e71aab9cc291 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:30:08 +0100 Subject: [PATCH 16/21] Simplify `Face::surface` --- crates/fj-kernel/src/objects/full/face.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/objects/full/face.rs b/crates/fj-kernel/src/objects/full/face.rs index fdf5b5647..f43c318fe 100644 --- a/crates/fj-kernel/src/objects/full/face.rs +++ b/crates/fj-kernel/src/objects/full/face.rs @@ -60,7 +60,7 @@ impl Face { /// Access the surface of the face pub fn surface(&self) -> &Handle { - self.exterior().surface() + &self.surface } /// Access the cycle that bounds the face on the outside From fe43248d94ff18890fc9b9758b3cea5689afab8e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:31:53 +0100 Subject: [PATCH 17/21] Prepare code for follow-on change --- crates/fj-kernel/src/algorithms/approx/cycle.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/cycle.rs b/crates/fj-kernel/src/algorithms/approx/cycle.rs index b819de3e3..29c2d0c54 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycle.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycle.rs @@ -21,12 +21,13 @@ impl Approx for &Cycle { tolerance: impl Into, cache: &mut Self::Cache, ) -> Self::Approximation { + let cycle = self; let tolerance = tolerance.into(); - let half_edges = self + let half_edges = cycle .half_edges() .map(|half_edge| { - (half_edge, self.surface().deref()) + (half_edge, cycle.surface().deref()) .approx_with_cache(tolerance, cache) }) .collect(); From fda058ccb4417adc598520f37ed202f1a22b6c04 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:33:54 +0100 Subject: [PATCH 18/21] Avoid use of `Cycle::surface` --- crates/fj-kernel/src/algorithms/approx/cycle.rs | 11 ++++------- crates/fj-kernel/src/algorithms/approx/face.rs | 8 +++++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/cycle.rs b/crates/fj-kernel/src/algorithms/approx/cycle.rs index 29c2d0c54..02c404311 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycle.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycle.rs @@ -2,17 +2,15 @@ //! //! See [`CycleApprox`]. -use std::ops::Deref; - use fj_math::Segment; -use crate::objects::Cycle; +use crate::objects::{Cycle, Surface}; use super::{ curve::CurveCache, edge::HalfEdgeApprox, Approx, ApproxPoint, Tolerance, }; -impl Approx for &Cycle { +impl Approx for (&Cycle, &Surface) { type Approximation = CycleApprox; type Cache = CurveCache; @@ -21,14 +19,13 @@ impl Approx for &Cycle { tolerance: impl Into, cache: &mut Self::Cache, ) -> Self::Approximation { - let cycle = self; + let (cycle, surface) = self; let tolerance = tolerance.into(); let half_edges = cycle .half_edges() .map(|half_edge| { - (half_edge, cycle.surface().deref()) - .approx_with_cache(tolerance, cache) + (half_edge, surface).approx_with_cache(tolerance, cache) }) .collect(); diff --git a/crates/fj-kernel/src/algorithms/approx/face.rs b/crates/fj-kernel/src/algorithms/approx/face.rs index 6ee5c7080..aa99ea0a9 100644 --- a/crates/fj-kernel/src/algorithms/approx/face.rs +++ b/crates/fj-kernel/src/algorithms/approx/face.rs @@ -2,7 +2,7 @@ //! //! See [`FaceApprox`]. -use std::collections::BTreeSet; +use std::{collections::BTreeSet, ops::Deref}; use fj_interop::mesh::Color; @@ -87,11 +87,13 @@ impl Approx for &Face { // would need to provide its own approximation, as the edges that bound // it have nothing to do with its curvature. - let exterior = self.exterior().approx_with_cache(tolerance, cache); + let exterior = (self.exterior().deref(), self.surface().deref()) + .approx_with_cache(tolerance, cache); let mut interiors = BTreeSet::new(); for cycle in self.interiors() { - let cycle = cycle.approx_with_cache(tolerance, cache); + let cycle = (cycle.deref(), self.surface().deref()) + .approx_with_cache(tolerance, cache); interiors.insert(cycle); } From 8f8e20bf97dc2c4777d00c968710b9b1120076ed Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:37:06 +0100 Subject: [PATCH 19/21] Remove unused code --- crates/fj-kernel/src/builder/cycle.rs | 81 --------------------------- 1 file changed, 81 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 3673b0ee8..906a365df 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -1,10 +1,6 @@ -use std::collections::VecDeque; - -use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - builder::SurfaceBuilder, objects::HalfEdge, partial::{Partial, PartialCycle}, }; @@ -64,35 +60,6 @@ pub trait CycleBuilder { /// Will update each half-edge in the cycle to be a line segment. fn update_as_polygon(&mut self); - /// Update cycle as a triangle, from global (3D) points - /// - /// Uses the three points to infer a plane that is used as the surface. - /// - /// # Implementation Note - /// - /// This method is probably just temporary, and will be generalized into a - /// "update as polygon from global points" method sooner or later. For now, - /// I didn't want to deal with the question of how to infer the surface, and - /// how to handle points that don't fit that surface. - fn update_as_triangle_from_global_points( - &mut self, - points: [impl Into>; 3], - ) -> [Partial; 3]; - - /// Connect the cycle to the provided half-edges - /// - /// Assumes that the provided half-edges, once translated into local - /// equivalents of this cycle, will not form a cycle themselves. - /// - /// Returns the local equivalents of the provided half-edges and, as the - /// last entry, an additional half-edge that closes the cycle. - fn connect_to_open_edges( - &mut self, - edges: O, - ) -> O::SizePlusOne> - where - O: ObjectArgument>; - /// Connect the cycles to the provided half-edges /// /// Assumes that the provided half-edges, once translated into local @@ -204,54 +171,6 @@ impl CycleBuilder for PartialCycle { } } - fn update_as_triangle_from_global_points( - &mut self, - points_global: [impl Into>; 3], - ) -> [Partial; 3] { - let points_global = points_global.map(Into::into); - - let (points_surface, _) = self - .surface - .write() - .update_as_plane_from_points(points_global); - - let half_edges = self.update_as_polygon_from_points(points_surface); - - for (mut half_edge, point) in half_edges.clone().zip_ext(points_global) - { - let [vertex, _] = &mut half_edge.write().vertices; - vertex.1.write().global_form.write().position = Some(point); - } - - half_edges - } - - fn connect_to_open_edges( - &mut self, - edges: O, - ) -> O::SizePlusOne> - where - O: ObjectArgument>, - { - // We need to create the additional half-edge last, but at the same time - // need to provide it to the `map_plus_one` method first. Really no - // choice but to create them all in one go, as we do here. - let mut half_edges = VecDeque::new(); - for _ in 0..edges.num_objects() { - half_edges.push_back(self.add_half_edge()); - } - let additional_half_edge = self.add_half_edge(); - - edges.map_plus_one(additional_half_edge, |other| { - let mut this = half_edges.pop_front().expect( - "Pushed correct number of half-edges; should be able to pop", - ); - this.write() - .update_from_other_edge(&other, &self.surface.read().geometry); - this - }) - } - fn connect_to_closed_edges( &mut self, edges: O, From 0817dd3c9a6b702353da775afd6dc908de2bf943 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:43:08 +0100 Subject: [PATCH 20/21] Avoid use of `PartialCycle`'s `geometry` field --- crates/fj-kernel/src/algorithms/sweep/face.rs | 4 +++- crates/fj-kernel/src/builder/cycle.rs | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index e72442b77..51731cbc5 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -85,7 +85,9 @@ impl Sweep for Handle { top_cycle.write().surface = Partial::from(top_surface.clone()); - top_cycle.write().connect_to_closed_edges(top_edges); + top_cycle + .write() + .connect_to_closed_edges(top_edges, &top_surface.geometry()); for half_edge in &mut top_cycle.write().half_edges { for (_, surface_vertex) in &mut half_edge.write().vertices { diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 906a365df..4bf5dd72c 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -1,6 +1,7 @@ use fj_math::Point; use crate::{ + geometry::surface::SurfaceGeometry, objects::HalfEdge, partial::{Partial, PartialCycle}, }; @@ -69,6 +70,7 @@ pub trait CycleBuilder { fn connect_to_closed_edges( &mut self, edges: O, + surface: &SurfaceGeometry, ) -> O::SameSize> where O: ObjectArgument>; @@ -174,14 +176,14 @@ impl CycleBuilder for PartialCycle { fn connect_to_closed_edges( &mut self, edges: O, + surface: &SurfaceGeometry, ) -> O::SameSize> where O: ObjectArgument>, { edges.map(|other| { let mut this = self.add_half_edge(); - this.write() - .update_from_other_edge(&other, &self.surface.read().geometry); + this.write().update_from_other_edge(&other, &Some(*surface)); this }) } From a7365e670e35a0258fcf63e51cc940b14b50ca97 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 17 Feb 2023 16:47:59 +0100 Subject: [PATCH 21/21] Simplify builder method --- crates/fj-kernel/src/builder/cycle.rs | 2 +- crates/fj-kernel/src/builder/edge.rs | 165 +++++++++++--------------- 2 files changed, 71 insertions(+), 96 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 4bf5dd72c..c20ed6256 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -183,7 +183,7 @@ impl CycleBuilder for PartialCycle { { edges.map(|other| { let mut this = self.add_half_edge(); - this.write().update_from_other_edge(&other, &Some(*surface)); + this.write().update_from_other_edge(&other, surface); this }) } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index c4f5b7402..2807164c2 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -58,7 +58,7 @@ pub trait HalfEdgeBuilder { fn update_from_other_edge( &mut self, other: &Partial, - surface: &Option, + surface: &SurfaceGeometry, ); } @@ -222,7 +222,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_from_other_edge( &mut self, other: &Partial, - surface: &Option, + surface: &SurfaceGeometry, ) { let global_curve = other.read().curve.read().global_form.clone(); self.curve.write().global_form = global_curve.clone(); @@ -230,103 +230,78 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.curve.write().path = other.read().curve.read().path.as_ref().and_then(|path| { - match surface { - Some(surface) => { - // We have information about the other edge's surface - // available. We need to use that to interpret what the - // other edge's curve path means for our curve path. - match surface.u { - GlobalPath::Circle(circle) => { - // The other surface is curved. We're entering - // some dodgy territory here, as only some edge - // cases can be represented using our current - // curve/surface representation. - match path { - MaybeSurfacePath::Defined( - SurfacePath::Line(_), - ) - | MaybeSurfacePath::UndefinedLine => { - // We're dealing with a line on a - // rounded surface. - // - // Based on the current uses of this - // method, we can make some assumptions: - // - // 1. The line is parallel to the u-axis - // of the other surface. - // 2. The surface that *our* edge is in - // is a plane that is parallel to the - // the plane of the circle that - // defines the curvature of the other - // surface. - // - // These assumptions are necessary - // preconditions for the following code - // to work. But unfortunately, I see no - // way to check those preconditions - // here, as neither the other line nor - // our surface is necessarily defined - // yet. - // - // Handling this case anyway feels like - // a grave sin, but I don't know what - // else to do. If you tracked some - // extremely subtle and annoying bug - // back to this code, I apologize. - // - // I hope that I'll come up with a - // better curve/surface representation - // before this becomes a problem. - Some( - MaybeSurfacePath::UndefinedCircle { - radius: circle.radius(), - }, - ) - } - _ => { - // The other edge is a line segment in a - // curved surface. No idea how to deal - // with this. - todo!( - "Can't connect edge to circle on \ - curved surface" - ) - } - } + // We have information about the other edge's surface available. + // We need to use that to interpret what the other edge's curve + // path means for our curve path. + match surface.u { + GlobalPath::Circle(circle) => { + // The other surface is curved. We're entering some + // dodgy territory here, as only some edge cases can be + // represented using our current curve/surface + // representation. + match path { + MaybeSurfacePath::Defined(SurfacePath::Line(_)) + | MaybeSurfacePath::UndefinedLine => { + // We're dealing with a line on a rounded + // surface. + // + // Based on the current uses of this method, we + // can make some assumptions: + // + // 1. The line is parallel to the u-axis of the + // other surface. + // 2. The surface that *our* edge is in is a + // plane that is parallel to the the plane of + // the circle that defines the curvature of + // the other surface. + // + // These assumptions are necessary preconditions + // for the following code to work. But + // unfortunately, I see no way to check those + // preconditions here, as neither the other line + // nor our surface is necessarily defined yet. + // + // Handling this case anyway feels like a grave + // sin, but I don't know what else to do. If you + // tracked some extremely subtle and annoying + // bug back to this code, I apologize. + // + // I hope that I'll come up with a better curve/ + // surface representation before this becomes a + // problem. + Some(MaybeSurfacePath::UndefinedCircle { + radius: circle.radius(), + }) } - GlobalPath::Line(_) => { - // The other edge is defined on a plane. - match path { - MaybeSurfacePath::Defined( - SurfacePath::Line(_), - ) - | MaybeSurfacePath::UndefinedLine => { - // The other edge is a line segment on - // a plane. That means our edge must be - // a line segment too. - Some(MaybeSurfacePath::UndefinedLine) - } - _ => { - // The other edge is a circle or arc on - // a plane. I'm actually not sure what - // that means for our edge. We might be - // able to represent it somehow, but - // let's leave that as an exercise for - // later. - todo!( - "Can't connect edge to circle on \ - plane" - ) - } - } + _ => { + // The other edge is a line segment in a curved + // surface. No idea how to deal with this. + todo!( + "Can't connect edge to circle on curved \ + surface" + ) } } } - None => { - // We know nothing about the surface the other edge is - // on. This means we can't infer anything about our - // curve from the other curve. - None + GlobalPath::Line(_) => { + // The other edge is defined on a plane. + match path { + MaybeSurfacePath::Defined(SurfacePath::Line(_)) + | MaybeSurfacePath::UndefinedLine => { + // The other edge is a line segment on a plane. + // That means our edge must be a line segment + // too. + Some(MaybeSurfacePath::UndefinedLine) + } + _ => { + // The other edge is a circle or arc on a plane. + // I'm actually not sure what that means for our + // edge. We might be able to represent it + // somehow, but let's leave that as an exercise + // for later. + todo!("Can't connect edge to circle on plane") + } + } } } });