From f98a2f48cf4877bf0309afaed04934bcfad7c9c8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 23 Mar 2022 15:18:58 +0100 Subject: [PATCH 01/15] Add dependency on AnyMap to `fj-kernel` --- Cargo.lock | 7 +++++++ fj-kernel/Cargo.toml | 1 + 2 files changed, 8 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index d72d4d487..37b9c29cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -59,6 +59,12 @@ version = "1.0.56" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4361135be9122e0870de935d7c439aef945b9f9ddd4199a553b5270b49c82a27" +[[package]] +name = "anymap" +version = "1.0.0-beta.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f1f8f5a6f3d50d89e3797d7593a50f96bb2aaa20ca0cc7be1fb673232c91d72" + [[package]] name = "approx" version = "0.3.2" @@ -671,6 +677,7 @@ name = "fj-kernel" version = "0.5.0" dependencies = [ "anyhow", + "anymap", "approx 0.5.1", "fj-debug", "fj-math", diff --git a/fj-kernel/Cargo.toml b/fj-kernel/Cargo.toml index c3a7ae6d8..3db1e76ff 100644 --- a/fj-kernel/Cargo.toml +++ b/fj-kernel/Cargo.toml @@ -13,6 +13,7 @@ categories = ["mathematics"] [dependencies] anyhow = "1.0.56" +anymap = "1.0.0-beta.2" approx = "0.5.1" map-macro = "0.2.0" nalgebra = "0.30.0" From 069c1f04e889453ae6447e6e9299893f5aa10f0c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 31 Mar 2022 15:44:13 +0200 Subject: [PATCH 02/15] Silence Clippy warning For reasons unknown, this warning isn't actually emitted right now, but changes I'm working on are changing that. --- fj-kernel/src/shape/stores.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fj-kernel/src/shape/stores.rs b/fj-kernel/src/shape/stores.rs index 0fb63cfbd..2889b2292 100644 --- a/fj-kernel/src/shape/stores.rs +++ b/fj-kernel/src/shape/stores.rs @@ -98,6 +98,12 @@ impl Clone for Store { } } +impl Default for Store { + fn default() -> Self { + Self::new() + } +} + impl PartialEq for Store { fn eq(&self, other: &Self) -> bool { self.ptr().eq(&other.ptr()) From 45b6db50cc8eb0a80ff1ee2b63a15c0a70bb129f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 31 Mar 2022 15:45:21 +0200 Subject: [PATCH 03/15] Implement `Validate` for geometric objects They aren't validated right now, but this makes objects more uniform, paving the way for further cleanups. --- fj-kernel/src/shape/validate.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/fj-kernel/src/shape/validate.rs b/fj-kernel/src/shape/validate.rs index 492cb07fd..fc9b4f17d 100644 --- a/fj-kernel/src/shape/validate.rs +++ b/fj-kernel/src/shape/validate.rs @@ -1,6 +1,6 @@ use std::collections::HashSet; -use fj_math::Scalar; +use fj_math::{Point, Scalar}; use crate::{ geometry::{Curve, Surface}, @@ -17,6 +17,24 @@ pub trait Validate { ) -> Result<(), ValidationError>; } +impl Validate for Point<3> { + fn validate(&self, _: Scalar, _: &Stores) -> Result<(), ValidationError> { + Ok(()) + } +} + +impl Validate for Curve { + fn validate(&self, _: Scalar, _: &Stores) -> Result<(), ValidationError> { + Ok(()) + } +} + +impl Validate for Surface { + fn validate(&self, _: Scalar, _: &Stores) -> Result<(), ValidationError> { + Ok(()) + } +} + impl Validate for Vertex { fn validate( &self, From 6f05b99023a1b1fbf9cb3715d3f2e4ea93614c53 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 31 Mar 2022 15:47:02 +0200 Subject: [PATCH 04/15] Implement `Object` trait for all objects --- fj-kernel/src/shape/mod.rs | 2 ++ fj-kernel/src/shape/object.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 fj-kernel/src/shape/object.rs diff --git a/fj-kernel/src/shape/mod.rs b/fj-kernel/src/shape/mod.rs index a99f7772b..d3fc0d636 100644 --- a/fj-kernel/src/shape/mod.rs +++ b/fj-kernel/src/shape/mod.rs @@ -4,6 +4,7 @@ mod api; mod geometry; +mod object; mod stores; mod topology; mod validate; @@ -11,6 +12,7 @@ mod validate; pub use self::{ api::Shape, geometry::Geometry, + object::Object, stores::{Handle, Iter}, topology::Topology, validate::{StructuralIssues, ValidationError, ValidationResult}, diff --git a/fj-kernel/src/shape/object.rs b/fj-kernel/src/shape/object.rs new file mode 100644 index 000000000..059ab3728 --- /dev/null +++ b/fj-kernel/src/shape/object.rs @@ -0,0 +1,33 @@ +use fj_math::Point; + +use crate::{ + geometry::{Curve, Surface}, + topology::{Cycle, Edge, Face, Vertex}, +}; + +use super::validate::Validate; + +/// Marker trait for geometric and topological objects +pub trait Object: 'static + Validate + private::Sealed {} + +impl private::Sealed for Point<3> {} +impl private::Sealed for Curve {} +impl private::Sealed for Surface {} + +impl private::Sealed for Vertex {} +impl private::Sealed for Edge {} +impl private::Sealed for Cycle {} +impl private::Sealed for Face {} + +impl Object for Point<3> {} +impl Object for Curve {} +impl Object for Surface {} + +impl Object for Vertex {} +impl Object for Edge {} +impl Object for Cycle {} +impl Object for Face {} + +mod private { + pub trait Sealed {} +} From 82543277ce42e56c695aebde4cc115cf77b1373e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:00:49 +0200 Subject: [PATCH 05/15] Add implementation notes to validation code These come from the documentation of the various `add_*` methods on `Topology`, which will be removed soon. --- fj-kernel/src/shape/validate.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fj-kernel/src/shape/validate.rs b/fj-kernel/src/shape/validate.rs index fc9b4f17d..5e3c69f12 100644 --- a/fj-kernel/src/shape/validate.rs +++ b/fj-kernel/src/shape/validate.rs @@ -36,6 +36,12 @@ impl Validate for Surface { } impl Validate for Vertex { + /// Validate the vertex + /// + /// # Implementation note + /// + /// In the future, this method is likely to validate more than it already + /// does. See documentation of [`crate::kernel`] for some context on that. fn validate( &self, min_distance: Scalar, @@ -90,6 +96,14 @@ impl Validate for Edge { } impl Validate for Cycle { + /// Validate the cycle + /// + /// # Implementation note + /// + /// The validation of the cycle should be extended to cover more cases: + /// - That those edges form a cycle. + /// - That the cycle is not self-overlapping. + /// - That there exists no duplicate cycle, with the same edges. fn validate( &self, _: Scalar, From 4df5b182ecb826ae289a5e441372dd56e396269e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:05:56 +0200 Subject: [PATCH 06/15] Document validation requirements of objects This duplicates the documentation of the various `add_*` methods, which are about to be removed. --- fj-kernel/src/topology/edges.rs | 11 +++++++++++ fj-kernel/src/topology/faces.rs | 5 +++++ fj-kernel/src/topology/vertices.rs | 11 +++++++++++ 3 files changed, 27 insertions(+) diff --git a/fj-kernel/src/topology/edges.rs b/fj-kernel/src/topology/edges.rs index 50c2f4c7f..7871481ba 100644 --- a/fj-kernel/src/topology/edges.rs +++ b/fj-kernel/src/topology/edges.rs @@ -17,6 +17,11 @@ use super::{vertices::Vertex, EdgeBuilder}; /// /// Please refer to [`crate::kernel::topology`] for documentation on the /// equality of topological objects. +/// +/// # Validation +/// +/// A cycle that is part of a [`Shape`] must be structurally sound. That means +/// the edges it refers to, must be part of the same shape. #[derive(Clone, Debug, Eq, Ord, PartialOrd)] pub struct Cycle { /// The edges that make up the cycle @@ -53,6 +58,12 @@ impl Hash for Cycle { /// /// Please refer to [`crate::kernel::topology`] for documentation on the /// equality of topological objects. +/// +/// # Validation +/// +/// An edge that is part of a [`Shape`] must be structurally sound. That means +/// the curve and any vertices that it refers to, must be part of the same +/// shape. #[derive(Clone, Debug, Eq, Ord, PartialOrd)] pub struct Edge { /// Access the curve that defines the edge's geometry diff --git a/fj-kernel/src/topology/faces.rs b/fj-kernel/src/topology/faces.rs index edbe0f2a0..62d600098 100644 --- a/fj-kernel/src/topology/faces.rs +++ b/fj-kernel/src/topology/faces.rs @@ -12,6 +12,11 @@ use super::edges::Cycle; /// /// Please refer to [`crate::kernel::topology`] for documentation on the /// equality of topological objects. +/// +/// # Validation +/// +/// A face that is part of a [`Shape`] must be structurally sound. That means +/// the surface and any cycles it refers to, must be part of the same shape. #[derive(Clone, Debug, Eq, Ord, PartialOrd)] pub enum Face { /// A face of a shape diff --git a/fj-kernel/src/topology/vertices.rs b/fj-kernel/src/topology/vertices.rs index fc4795826..814bd6c1e 100644 --- a/fj-kernel/src/topology/vertices.rs +++ b/fj-kernel/src/topology/vertices.rs @@ -19,6 +19,17 @@ use super::VertexBuilder; /// /// Please refer to [`crate::kernel::topology`] for documentation on the /// equality of topological objects. +/// +/// # Validation +/// +/// A vertex that is part of a [`Shape`] must be structurally sound. That means +/// the point it refers to must be part of the same shape. +/// +/// Vertices must be unique within a shape, meaning another vertex defined by +/// the same shape must not exist. In the context of vertex uniqueness, points +/// that are close to each other are considered identical. The minimum distance +/// between distinct vertices can be configured using +/// [`Shape::with_minimum_distance`]. #[derive(Clone, Debug, Eq, Ord, PartialOrd)] pub struct Vertex { /// The point that defines the location of the vertex From d4ebee3a7d141c48975a45e1e20f4efee81163b3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:07:11 +0200 Subject: [PATCH 07/15] Add `Shape::insert` --- fj-kernel/src/shape/api.rs | 15 ++++++++++++++- fj-kernel/src/shape/stores.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/fj-kernel/src/shape/api.rs b/fj-kernel/src/shape/api.rs index 95a09fd9c..659a93d42 100644 --- a/fj-kernel/src/shape/api.rs +++ b/fj-kernel/src/shape/api.rs @@ -6,7 +6,7 @@ use super::{ stores::{ Curves, Cycles, Edges, Faces, Points, Stores, Surfaces, Vertices, }, - Geometry, Topology, + Geometry, Object, Topology, ValidationResult, }; /// The boundary representation of a shape @@ -55,6 +55,19 @@ impl Shape { self } + /// Insert an object into the shape + /// + /// Validates the object, and returns an error if it is not valid. See the + /// documentation of each object for validation requirements. + pub fn insert(&mut self, object: T) -> ValidationResult + where + T: Object, + { + object.validate(self.min_distance, &self.stores)?; + let handle = self.stores.get::().insert(object); + Ok(handle) + } + /// Access the shape's geometry pub fn geometry(&mut self) -> Geometry { Geometry { diff --git a/fj-kernel/src/shape/stores.rs b/fj-kernel/src/shape/stores.rs index 2889b2292..a612d8d64 100644 --- a/fj-kernel/src/shape/stores.rs +++ b/fj-kernel/src/shape/stores.rs @@ -3,6 +3,7 @@ use std::{ sync::Arc, }; +use anymap::AnyMap; use fj_math::Point; use parking_lot::{RwLock, RwLockReadGuard}; use slotmap::{DefaultKey, SlotMap}; @@ -12,6 +13,8 @@ use crate::{ topology::{Cycle, Edge, Face, Vertex}, }; +use super::Object; + #[derive(Clone, Debug)] pub struct Stores { pub points: Points, @@ -24,6 +27,30 @@ pub struct Stores { pub faces: Faces, } +impl Stores { + pub fn get(&mut self) -> Store + where + T: Object, + { + let mut stores = AnyMap::new(); + + stores.insert(self.points.clone()); + stores.insert(self.curves.clone()); + stores.insert(self.surfaces.clone()); + + stores.insert(self.vertices.clone()); + stores.insert(self.edges.clone()); + stores.insert(self.cycles.clone()); + stores.insert(self.faces.clone()); + + stores + .remove::>() + // Can't panic, as `T` is bound by `Object`, and we added the stores + // for all types of objects above. + .expect("Invalid object type") + } +} + pub type Points = Store>; pub type Curves = Store; pub type Surfaces = Store; From 59610358441df5bc409c8582a9536a2c903b33d2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:11:58 +0200 Subject: [PATCH 08/15] Replace `Geometry::add_point` with `Shape::insert` --- fj-kernel/src/algorithms/sweep.rs | 11 +++++------ fj-kernel/src/shape/geometry.rs | 5 ----- fj-kernel/src/shape/topology.rs | 10 +++++----- fj-kernel/src/topology/builder.rs | 2 +- fj-operations/src/difference_2d.rs | 2 +- fj-operations/src/group.rs | 2 +- fj-operations/src/sketch.rs | 2 +- 7 files changed, 14 insertions(+), 20 deletions(-) diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index 8cb267a2c..e229a25eb 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -26,9 +26,8 @@ pub fn sweep_shape( // Create the new vertices. for vertex_source in source.topology().vertices() { - let point_bottom = - target.geometry().add_point(vertex_source.get().point()); - let point_top = target.geometry().add_point(point_bottom.get() + path); + let point_bottom = target.insert(vertex_source.get().point()).unwrap(); + let point_top = target.insert(point_bottom.get() + path).unwrap(); let vertex_bottom = target .topology() @@ -401,9 +400,9 @@ mod tests { fn new([a, b, c]: [impl Into>; 3]) -> anyhow::Result { let mut shape = Shape::new(); - let a = shape.geometry().add_point(a.into()); - let b = shape.geometry().add_point(b.into()); - let c = shape.geometry().add_point(c.into()); + let a = shape.insert(a.into())?; + let b = shape.insert(b.into())?; + let c = shape.insert(c.into())?; let a = shape.topology().add_vertex(Vertex { point: a })?; let b = shape.topology().add_vertex(Vertex { point: b })?; diff --git a/fj-kernel/src/shape/geometry.rs b/fj-kernel/src/shape/geometry.rs index 007e94422..f93cedcf4 100644 --- a/fj-kernel/src/shape/geometry.rs +++ b/fj-kernel/src/shape/geometry.rs @@ -41,11 +41,6 @@ pub struct Geometry<'r> { } impl Geometry<'_> { - /// Add a point to the shape - pub fn add_point(&mut self, point: Point<3>) -> Handle> { - self.points.insert(point) - } - /// Add a curve to the shape pub fn add_curve(&mut self, curve: Curve) -> Handle { self.curves.insert(curve) diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index 3cad423ae..2d654e917 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -138,23 +138,23 @@ mod tests { let mut shape = Shape::new().with_min_distance(MIN_DISTANCE); let mut other = Shape::new(); - let point = shape.geometry().add_point(Point::from([0., 0., 0.])); + let point = shape.insert(Point::from([0., 0., 0.]))?; shape.topology().add_vertex(Vertex { point })?; // Should fail, as `point` is not part of the shape. - let point = other.geometry().add_point(Point::from([1., 0., 0.])); + let point = other.insert(Point::from([1., 0., 0.]))?; let result = shape.topology().add_vertex(Vertex { point }); assert!(matches!(result, Err(ValidationError::Structural(_)))); // `point` is too close to the original point. `assert!` is commented, // because that only causes a warning to be logged right now. - let point = shape.geometry().add_point(Point::from([5e-8, 0., 0.])); + let point = shape.insert(Point::from([5e-8, 0., 0.]))?; let result = shape.topology().add_vertex(Vertex { point }); assert!(matches!(result, Err(ValidationError::Uniqueness))); // `point` is farther than `MIN_DISTANCE` away from original point. // Should work. - let point = shape.geometry().add_point(Point::from([5e-6, 0., 0.])); + let point = shape.insert(Point::from([5e-6, 0., 0.]))?; shape.topology().add_vertex(Vertex { point })?; Ok(()) @@ -277,7 +277,7 @@ mod tests { let point = self.next_point; self.next_point.x += Scalar::ONE; - let point = self.geometry().add_point(point); + let point = self.insert(point).unwrap(); self.topology().add_vertex(Vertex { point }).unwrap() }); let edge = Edge::build(&mut self.inner) diff --git a/fj-kernel/src/topology/builder.rs b/fj-kernel/src/topology/builder.rs index 3a65b43b5..2fe73c09a 100644 --- a/fj-kernel/src/topology/builder.rs +++ b/fj-kernel/src/topology/builder.rs @@ -23,7 +23,7 @@ impl<'r> VertexBuilder<'r> { self, point: impl Into>, ) -> ValidationResult { - let point = self.shape.geometry().add_point(point.into()); + let point = self.shape.insert(point.into())?; let vertex = self.shape.topology().add_vertex(Vertex { point })?; Ok(vertex) diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index bf0cd58fe..dafa15394 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -98,7 +98,7 @@ fn add_cycle( vertices .entry(vertex.clone()) .or_insert_with(|| { - let point = shape.geometry().add_point(vertex.point()); + let point = shape.insert(vertex.point()).unwrap(); shape.topology().add_vertex(Vertex { point }).unwrap() }) .clone() diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index f074f9495..218434116 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -40,7 +40,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { let mut cycles = HashMap::new(); for point_orig in orig.geometry().points() { - let point = target.geometry().add_point(point_orig.get()); + let point = target.insert(point_orig.get()).unwrap(); points.insert(point_orig, point); } for curve_orig in orig.geometry().curves() { diff --git a/fj-operations/src/sketch.rs b/fj-operations/src/sketch.rs index 47f5d8cbc..247db1766 100644 --- a/fj-operations/src/sketch.rs +++ b/fj-operations/src/sketch.rs @@ -14,7 +14,7 @@ impl ToShape for fj::Sketch { let mut vertices = Vec::new(); for [x, y] in self.to_points() { - let point = shape.geometry().add_point(Point::from([x, y, 0.])); + let point = shape.insert(Point::from([x, y, 0.])).unwrap(); let vertex = shape.topology().add_vertex(Vertex { point }).unwrap(); vertices.push(vertex); } From e83696259a1d09cb31ad960b7e45fb3b40e4d28d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:14:21 +0200 Subject: [PATCH 09/15] Replace `Geometry::add_curve` with `Shape::insert` --- fj-kernel/src/algorithms/sweep.rs | 11 +++++------ fj-kernel/src/shape/geometry.rs | 5 ----- fj-kernel/src/shape/topology.rs | 2 +- fj-kernel/src/topology/builder.rs | 13 +++++-------- fj-operations/src/difference_2d.rs | 2 +- fj-operations/src/group.rs | 2 +- 6 files changed, 13 insertions(+), 22 deletions(-) diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index e229a25eb..aeae07044 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -48,11 +48,10 @@ pub fn sweep_shape( // Create the new edges. for edge_source in source.topology().edges() { - let curve_bottom = - target.geometry().add_curve(edge_source.get().curve()); + let curve_bottom = target.insert(edge_source.get().curve()).unwrap(); let curve_top = target - .geometry() - .add_curve(curve_bottom.get().transform(&translation)); + .insert(curve_bottom.get().transform(&translation)) + .unwrap(); let vertices_bottom = source_to_bottom.vertices_for_edge(&edge_source); let vertices_top = source_to_top.vertices_for_edge(&edge_source); @@ -207,8 +206,8 @@ pub fn sweep_shape( .entry(vertex_bottom.clone()) .or_insert_with(|| { let curve = target - .geometry() - .add_curve(edge_source.get().curve()); + .insert(edge_source.get().curve()) + .unwrap(); let vertex_top = source_to_top .vertices diff --git a/fj-kernel/src/shape/geometry.rs b/fj-kernel/src/shape/geometry.rs index f93cedcf4..e29c6f637 100644 --- a/fj-kernel/src/shape/geometry.rs +++ b/fj-kernel/src/shape/geometry.rs @@ -41,11 +41,6 @@ pub struct Geometry<'r> { } impl Geometry<'_> { - /// Add a curve to the shape - pub fn add_curve(&mut self, curve: Curve) -> Handle { - self.curves.insert(curve) - } - /// Add a surface to the shape pub fn add_surface(&mut self, surface: Surface) -> Handle { self.surfaces.insert(surface) diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index 2d654e917..358a06d33 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -265,7 +265,7 @@ mod tests { } fn add_curve(&mut self) -> Handle { - self.geometry().add_curve(Curve::x_axis()) + self.insert(Curve::x_axis()).unwrap() } fn add_surface(&mut self) -> Handle { diff --git a/fj-kernel/src/topology/builder.rs b/fj-kernel/src/topology/builder.rs index 2fe73c09a..def23ee8d 100644 --- a/fj-kernel/src/topology/builder.rs +++ b/fj-kernel/src/topology/builder.rs @@ -43,10 +43,10 @@ impl<'r> EdgeBuilder<'r> { /// Build a circle from a radius pub fn circle(self, radius: Scalar) -> ValidationResult { - let curve = self.shape.geometry().add_curve(Curve::Circle(Circle { + let curve = self.shape.insert(Curve::Circle(Circle { center: Point::origin(), radius: Vector::from([radius, Scalar::ZERO]), - })); + }))?; let edge = self.shape.topology().add_edge(Edge { curve, vertices: None, @@ -60,12 +60,9 @@ impl<'r> EdgeBuilder<'r> { self, vertices: [Handle; 2], ) -> ValidationResult { - let curve = - self.shape - .geometry() - .add_curve(Curve::Line(Line::from_points( - vertices.clone().map(|vertex| vertex.get().point()), - ))); + let curve = self.shape.insert(Curve::Line(Line::from_points( + vertices.clone().map(|vertex| vertex.get().point()), + )))?; let edge = self.shape.topology().add_edge(Edge { curve, vertices: Some(vertices), diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index dafa15394..7f10f816b 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -91,7 +91,7 @@ fn add_cycle( ) -> Handle { let mut edges = Vec::new(); for edge in cycle.get().edges() { - let curve = shape.geometry().add_curve(edge.curve()); + let curve = shape.insert(edge.curve()).unwrap(); let vertices = edge.vertices().clone().map(|vs| { vs.map(|vertex| { diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 218434116..1537a5a61 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -44,7 +44,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { points.insert(point_orig, point); } for curve_orig in orig.geometry().curves() { - let curve = target.geometry().add_curve(curve_orig.get()); + let curve = target.insert(curve_orig.get()).unwrap(); curves.insert(curve_orig, curve); } for surface_orig in orig.geometry().surfaces() { From 222bbc40cf62f2e0e937da16c9eaa8ba8e4d578c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:17:31 +0200 Subject: [PATCH 10/15] Replace `add_surface` with `Shape::insert` --- fj-kernel/src/algorithms/approximation.rs | 2 +- fj-kernel/src/algorithms/sweep.rs | 19 +++++++++---------- fj-kernel/src/shape/geometry.rs | 7 +------ fj-kernel/src/shape/topology.rs | 2 +- fj-operations/src/circle.rs | 2 +- fj-operations/src/difference_2d.rs | 2 +- fj-operations/src/group.rs | 2 +- fj-operations/src/sketch.rs | 2 +- 8 files changed, 16 insertions(+), 22 deletions(-) diff --git a/fj-kernel/src/algorithms/approximation.rs b/fj-kernel/src/algorithms/approximation.rs index 514e2e9f6..be105b297 100644 --- a/fj-kernel/src/algorithms/approximation.rs +++ b/fj-kernel/src/algorithms/approximation.rs @@ -180,7 +180,7 @@ mod tests { edges: vec![ab, bc, cd, da], })?; - let surface = shape.geometry().add_surface(Surface::x_y_plane()); + let surface = shape.insert(Surface::x_y_plane())?; let face = Face::Face { surface, exteriors: vec![abcd], diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index aeae07044..451bef6c0 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -101,11 +101,10 @@ pub fn sweep_shape( // Create top faces. for face_source in source.topology().faces().values() { - let surface_bottom = - target.geometry().add_surface(face_source.surface()); + let surface_bottom = target.insert(face_source.surface()).unwrap(); let surface_top = target - .geometry() - .add_surface(surface_bottom.get().transform(&translation)); + .insert(surface_bottom.get().transform(&translation)) + .unwrap(); let exteriors_bottom = source_to_bottom.exteriors_for_face(&face_source); @@ -237,12 +236,12 @@ pub fn sweep_shape( let top_edge = source_to_top.edges.get(edge_source).unwrap().clone(); - let surface = target.geometry().add_surface( - Surface::SweptCurve(SweptCurve { + let surface = target + .insert(Surface::SweptCurve(SweptCurve { curve: bottom_edge.get().curve(), path, - }), - ); + })) + .unwrap(); let cycle = target .topology() @@ -418,11 +417,11 @@ mod tests { edges: vec![ab, bc, ca], })?; - let surface = shape.geometry().add_surface(Surface::SweptCurve( + let surface = shape.insert(Surface::SweptCurve( SweptCurve::plane_from_points( [a, b, c].map(|vertex| vertex.get().point()), ), - )); + ))?; let abc = Face::Face { surface, exteriors: vec![cycles], diff --git a/fj-kernel/src/shape/geometry.rs b/fj-kernel/src/shape/geometry.rs index e29c6f637..9875d5de4 100644 --- a/fj-kernel/src/shape/geometry.rs +++ b/fj-kernel/src/shape/geometry.rs @@ -7,7 +7,7 @@ use crate::{ use super::{ stores::{Curves, Faces, Points, Surfaces}, - Handle, Iter, + Iter, }; /// API to access a shape's geometry @@ -41,11 +41,6 @@ pub struct Geometry<'r> { } impl Geometry<'_> { - /// Add a surface to the shape - pub fn add_surface(&mut self, surface: Surface) -> Handle { - self.surfaces.insert(surface) - } - /// Transform the geometry of the shape /// /// Since the topological types refer to geometry, and don't contain any diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index 358a06d33..ff1881b02 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -269,7 +269,7 @@ mod tests { } fn add_surface(&mut self) -> Handle { - self.geometry().add_surface(Surface::x_y_plane()) + self.insert(Surface::x_y_plane()).unwrap() } fn add_edge(&mut self) -> anyhow::Result> { diff --git a/fj-operations/src/circle.rs b/fj-operations/src/circle.rs index 705e7b075..9dadbfc91 100644 --- a/fj-operations/src/circle.rs +++ b/fj-operations/src/circle.rs @@ -24,7 +24,7 @@ impl ToShape for fj::Circle { .unwrap(); let cycles = shape.topology().cycles().collect(); - let surface = shape.geometry().add_surface(Surface::x_y_plane()); + let surface = shape.insert(Surface::x_y_plane()).unwrap(); shape .topology() .add_face(Face::Face { diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index 7f10f816b..b79c32253 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -61,7 +61,7 @@ impl ToShape for fj::Difference2d { face_a.surface() == face_b.surface(), "Trying to subtract sketches with different surfaces." ); - let surface = shape.geometry().add_surface(face_a.surface()); + let surface = shape.insert(face_a.surface()).unwrap(); shape .topology() diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 1537a5a61..9d05c3fe3 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -48,7 +48,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { curves.insert(curve_orig, curve); } for surface_orig in orig.geometry().surfaces() { - let surface = target.geometry().add_surface(surface_orig.get()); + let surface = target.insert(surface_orig.get()).unwrap(); surfaces.insert(surface_orig, surface); } diff --git a/fj-operations/src/sketch.rs b/fj-operations/src/sketch.rs index 247db1766..c1abfe28a 100644 --- a/fj-operations/src/sketch.rs +++ b/fj-operations/src/sketch.rs @@ -45,7 +45,7 @@ impl ToShape for fj::Sketch { shape.topology().add_cycle(Cycle { edges }).unwrap(); }; - let surface = shape.geometry().add_surface(Surface::x_y_plane()); + let surface = shape.insert(Surface::x_y_plane()).unwrap(); let face = Face::Face { exteriors: shape.topology().cycles().collect(), interiors: Vec::new(), From 360927a97990edf7f7ba5e5fe75142d0df69e2b8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:18:24 +0200 Subject: [PATCH 11/15] Remove obsolete documentation --- fj-kernel/src/shape/geometry.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/fj-kernel/src/shape/geometry.rs b/fj-kernel/src/shape/geometry.rs index 9875d5de4..2e55dc7d6 100644 --- a/fj-kernel/src/shape/geometry.rs +++ b/fj-kernel/src/shape/geometry.rs @@ -11,20 +11,6 @@ use super::{ }; /// API to access a shape's geometry -/// -/// Other than topology, geometry doesn't need to be validated. Hence adding -/// geometry is infallible. -/// -/// There are several reasons for this: -/// - Geometry doesn't refer to other objects, so structural validation doesn't -/// apply. -/// - There simply no reason that geometry needs to be unique. In addition, it's -/// probably quite hard to rule out generating duplicate geometry. Think about -/// line segment edges that are on identical lines, but are created -/// separately. -/// - Geometric validation doesn't apply either. It simply doesn't matter, if -/// curves or surfaces intersect, for example, as long as they don't do that -/// where an edge or face is defined. pub struct Geometry<'r> { pub(super) points: &'r mut Points, pub(super) curves: &'r mut Curves, From 359ff019dee91bead903930b3310db439e30a7c2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:20:26 +0200 Subject: [PATCH 12/15] Replace `add_vertex` with `Shape::insert` --- fj-kernel/src/algorithms/sweep.rs | 14 ++++------- fj-kernel/src/shape/topology.rs | 38 ++++-------------------------- fj-kernel/src/topology/builder.rs | 2 +- fj-operations/src/difference_2d.rs | 2 +- fj-operations/src/group.rs | 3 +-- fj-operations/src/sketch.rs | 2 +- 6 files changed, 14 insertions(+), 47 deletions(-) diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index 451bef6c0..62763c3c8 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -30,15 +30,11 @@ pub fn sweep_shape( let point_top = target.insert(point_bottom.get() + path).unwrap(); let vertex_bottom = target - .topology() - .add_vertex(Vertex { + .insert(Vertex { point: point_bottom, }) .unwrap(); - let vertex_top = target - .topology() - .add_vertex(Vertex { point: point_top }) - .unwrap(); + let vertex_top = target.insert(Vertex { point: point_top }).unwrap(); source_to_bottom .vertices @@ -402,9 +398,9 @@ mod tests { let b = shape.insert(b.into())?; let c = shape.insert(c.into())?; - let a = shape.topology().add_vertex(Vertex { point: a })?; - let b = shape.topology().add_vertex(Vertex { point: b })?; - let c = shape.topology().add_vertex(Vertex { point: c })?; + let a = shape.insert(Vertex { point: a })?; + let b = shape.insert(Vertex { point: b })?; + let c = shape.insert(Vertex { point: c })?; let ab = Edge::build(&mut shape) .line_segment_from_vertices([a.clone(), b.clone()])?; diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index ff1881b02..468c327f2 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -14,34 +14,6 @@ pub struct Topology<'r> { } impl Topology<'_> { - /// Add a vertex to the shape - /// - /// Validates that the vertex is structurally sound (i.e. the point it - /// refers to is part of the shape). Returns an error, if that is not the - /// case. - /// - /// Logs a warning, if the vertex is not unique, meaning if another vertex - /// defined by the same point already exists. - /// - /// In the context of of vertex uniqueness, points that are close to each - /// other are considered identical. The minimum distance between distinct - /// vertices can be configured using [`Shape::with_minimum_distance`]. - /// - /// # Implementation note - /// - /// This method is intended to actually validate vertex uniqueness: To - /// panic, if duplicate vertices are found. This is currently not possible, - /// as the presence of bugs in the sweep and transform code would basically - /// break ever model, due to validation errors. - /// - /// In the future, this method is likely to validate more than it already - /// does. See documentation of [`crate::kernel`] for some context on that. - pub fn add_vertex(&mut self, vertex: Vertex) -> ValidationResult { - vertex.validate(self.min_distance, &self.stores)?; - let handle = self.stores.vertices.insert(vertex); - Ok(handle) - } - /// Add an edge to the shape /// /// Validates that the edge is structurally sound (i.e. the curve and @@ -139,23 +111,23 @@ mod tests { let mut other = Shape::new(); let point = shape.insert(Point::from([0., 0., 0.]))?; - shape.topology().add_vertex(Vertex { point })?; + shape.insert(Vertex { point })?; // Should fail, as `point` is not part of the shape. let point = other.insert(Point::from([1., 0., 0.]))?; - let result = shape.topology().add_vertex(Vertex { point }); + let result = shape.insert(Vertex { point }); assert!(matches!(result, Err(ValidationError::Structural(_)))); // `point` is too close to the original point. `assert!` is commented, // because that only causes a warning to be logged right now. let point = shape.insert(Point::from([5e-8, 0., 0.]))?; - let result = shape.topology().add_vertex(Vertex { point }); + let result = shape.insert(Vertex { point }); assert!(matches!(result, Err(ValidationError::Uniqueness))); // `point` is farther than `MIN_DISTANCE` away from original point. // Should work. let point = shape.insert(Point::from([5e-6, 0., 0.]))?; - shape.topology().add_vertex(Vertex { point })?; + shape.insert(Vertex { point })?; Ok(()) } @@ -278,7 +250,7 @@ mod tests { self.next_point.x += Scalar::ONE; let point = self.insert(point).unwrap(); - self.topology().add_vertex(Vertex { point }).unwrap() + self.insert(Vertex { point }).unwrap() }); let edge = Edge::build(&mut self.inner) .line_segment_from_vertices(vertices)?; diff --git a/fj-kernel/src/topology/builder.rs b/fj-kernel/src/topology/builder.rs index def23ee8d..431f052f4 100644 --- a/fj-kernel/src/topology/builder.rs +++ b/fj-kernel/src/topology/builder.rs @@ -24,7 +24,7 @@ impl<'r> VertexBuilder<'r> { point: impl Into>, ) -> ValidationResult { let point = self.shape.insert(point.into())?; - let vertex = self.shape.topology().add_vertex(Vertex { point })?; + let vertex = self.shape.insert(Vertex { point })?; Ok(vertex) } diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index b79c32253..407f466f2 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -99,7 +99,7 @@ fn add_cycle( .entry(vertex.clone()) .or_insert_with(|| { let point = shape.insert(vertex.point()).unwrap(); - shape.topology().add_vertex(Vertex { point }).unwrap() + shape.insert(Vertex { point }).unwrap() }) .clone() }) diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 9d05c3fe3..03c263000 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -54,8 +54,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { for vertex_orig in orig.topology().vertices() { let vertex = target - .topology() - .add_vertex(Vertex { + .insert(Vertex { point: points[&vertex_orig.get().point].clone(), }) .unwrap(); diff --git a/fj-operations/src/sketch.rs b/fj-operations/src/sketch.rs index c1abfe28a..aa8e5515d 100644 --- a/fj-operations/src/sketch.rs +++ b/fj-operations/src/sketch.rs @@ -15,7 +15,7 @@ impl ToShape for fj::Sketch { for [x, y] in self.to_points() { let point = shape.insert(Point::from([x, y, 0.])).unwrap(); - let vertex = shape.topology().add_vertex(Vertex { point }).unwrap(); + let vertex = shape.insert(Vertex { point }).unwrap(); vertices.push(vertex); } From 69409a6210caf91e0d1696d64f6303b0544d758e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:22:02 +0200 Subject: [PATCH 13/15] Replace `Topology::add_edge` with `Shape::insert` --- fj-kernel/src/algorithms/sweep.rs | 9 +++------ fj-kernel/src/shape/topology.rs | 25 ++----------------------- fj-kernel/src/topology/builder.rs | 4 ++-- fj-operations/src/difference_2d.rs | 2 +- fj-operations/src/group.rs | 3 +-- 5 files changed, 9 insertions(+), 34 deletions(-) diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index 62763c3c8..f86e71140 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -53,15 +53,13 @@ pub fn sweep_shape( let vertices_top = source_to_top.vertices_for_edge(&edge_source); let edge_bottom = target - .topology() - .add_edge(Edge { + .insert(Edge { curve: curve_bottom, vertices: vertices_bottom, }) .unwrap(); let edge_top = target - .topology() - .add_edge(Edge { + .insert(Edge { curve: curve_top, vertices: vertices_top, }) @@ -211,8 +209,7 @@ pub fn sweep_shape( .clone(); target - .topology() - .add_edge(Edge { + .insert(Edge { curve, vertices: Some([ vertex_bottom, diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index 468c327f2..e6a612000 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -14,26 +14,6 @@ pub struct Topology<'r> { } impl Topology<'_> { - /// Add an edge to the shape - /// - /// Validates that the edge is structurally sound (i.e. the curve and - /// vertices it refers to are part of the shape). Returns an error, if that - /// is not the case. - /// - /// # Vertices - /// - /// If vertices are provided in `vertices`, they must be on `curve`. - /// - /// This constructor will convert the vertices into curve coordinates. If - /// they are not on the curve, this will result in their projection being - /// converted into curve coordinates, which is likely not the caller's - /// intention. - pub fn add_edge(&mut self, edge: Edge) -> ValidationResult { - edge.validate(self.min_distance, &self.stores)?; - let handle = self.stores.edges.insert(edge); - Ok(handle) - } - /// Add a cycle to the shape /// /// Validates that the cycle is structurally sound (i.e. the edges it refers @@ -143,8 +123,7 @@ mod tests { // Shouldn't work. Nothing has been added to `shape`. let err = shape - .topology() - .add_edge(Edge { + .insert(Edge { curve: curve.clone(), vertices: Some([a.clone(), b.clone()]), }) @@ -158,7 +137,7 @@ mod tests { let b = Vertex::build(&mut shape).from_point([2., 0., 0.])?; // Everything has been added to `shape` now. Should work! - shape.topology().add_edge(Edge { + shape.insert(Edge { curve, vertices: Some([a, b]), })?; diff --git a/fj-kernel/src/topology/builder.rs b/fj-kernel/src/topology/builder.rs index 431f052f4..7b55db4e0 100644 --- a/fj-kernel/src/topology/builder.rs +++ b/fj-kernel/src/topology/builder.rs @@ -47,7 +47,7 @@ impl<'r> EdgeBuilder<'r> { center: Point::origin(), radius: Vector::from([radius, Scalar::ZERO]), }))?; - let edge = self.shape.topology().add_edge(Edge { + let edge = self.shape.insert(Edge { curve, vertices: None, })?; @@ -63,7 +63,7 @@ impl<'r> EdgeBuilder<'r> { let curve = self.shape.insert(Curve::Line(Line::from_points( vertices.clone().map(|vertex| vertex.get().point()), )))?; - let edge = self.shape.topology().add_edge(Edge { + let edge = self.shape.insert(Edge { curve, vertices: Some(vertices), })?; diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index 407f466f2..106691e23 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -105,7 +105,7 @@ fn add_cycle( }) }); - let edge = shape.topology().add_edge(Edge { curve, vertices }).unwrap(); + let edge = shape.insert(Edge { curve, vertices }).unwrap(); edges.push(edge); } diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 03c263000..0ccd59f85 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -62,8 +62,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { } for edge_orig in orig.topology().edges() { let edge = target - .topology() - .add_edge(Edge { + .insert(Edge { curve: curves[&edge_orig.get().curve].clone(), vertices: edge_orig.get().vertices.as_ref().map(|vs| { vs.clone().map(|vertex| vertices[&vertex].clone()) From d657ecec4addae3fc1b51c4e28b29494e1f58021 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:23:58 +0200 Subject: [PATCH 14/15] Replace `Topology::add_cycle` with `Shape::insert` --- fj-kernel/src/algorithms/approximation.rs | 2 +- fj-kernel/src/algorithms/sweep.rs | 13 ++++-------- fj-kernel/src/shape/topology.rs | 25 +++-------------------- fj-operations/src/circle.rs | 5 +---- fj-operations/src/difference_2d.rs | 2 +- fj-operations/src/group.rs | 3 +-- fj-operations/src/sketch.rs | 2 +- 7 files changed, 12 insertions(+), 40 deletions(-) diff --git a/fj-kernel/src/algorithms/approximation.rs b/fj-kernel/src/algorithms/approximation.rs index be105b297..bb5e5b42e 100644 --- a/fj-kernel/src/algorithms/approximation.rs +++ b/fj-kernel/src/algorithms/approximation.rs @@ -176,7 +176,7 @@ mod tests { let da = Edge::build(&mut shape).line_segment_from_vertices([v4, v1])?; - let abcd = shape.topology().add_cycle(Cycle { + let abcd = shape.insert(Cycle { edges: vec![ab, bc, cd, da], })?; diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index f86e71140..8ed99213a 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -77,15 +77,11 @@ pub fn sweep_shape( let edges_top = source_to_top.edges_for_cycle(&cycle_source); let cycle_bottom = target - .topology() - .add_cycle(Cycle { + .insert(Cycle { edges: edges_bottom, }) .unwrap(); - let cycle_top = target - .topology() - .add_cycle(Cycle { edges: edges_top }) - .unwrap(); + let cycle_top = target.insert(Cycle { edges: edges_top }).unwrap(); source_to_bottom .cycles @@ -237,8 +233,7 @@ pub fn sweep_shape( .unwrap(); let cycle = target - .topology() - .add_cycle(Cycle { + .insert(Cycle { edges: vec![ bottom_edge, top_edge, @@ -406,7 +401,7 @@ mod tests { let ca = Edge::build(&mut shape) .line_segment_from_vertices([c.clone(), a.clone()])?; - let cycles = shape.topology().add_cycle(Cycle { + let cycles = shape.insert(Cycle { edges: vec![ab, bc, ca], })?; diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index e6a612000..e08638bed 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -14,23 +14,6 @@ pub struct Topology<'r> { } impl Topology<'_> { - /// Add a cycle to the shape - /// - /// Validates that the cycle is structurally sound (i.e. the edges it refers - /// to are part of the shape). Returns an error, if that is not the case. - /// - /// # Implementation note - /// - /// The validation of the cycle should be extended to cover more cases: - /// - That those edges form a cycle. - /// - That the cycle is not self-overlapping. - /// - That there exists no duplicate cycle, with the same edges. - pub fn add_cycle(&mut self, cycle: Cycle) -> ValidationResult { - cycle.validate(self.min_distance, &self.stores)?; - let handle = self.stores.cycles.insert(cycle); - Ok(handle) - } - /// Add a face to the shape /// /// Validates that the face is structurally sound (i.e. the surface and @@ -153,8 +136,7 @@ mod tests { // Trying to refer to edge that is not from the same shape. Should fail. let edge = other.add_edge()?; let err = shape - .topology() - .add_cycle(Cycle { + .insert(Cycle { edges: vec![edge.clone()], }) .unwrap_err(); @@ -162,7 +144,7 @@ mod tests { // Referring to edge that *is* from the same shape. Should work. let edge = shape.add_edge()?; - shape.topology().add_cycle(Cycle { edges: vec![edge] })?; + shape.insert(Cycle { edges: vec![edge] })?; Ok(()) } @@ -239,8 +221,7 @@ mod tests { fn add_cycle(&mut self) -> anyhow::Result> { let edge = self.add_edge()?; - let cycle = - self.topology().add_cycle(Cycle { edges: vec![edge] })?; + let cycle = self.insert(Cycle { edges: vec![edge] })?; Ok(cycle) } } diff --git a/fj-operations/src/circle.rs b/fj-operations/src/circle.rs index 9dadbfc91..9f301ee04 100644 --- a/fj-operations/src/circle.rs +++ b/fj-operations/src/circle.rs @@ -18,10 +18,7 @@ impl ToShape for fj::Circle { let edge = Edge::build(&mut shape) .circle(Scalar::from_f64(self.radius())) .unwrap(); - shape - .topology() - .add_cycle(Cycle { edges: vec![edge] }) - .unwrap(); + shape.insert(Cycle { edges: vec![edge] }).unwrap(); let cycles = shape.topology().cycles().collect(); let surface = shape.insert(Surface::x_y_plane()).unwrap(); diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index 106691e23..2001d7870 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -109,5 +109,5 @@ fn add_cycle( edges.push(edge); } - shape.topology().add_cycle(Cycle { edges }).unwrap() + shape.insert(Cycle { edges }).unwrap() } diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 0ccd59f85..168ce1893 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -73,8 +73,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { } for cycle_orig in orig.topology().cycles() { let cycle = target - .topology() - .add_cycle(Cycle { + .insert(Cycle { edges: cycle_orig .get() .edges diff --git a/fj-operations/src/sketch.rs b/fj-operations/src/sketch.rs index aa8e5515d..6e48c484e 100644 --- a/fj-operations/src/sketch.rs +++ b/fj-operations/src/sketch.rs @@ -42,7 +42,7 @@ impl ToShape for fj::Sketch { edges.push(edge); } - shape.topology().add_cycle(Cycle { edges }).unwrap(); + shape.insert(Cycle { edges }).unwrap(); }; let surface = shape.insert(Surface::x_y_plane()).unwrap(); From 92df8ef749d71d6c307055ecbfb14c68d1506cd8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 1 Apr 2022 17:26:19 +0200 Subject: [PATCH 15/15] Replace `Topology::add_face` with `Shape::insert` --- fj-kernel/src/algorithms/sweep.rs | 16 +++++----------- fj-kernel/src/shape/api.rs | 1 - fj-kernel/src/shape/topology.rs | 21 +++------------------ fj-operations/src/circle.rs | 3 +-- fj-operations/src/difference_2d.rs | 3 +-- fj-operations/src/group.rs | 5 ++--- fj-operations/src/sketch.rs | 2 +- 7 files changed, 13 insertions(+), 38 deletions(-) diff --git a/fj-kernel/src/algorithms/sweep.rs b/fj-kernel/src/algorithms/sweep.rs index 8ed99213a..d99be2976 100644 --- a/fj-kernel/src/algorithms/sweep.rs +++ b/fj-kernel/src/algorithms/sweep.rs @@ -104,8 +104,7 @@ pub fn sweep_shape( let interiors_top = source_to_top.interiors_for_face(&face_source); target - .topology() - .add_face(Face::Face { + .insert(Face::Face { surface: surface_bottom, exteriors: exteriors_bottom, interiors: interiors_bottom, @@ -113,8 +112,7 @@ pub fn sweep_shape( }) .unwrap(); target - .topology() - .add_face(Face::Face { + .insert(Face::Face { surface: surface_top, exteriors: exteriors_top, interiors: interiors_top, @@ -165,10 +163,7 @@ pub fn sweep_shape( s.set_color(color); } - target - .topology() - .add_face(Face::Triangles(side_face)) - .unwrap(); + target.insert(Face::Triangles(side_face)).unwrap(); } else { // If there's no continuous edge, we can create the non- // continuous faces using boundary representation. @@ -244,8 +239,7 @@ pub fn sweep_shape( .unwrap(); target - .topology() - .add_face(Face::Face { + .insert(Face::Face { surface, exteriors: vec![cycle], interiors: Vec::new(), @@ -417,7 +411,7 @@ mod tests { color: [255, 0, 0, 255], }; - let face = shape.topology().add_face(abc)?; + let face = shape.insert(abc)?; Ok(Self { shape, face }) } diff --git a/fj-kernel/src/shape/api.rs b/fj-kernel/src/shape/api.rs index 659a93d42..6b9461ca1 100644 --- a/fj-kernel/src/shape/api.rs +++ b/fj-kernel/src/shape/api.rs @@ -82,7 +82,6 @@ impl Shape { /// Access the shape's topology pub fn topology(&mut self) -> Topology { Topology { - min_distance: self.min_distance, stores: self.stores.clone(), _lifetime: PhantomData, } diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index e08638bed..ec63bdca5 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -1,30 +1,16 @@ use std::marker::PhantomData; -use fj_math::Scalar; - use crate::topology::{Cycle, Edge, Face, Vertex}; -use super::{stores::Stores, validate::Validate as _, Iter, ValidationResult}; +use super::{stores::Stores, Iter}; /// The vertices of a shape pub struct Topology<'r> { - pub(super) min_distance: Scalar, pub(super) stores: Stores, pub(super) _lifetime: PhantomData<&'r ()>, } impl Topology<'_> { - /// Add a face to the shape - /// - /// Validates that the face is structurally sound (i.e. the surface and - /// cycles it refers to are part of the shape). Returns an error, if that is - /// not the case. - pub fn add_face(&mut self, face: Face) -> ValidationResult { - face.validate(self.min_distance, &self.stores)?; - let handle = self.stores.faces.insert(face); - Ok(handle) - } - /// Access iterator over all vertices /// /// The caller must not make any assumptions about the order of vertices. @@ -159,8 +145,7 @@ mod tests { // Nothing has been added to `shape`. Should fail. let err = shape - .topology() - .add_face(Face::Face { + .insert(Face::Face { surface: surface.clone(), exteriors: vec![cycle.clone()], interiors: Vec::new(), @@ -174,7 +159,7 @@ mod tests { let cycle = shape.add_cycle()?; // Everything has been added to `shape` now. Should work! - shape.topology().add_face(Face::Face { + shape.insert(Face::Face { surface, exteriors: vec![cycle], interiors: Vec::new(), diff --git a/fj-operations/src/circle.rs b/fj-operations/src/circle.rs index 9f301ee04..522888d76 100644 --- a/fj-operations/src/circle.rs +++ b/fj-operations/src/circle.rs @@ -23,8 +23,7 @@ impl ToShape for fj::Circle { let cycles = shape.topology().cycles().collect(); let surface = shape.insert(Surface::x_y_plane()).unwrap(); shape - .topology() - .add_face(Face::Face { + .insert(Face::Face { exteriors: cycles, interiors: Vec::new(), surface, diff --git a/fj-operations/src/difference_2d.rs b/fj-operations/src/difference_2d.rs index 2001d7870..06aee0848 100644 --- a/fj-operations/src/difference_2d.rs +++ b/fj-operations/src/difference_2d.rs @@ -64,8 +64,7 @@ impl ToShape for fj::Difference2d { let surface = shape.insert(face_a.surface()).unwrap(); shape - .topology() - .add_face(Face::Face { + .insert(Face::Face { surface, exteriors, interiors, diff --git a/fj-operations/src/group.rs b/fj-operations/src/group.rs index 168ce1893..f99bbb22d 100644 --- a/fj-operations/src/group.rs +++ b/fj-operations/src/group.rs @@ -94,8 +94,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { color, } => { target - .topology() - .add_face(Face::Face { + .insert(Face::Face { surface: surfaces[&surface].clone(), exteriors: exteriors .iter() @@ -110,7 +109,7 @@ fn copy_shape(mut orig: Shape, target: &mut Shape) { .unwrap(); } face @ Face::Triangles(_) => { - target.topology().add_face(face.clone()).unwrap(); + target.insert(face.clone()).unwrap(); } } } diff --git a/fj-operations/src/sketch.rs b/fj-operations/src/sketch.rs index 6e48c484e..f7149d212 100644 --- a/fj-operations/src/sketch.rs +++ b/fj-operations/src/sketch.rs @@ -52,7 +52,7 @@ impl ToShape for fj::Sketch { surface, color: self.color(), }; - shape.topology().add_face(face).unwrap(); + shape.insert(face).unwrap(); shape }