From f626f4aeb7d91be1e1875e08087df5c049d1160f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 Jan 2023 15:40:13 +0100 Subject: [PATCH 1/6] Simplify `RayFaceIntersection` Since the intersection result concerns a face, we don't need `HalfEdge`- specific information in there. --- crates/fj-kernel/src/algorithms/intersect/ray_face.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index de54e5cc4..6397bd41d 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -114,7 +114,7 @@ impl Intersect for (&HorizontalRayToTheRight<3>, &Handle) { FacePointIntersection::PointIsOnEdge(edge) => { RayFaceIntersection::RayHitsEdge(edge) } - FacePointIntersection::PointIsOnVertex(vertex) => { + FacePointIntersection::PointIsOnVertex((_, vertex)) => { RayFaceIntersection::RayHitsVertex(vertex) } }; @@ -136,12 +136,11 @@ pub enum RayFaceIntersection { RayHitsEdge(Handle), /// The ray hits a vertex - RayHitsVertex((Point<1>, Handle)), + RayHitsVertex(Handle), } #[cfg(test)] mod tests { - use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ @@ -289,11 +288,9 @@ mod tests { .exterior() .half_edges() .flat_map(|half_edge| { - half_edge - .boundary() - .zip_ext(half_edge.surface_vertices().map(Clone::clone)) + half_edge.surface_vertices().map(Clone::clone) }) - .find(|(_, surface_vertex)| { + .find(|surface_vertex| { surface_vertex.position() == Point::from([-1., -1.]) }) .unwrap(); From f68a21410f9ee62c00d04f15abbdda75589e20b1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 Jan 2023 15:40:13 +0100 Subject: [PATCH 2/6] Simplify `FacePointIntersection` Since the intersection result concerns a face, we don't need `HalfEdge`- specific information in there. --- .../src/algorithms/intersect/face_point.rs | 20 +++++++------------ .../src/algorithms/intersect/ray_face.rs | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index 730162d1f..39b1fd491 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -1,6 +1,5 @@ //! Intersection between faces and points in 2D -use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ @@ -49,17 +48,15 @@ impl Intersect for (&Handle, &Point<2>) { )); } (Some(RaySegmentIntersection::RayStartsOnOnFirstVertex), _) => { - let [vertex, _] = half_edge.boundary().zip_ext( - half_edge.surface_vertices().map(Clone::clone) - ); + let [vertex, _] = + half_edge.surface_vertices().map(Clone::clone); return Some( FacePointIntersection::PointIsOnVertex(vertex) ); } (Some(RaySegmentIntersection::RayStartsOnSecondVertex), _) => { - let [_, vertex] = half_edge.boundary().zip_ext( - half_edge.surface_vertices().map(Clone::clone) - ); + let [_, vertex] = + half_edge.surface_vertices().map(Clone::clone); return Some( FacePointIntersection::PointIsOnVertex(vertex) ); @@ -130,12 +127,11 @@ pub enum FacePointIntersection { PointIsOnEdge(Handle), /// The point is coincident with a vertex - PointIsOnVertex((Point<1>, Handle)), + PointIsOnVertex(Handle), } #[cfg(test)] mod tests { - use fj_interop::ext::ArrayExt; use fj_math::Point; use pretty_assertions::assert_eq; @@ -349,11 +345,9 @@ mod tests { .exterior() .half_edges() .flat_map(|half_edge| { - half_edge - .boundary() - .zip_ext(half_edge.surface_vertices().map(Clone::clone)) + half_edge.surface_vertices().map(Clone::clone) }) - .find(|(_, surface_vertex)| { + .find(|surface_vertex| { surface_vertex.position() == Point::from([1., 0.]) }) .unwrap(); diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 6397bd41d..3ffd68a4d 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -114,7 +114,7 @@ impl Intersect for (&HorizontalRayToTheRight<3>, &Handle) { FacePointIntersection::PointIsOnEdge(edge) => { RayFaceIntersection::RayHitsEdge(edge) } - FacePointIntersection::PointIsOnVertex((_, vertex)) => { + FacePointIntersection::PointIsOnVertex(vertex) => { RayFaceIntersection::RayHitsVertex(vertex) } }; From 8e00ebac60154b8930a565d1a405c5fa4fd5c8a7 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 Jan 2023 15:30:45 +0100 Subject: [PATCH 3/6] Simplify tests --- crates/fj-kernel/src/algorithms/intersect/face_point.rs | 5 ++--- crates/fj-kernel/src/algorithms/intersect/ray_face.rs | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index 39b1fd491..6eb03e805 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -311,9 +311,8 @@ mod tests { .exterior() .half_edges() .find(|edge| { - let [a, b] = edge.surface_vertices(); - a.position() == Point::from([0., 0.]) - && b.position() == Point::from([2., 0.]) + let [vertex, _] = edge.surface_vertices(); + vertex.position() == Point::from([0., 0.]) }) .unwrap(); assert_eq!( diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 3ffd68a4d..04095587f 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -253,9 +253,8 @@ mod tests { .exterior() .half_edges() .find(|edge| { - let [a, b] = edge.surface_vertices(); - a.position() == Point::from([-1., 1.]) - && b.position() == Point::from([-1., -1.]) + let [vertex, _] = edge.surface_vertices(); + vertex.position() == Point::from([-1., 1.]) }) .unwrap(); assert_eq!( From 125f8aa246c6694dfa51cf2da375cd10f2facfbf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 Jan 2023 15:51:54 +0100 Subject: [PATCH 4/6] Add `HalfEdge::start_vertex` The plan is to eventually make this the only way of accessing `SurfaceVertex` instances through `HalfEdge`, which would remove the redundancy between neighboring `HalfEdge`s. --- crates/fj-kernel/src/objects/full/edge.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 95f678b35..390ec41e3 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -40,6 +40,13 @@ impl HalfEdge { self.boundary.each_ref_ext().map(|&(point, _)| point) } + /// Access the vertex from where this half-edge starts + pub fn start_vertex(&self) -> &Handle { + let [vertex, _] = + self.boundary.each_ref_ext().map(|(_, vertex)| vertex); + vertex + } + /// Access the surface vertices that bound the half-edge pub fn surface_vertices(&self) -> [&Handle; 2] { self.boundary From 53ec3878884c3b6c50d7751cae5913d2aa2eec12 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 Jan 2023 15:54:51 +0100 Subject: [PATCH 5/6] Avoid using `HalfEdge::surface_vertices` I plan to remove that method as soon as possible. This commit covers some easy cases, that didn't really need access to both vertices in the first place. --- crates/fj-kernel/src/algorithms/approx/edge.rs | 2 +- crates/fj-kernel/src/algorithms/intersect/face_point.rs | 6 ++---- crates/fj-kernel/src/algorithms/intersect/ray_face.rs | 6 ++---- crates/fj-kernel/src/objects/full/cycle.rs | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 27ee3c0e3..628bbb391 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -25,7 +25,7 @@ impl Approx for &HalfEdge { let boundary = self.boundary(); let range = RangeOnPath { boundary }; - let [first, _] = self.surface_vertices(); + let first = self.start_vertex(); let first = ApproxPoint::new(first.position(), first.global_form().position()); let curve_approx = diff --git a/crates/fj-kernel/src/algorithms/intersect/face_point.rs b/crates/fj-kernel/src/algorithms/intersect/face_point.rs index 6eb03e805..1e018e9df 100644 --- a/crates/fj-kernel/src/algorithms/intersect/face_point.rs +++ b/crates/fj-kernel/src/algorithms/intersect/face_point.rs @@ -311,7 +311,7 @@ mod tests { .exterior() .half_edges() .find(|edge| { - let [vertex, _] = edge.surface_vertices(); + let vertex = edge.start_vertex(); vertex.position() == Point::from([0., 0.]) }) .unwrap(); @@ -343,9 +343,7 @@ mod tests { let vertex = face .exterior() .half_edges() - .flat_map(|half_edge| { - half_edge.surface_vertices().map(Clone::clone) - }) + .map(|half_edge| half_edge.start_vertex().clone()) .find(|surface_vertex| { surface_vertex.position() == Point::from([1., 0.]) }) diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs index 04095587f..336ffb5be 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_face.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_face.rs @@ -253,7 +253,7 @@ mod tests { .exterior() .half_edges() .find(|edge| { - let [vertex, _] = edge.surface_vertices(); + let vertex = edge.start_vertex(); vertex.position() == Point::from([-1., 1.]) }) .unwrap(); @@ -286,9 +286,7 @@ mod tests { let vertex = face .exterior() .half_edges() - .flat_map(|half_edge| { - half_edge.surface_vertices().map(Clone::clone) - }) + .map(|half_edge| half_edge.start_vertex().clone()) .find(|surface_vertex| { surface_vertex.position() == Point::from([-1., -1.]) }) diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index dc9f8b192..ef93dbfc8 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -93,7 +93,7 @@ impl Cycle { for [a, b] in self.half_edges.as_slice().array_windows_ext() { let [a, b] = [a, b].map(|half_edge| { - let [surface_vertex, _] = half_edge.surface_vertices(); + let surface_vertex = half_edge.start_vertex(); surface_vertex.position() }); From ab6f537d50b3e665b0e3f60607e773320384e15e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 24 Jan 2023 15:57:09 +0100 Subject: [PATCH 6/6] Inline variables --- crates/fj-kernel/src/algorithms/approx/edge.rs | 7 ++++--- crates/fj-kernel/src/objects/full/cycle.rs | 6 ++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 628bbb391..58f6204a2 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -25,9 +25,10 @@ impl Approx for &HalfEdge { let boundary = self.boundary(); let range = RangeOnPath { boundary }; - let first = self.start_vertex(); - let first = - ApproxPoint::new(first.position(), first.global_form().position()); + let first = ApproxPoint::new( + self.start_vertex().position(), + self.start_vertex().global_form().position(), + ); let curve_approx = (self.curve(), range).approx_with_cache(tolerance, cache); diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index ef93dbfc8..2a829a8a5 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -92,10 +92,8 @@ impl Cycle { let mut sum = Scalar::ZERO; for [a, b] in self.half_edges.as_slice().array_windows_ext() { - let [a, b] = [a, b].map(|half_edge| { - let surface_vertex = half_edge.start_vertex(); - surface_vertex.position() - }); + let [a, b] = + [a, b].map(|half_edge| half_edge.start_vertex().position()); sum += (b.u - a.u) * (b.v + a.v); }