From 62a931a00b79551825f0121e63266f886c0ce623 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 16 Feb 2023 13:17:19 +0100 Subject: [PATCH 1/7] Avoid indexing into array This is potentially panicky, while the destructuring will fail at compile-time. --- crates/fj-kernel/src/builder/edge.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 87c4fb2fb..229bbaab6 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -100,7 +100,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { if angle_rad <= -Scalar::TAU || angle_rad >= Scalar::TAU { panic!("arc angle must be in the range (-2pi, 2pi) radians"); } - let points_surface = self.vertices.each_ref_ext().map(|vertex| { + let [start, end] = self.vertices.each_ref_ext().map(|vertex| { vertex .1 .read() @@ -108,11 +108,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { .expect("Can't infer arc without surface position") }); - let arc = fj_math::Arc::from_endpoints_and_angle( - points_surface[0], - points_surface[1], - angle_rad, - ); + let arc = fj_math::Arc::from_endpoints_and_angle(start, end, angle_rad); let path = self .curve From 93ce06276fda56e90cc8d268a4bc510e53c29ef9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 25 Jan 2023 13:54:14 +0100 Subject: [PATCH 2/7] Refactor --- 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 32f209174..63d2caab8 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -62,11 +62,11 @@ impl Sweep for Handle { // Generate side faces for cycle in self.all_cycles() { - for half_edge in cycle.half_edges() { + for half_edge in cycle.half_edges().cloned() { let half_edge = if is_negative_sweep { - half_edge.clone().reverse(objects) + half_edge.reverse(objects) } else { - half_edge.clone() + half_edge }; let face = (half_edge, self.color()) From b4f21fb677ad872d3f951faead97a358570cc78e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 25 Jan 2023 13:54:54 +0100 Subject: [PATCH 3/7] Avoid using `Reverse` impl of `HalfEdge` This implementation will be removed as part of the ongoing `HalfEdge` cleanup. --- crates/fj-kernel/src/algorithms/sweep/face.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 63d2caab8..f42cbb2f7 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -61,14 +61,14 @@ impl Sweep for Handle { faces.push(top_face); // Generate side faces - for cycle in self.all_cycles() { - for half_edge in cycle.half_edges().cloned() { - let half_edge = if is_negative_sweep { - half_edge.reverse(objects) - } else { - half_edge - }; + for cycle in self.all_cycles().cloned() { + let cycle = if is_negative_sweep { + cycle.reverse(objects) + } else { + cycle + }; + for half_edge in cycle.half_edges().cloned() { let face = (half_edge, self.color()) .sweep_with_cache(path, cache, objects); From b04638ea0ab6f1eda9b8ed3fb3a8dea502550eb9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 8 Feb 2023 12:07:37 +0100 Subject: [PATCH 4/7] Refactor --- crates/fj-kernel/src/algorithms/sweep/face.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index f42cbb2f7..7cf5188df 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -47,7 +47,7 @@ impl Sweep for Handle { self.clone().reverse(objects) } }; - faces.push(bottom_face); + faces.push(bottom_face.clone()); let top_face = { let mut face = self.clone().translate(path, objects); @@ -61,12 +61,8 @@ impl Sweep for Handle { faces.push(top_face); // Generate side faces - for cycle in self.all_cycles().cloned() { - let cycle = if is_negative_sweep { - cycle.reverse(objects) - } else { - cycle - }; + for cycle in bottom_face.all_cycles().cloned() { + let cycle = cycle.reverse(objects); for half_edge in cycle.half_edges().cloned() { let face = (half_edge, self.color()) From 819001eb0bcdfd583c67ed4dd60d68d7d3491007 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 27 Jan 2023 14:52:33 +0100 Subject: [PATCH 5/7] Return top edge from half-edge sweep This is going to be required in the face sweep code, to make sure the top of the solid is properly stitched up with the sides. --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 10 ++++++---- crates/fj-kernel/src/algorithms/sweep/face.rs | 10 +++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index c370e01c0..cb56f1fbd 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -17,7 +17,7 @@ use crate::{ use super::{Sweep, SweepCache}; impl Sweep for (Handle, Color) { - type Swept = Handle; + type Swept = (Handle, Handle); fn sweep_with_cache( self, @@ -137,7 +137,7 @@ impl Sweep for (Handle, Color) { let cycle = { let a = bottom_edge; let [d, b] = side_edges; - let c = top_edge; + let c = top_edge.clone(); let mut edges = [a, b, c, d]; @@ -167,7 +167,9 @@ impl Sweep for (Handle, Color) { color: Some(color), ..Default::default() }; - face.build(objects).insert(objects) + let face = face.build(objects).insert(objects); + + (face, top_edge) } } @@ -202,7 +204,7 @@ mod tests { .insert(&mut services.objects) }; - let face = (half_edge, Color::default()) + let (face, _) = (half_edge, Color::default()) .sweep([0., 0., 1.], &mut services.objects); let expected_face = { diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 7cf5188df..4a00335db 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -65,7 +65,7 @@ impl Sweep for Handle { let cycle = cycle.reverse(objects); for half_edge in cycle.half_edges().cloned() { - let face = (half_edge, self.color()) + let (face, _) = (half_edge, self.color()) .sweep_with_cache(path, cache, objects); faces.push(face); @@ -155,7 +155,9 @@ mod tests { .build(&mut services.objects) .insert(&mut services.objects) }; - (half_edge, Color::default()).sweep(UP, &mut services.objects) + let (face, _) = + (half_edge, Color::default()).sweep(UP, &mut services.objects); + face }); assert!(side_faces @@ -222,7 +224,9 @@ mod tests { .insert(&mut services.objects) .reverse(&mut services.objects) }; - (half_edge, Color::default()).sweep(DOWN, &mut services.objects) + let (face, _) = (half_edge, Color::default()) + .sweep(DOWN, &mut services.objects); + face }); assert!(side_faces From 6ceadc6830118a2d774e7cb2fbcbc9a9d2c4a4f8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 30 Jan 2023 14:57:38 +0100 Subject: [PATCH 6/7] Rewrite parts of sweep code The rewrite of the edge sweeping code uses the builder API, making the code more compact, and making the sweep code for `Vertex` redundant (it can be removed in a follow-up commit. The rewrite of the face sweeping code is less of a slam dunk. It adds a fair bit of complexity over the previous version, but I think this is okay, due to two factors: 1. I believe that this complexity can be reigned in later, once the core data structures have been simplified. This is an ongoing effort. 2. It fixes existing bugs. See below. The initial reason for the rewrite was the ongoing `HalfEdge` cleanup: As part of the celanup, the `Reverse` implementation for `HalfEdge` needs to be removed, and this new code no longer uses it. However, as it turned out, there is a significant side effect: The new code generates different (but still correct) curves, which are less uniform than the ones generated by the old code, which happens to trigger approximation failures that exposed existing bugs. Those bugs basically boil down to the previous sweep code generating coincident edges that don't refer to the same global edge. This was already known (#1162), but so far a proper fix was blocked on missing validation code, to ensure all instances of this bug are fixed. That validation code still needs to be written, but the approximation failures already did a pretty good job of guiding me towards the various sources of the bug. --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 300 ++++++++---------- crates/fj-kernel/src/algorithms/sweep/face.rs | 75 ++++- 2 files changed, 198 insertions(+), 177 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index cb56f1fbd..e4319de67 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -1,14 +1,10 @@ use fj_interop::{ext::ArrayExt, mesh::Color}; -use fj_math::{Line, Scalar, Vector}; -use iter_fixed::IntoIteratorFixed; +use fj_math::{Point, Scalar, Vector}; use crate::{ - algorithms::{reverse::Reverse, transform::TransformObject}, - geometry::path::SurfacePath, + builder::{CycleBuilder, HalfEdgeBuilder}, insert::Insert, - objects::{ - Curve, Cycle, Face, GlobalEdge, HalfEdge, Objects, SurfaceVertex, - }, + objects::{Face, HalfEdge, Objects}, partial::{Partial, PartialFace, PartialObject}, services::Service, storage::Handle, @@ -28,158 +24,135 @@ impl Sweep for (Handle, Color) { let (edge, color) = self; let path = path.into(); - let surface = - edge.curve().clone().sweep_with_cache(path, cache, objects); - - // We can't use the edge we're sweeping from as the bottom edge, as that - // is not defined in the right surface. Let's create a new bottom edge, - // by swapping the surface of the original. - let bottom_edge = { - let points_curve_and_surface = edge - .boundary() - .map(|point| (point, [point.t, Scalar::ZERO])); - - let curve = { - // Please note that creating a line here is correct, even if the - // global curve is a circle. Projected into the side surface, it - // is going to be a line either way. - let path = - SurfacePath::Line(Line::from_points_with_line_coords( - points_curve_and_surface, - )); - - Curve::new( - surface.clone(), - path, - edge.curve().global_form().clone(), - ) - .insert(objects) - }; - - let boundary = { - let points_surface = points_curve_and_surface - .map(|(_, point_surface)| point_surface); - - edge.boundary() - .zip_ext(edge.surface_vertices()) - .into_iter_fixed() - .zip(points_surface) - .collect::<[_; 2]>() - .map(|((point, surface_vertex), point_surface)| { - let surface_vertex = SurfaceVertex::new( - point_surface, - surface.clone(), - surface_vertex.global_form().clone(), - ) - .insert(objects); - - (point, surface_vertex) - }) - }; - - HalfEdge::new(curve, boundary, edge.global_form().clone()) - .insert(objects) - }; - - let side_edges = bottom_edge - .boundary() - .zip_ext(bottom_edge.surface_vertices()) - .map(|(point, surface_vertex)| { - (point, surface_vertex.clone(), surface.clone()) - .sweep_with_cache(path, cache, objects) - }); - - let top_edge = { - let surface_vertices = side_edges.clone().map(|edge| { - let [_, surface_vertex] = edge.surface_vertices(); - surface_vertex.clone() - }); - - let points_curve_and_surface = bottom_edge - .boundary() - .map(|point| (point, [point.t, Scalar::ONE])); - - let curve = { - let global = bottom_edge - .curve() - .global_form() - .clone() - .translate(path, objects); - - // Please note that creating a line here is correct, even if the - // global curve is a circle. Projected into the side surface, it - // is going to be a line either way. - let path = - SurfacePath::Line(Line::from_points_with_line_coords( - points_curve_and_surface, - )); - - Curve::new(surface, path, global).insert(objects) - }; - - let global = GlobalEdge::new( - curve.global_form().clone(), - surface_vertices - .clone() - .map(|surface_vertex| surface_vertex.global_form().clone()), - ) - .insert(objects); - - let boundary = bottom_edge - .boundary() - .into_iter_fixed() - .zip(surface_vertices) - .collect::<[_; 2]>(); - - HalfEdge::new(curve, boundary, global).insert(objects) - }; - - let cycle = { - let a = bottom_edge; - let [d, b] = side_edges; - let c = top_edge.clone(); - - let mut edges = [a, b, c, d]; - - // Make sure that edges are oriented correctly. - let mut i = 0; - while i < edges.len() { - let j = (i + 1) % edges.len(); - - let [_, prev_last] = edges[i].surface_vertices(); - let [next_first, _] = edges[j].surface_vertices(); - - // Need to compare surface forms here, as the global forms might - // be coincident when sweeping circles, despite the vertices - // being different! - if prev_last.id() != next_first.id() { - edges[j] = edges[j].clone().reverse(objects); - } - - i += 1; - } - - Cycle::new(edges).insert(objects) - }; - - let face = PartialFace { - exterior: Partial::from(cycle), + // The result of sweeping an edge is a face. Let's create that. + let mut face = PartialFace { color: Some(color), ..Default::default() }; - let face = face.build(objects).insert(objects); - (face, top_edge) + // 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().sweep_with_cache(path, cache, objects), + ); + + // Now we're ready to create the edges. + let mut edge_bottom = face.exterior.write().add_half_edge(); + let mut edge_up = face.exterior.write().add_half_edge(); + let mut edge_top = face.exterior.write().add_half_edge(); + let mut edge_down = face.exterior.write().add_half_edge(); + + // Those edges aren't fully defined yet. We'll do that shortly, but + // first we have to figure a few things out. + // + // Let's start with the global vertices and edges. + let (global_vertices, global_edges) = { + let [a, b] = edge + .surface_vertices() + .map(|surface_vertex| surface_vertex.global_form().clone()); + let (edge_right, [_, c]) = + b.clone().sweep_with_cache(path, cache, objects); + let (edge_left, [_, d]) = + a.clone().sweep_with_cache(path, cache, objects); + + ( + [a, b, c, d], + [edge.global_form().clone(), edge_right, edge_left], + ) + }; + + // Next, let's figure out the surface coordinates of the edge vertices. + let surface_points = { + let [a, b] = edge.boundary(); + + [ + [a.t, Scalar::ZERO], + [b.t, Scalar::ZERO], + [b.t, Scalar::ONE], + [a.t, Scalar::ONE], + ] + .map(Point::from) + }; + + // Now, the boundaries of each edge. + let boundaries = { + let [a, b] = edge.boundary(); + let [c, d] = [0., 1.].map(|coord| Point::from([coord])); + + [[a, b], [c, d], [b, a], [d, c]] + }; + + // Armed with all of that, we can set the edge's vertices. + [ + edge_bottom.write(), + edge_up.write(), + edge_top.write(), + edge_down.write(), + ] + .zip_ext(boundaries) + .zip_ext(surface_points) + .zip_ext(global_vertices) + .map( + |(((mut half_edge, boundary), surface_point), global_vertex)| { + for ((a, _), b) in + half_edge.vertices.each_mut_ext().zip_ext(boundary) + { + *a = Some(b); + } + + // Writing to the start vertices is enough. Neighboring half- + // edges share surface vertices, so writing the start vertex of + // each half-edge writes to all vertices. + let mut vertex = half_edge.vertices[0].1.write(); + vertex.position = Some(surface_point); + vertex.global_form = Partial::from(global_vertex); + }, + ); + + // With the vertices set, we can now update the curves. + // + // Those are all line segments. For the bottom and top curve, because + // even if the original edge was a circle, it's still going to be a line + // when projected into the new surface. For the side edges, because + // we're sweeping along a straight path. + for mut edge in [ + edge_bottom.write(), + edge_up.write(), + edge_top.write(), + edge_down.write(), + ] { + edge.update_as_line_segment(); + } + + // Finally, we can make sure that all edges refer to the correct global + // edges. + [edge_bottom.write(), edge_up.write(), edge_down.write()] + .zip_ext(global_edges) + .map(|(mut half_edge, global_edge)| { + let global_edge = Partial::from(global_edge); + + half_edge.curve.write().global_form = + global_edge.read().curve.clone(); + half_edge.global_form = global_edge; + }); + + // And we're done creating the face! All that's left to do is build our + // return values. + let face = face.build(objects).insert(objects); + let edge_top = edge_top.build(objects); + (face, edge_top) } } #[cfg(test)] mod tests { use fj_interop::{ext::ArrayExt, mesh::Color}; + use fj_math::Point; use pretty_assertions::assert_eq; use crate::{ - algorithms::{reverse::Reverse, sweep::Sweep}, + algorithms::sweep::Sweep, builder::HalfEdgeBuilder, insert::Insert, partial::{ @@ -246,16 +219,16 @@ mod tests { top.curve.write().surface = surface.clone(); { - let [back, front] = top - .vertices - .each_mut_ext() - .map(|(_, surface_vertex)| surface_vertex); + let [(back, back_surface), (front, front_surface)] = + top.vertices.each_mut_ext(); - let mut back = back.write(); - back.position = Some([0., 1.].into()); - back.surface = surface.clone(); + *back = Some(Point::from([1.])); + *back_surface = side_up.vertices[1].1.clone(); - *front = side_up.vertices[1].1.clone(); + *front = Some(Point::from([0.])); + let mut front_surface = front_surface.write(); + front_surface.position = Some([0., 1.].into()); + front_surface.surface = surface.clone(); } top.infer_global_form(); @@ -263,8 +236,7 @@ mod tests { Partial::from( top.build(&mut services.objects) - .insert(&mut services.objects) - .reverse(&mut services.objects), + .insert(&mut services.objects), ) .read() .clone() @@ -273,13 +245,14 @@ mod tests { let mut side_down = PartialHalfEdge::default(); side_down.curve.write().surface = surface; - let [back, front] = side_down - .vertices - .each_mut_ext() - .map(|(_, surface_vertex)| surface_vertex); + let [(back, back_surface), (front, front_surface)] = + side_down.vertices.each_mut_ext(); - *back = bottom.vertices[0].1.clone(); - *front = top.vertices[1].1.clone(); + *back = Some(Point::from([1.])); + *front = Some(Point::from([0.])); + + *back_surface = top.vertices[1].1.clone(); + *front_surface = bottom.vertices[0].1.clone(); side_down.infer_global_form(); side_down.update_as_line_segment(); @@ -287,8 +260,7 @@ mod tests { Partial::from( side_down .build(&mut services.objects) - .insert(&mut services.objects) - .reverse(&mut services.objects), + .insert(&mut services.objects), ) .read() .clone() diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 4a00335db..db79e37f2 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -1,11 +1,13 @@ +use fj_interop::ext::ArrayExt; use fj_math::{Scalar, Vector}; use crate::{ algorithms::{reverse::Reverse, transform::TransformObject}, + builder::{CycleBuilder, FaceBuilder}, geometry::path::GlobalPath, insert::Insert, objects::{Face, Objects, Shell}, - partial::{Partial, PartialObject, PartialShell}, + partial::{Partial, PartialFace, PartialObject, PartialShell}, services::Service, storage::Handle, }; @@ -49,29 +51,76 @@ impl Sweep for Handle { }; faces.push(bottom_face.clone()); - let top_face = { - let mut face = self.clone().translate(path, objects); - - if is_negative_sweep { - face = face.reverse(objects); - }; - - face + let mut top_face = PartialFace { + color: Some(self.color()), + ..PartialFace::default() }; - faces.push(top_face); + let top_surface = + bottom_face.surface().clone().translate(path, objects); - // Generate side faces - for cycle in bottom_face.all_cycles().cloned() { + for (i, cycle) in bottom_face.all_cycles().cloned().enumerate() { let cycle = cycle.reverse(objects); + let mut top_cycle = if i == 0 { + top_face.exterior.clone() + } else { + top_face.add_interior() + }; + + let mut original_edges = Vec::new(); + let mut top_edges = Vec::new(); for half_edge in cycle.half_edges().cloned() { - let (face, _) = (half_edge, self.color()) + let (face, top_edge) = (half_edge.clone(), self.color()) .sweep_with_cache(path, cache, objects); faces.push(face); + + original_edges.push(half_edge); + top_edges.push(Partial::from(top_edge)); + } + + top_cycle.write().surface = Partial::from(top_surface.clone()); + + top_cycle.write().connect_to_closed_edges(top_edges); + + for half_edge in &mut top_cycle.write().half_edges { + for (_, surface_vertex) in &mut half_edge.write().vertices { + let mut surface_vertex = surface_vertex.write(); + let global_point = + surface_vertex.global_form.read().position; + + if surface_vertex.position.is_none() { + if let Some(global_point) = global_point { + surface_vertex.position = Some( + top_surface + .geometry() + .project_global_point(global_point), + ); + } + } + } + } + + for (bottom, top) in original_edges + .into_iter() + .zip(top_cycle.write().half_edges.iter_mut()) + { + top.write().curve.write().path = + Some(bottom.curve().path().into()); + + let boundary = bottom.boundary(); + + for ((top, _), bottom) in + top.write().vertices.each_mut_ext().zip_ext(boundary) + { + *top = Some(bottom); + } } } + let top_face = top_face.build(objects).insert(objects); + faces.push(top_face); + let faces = faces.into_iter().map(Partial::from).collect(); PartialShell { faces }.build(objects).insert(objects) } From bf75334e70e890e18fd7de7daee8443b2bc01a4e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 16 Feb 2023 14:56:30 +0100 Subject: [PATCH 7/7] Remove unused code --- .../fj-kernel/src/algorithms/sweep/vertex.rs | 168 +----------------- 1 file changed, 2 insertions(+), 166 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 558a93fdb..4bf5f662a 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -1,120 +1,14 @@ -use fj_math::{Point, Scalar, Vector}; +use fj_math::Vector; use crate::{ - geometry::path::SurfacePath, insert::Insert, - objects::{ - Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, - Surface, SurfaceVertex, - }, + objects::{GlobalCurve, GlobalEdge, GlobalVertex, Objects}, services::Service, storage::Handle, }; use super::{Sweep, SweepCache}; -impl Sweep for (Point<1>, Handle, Handle) { - type Swept = Handle; - - fn sweep_with_cache( - self, - path: impl Into>, - cache: &mut SweepCache, - objects: &mut Service, - ) -> Self::Swept { - let (point, surface_vertex, surface) = self; - let path = path.into(); - - // The result of sweeping a `Vertex` is an `Edge`. Seems - // straight-forward at first, but there are some subtleties we need to - // understand: - // - // 1. To create an `Edge`, we need the `Curve` that defines it. A - // `Curve` is defined in a `Surface`, and we're going to need that to - // create the `Curve`. Which is why this `Sweep` implementation is - // for `(Vertex, Surface)`, and not just for `Vertex`. - // 2. Please note that, while the output `Edge` has two vertices, our - // input `Vertex` is not one of them! It can't be, unless the `Curve` - // of the output `Edge` happens to be the same `Curve` that the input - // `Vertex` is defined on. That would be an edge case that probably - // can't result in anything valid, and we're going to ignore it for - // now. - // 3. This means, we have to compute everything that defines the - // output `Edge`: The `Curve`, the vertices, and the `GlobalCurve`. - // - // Before we get to that though, let's make sure that whoever called - // this didn't give us bad input. - - // So, we're supposed to create the `Edge` by sweeping a `Vertex` using - // `path`. Unless `path` is identical to the path that created the - // `Surface`, this doesn't make any sense. Let's make sure this - // requirement is met. - // - // Further, the `Curve` that was swept to create the `Surface` needs to - // be the same `Curve` that the input `Vertex` is defined on. If it's - // not, we have no way of knowing the surface coordinates of the input - // `Vertex` on the `Surface`, and we're going to need to do that further - // down. There's no way to check for that, unfortunately. - assert_eq!(path, surface.geometry().v); - - // With that out of the way, let's start by creating the `GlobalEdge`, - // as that is the most straight-forward part of this operations, and - // we're going to need it soon anyway. - let (edge_global, vertices_global) = surface_vertex - .global_form() - .clone() - .sweep_with_cache(path, cache, objects); - - // Next, let's compute the surface coordinates of the two vertices of - // the output `Edge`, as we're going to need these for the rest of this - // operation. - // - // They both share a u-coordinate, which is the t-coordinate of our - // input `Vertex`. Remember, we validated above, that the `Curve` of the - // `Surface` and the curve of the input `Vertex` are the same, so we can - // do that. - // - // Now remember what we also validated above: That `path`, which we're - // using to create the output `Edge`, also created the `Surface`, and - // thereby defined its coordinate system. That makes the v-coordinates - // straight-forward: The start of the edge is at zero, the end is at - // one. - let points_surface = [ - Point::from([point.t, Scalar::ZERO]), - Point::from([point.t, Scalar::ONE]), - ]; - - // Armed with those coordinates, creating the `Curve` of the output - // `Edge` is straight-forward. - let curve = { - let (path, _) = SurfacePath::line_from_points(points_surface); - - Curve::new(surface.clone(), path, edge_global.curve().clone()) - .insert(objects) - }; - - let vertices_surface = { - let [_, position] = points_surface; - let [_, global_form] = vertices_global; - - [ - surface_vertex, - SurfaceVertex::new(position, surface, global_form) - .insert(objects), - ] - }; - - // And now the vertices. Again, nothing wild here. - let boundary = vertices_surface.map(|surface_vertex| { - (Point::from([surface_vertex.position().v]), surface_vertex) - }); - - // And finally, creating the output `Edge` is just a matter of - // assembling the pieces we've already created. - HalfEdge::new(curve, boundary, edge_global).insert(objects) - } -} - impl Sweep for Handle { type Swept = (Handle, [Self; 2]); @@ -145,61 +39,3 @@ impl Sweep for Handle { (global_edge, vertices) } } - -#[cfg(test)] -mod tests { - use fj_math::Point; - use pretty_assertions::assert_eq; - - use crate::{ - algorithms::sweep::Sweep, - builder::{CurveBuilder, HalfEdgeBuilder}, - insert::Insert, - partial::{ - Partial, PartialCurve, PartialGlobalVertex, PartialHalfEdge, - PartialObject, PartialSurfaceVertex, - }, - services::Services, - }; - - #[test] - fn vertex_surface() { - let mut services = Services::new(); - - let surface = services.objects.surfaces.xz_plane(); - let (position, surface_vertex) = { - let mut curve = PartialCurve { - surface: Partial::from(surface.clone()), - ..Default::default() - }; - curve.update_as_u_axis(); - - let surface_form = Partial::from_partial(PartialSurfaceVertex { - position: Some(Point::from([0., 0.])), - surface: Partial::from(surface.clone()), - global_form: Partial::from_partial(PartialGlobalVertex { - position: Some(Point::from([0., 0., 0.])), - }), - }) - .build(&mut services.objects); - - (Point::from([0.]), surface_form) - }; - - let half_edge = (position, surface_vertex, surface.clone()) - .sweep([0., 0., 1.], &mut services.objects); - - let expected_half_edge = { - let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface, - [[0., 0.], [0., 1.]], - ); - - half_edge - .build(&mut services.objects) - .insert(&mut services.objects) - }; - assert_eq!(half_edge, expected_half_edge); - } -}