From 0ef549250d505dd8e72b00b1634194c31c684d83 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:22:06 +0100 Subject: [PATCH 01/14] Fix wording in doc comments --- crates/fj-kernel/src/builder/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 294cd8350..d39dce4d4 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -12,7 +12,7 @@ use super::{CycleBuilder, HalfEdgeBuilder, ObjectArgument, SurfaceBuilder}; /// Builder API for [`PartialFace`] pub trait FaceBuilder { - /// Connect the face to another face at the provided half-edges + /// Connect the face to other faces at the provided half-edges /// /// Assumes that the provided half-edges, once translated into local /// equivalents of this face, will not form a cycle. @@ -26,7 +26,7 @@ pub trait FaceBuilder { where O: ObjectArgument>; - /// Connect the face to another face at the provided half-edges + /// Connect the face to other faces at the provided half-edges /// /// Assumes that the provided half-edges, once translated into local /// equivalents of this face, form a cycle. From 2143452e9207976e97acdeaf62e31c075a3dc68d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 14:44:27 +0100 Subject: [PATCH 02/14] Make variable name more explicit --- crates/fj-kernel/src/builder/face.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index d39dce4d4..db379af23 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -128,7 +128,7 @@ impl FaceBuilder for PartialFace { array }; - let points = vertices.each_ref_ext().map(|vertex| { + let points_global = vertices.each_ref_ext().map(|vertex| { vertex .read() .global_form @@ -137,8 +137,10 @@ impl FaceBuilder for PartialFace { .expect("Need global position to infer plane") }); - let points_surface = - exterior.surface.write().update_as_plane_from_points(points); + let points_surface = exterior + .surface + .write() + .update_as_plane_from_points(points_global); for (mut surface_vertex, point) in vertices.zip_ext(points_surface) { surface_vertex.write().position = Some(point); From 10239389e69b3dfeebc48d90f92f7dd745e08797 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 14:45:53 +0100 Subject: [PATCH 03/14] Refactor --- crates/fj-kernel/src/builder/face.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index db379af23..b1f202880 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -128,19 +128,22 @@ impl FaceBuilder for PartialFace { array }; - let points_global = vertices.each_ref_ext().map(|vertex| { - vertex - .read() - .global_form - .read() - .position - .expect("Need global position to infer plane") - }); - let points_surface = exterior - .surface - .write() - .update_as_plane_from_points(points_global); + let points_surface = { + let points_global = vertices.each_ref_ext().map(|vertex| { + vertex + .read() + .global_form + .read() + .position + .expect("Need global position to infer plane") + }); + + exterior + .surface + .write() + .update_as_plane_from_points(points_global) + }; for (mut surface_vertex, point) in vertices.zip_ext(points_surface) { surface_vertex.write().position = Some(point); From 459bcea5e45182d3edf5eb3dd8cb70c3fc7746ad Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 14:57:41 +0100 Subject: [PATCH 04/14] Return updated `SurfaceGeometry` from builder --- crates/fj-kernel/src/builder/cycle.rs | 2 +- crates/fj-kernel/src/builder/face.rs | 2 +- crates/fj-kernel/src/builder/surface.rs | 13 ++++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index c0eb39796..e35c95eb4 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -182,7 +182,7 @@ impl CycleBuilder for PartialCycle { ) -> [Partial; 3] { let points_global = points_global.map(Into::into); - let points_surface = self + let (points_surface, _) = self .surface .write() .update_as_plane_from_points(points_global); diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index b1f202880..d49ae4db5 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -129,7 +129,7 @@ impl FaceBuilder for PartialFace { array }; - let points_surface = { + let (points_surface, _) = { let points_global = vertices.each_ref_ext().map(|vertex| { vertex .read() diff --git a/crates/fj-kernel/src/builder/surface.rs b/crates/fj-kernel/src/builder/surface.rs index 358e60589..1d5e23ee7 100644 --- a/crates/fj-kernel/src/builder/surface.rs +++ b/crates/fj-kernel/src/builder/surface.rs @@ -14,7 +14,7 @@ pub trait SurfaceBuilder: Sized { fn update_as_plane_from_points( &mut self, points: [impl Into>; 3], - ) -> [Point<2>; 3]; + ) -> ([Point<2>; 3], SurfaceGeometry); } impl SurfaceBuilder for PartialSurface { @@ -29,16 +29,19 @@ impl SurfaceBuilder for PartialSurface { fn update_as_plane_from_points( &mut self, points: [impl Into>; 3], - ) -> [Point<2>; 3] { + ) -> ([Point<2>; 3], SurfaceGeometry) { let [a, b, c] = points.map(Into::into); let (u, u_coords) = GlobalPath::line_from_points([a, b]); let v = c - a; - self.geometry = Some(SurfaceGeometry { u, v }); + let geometry = SurfaceGeometry { u, v }; + self.geometry = Some(geometry); let [a, b] = u_coords.map(|point| point.t); - [[a, Scalar::ZERO], [b, Scalar::ZERO], [a, Scalar::ONE]] - .map(Point::from) + let points = [[a, Scalar::ZERO], [b, Scalar::ZERO], [a, Scalar::ONE]] + .map(Point::from); + + (points, geometry) } } From eae906e757b0fe208dcb5dd9d7623f294ae9b87c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 15:04:43 +0100 Subject: [PATCH 05/14] Refactor --- crates/fj-kernel/src/builder/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index d49ae4db5..0bfb1881f 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -110,8 +110,8 @@ impl FaceBuilder for PartialFace { fn update_surface_as_plane(&mut self) -> Partial { let mut exterior = self.exterior.write(); let mut vertices = exterior.half_edges.iter().map(|half_edge| { - let [vertex, _] = &half_edge.read().vertices; - vertex.1.clone() + let [(_, surface_vertex), _] = &half_edge.read().vertices; + surface_vertex.clone() }); let vertices = { From 8ccc23f7590e5d9aea0857008b0ab7babac9b49a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 15:50:26 +0100 Subject: [PATCH 06/14] Refactor --- crates/fj-kernel/src/builder/face.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 0bfb1881f..7f15da537 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -111,7 +111,14 @@ impl FaceBuilder for PartialFace { let mut exterior = self.exterior.write(); let mut vertices = exterior.half_edges.iter().map(|half_edge| { let [(_, surface_vertex), _] = &half_edge.read().vertices; - surface_vertex.clone() + let global_position = surface_vertex + .read() + .global_form + .read() + .position + .expect("Need global position to infer plane"); + + (surface_vertex.clone(), global_position) }); let vertices = { @@ -130,14 +137,8 @@ impl FaceBuilder for PartialFace { }; let (points_surface, _) = { - let points_global = vertices.each_ref_ext().map(|vertex| { - vertex - .read() - .global_form - .read() - .position - .expect("Need global position to infer plane") - }); + let points_global = + vertices.each_ref_ext().map(|(_, point)| *point); exterior .surface @@ -145,7 +146,8 @@ impl FaceBuilder for PartialFace { .update_as_plane_from_points(points_global) }; - for (mut surface_vertex, point) in vertices.zip_ext(points_surface) { + for ((mut surface_vertex, _), point) in vertices.zip_ext(points_surface) + { surface_vertex.write().position = Some(point); } From b5dfb89419304ea4ca0f294cce9e32260c345a9b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 15:52:12 +0100 Subject: [PATCH 07/14] Refactor --- crates/fj-kernel/src/builder/face.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 7f15da537..a34375a5a 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -1,4 +1,4 @@ -use std::collections::VecDeque; +use std::{array, collections::VecDeque}; use fj_interop::ext::ArrayExt; @@ -122,11 +122,9 @@ impl FaceBuilder for PartialFace { }); let vertices = { - let array = [ - vertices.next().expect("Expected exactly three vertices"), - vertices.next().expect("Expected exactly three vertices"), - vertices.next().expect("Expected exactly three vertices"), - ]; + let array = array::from_fn(|_| { + vertices.next().expect("Expected exactly three vertices") + }); assert!( vertices.next().is_none(), From b0a2a88c8f99208f546e50f698c28d5a3ee31158 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:25:49 +0100 Subject: [PATCH 08/14] Make variable name more explicit --- crates/fj-kernel/src/builder/face.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index a34375a5a..034695159 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -121,7 +121,7 @@ impl FaceBuilder for PartialFace { (surface_vertex.clone(), global_position) }); - let vertices = { + let first_three_vertices = { let array = array::from_fn(|_| { vertices.next().expect("Expected exactly three vertices") }); @@ -136,7 +136,7 @@ impl FaceBuilder for PartialFace { let (points_surface, _) = { let points_global = - vertices.each_ref_ext().map(|(_, point)| *point); + first_three_vertices.each_ref_ext().map(|(_, point)| *point); exterior .surface @@ -144,7 +144,8 @@ impl FaceBuilder for PartialFace { .update_as_plane_from_points(points_global) }; - for ((mut surface_vertex, _), point) in vertices.zip_ext(points_surface) + for ((mut surface_vertex, _), point) in + first_three_vertices.zip_ext(points_surface) { surface_vertex.write().position = Some(point); } From 5b6f52d46c9ee1d8cfd7ceaf654d662b3b28ec18 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:26:55 +0100 Subject: [PATCH 09/14] Make variable name more explicit --- crates/fj-kernel/src/builder/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 034695159..fe147bf9c 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -135,13 +135,13 @@ impl FaceBuilder for PartialFace { }; let (points_surface, _) = { - let points_global = + let first_three_points_global = first_three_vertices.each_ref_ext().map(|(_, point)| *point); exterior .surface .write() - .update_as_plane_from_points(points_global) + .update_as_plane_from_points(first_three_points_global) }; for ((mut surface_vertex, _), point) in From 71db5be97fb7eacf0c749097f113d1b41429ee39 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:33:01 +0100 Subject: [PATCH 10/14] Make variable name more explicit --- crates/fj-kernel/src/builder/face.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index fe147bf9c..865e1d5ea 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -122,7 +122,7 @@ impl FaceBuilder for PartialFace { }); let first_three_vertices = { - let array = array::from_fn(|_| { + let first_three_vertices = array::from_fn(|_| { vertices.next().expect("Expected exactly three vertices") }); @@ -131,7 +131,7 @@ impl FaceBuilder for PartialFace { "Expected exactly three vertices" ); - array + first_three_vertices }; let (points_surface, _) = { From 0994519e3a9ddc08212ed07330fdbebe05a71ae2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:34:05 +0100 Subject: [PATCH 11/14] Refactor --- crates/fj-kernel/src/builder/face.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 865e1d5ea..50b0ed469 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -131,22 +131,18 @@ impl FaceBuilder for PartialFace { "Expected exactly three vertices" ); - first_three_vertices - }; - - let (points_surface, _) = { let first_three_points_global = first_three_vertices.each_ref_ext().map(|(_, point)| *point); - exterior + let (first_three_points_surface, _) = exterior .surface .write() - .update_as_plane_from_points(first_three_points_global) + .update_as_plane_from_points(first_three_points_global); + + first_three_vertices.zip_ext(first_three_points_surface) }; - for ((mut surface_vertex, _), point) in - first_three_vertices.zip_ext(points_surface) - { + for ((mut surface_vertex, _), point) in first_three_vertices { surface_vertex.write().position = Some(point); } From 078ff579ee2a896c044f106839ddd19947084567 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:48:21 +0100 Subject: [PATCH 12/14] Refactor --- crates/fj-kernel/src/builder/face.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 50b0ed469..0ef797584 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -139,10 +139,12 @@ impl FaceBuilder for PartialFace { .write() .update_as_plane_from_points(first_three_points_global); - first_three_vertices.zip_ext(first_three_points_surface) + first_three_vertices + .zip_ext(first_three_points_surface) + .map(|((vertex, _), point_global)| (vertex, point_global)) }; - for ((mut surface_vertex, _), point) in first_three_vertices { + for (mut surface_vertex, point) in first_three_vertices { surface_vertex.write().position = Some(point); } From 52044f1ccaf6224d178969f48a0b362008c4f9dc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 31 Jan 2023 15:06:08 +0100 Subject: [PATCH 13/14] Add `SurfaceGeometry::project_global_point` --- crates/fj-kernel/src/geometry/surface.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/geometry/surface.rs b/crates/fj-kernel/src/geometry/surface.rs index c99a4fc02..612387e1d 100644 --- a/crates/fj-kernel/src/geometry/surface.rs +++ b/crates/fj-kernel/src/geometry/surface.rs @@ -1,6 +1,6 @@ //! The geometry that defines a surface -use fj_math::{Line, Point, Transform, Vector}; +use fj_math::{Line, Plane, Point, Transform, Vector}; use super::path::GlobalPath; @@ -39,6 +39,17 @@ impl SurfaceGeometry { Line::from_origin_and_direction(self.u.origin(), self.v) } + /// Project the global point into the surface + pub fn project_global_point(&self, point: impl Into>) -> Point<2> { + let GlobalPath::Line(line) = self.u else { + todo!("Projecting point into non-plane surface is not supported") + }; + + let plane = + Plane::from_parametric(line.origin(), line.direction(), self.v); + plane.project_point(point) + } + /// Transform the surface geometry #[must_use] pub fn transform(self, transform: &Transform) -> Self { From 2cd0b503eea0cf3f5c8fd88b6ca12e0dc14868ea Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 1 Feb 2023 12:58:35 +0100 Subject: [PATCH 14/14] Lift limitation when inferring surface as plane Previously, it only supported inferring a plane from exactly three vertices. Now it supports faces with more vertices. --- crates/fj-kernel/src/builder/face.rs | 56 ++++++++++++++++------------ 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 0ef797584..95e639f16 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -109,42 +109,52 @@ impl FaceBuilder for PartialFace { fn update_surface_as_plane(&mut self) -> Partial { let mut exterior = self.exterior.write(); - let mut vertices = exterior.half_edges.iter().map(|half_edge| { - let [(_, surface_vertex), _] = &half_edge.read().vertices; - let global_position = surface_vertex - .read() - .global_form - .read() - .position - .expect("Need global position to infer plane"); - - (surface_vertex.clone(), global_position) - }); - - let first_three_vertices = { + let mut vertices = exterior + .half_edges + .iter() + .map(|half_edge| { + let [(_, surface_vertex), _] = &half_edge.read().vertices; + let global_position = surface_vertex + .read() + .global_form + .read() + .position + .expect("Need global position to infer plane"); + + (surface_vertex.clone(), global_position) + }) + .collect::>(); + + let (first_three_vertices, surface) = { let first_three_vertices = array::from_fn(|_| { - vertices.next().expect("Expected exactly three vertices") + vertices + .pop_front() + .expect("Expected at least three vertices") }); - assert!( - vertices.next().is_none(), - "Expected exactly three vertices" - ); - let first_three_points_global = first_three_vertices.each_ref_ext().map(|(_, point)| *point); - let (first_three_points_surface, _) = exterior + let (first_three_points_surface, surface) = exterior .surface .write() .update_as_plane_from_points(first_three_points_global); - first_three_vertices + let first_three_vertices = first_three_vertices .zip_ext(first_three_points_surface) - .map(|((vertex, _), point_global)| (vertex, point_global)) + .map(|((vertex, _), point_global)| (vertex, point_global)); + + (first_three_vertices, surface) }; + let rest_of_vertices = + vertices.into_iter().map(|(vertex, point_global)| { + let point_surface = surface.project_global_point(point_global); + (vertex, point_surface) + }); - for (mut surface_vertex, point) in first_three_vertices { + for (mut surface_vertex, point) in + first_three_vertices.into_iter().chain(rest_of_vertices) + { surface_vertex.write().position = Some(point); }