From 34b038e193e66ab671238f40bdfb10f6e58da236 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 10:17:45 +0200 Subject: [PATCH 1/9] Refactor to prepare for follow-on change --- crates/fj-core/src/operations/build/surface.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/fj-core/src/operations/build/surface.rs b/crates/fj-core/src/operations/build/surface.rs index bd7bab739..9c64dc508 100644 --- a/crates/fj-core/src/operations/build/surface.rs +++ b/crates/fj-core/src/operations/build/surface.rs @@ -11,11 +11,10 @@ pub trait BuildSurface { fn plane_from_points(points: [impl Into>; 3]) -> Surface { let [a, b, c] = points.map(Into::into); - let geometry = SurfaceGeometry { - u: GlobalPath::line_from_points([a, b]).0, - v: c - a, - }; + let u = GlobalPath::line_from_points([a, b]).0; + let v = c - a; + let geometry = SurfaceGeometry { u, v }; Surface::new(geometry) } } From 74ec509e948ed4b944fa73088b250ca14720449d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 10:24:17 +0200 Subject: [PATCH 2/9] Refactor to prepare for follow-on change --- crates/fj-core/src/operations/build/face.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs index 91b02bfbc..8dfe1253f 100644 --- a/crates/fj-core/src/operations/build/face.rs +++ b/crates/fj-core/src/operations/build/face.rs @@ -27,7 +27,9 @@ pub trait BuildFace { ) -> Polygon<3> { let [a, b, c] = points.map(Into::into); - let surface = Surface::plane_from_points([a, b, c]).insert(services); + let surface = Surface::plane_from_points([a, b, c]); + let surface = surface.insert(services); + let (exterior, edges, vertices) = { let half_edges = [[a, b], [b, c], [c, a]].map(|points| { let half_edge = HalfEdge::line_segment_from_global_points( From d74feb78a16b9beb253e174e2fdb6df56360ef82 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 10:26:46 +0200 Subject: [PATCH 3/9] Return surface coords from `BuildSurface` method --- crates/fj-core/src/operations/build/face.rs | 2 +- .../fj-core/src/operations/build/surface.rs | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs index 8dfe1253f..466ea27b0 100644 --- a/crates/fj-core/src/operations/build/face.rs +++ b/crates/fj-core/src/operations/build/face.rs @@ -27,7 +27,7 @@ pub trait BuildFace { ) -> Polygon<3> { let [a, b, c] = points.map(Into::into); - let surface = Surface::plane_from_points([a, b, c]); + let (surface, _) = Surface::plane_from_points([a, b, c]); let surface = surface.insert(services); let (exterior, edges, vertices) = { diff --git a/crates/fj-core/src/operations/build/surface.rs b/crates/fj-core/src/operations/build/surface.rs index 9c64dc508..9e74f0fd2 100644 --- a/crates/fj-core/src/operations/build/surface.rs +++ b/crates/fj-core/src/operations/build/surface.rs @@ -1,4 +1,4 @@ -use fj_math::Point; +use fj_math::{Point, Scalar}; use crate::{ geometry::{curve::GlobalPath, surface::SurfaceGeometry}, @@ -8,14 +8,26 @@ use crate::{ /// Build a [`Surface`] pub trait BuildSurface { /// Build a plane from the provided points - fn plane_from_points(points: [impl Into>; 3]) -> Surface { + fn plane_from_points( + points: [impl Into>; 3], + ) -> (Surface, [Point<2>; 3]) { let [a, b, c] = points.map(Into::into); - let u = GlobalPath::line_from_points([a, b]).0; + let (u, u_line) = GlobalPath::line_from_points([a, b]); let v = c - a; let geometry = SurfaceGeometry { u, v }; - Surface::new(geometry) + let surface = Surface::new(geometry); + + let points_surface = { + let [a, b] = + u_line.map(|point| Point::from([point.t, Scalar::ZERO])); + let c = Point::from([a.u, Scalar::ONE]); + + [a, b, c] + }; + + (surface, points_surface) } } From 2179105f60d740053e59e0b621ca350b51142776 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 10:51:23 +0200 Subject: [PATCH 4/9] Remove redundant information from error messages I feel like I'm changing this back and forth every few months. I don't know what's changing or if I'm just being an idiot, but fact is, right now I'm seeing test failures where the same information is printed twice. --- crates/fj-core/src/validate/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index ea8ea3c45..27c8f601c 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -99,23 +99,23 @@ impl Default for ValidationConfig { #[derive(Clone, Debug, thiserror::Error)] pub enum ValidationError { /// `Cycle` validation error - #[error("`Cycle` validation error:\n {0}")] + #[error("`Cycle` validation error")] Cycle(#[from] CycleValidationError), /// `Face` validation error - #[error("`Face` validation error\n {0}")] + #[error("`Face` validation error")] Face(#[from] FaceValidationError), /// `HalfEdge` validation error - #[error("`HalfEdge` validation error\n {0}")] + #[error("`HalfEdge` validation error")] HalfEdge(#[from] HalfEdgeValidationError), /// `Shell` validation error - #[error("`Shell` validation error\n {0}")] + #[error("`Shell` validation error")] Shell(#[from] ShellValidationError), /// `Solid` validation error - #[error("`Solid` validation error\n {0}")] + #[error("`Solid` validation error")] Solid(#[from] SolidValidationError), } From 98da391e65c06460f6c097554a08519491402d76 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 11:29:03 +0200 Subject: [PATCH 5/9] Simplify `BuildFace::triangle` --- crates/fj-core/src/operations/build/face.rs | 36 +++++++++------------ 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs index 466ea27b0..e21faa087 100644 --- a/crates/fj-core/src/operations/build/face.rs +++ b/crates/fj-core/src/operations/build/face.rs @@ -4,8 +4,7 @@ use fj_math::Point; use crate::{ objects::{Cycle, Face, HalfEdge, Region, Surface, Vertex}, operations::{ - BuildCycle, BuildHalfEdge, BuildSurface, Insert, IsInserted, - IsInsertedNo, + BuildCycle, BuildRegion, BuildSurface, Insert, IsInserted, IsInsertedNo, }, services::Services, storage::Handle, @@ -27,29 +26,26 @@ pub trait BuildFace { ) -> Polygon<3> { let [a, b, c] = points.map(Into::into); - let (surface, _) = Surface::plane_from_points([a, b, c]); + let (surface, points_surface) = Surface::plane_from_points([a, b, c]); let surface = surface.insert(services); - let (exterior, edges, vertices) = { - let half_edges = [[a, b], [b, c], [c, a]].map(|points| { - let half_edge = HalfEdge::line_segment_from_global_points( - points, &surface, None, services, - ); - - half_edge.insert(services) - }); - let vertices = half_edges - .each_ref_ext() - .map(|half_edge| half_edge.start_vertex().clone()); + let region = Region::polygon(points_surface, services).insert(services); + let face = Face::new(surface, region); - let cycle = Cycle::new(half_edges.clone()).insert(services); + let edges = { + let mut half_edges = face.region().exterior().half_edges().cloned(); + assert_eq!(half_edges.clone().count(), 3); - (cycle, half_edges, vertices) + [half_edges.next(), half_edges.next(), half_edges.next()].map( + |half_edge| { + half_edge + .expect("Just asserted that there are three half-edges") + }, + ) }; - - let region = Region::new(exterior, [], None).insert(services); - - let face = Face::new(surface, region); + let vertices = edges + .each_ref_ext() + .map(|half_edge| half_edge.start_vertex().clone()); Polygon { face, From 7af94963d58b43f09792865ee232b415ffedb7da Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 11:30:55 +0200 Subject: [PATCH 6/9] Remove redundant variables --- crates/fj-core/src/operations/build/face.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs index e21faa087..233867c9b 100644 --- a/crates/fj-core/src/operations/build/face.rs +++ b/crates/fj-core/src/operations/build/face.rs @@ -24,9 +24,7 @@ pub trait BuildFace { points: [impl Into>; 3], services: &mut Services, ) -> Polygon<3> { - let [a, b, c] = points.map(Into::into); - - let (surface, points_surface) = Surface::plane_from_points([a, b, c]); + let (surface, points_surface) = Surface::plane_from_points(points); let surface = surface.insert(services); let region = Region::polygon(points_surface, services).insert(services); From cdcc30ae632fb3b7c6eda2467ad29168fb2580ab Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 11:31:40 +0200 Subject: [PATCH 7/9] Remove unused builder method --- crates/fj-core/src/operations/build/edge.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/fj-core/src/operations/build/edge.rs b/crates/fj-core/src/operations/build/edge.rs index dc6efe436..0fdda9d55 100644 --- a/crates/fj-core/src/operations/build/edge.rs +++ b/crates/fj-core/src/operations/build/edge.rs @@ -3,7 +3,7 @@ use fj_math::{Arc, Point, Scalar}; use crate::{ geometry::curve::Curve, - objects::{GlobalEdge, HalfEdge, Surface, Vertex}, + objects::{GlobalEdge, HalfEdge, Vertex}, operations::Insert, services::Services, }; @@ -75,18 +75,6 @@ pub trait BuildHalfEdge { HalfEdge::unjoined(curve, boundary, services) } - - /// Create a line segment from global points - fn line_segment_from_global_points( - points_global: [impl Into>; 2], - surface: &Surface, - boundary: Option<[Point<1>; 2]>, - services: &mut Services, - ) -> HalfEdge { - let points_surface = points_global - .map(|point| surface.geometry().project_global_point(point)); - HalfEdge::line_segment(points_surface, boundary, services) - } } impl BuildHalfEdge for HalfEdge {} From ec0987ceae5cddb78bcc869da451781f8ffee503 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 11:34:55 +0200 Subject: [PATCH 8/9] Add `BuildFace::polygon` --- crates/fj-core/src/operations/build/face.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs index 233867c9b..d67bb53d8 100644 --- a/crates/fj-core/src/operations/build/face.rs +++ b/crates/fj-core/src/operations/build/face.rs @@ -27,8 +27,7 @@ pub trait BuildFace { let (surface, points_surface) = Surface::plane_from_points(points); let surface = surface.insert(services); - let region = Region::polygon(points_surface, services).insert(services); - let face = Face::new(surface, region); + let face = Face::polygon(surface, points_surface, services); let edges = { let mut half_edges = face.region().exterior().half_edges().cloned(); @@ -51,6 +50,21 @@ pub trait BuildFace { vertices, } } + + /// Build a polygon + fn polygon( + surface: Handle, + points: Ps, + services: &mut Services, + ) -> Face + where + P: Into>, + Ps: IntoIterator, + Ps::IntoIter: Clone + ExactSizeIterator, + { + let region = Region::polygon(points, services).insert(services); + Face::new(surface, region) + } } impl BuildFace for Face {} From 2289859e43f795998926d911cf54cc4008d53b38 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Mon, 26 Jun 2023 11:44:10 +0200 Subject: [PATCH 9/9] Refactor --- crates/fj-core/src/operations/build/face.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/fj-core/src/operations/build/face.rs b/crates/fj-core/src/operations/build/face.rs index d67bb53d8..5c8f0c810 100644 --- a/crates/fj-core/src/operations/build/face.rs +++ b/crates/fj-core/src/operations/build/face.rs @@ -1,3 +1,5 @@ +use std::array; + use fj_interop::ext::ArrayExt; use fj_math::Point; @@ -33,16 +35,15 @@ pub trait BuildFace { let mut half_edges = face.region().exterior().half_edges().cloned(); assert_eq!(half_edges.clone().count(), 3); - [half_edges.next(), half_edges.next(), half_edges.next()].map( - |half_edge| { - half_edge - .expect("Just asserted that there are three half-edges") - }, - ) + array::from_fn(|_| half_edges.next()).map(|half_edge| { + half_edge + .expect("Just asserted that there are three half-edges") + }) }; - let vertices = edges - .each_ref_ext() - .map(|half_edge| half_edge.start_vertex().clone()); + let vertices = + edges.each_ref_ext().map(|half_edge: &Handle| { + half_edge.start_vertex().clone() + }); Polygon { face,