From 89f176b5d8b299154f201005a8f6e8ca7169fa1f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:25:10 +0200 Subject: [PATCH 01/13] Add `Vertex::from_point` --- crates/fj-kernel/src/algorithms/approx/edges.rs | 8 +++----- crates/fj-kernel/src/iter.rs | 5 +---- crates/fj-kernel/src/objects/vertex.rs | 6 ++++++ crates/fj-kernel/src/validation/mod.rs | 17 ++++------------- 4 files changed, 14 insertions(+), 22 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edges.rs b/crates/fj-kernel/src/algorithms/approx/edges.rs index f570ff843..074cd947c 100644 --- a/crates/fj-kernel/src/algorithms/approx/edges.rs +++ b/crates/fj-kernel/src/algorithms/approx/edges.rs @@ -37,20 +37,18 @@ mod test { use crate::{ geometry, objects::{Vertex, VerticesOfEdge}, - shape::{LocalForm, Shape}, + shape::LocalForm, }; #[test] fn approx_edge() { - let mut shape = Shape::new(); - let a = Point::from([1., 2., 3.]); let b = Point::from([2., 3., 5.]); let c = Point::from([3., 5., 8.]); let d = Point::from([5., 8., 13.]); - let v1 = Vertex::builder(&mut shape).build_from_point(a).get(); - let v2 = Vertex::builder(&mut shape).build_from_point(d).get(); + let v1 = Vertex::from_point(a); + let v2 = Vertex::from_point(d); let vertices = VerticesOfEdge::from_vertices([ LocalForm::new(Point::from([0.]), v1), diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 30b4939ae..ab26368e8 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -518,10 +518,7 @@ mod tests { #[test] fn vertex() { - let mut shape = Shape::new(); - let vertex = Vertex::builder(&mut shape) - .build_from_point([0., 0., 0.]) - .get(); + let vertex = Vertex::from_point([0., 0., 0.]); assert_eq!(0, vertex.curve_iter().count()); assert_eq!(0, vertex.cycle_iter().count()); diff --git a/crates/fj-kernel/src/objects/vertex.rs b/crates/fj-kernel/src/objects/vertex.rs index 1f719a882..354b23aef 100644 --- a/crates/fj-kernel/src/objects/vertex.rs +++ b/crates/fj-kernel/src/objects/vertex.rs @@ -27,6 +27,12 @@ pub struct Vertex { } impl Vertex { + /// Construct a `Vertex` from a point + pub fn from_point(point: impl Into>) -> Self { + let point = point.into(); + Self { point } + } + /// Build a vertex using the [`VertexBuilder`] API pub fn builder(shape: &mut Shape) -> VertexBuilder { VertexBuilder::new(shape) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 744a6e7ea..90f52f0f6 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -294,15 +294,10 @@ mod tests { #[test] fn structural_edge() { let mut shape = Shape::new(); - let mut other = Shape::new(); let curve = Curve::x_axis(); - let a = Vertex::builder(&mut other) - .build_from_point([1., 0., 0.]) - .get(); - let b = Vertex::builder(&mut other) - .build_from_point([2., 0., 0.]) - .get(); + let a = Vertex::from_point([1., 0., 0.]); + let b = Vertex::from_point([2., 0., 0.]); let a = LocalForm::new(Point::from([1.]), a); let b = LocalForm::new(Point::from([2.]), b); @@ -319,12 +314,8 @@ mod tests { assert!(err.missing_vertex(&b.canonical())); let curve = Curve::x_axis(); - let a = Vertex::builder(&mut shape) - .build_from_point([1., 0., 0.]) - .get(); - let b = Vertex::builder(&mut shape) - .build_from_point([2., 0., 0.]) - .get(); + let a = Vertex::from_point([1., 0., 0.]); + let b = Vertex::from_point([2., 0., 0.]); let a = LocalForm::new(Point::from([1.]), a); let b = LocalForm::new(Point::from([2.]), b); From c60cf9d2f3f5a208692e5334727838d5b9992b09 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:25:51 +0200 Subject: [PATCH 02/13] Remove `VertexBuilder` It has been made redundant by the new `Vertex` constructor. --- crates/fj-kernel/src/builder.rs | 25 ------------------------- crates/fj-kernel/src/objects/vertex.rs | 7 ------- 2 files changed, 32 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index e2dad7397..8c61ce551 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -7,31 +7,6 @@ use crate::{ shape::{Handle, LocalForm, Shape}, }; -/// API for building a [`Vertex`] -#[must_use] -pub struct VertexBuilder<'r> { - shape: &'r mut Shape, -} - -impl<'r> VertexBuilder<'r> { - /// Construct a new instance of `VertexBuilder` - pub fn new(shape: &'r mut Shape) -> Self { - Self { shape } - } - - /// Build a [`Vertex`] from a point - /// - /// If an identical point or vertex are already part of the shape, those - /// objects are re-used. - pub fn build_from_point( - self, - point: impl Into>, - ) -> Handle { - let point = point.into(); - self.shape.get_handle_or_insert(Vertex { point }) - } -} - /// API for building an [`Edge`] #[must_use] pub struct EdgeBuilder<'r> { diff --git a/crates/fj-kernel/src/objects/vertex.rs b/crates/fj-kernel/src/objects/vertex.rs index 354b23aef..9252c8e69 100644 --- a/crates/fj-kernel/src/objects/vertex.rs +++ b/crates/fj-kernel/src/objects/vertex.rs @@ -2,8 +2,6 @@ use std::hash::Hash; use fj_math::Point; -use crate::{builder::VertexBuilder, shape::Shape}; - /// A vertex /// /// This struct exists to distinguish between vertices and points at the type @@ -32,9 +30,4 @@ impl Vertex { let point = point.into(); Self { point } } - - /// Build a vertex using the [`VertexBuilder`] API - pub fn builder(shape: &mut Shape) -> VertexBuilder { - VertexBuilder::new(shape) - } } From e167832be559ca6125975ec8104e2ece560fe6ac Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:30:28 +0200 Subject: [PATCH 03/13] Add `Edge::circle_from_radius` --- crates/fj-kernel/src/objects/edge.rs | 29 +++++++++++++++++++++++++++- crates/fj-operations/src/circle.rs | 3 +-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 9d62bfc10..05dc41964 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -1,6 +1,6 @@ use std::fmt; -use fj_math::Point; +use fj_math::{Circle, Point, Scalar, Vector}; use crate::{ builder::EdgeBuilder, @@ -53,6 +53,33 @@ impl Edge { } } +impl Edge<2> { + /// Create a circle from the given radius + pub fn circle_from_radius(radius: Scalar) -> LocalForm, Edge<3>> { + let curve_local = Curve::Circle(Circle { + center: Point::origin(), + a: Vector::from([radius, Scalar::ZERO]), + b: Vector::from([Scalar::ZERO, radius]), + }); + let curve_canonical = Curve::Circle(Circle { + center: Point::origin(), + a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]), + b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]), + }); + + let edge_local = Edge { + curve: LocalForm::new(curve_local, curve_canonical), + vertices: VerticesOfEdge::none(), + }; + let edge_canonical = Edge { + curve: LocalForm::canonical_only(curve_canonical), + vertices: VerticesOfEdge::none(), + }; + + LocalForm::new(edge_local, edge_canonical) + } +} + impl Edge<3> { /// Build an edge using the [`EdgeBuilder`] API pub fn builder(shape: &mut Shape) -> EdgeBuilder { diff --git a/crates/fj-operations/src/circle.rs b/crates/fj-operations/src/circle.rs index daba5ea25..83ddd28dd 100644 --- a/crates/fj-operations/src/circle.rs +++ b/crates/fj-operations/src/circle.rs @@ -21,8 +21,7 @@ impl ToShape for fj::Circle { // Circles have just a single round edge with no vertices. So none need // to be added here. - let edge = Edge::builder(&mut tmp) - .build_circle(Scalar::from_f64(self.radius())); + let edge = Edge::circle_from_radius(Scalar::from_f64(self.radius())); let cycle_local = Cycle { edges: vec![edge.clone()], From a4426cf38eac9ce139335597288bda19e647e117 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:34:26 +0200 Subject: [PATCH 04/13] Add `Edge::line_segment_from_vertices` --- crates/fj-kernel/src/objects/edge.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 05dc41964..80bd2bea4 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -1,6 +1,6 @@ use std::fmt; -use fj_math::{Circle, Point, Scalar, Vector}; +use fj_math::{Circle, Line, Point, Scalar, Vector}; use crate::{ builder::EdgeBuilder, @@ -81,6 +81,24 @@ impl Edge<2> { } impl Edge<3> { + /// Create a line segment from two vertices + pub fn line_segment_from_vertices([a, b]: [Vertex; 2]) -> Self { + let curve = { + let points = [a, b].map(|vertex| vertex.point); + Curve::Line(Line::from_points(points)) + }; + + let vertices = [ + LocalForm::new(Point::from([0.]), a), + LocalForm::new(Point::from([1.]), b), + ]; + + Self { + curve: LocalForm::canonical_only(curve), + vertices: VerticesOfEdge::from_vertices(vertices), + } + } + /// Build an edge using the [`EdgeBuilder`] API pub fn builder(shape: &mut Shape) -> EdgeBuilder { EdgeBuilder::new(shape) From b585cb91530b64b97dcc196bb7ed55a1a272f500 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:38:32 +0200 Subject: [PATCH 05/13] Add `Edge::line_segment_from_points` --- crates/fj-kernel/src/builder.rs | 11 +++++------ crates/fj-kernel/src/iter.rs | 5 +---- crates/fj-kernel/src/objects/edge.rs | 12 ++++++++++++ crates/fj-kernel/src/validation/mod.rs | 9 ++------- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index 8c61ce551..3fe245f04 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -83,13 +83,13 @@ impl<'r> EdgeBuilder<'r> { #[must_use] pub struct CycleBuilder<'r> { surface: Surface, - shape: &'r mut Shape, + _shape: &'r mut Shape, } impl<'r> CycleBuilder<'r> { /// Construct a new instance of `CycleBuilder` - pub fn new(surface: Surface, shape: &'r mut Shape) -> Self { - Self { surface, shape } + pub fn new(surface: Surface, _shape: &'r mut Shape) -> Self { + Self { surface, _shape } } /// Build a polygon from a list of points @@ -114,9 +114,8 @@ impl<'r> CycleBuilder<'r> { let points_canonical = points .map(|point| self.surface.point_from_surface_coords(point)); - let edge_canonical = Edge::builder(self.shape) - .build_line_segment_from_points(points_canonical) - .get(); + let edge_canonical = + Edge::line_segment_from_points(points_canonical); let edge_local = Edge { curve: LocalForm::new( diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index ab26368e8..ad29f48e4 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -475,10 +475,7 @@ mod tests { #[test] fn edge() { - let mut shape = Shape::new(); - let edge = Edge::builder(&mut shape) - .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]) - .get(); + let edge = Edge::line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]); assert_eq!(1, edge.curve_iter().count()); assert_eq!(0, edge.cycle_iter().count()); diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 80bd2bea4..4d9064529 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -81,6 +81,18 @@ impl Edge<2> { } impl Edge<3> { + /// Create a line segment from two points + pub fn line_segment_from_points( + vertices: [impl Into>; 2], + ) -> Self { + let vertices = vertices.map(|point| { + let point = point.into(); + Vertex { point } + }); + + Self::line_segment_from_vertices(vertices) + } + /// Create a line segment from two vertices pub fn line_segment_from_vertices([a, b]: [Vertex; 2]) -> Self { let curve = { diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 90f52f0f6..2fea97df0 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -273,21 +273,16 @@ mod tests { #[test] fn structural_cycle() { let mut shape = Shape::new(); - let mut other = Shape::new(); // Trying to refer to edge that is not from the same shape. Should fail. - let edge = Edge::builder(&mut other) - .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]) - .get(); + let edge = Edge::line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]); shape.insert(Cycle::new(vec![edge.clone()])); let err = validate(shape.clone(), &ValidationConfig::default()).unwrap_err(); assert!(err.missing_edge(&edge)); // Referring to edge that *is* from the same shape. Should work. - let edge = Edge::builder(&mut shape) - .build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]) - .get(); + let edge = Edge::line_segment_from_points([[0., 0., 0.], [1., 0., 0.]]); shape.insert(Cycle::new(vec![edge])); } From 998d2aa144b109da275de55d2a5da94cc9030787 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:51:56 +0200 Subject: [PATCH 06/13] Remove unused struct field --- crates/fj-kernel/src/builder.rs | 9 ++++----- crates/fj-kernel/src/objects/cycle.rs | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index 3fe245f04..3245392be 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -81,15 +81,14 @@ impl<'r> EdgeBuilder<'r> { /// API for building a [`Cycle`] #[must_use] -pub struct CycleBuilder<'r> { +pub struct CycleBuilder { surface: Surface, - _shape: &'r mut Shape, } -impl<'r> CycleBuilder<'r> { +impl CycleBuilder { /// Construct a new instance of `CycleBuilder` - pub fn new(surface: Surface, _shape: &'r mut Shape) -> Self { - Self { surface, _shape } + pub fn new(surface: Surface) -> Self { + Self { surface } } /// Build a polygon from a list of points diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index fe6c31ad3..06ec54a5a 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -30,8 +30,8 @@ impl Cycle<3> { } /// Build a cycle using the [`CycleBuilder`] API - pub fn builder(surface: Surface, shape: &mut Shape) -> CycleBuilder { - CycleBuilder::new(surface, shape) + pub fn builder(surface: Surface, _shape: &mut Shape) -> CycleBuilder { + CycleBuilder::new(surface) } /// Access the edges that this cycle refers to From b4e04bc95b8ecf587c13fbc94d1a8971e280929c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:52:38 +0200 Subject: [PATCH 07/13] Remove unused function argument --- crates/fj-kernel/src/builder.rs | 6 ++---- crates/fj-kernel/src/iter.rs | 3 +-- crates/fj-kernel/src/objects/cycle.rs | 7 ++----- crates/fj-kernel/src/validation/mod.rs | 5 ++--- 4 files changed, 7 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index 3245392be..e49ba15c8 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -200,15 +200,13 @@ impl<'r> FaceBuilder<'r> { let mut exteriors = Vec::new(); if let Some(points) = self.exterior { - let cycle = - Cycle::builder(self.surface, self.shape).build_polygon(points); + let cycle = Cycle::builder(self.surface).build_polygon(points); exteriors.push(cycle); } let mut interiors = Vec::new(); for points in self.interiors { - let cycle = - Cycle::builder(self.surface, self.shape).build_polygon(points); + let cycle = Cycle::builder(self.surface).build_polygon(points); interiors.push(cycle); } diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index ad29f48e4..0a1726cf5 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -460,8 +460,7 @@ mod tests { #[test] fn cycle() { - let mut shape = Shape::new(); - let cycle = Cycle::builder(Surface::xy_plane(), &mut shape) + let cycle = Cycle::builder(Surface::xy_plane()) .build_polygon([[0., 0.], [1., 0.], [0., 1.]]) .canonical(); diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index 06ec54a5a..388ee7947 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -1,7 +1,4 @@ -use crate::{ - builder::CycleBuilder, - shape::{LocalForm, Shape}, -}; +use crate::{builder::CycleBuilder, shape::LocalForm}; use super::{Edge, Surface}; @@ -30,7 +27,7 @@ impl Cycle<3> { } /// Build a cycle using the [`CycleBuilder`] API - pub fn builder(surface: Surface, _shape: &mut Shape) -> CycleBuilder { + pub fn builder(surface: Surface) -> CycleBuilder { CycleBuilder::new(surface) } diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 2fea97df0..5d6d9d247 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -325,12 +325,11 @@ mod tests { #[test] fn structural_face() { let mut shape = Shape::new(); - let mut other = Shape::new(); let triangle = [[0., 0.], [1., 0.], [0., 1.]]; let surface = Surface::xy_plane(); - let cycle = Cycle::builder(surface, &mut other).build_polygon(triangle); + let cycle = Cycle::builder(surface).build_polygon(triangle); // Nothing has been added to `shape`. Should fail. shape.insert(Face::new( @@ -345,7 +344,7 @@ mod tests { assert!(err.missing_cycle(&cycle.canonical())); let surface = Surface::xy_plane(); - let cycle = Cycle::builder(surface, &mut shape).build_polygon(triangle); + let cycle = Cycle::builder(surface).build_polygon(triangle); // Everything has been added to `shape` now. Should work! shape.insert(Face::new( From 1dc7ba59e216871b03d5549b809987deb73edc07 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:41:45 +0200 Subject: [PATCH 08/13] Remove `EdgeBuilder` It has been made redundant by the new `Edge` constructors. --- crates/fj-kernel/src/builder.rs | 76 +--------------------------- crates/fj-kernel/src/objects/edge.rs | 10 +--- 2 files changed, 3 insertions(+), 83 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index e49ba15c8..a6f509f2b 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -1,84 +1,12 @@ //! Convenient API to build objects -use fj_math::{Circle, Line, Point, Scalar, Vector}; +use fj_math::{Line, Point}; use crate::{ - objects::{Curve, Cycle, Edge, Face, Surface, Vertex, VerticesOfEdge}, + objects::{Curve, Cycle, Edge, Face, Surface}, shape::{Handle, LocalForm, Shape}, }; -/// API for building an [`Edge`] -#[must_use] -pub struct EdgeBuilder<'r> { - shape: &'r mut Shape, -} - -impl<'r> EdgeBuilder<'r> { - /// Construct a new instance of `EdgeBuilder` - pub fn new(shape: &'r mut Shape) -> Self { - Self { shape } - } - - /// Build a circle from a radius - pub fn build_circle(self, radius: Scalar) -> LocalForm, Edge<3>> { - let curve_local = Curve::Circle(Circle { - center: Point::origin(), - a: Vector::from([radius, Scalar::ZERO]), - b: Vector::from([Scalar::ZERO, radius]), - }); - let curve_canonical = Curve::Circle(Circle { - center: Point::origin(), - a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]), - b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]), - }); - - let edge_local = Edge { - curve: LocalForm::new(curve_local, curve_canonical), - vertices: VerticesOfEdge::none(), - }; - let edge_canonical = Edge { - curve: LocalForm::canonical_only(curve_canonical), - vertices: VerticesOfEdge::none(), - }; - - LocalForm::new(edge_local, edge_canonical) - } - - /// Build a line segment from two points - pub fn build_line_segment_from_points( - self, - vertices: [impl Into>; 2], - ) -> Handle> { - let vertices = vertices.map(|point| { - let point = point.into(); - Vertex { point } - }); - - self.build_line_segment_from_vertices(vertices) - } - - /// Build a line segment from two vertices - pub fn build_line_segment_from_vertices( - self, - [a, b]: [Vertex; 2], - ) -> Handle> { - let curve = { - let points = [a, b].map(|vertex| vertex.point); - Curve::Line(Line::from_points(points)) - }; - - let vertices = [ - LocalForm::new(Point::from([0.]), a), - LocalForm::new(Point::from([1.]), b), - ]; - - self.shape.get_handle_or_insert(Edge { - curve: LocalForm::canonical_only(curve), - vertices: VerticesOfEdge::from_vertices(vertices), - }) - } -} - /// API for building a [`Cycle`] #[must_use] pub struct CycleBuilder { diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index 4d9064529..ea2998998 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -2,10 +2,7 @@ use std::fmt; use fj_math::{Circle, Line, Point, Scalar, Vector}; -use crate::{ - builder::EdgeBuilder, - shape::{LocalForm, Shape}, -}; +use crate::shape::LocalForm; use super::{Curve, Vertex}; @@ -110,11 +107,6 @@ impl Edge<3> { vertices: VerticesOfEdge::from_vertices(vertices), } } - - /// Build an edge using the [`EdgeBuilder`] API - pub fn builder(shape: &mut Shape) -> EdgeBuilder { - EdgeBuilder::new(shape) - } } impl fmt::Display for Edge { From 888387916335ca47dccbec4ef41496a7ae495118 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:45:22 +0200 Subject: [PATCH 09/13] Add `Cycle::polygon_from_points` --- crates/fj-kernel/src/builder.rs | 4 +-- crates/fj-kernel/src/iter.rs | 8 +++-- crates/fj-kernel/src/objects/cycle.rs | 50 +++++++++++++++++++++++++- crates/fj-kernel/src/validation/mod.rs | 4 +-- 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index a6f509f2b..556b17153 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -128,13 +128,13 @@ impl<'r> FaceBuilder<'r> { let mut exteriors = Vec::new(); if let Some(points) = self.exterior { - let cycle = Cycle::builder(self.surface).build_polygon(points); + let cycle = Cycle::polygon_from_points(&self.surface, points); exteriors.push(cycle); } let mut interiors = Vec::new(); for points in self.interiors { - let cycle = Cycle::builder(self.surface).build_polygon(points); + let cycle = Cycle::polygon_from_points(&self.surface, points); interiors.push(cycle); } diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 0a1726cf5..9f45d42b8 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -460,9 +460,11 @@ mod tests { #[test] fn cycle() { - let cycle = Cycle::builder(Surface::xy_plane()) - .build_polygon([[0., 0.], [1., 0.], [0., 1.]]) - .canonical(); + let cycle = Cycle::polygon_from_points( + &Surface::xy_plane(), + [[0., 0.], [1., 0.], [0., 1.]], + ) + .canonical(); assert_eq!(3, cycle.curve_iter().count()); assert_eq!(1, cycle.cycle_iter().count()); diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index 388ee7947..cabe8036a 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -1,6 +1,8 @@ +use fj_math::{Line, Point}; + use crate::{builder::CycleBuilder, shape::LocalForm}; -use super::{Edge, Surface}; +use super::{Curve, Edge, Surface}; /// A cycle of connected edges /// @@ -26,6 +28,52 @@ impl Cycle<3> { Self { edges } } + /// Create a polygon from a list of points + pub fn polygon_from_points( + surface: &Surface, + points: impl IntoIterator>>, + ) -> LocalForm, Cycle<3>> { + let mut points: Vec<_> = points.into_iter().map(Into::into).collect(); + + // A polygon is closed, so we need to add the first point at the end + // again, for the next step. + if let Some(point) = points.first().cloned() { + points.push(point); + } + + let mut edges = Vec::new(); + for points in points.windows(2) { + // Can't panic, as we passed `2` to `windows`. + // + // Can be cleaned up, once `array_windows` is stable. + let points = [points[0], points[1]]; + + let points_canonical = + points.map(|point| surface.point_from_surface_coords(point)); + let edge_canonical = + Edge::line_segment_from_points(points_canonical); + + let edge_local = Edge { + curve: LocalForm::new( + Curve::Line(Line::from_points(points)), + edge_canonical.curve.canonical(), + ), + vertices: edge_canonical.vertices.clone(), + }; + + edges.push(LocalForm::new(edge_local, edge_canonical)); + } + + let local = Cycle { + edges: edges.clone(), + }; + + let edges_canonical = edges.into_iter().map(|edge| edge.canonical()); + let canonical = Cycle::new(edges_canonical); + + LocalForm::new(local, canonical) + } + /// Build a cycle using the [`CycleBuilder`] API pub fn builder(surface: Surface) -> CycleBuilder { CycleBuilder::new(surface) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 5d6d9d247..4a1ba5ebf 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -329,7 +329,7 @@ mod tests { let triangle = [[0., 0.], [1., 0.], [0., 1.]]; let surface = Surface::xy_plane(); - let cycle = Cycle::builder(surface).build_polygon(triangle); + let cycle = Cycle::polygon_from_points(&surface, triangle); // Nothing has been added to `shape`. Should fail. shape.insert(Face::new( @@ -344,7 +344,7 @@ mod tests { assert!(err.missing_cycle(&cycle.canonical())); let surface = Surface::xy_plane(); - let cycle = Cycle::builder(surface).build_polygon(triangle); + let cycle = Cycle::polygon_from_points(&surface, triangle); // Everything has been added to `shape` now. Should work! shape.insert(Face::new( From 07e1fe3f0e8b84f12db4f6ca3095b1ab3b69fdbe Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:46:21 +0200 Subject: [PATCH 10/13] Remove `CycleBuilder` It has been made redundant by the new `Cycle` constructor. --- crates/fj-kernel/src/builder.rs | 65 ++------------------------- crates/fj-kernel/src/objects/cycle.rs | 7 +-- 2 files changed, 4 insertions(+), 68 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index 556b17153..e8711385b 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -1,71 +1,12 @@ //! Convenient API to build objects -use fj_math::{Line, Point}; +use fj_math::Point; use crate::{ - objects::{Curve, Cycle, Edge, Face, Surface}, - shape::{Handle, LocalForm, Shape}, + objects::{Cycle, Face, Surface}, + shape::{Handle, Shape}, }; -/// API for building a [`Cycle`] -#[must_use] -pub struct CycleBuilder { - surface: Surface, -} - -impl CycleBuilder { - /// Construct a new instance of `CycleBuilder` - pub fn new(surface: Surface) -> Self { - Self { surface } - } - - /// Build a polygon from a list of points - pub fn build_polygon( - self, - points: impl IntoIterator>>, - ) -> LocalForm, Cycle<3>> { - let mut points: Vec<_> = points.into_iter().map(Into::into).collect(); - - // A polygon is closed, so we need to add the first point at the end - // again, for the next step. - if let Some(point) = points.first().cloned() { - points.push(point); - } - - let mut edges = Vec::new(); - for points in points.windows(2) { - // Can't panic, as we passed `2` to `windows`. - // - // Can be cleaned up, once `array_windows` is stable. - let points = [points[0], points[1]]; - - let points_canonical = points - .map(|point| self.surface.point_from_surface_coords(point)); - let edge_canonical = - Edge::line_segment_from_points(points_canonical); - - let edge_local = Edge { - curve: LocalForm::new( - Curve::Line(Line::from_points(points)), - edge_canonical.curve.canonical(), - ), - vertices: edge_canonical.vertices.clone(), - }; - - edges.push(LocalForm::new(edge_local, edge_canonical)); - } - - let local = Cycle { - edges: edges.clone(), - }; - - let edges_canonical = edges.into_iter().map(|edge| edge.canonical()); - let canonical = Cycle::new(edges_canonical); - - LocalForm::new(local, canonical) - } -} - /// API for building a [`Face`] #[must_use] pub struct FaceBuilder<'r> { diff --git a/crates/fj-kernel/src/objects/cycle.rs b/crates/fj-kernel/src/objects/cycle.rs index cabe8036a..98b2d82db 100644 --- a/crates/fj-kernel/src/objects/cycle.rs +++ b/crates/fj-kernel/src/objects/cycle.rs @@ -1,6 +1,6 @@ use fj_math::{Line, Point}; -use crate::{builder::CycleBuilder, shape::LocalForm}; +use crate::shape::LocalForm; use super::{Curve, Edge, Surface}; @@ -74,11 +74,6 @@ impl Cycle<3> { LocalForm::new(local, canonical) } - /// Build a cycle using the [`CycleBuilder`] API - pub fn builder(surface: Surface) -> CycleBuilder { - CycleBuilder::new(surface) - } - /// Access the edges that this cycle refers to /// /// This is a convenience method that saves the caller from dealing with the From bac7c658e6ebfc12ae3279b940745c13ac943244 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:48:09 +0200 Subject: [PATCH 11/13] Simplify return value of `FaceBuilder::build` --- crates/fj-kernel/src/algorithms/approx/faces.rs | 2 +- crates/fj-kernel/src/algorithms/sweep.rs | 4 +--- .../fj-kernel/src/algorithms/triangulation/mod.rs | 6 ++---- crates/fj-kernel/src/builder.rs | 14 ++++++-------- crates/fj-kernel/src/iter.rs | 3 +-- crates/fj-operations/src/sketch.rs | 3 +-- 6 files changed, 12 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/faces.rs b/crates/fj-kernel/src/algorithms/approx/faces.rs index 4642dafd9..ad977d0e4 100644 --- a/crates/fj-kernel/src/algorithms/approx/faces.rs +++ b/crates/fj-kernel/src/algorithms/approx/faces.rs @@ -130,7 +130,7 @@ mod tests { let g = geometry::Point::new(g, g); let h = geometry::Point::new(h, h); - let approx = FaceApprox::new(&face.get(), tolerance); + let approx = FaceApprox::new(&face, tolerance); let expected = FaceApprox { points: set![a, b, c, d, e, f, g, h], exterior: CycleApprox { diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 3a56d057b..2ebc7a77b 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -366,8 +366,7 @@ mod tests { let sketch = Face::builder(Surface::xy_plane(), &mut shape) .with_exterior_polygon([[0., 0.], [1., 0.], [0., 1.]]) - .build() - .get(); + .build(); let solid = super::sweep(vec![sketch], direction, tolerance, [255, 0, 0, 255]); @@ -384,7 +383,6 @@ mod tests { Face::builder(surface, &mut shape) .with_exterior_polygon(expected_vertices.clone()) .build() - .get() }); for face in faces { diff --git a/crates/fj-kernel/src/algorithms/triangulation/mod.rs b/crates/fj-kernel/src/algorithms/triangulation/mod.rs index cdede849a..7ca4f6ff9 100644 --- a/crates/fj-kernel/src/algorithms/triangulation/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulation/mod.rs @@ -103,8 +103,7 @@ mod tests { let face = Face::builder(Surface::xy_plane(), &mut shape) .with_exterior_polygon([a, b, c, d]) - .build() - .get(); + .build(); let a = Point::from(a).to_xyz(); let b = Point::from(b).to_xyz(); @@ -138,8 +137,7 @@ mod tests { let face = Face::builder(Surface::xy_plane(), &mut shape) .with_exterior_polygon([a, b, c, d]) .with_interior_polygon([e, f, g, h]) - .build() - .get(); + .build(); let triangles = triangulate(face)?; diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index e8711385b..acfb58a10 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -4,7 +4,7 @@ use fj_math::Point; use crate::{ objects::{Cycle, Face, Surface}, - shape::{Handle, Shape}, + shape::Shape, }; /// API for building a [`Face`] @@ -15,19 +15,19 @@ pub struct FaceBuilder<'r> { interiors: Vec>>, color: Option<[u8; 4]>, - shape: &'r mut Shape, + _shape: &'r mut Shape, } impl<'r> FaceBuilder<'r> { /// Construct a new instance of `FaceBuilder` - pub fn new(surface: Surface, shape: &'r mut Shape) -> Self { + pub fn new(surface: Surface, _shape: &'r mut Shape) -> Self { Self { surface, exterior: None, interiors: Vec::new(), color: None, - shape, + _shape, } } @@ -64,7 +64,7 @@ impl<'r> FaceBuilder<'r> { } /// Build the face - pub fn build(self) -> Handle { + pub fn build(self) -> Face { let surface = self.surface; let mut exteriors = Vec::new(); @@ -81,8 +81,6 @@ impl<'r> FaceBuilder<'r> { let color = self.color.unwrap_or([255, 0, 0, 255]); - self.shape.get_handle_or_insert(Face::new( - surface, exteriors, interiors, color, - )) + Face::new(surface, exteriors, interiors, color) } } diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 9f45d42b8..49756fe02 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -491,8 +491,7 @@ mod tests { let mut shape = Shape::new(); let face = Face::builder(Surface::xy_plane(), &mut shape) .with_exterior_polygon([[0., 0.], [1., 0.], [0., 1.]]) - .build() - .get(); + .build(); assert_eq!(3, face.curve_iter().count()); assert_eq!(1, face.cycle_iter().count()); diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 40678a332..26bc242fd 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -24,8 +24,7 @@ impl ToShape for fj::Sketch { let sketch = Face::builder(surface, &mut tmp) .with_exterior_polygon(points) .with_color(self.color()) - .build() - .get(); + .build(); validate(vec![sketch], config) } From 19724a3010ce6016e303e6187b65823a418f5a9a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:48:58 +0200 Subject: [PATCH 12/13] Remove unused struct field --- crates/fj-kernel/src/builder.rs | 15 ++++----------- crates/fj-kernel/src/objects/face.rs | 4 ++-- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/crates/fj-kernel/src/builder.rs b/crates/fj-kernel/src/builder.rs index acfb58a10..0d8744199 100644 --- a/crates/fj-kernel/src/builder.rs +++ b/crates/fj-kernel/src/builder.rs @@ -2,32 +2,25 @@ use fj_math::Point; -use crate::{ - objects::{Cycle, Face, Surface}, - shape::Shape, -}; +use crate::objects::{Cycle, Face, Surface}; /// API for building a [`Face`] #[must_use] -pub struct FaceBuilder<'r> { +pub struct FaceBuilder { surface: Surface, exterior: Option>>, interiors: Vec>>, color: Option<[u8; 4]>, - - _shape: &'r mut Shape, } -impl<'r> FaceBuilder<'r> { +impl FaceBuilder { /// Construct a new instance of `FaceBuilder` - pub fn new(surface: Surface, _shape: &'r mut Shape) -> Self { + pub fn new(surface: Surface) -> Self { Self { surface, exterior: None, interiors: Vec::new(), color: None, - - _shape, } } diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index acd462e66..ceae1f963 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -57,8 +57,8 @@ impl Face { }) } /// Build a face using the [`FaceBuilder`] API - pub fn builder(surface: Surface, shape: &mut Shape) -> FaceBuilder { - FaceBuilder::new(surface, shape) + pub fn builder(surface: Surface, _shape: &mut Shape) -> FaceBuilder { + FaceBuilder::new(surface) } /// Access the boundary representation of the face From 758fbe9308fc7fc0c93929697f1ba626cda354b9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 28 Jun 2022 18:50:38 +0200 Subject: [PATCH 13/13] Remove unused function argument --- crates/fj-kernel/src/algorithms/approx/faces.rs | 5 +---- crates/fj-kernel/src/algorithms/sweep.rs | 8 ++------ crates/fj-kernel/src/algorithms/triangulation/mod.rs | 9 ++------- crates/fj-kernel/src/iter.rs | 8 ++------ crates/fj-kernel/src/objects/face.rs | 7 ++----- crates/fj-operations/src/sketch.rs | 5 +---- 6 files changed, 10 insertions(+), 32 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/faces.rs b/crates/fj-kernel/src/algorithms/approx/faces.rs index ad977d0e4..54295316b 100644 --- a/crates/fj-kernel/src/algorithms/approx/faces.rs +++ b/crates/fj-kernel/src/algorithms/approx/faces.rs @@ -84,7 +84,6 @@ mod tests { use crate::{ geometry, objects::{Face, Surface}, - shape::Shape, }; use super::{CycleApprox, FaceApprox, Tolerance}; @@ -95,8 +94,6 @@ mod tests { let tolerance = Tolerance::from_scalar(Scalar::ONE)?; - let mut shape = Shape::new(); - let a = Point::from([0., 0.]); let b = Point::from([3., 0.]); let c = Point::from([3., 3.]); @@ -107,7 +104,7 @@ mod tests { let g = Point::from([2., 2.]); let h = Point::from([1., 2.]); - let face = Face::builder(Surface::xy_plane(), &mut shape) + let face = Face::builder(Surface::xy_plane()) .with_exterior_polygon([a, b, c, d]) .with_interior_polygon([e, f, g, h]) .build(); diff --git a/crates/fj-kernel/src/algorithms/sweep.rs b/crates/fj-kernel/src/algorithms/sweep.rs index 2ebc7a77b..4ffee264f 100644 --- a/crates/fj-kernel/src/algorithms/sweep.rs +++ b/crates/fj-kernel/src/algorithms/sweep.rs @@ -273,7 +273,6 @@ mod tests { algorithms::Tolerance, iter::ObjectIters, objects::{Face, Surface}, - shape::Shape, }; #[test] @@ -362,9 +361,7 @@ mod tests { ) -> anyhow::Result<()> { let tolerance = Tolerance::from_scalar(Scalar::ONE)?; - let mut shape = Shape::new(); - - let sketch = Face::builder(Surface::xy_plane(), &mut shape) + let sketch = Face::builder(Surface::xy_plane()) .with_exterior_polygon([[0., 0.], [1., 0.], [0., 1.]]) .build(); @@ -376,11 +373,10 @@ mod tests { .map(|vertex| vertex.into()) .collect(); - let mut shape = Shape::new(); let faces = expected_surfaces.into_iter().map(|surface| { let surface = Surface::plane_from_points(surface); - Face::builder(surface, &mut shape) + Face::builder(surface) .with_exterior_polygon(expected_vertices.clone()) .build() }); diff --git a/crates/fj-kernel/src/algorithms/triangulation/mod.rs b/crates/fj-kernel/src/algorithms/triangulation/mod.rs index 7ca4f6ff9..77a37ab07 100644 --- a/crates/fj-kernel/src/algorithms/triangulation/mod.rs +++ b/crates/fj-kernel/src/algorithms/triangulation/mod.rs @@ -89,19 +89,16 @@ mod tests { use crate::{ algorithms::Tolerance, objects::{Face, Surface}, - shape::Shape, }; #[test] fn simple() -> anyhow::Result<()> { - let mut shape = Shape::new(); - let a = [0., 0.]; let b = [2., 0.]; let c = [2., 2.]; let d = [0., 1.]; - let face = Face::builder(Surface::xy_plane(), &mut shape) + let face = Face::builder(Surface::xy_plane()) .with_exterior_polygon([a, b, c, d]) .build(); @@ -122,8 +119,6 @@ mod tests { #[test] fn simple_hole() -> anyhow::Result<()> { - let mut shape = Shape::new(); - let a = [0., 0.]; let b = [4., 0.]; let c = [4., 4.]; @@ -134,7 +129,7 @@ mod tests { let g = [3., 3.]; let h = [1., 2.]; - let face = Face::builder(Surface::xy_plane(), &mut shape) + let face = Face::builder(Surface::xy_plane()) .with_exterior_polygon([a, b, c, d]) .with_interior_polygon([e, f, g, h]) .build(); diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 49756fe02..f5c9f1cfa 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -439,10 +439,7 @@ impl Iterator for Iter { #[cfg(test)] mod tests { - use crate::{ - objects::{Curve, Cycle, Edge, Face, Surface, Vertex}, - shape::Shape, - }; + use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; use super::ObjectIters as _; @@ -488,8 +485,7 @@ mod tests { #[test] fn face() { - let mut shape = Shape::new(); - let face = Face::builder(Surface::xy_plane(), &mut shape) + let face = Face::builder(Surface::xy_plane()) .with_exterior_polygon([[0., 0.], [1., 0.], [0., 1.]]) .build(); diff --git a/crates/fj-kernel/src/objects/face.rs b/crates/fj-kernel/src/objects/face.rs index ceae1f963..e5bcee02a 100644 --- a/crates/fj-kernel/src/objects/face.rs +++ b/crates/fj-kernel/src/objects/face.rs @@ -3,10 +3,7 @@ use std::hash::{Hash, Hasher}; use fj_interop::mesh::Color; use fj_math::Triangle; -use crate::{ - builder::FaceBuilder, - shape::{LocalForm, Shape}, -}; +use crate::{builder::FaceBuilder, shape::LocalForm}; use super::{Cycle, Surface}; @@ -57,7 +54,7 @@ impl Face { }) } /// Build a face using the [`FaceBuilder`] API - pub fn builder(surface: Surface, _shape: &mut Shape) -> FaceBuilder { + pub fn builder(surface: Surface) -> FaceBuilder { FaceBuilder::new(surface) } diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 26bc242fd..8f032a553 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -2,7 +2,6 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::Tolerance, objects::{Face, Surface}, - shape::Shape, validation::{validate, Validated, ValidationConfig, ValidationError}, }; use fj_math::{Aabb, Point}; @@ -16,12 +15,10 @@ impl ToShape for fj::Sketch { _: Tolerance, _: &mut DebugInfo, ) -> Result>, ValidationError> { - let mut tmp = Shape::new(); - let surface = Surface::xy_plane(); let points = self.to_points().into_iter().map(Point::from); - let sketch = Face::builder(surface, &mut tmp) + let sketch = Face::builder(surface) .with_exterior_polygon(points) .with_color(self.color()) .build();