From e11266d13091f68a8bbca264bd4c482c201cc007 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 7 Oct 2022 11:44:53 +0200 Subject: [PATCH 1/2] Add `global_vertices` field to `Stores` --- crates/fj-kernel/src/stores/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/stores/mod.rs b/crates/fj-kernel/src/stores/mod.rs index 983361282..0e42ea5ea 100644 --- a/crates/fj-kernel/src/stores/mod.rs +++ b/crates/fj-kernel/src/stores/mod.rs @@ -4,7 +4,7 @@ mod blocks; mod handle; mod store; -use crate::objects::{Curve, GlobalCurve, Surface}; +use crate::objects::{Curve, GlobalCurve, GlobalVertex, Surface}; pub use self::{ handle::{Handle, HandleWrapper, ObjectId}, @@ -27,6 +27,9 @@ pub struct Stores { /// Store for global curves pub global_curves: Store, + /// Store for global vertices + pub global_vertices: Store, + /// Store for surfaces pub surfaces: Store, } From f93977a7a0c7833eed280004242dd80c930bd8f4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 7 Oct 2022 12:16:42 +0200 Subject: [PATCH 2/2] Add `GlobalVertex` to centralized object storage --- crates/fj-kernel/src/algorithms/sweep/edge.rs | 10 +++++---- .../fj-kernel/src/algorithms/sweep/vertex.rs | 13 ++++++----- .../fj-kernel/src/algorithms/validate/mod.rs | 22 ++++++++++++++----- crates/fj-kernel/src/builder/shell.rs | 8 ++++--- crates/fj-kernel/src/iter.rs | 8 ++++--- crates/fj-kernel/src/objects/edge.rs | 12 +++++----- crates/fj-kernel/src/objects/vertex.rs | 17 ++++++++------ crates/fj-kernel/src/partial/objects/edge.rs | 10 ++++----- crates/fj-kernel/src/partial/objects/mod.rs | 2 +- .../fj-kernel/src/partial/objects/vertex.rs | 22 +++++++++---------- 10 files changed, 74 insertions(+), 50 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 42da2f812..268f5793b 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -63,7 +63,7 @@ impl Sweep for (HalfEdge, Color) { let surface_vertex = SurfaceVertex::new( point_surface, surface.clone(), - *vertex.global_form(), + vertex.global_form().clone(), ); Vertex::new( @@ -87,7 +87,7 @@ impl Sweep for (HalfEdge, Color) { let global_vertices = side_edges.clone().map(|edge| { let [_, vertex] = edge.vertices(); - *vertex.global_form() + vertex.global_form().clone() }); let points_curve_and_surface = @@ -113,8 +113,10 @@ impl Sweep for (HalfEdge, Color) { Curve::new(surface.clone(), path, global, stores) }; - let global = - GlobalEdge::new(curve.global_form().clone(), global_vertices); + let global = GlobalEdge::new( + curve.global_form().clone(), + global_vertices.clone(), + ); let vertices = { let surface_points = points_curve_and_surface diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index 7f541c1dd..15c1a7c30 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -54,7 +54,7 @@ impl Sweep for (Vertex, Handle) { // as that is the most straight-forward part of this operations, and // we're going to need it soon anyway. let (edge_global, vertices_global) = - vertex.global_form().sweep(path, stores); + vertex.global_form().clone().sweep(path, stores); // Next, let's compute the surface coordinates of the two vertices of // the output `Edge`, as we're going to need these for the rest of this @@ -120,17 +120,18 @@ impl Sweep for (Vertex, Handle) { } } -impl Sweep for GlobalVertex { - type Swept = (GlobalEdge, [GlobalVertex; 2]); +impl Sweep for Handle { + type Swept = (GlobalEdge, [Handle; 2]); fn sweep(self, path: impl Into>, stores: &Stores) -> Self::Swept { let curve = GlobalCurve::new(stores); - let a = self; - let b = GlobalVertex::from_position(self.position() + path.into()); + let a = self.clone(); + let b = + GlobalVertex::from_position(self.position() + path.into(), stores); let vertices = [a, b]; - let global_edge = GlobalEdge::new(curve, vertices); + let global_edge = GlobalEdge::new(curve, vertices.clone()); // The vertices of the returned `GlobalEdge` are in normalized order, // which means the order can't be relied upon by the caller. Return the diff --git a/crates/fj-kernel/src/algorithms/validate/mod.rs b/crates/fj-kernel/src/algorithms/validate/mod.rs index 80fc99529..e10e5f1c9 100644 --- a/crates/fj-kernel/src/algorithms/validate/mod.rs +++ b/crates/fj-kernel/src/algorithms/validate/mod.rs @@ -37,9 +37,20 @@ pub trait Validate: Sized { /// # use fj_kernel::{ /// # algorithms::validate::{Validate, ValidationConfig}, /// # objects::GlobalVertex, + /// # stores::Stores, /// # }; - /// # let object = GlobalVertex::from_position([0., 0., 0.]); + /// # let stores = Stores::new(); + /// # let object = GlobalVertex::from_position([0., 0., 0.], &stores); /// object.validate(); + /// ``` + /// ``` rust + /// # use fj_kernel::{ + /// # algorithms::validate::{Validate, ValidationConfig}, + /// # objects::GlobalVertex, + /// # stores::Stores, + /// # }; + /// # let stores = Stores::new(); + /// # let object = GlobalVertex::from_position([0., 0., 0.], &stores); /// object.validate_with_config(&ValidationConfig::default()); /// ``` fn validate(self) -> Result, ValidationError> { @@ -181,8 +192,8 @@ mod tests { Curve::new(surface.clone(), path, global_form, &stores) }; - let [a_global, b_global] = - points_global.map(GlobalVertex::from_position); + let [a_global, b_global] = points_global + .map(|point| GlobalVertex::from_position(point, &stores)); let [a_surface, b_surface] = { // Can be cleaned up, once `zip` is stable: @@ -231,6 +242,7 @@ mod tests { #[test] fn uniqueness_vertex() -> anyhow::Result<()> { + let stores = Stores::new(); let mut shape = Vec::new(); let deviation = Scalar::from_f64(0.25); @@ -246,11 +258,11 @@ mod tests { }; // Adding a vertex should work. - shape.push(GlobalVertex::from_position(a)); + shape.push(GlobalVertex::from_position(a, &stores)); shape.clone().validate_with_config(&config)?; // Adding a second vertex that is considered identical should fail. - shape.push(GlobalVertex::from_position(b)); + shape.push(GlobalVertex::from_position(b, &stores)); let result = shape.validate_with_config(&config); assert!(matches!(result, Err(ValidationError::Uniqueness(_)))); diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index 71fb9e8e9..2a0707d12 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -117,7 +117,7 @@ impl<'a> ShellBuilder<'a> { to.position() + [Z, edge_length], )) .with_surface(Some(surface.clone())) - .with_global_form(Some(*from.global_form())); + .with_global_form(Some(from.global_form().clone())); let curve = Handle::::partial() .with_global_form(Some( @@ -151,7 +151,7 @@ impl<'a> ShellBuilder<'a> { from.position() + [-edge_length, Z], )) .with_surface(Some(surface.clone())) - .with_global_form(Some(*to.global_form())); + .with_global_form(Some(to.global_form().clone())); let from = Vertex::partial().with_surface_form(Some(from)); let to = Vertex::partial().with_surface_form(Some(to)); @@ -208,7 +208,9 @@ impl<'a> ShellBuilder<'a> { let surface_form = SurfaceVertex::partial() .with_position(Some(point)) .with_surface(Some(surface.clone())) - .with_global_form(Some(*vertex.global_form())) + .with_global_form(Some( + vertex.global_form().clone(), + )) .build(self.stores); Vertex::partial() .with_position(Some(vertex.position())) diff --git a/crates/fj-kernel/src/iter.rs b/crates/fj-kernel/src/iter.rs index 2c3ba1047..52896b4b3 100644 --- a/crates/fj-kernel/src/iter.rs +++ b/crates/fj-kernel/src/iter.rs @@ -192,7 +192,7 @@ impl<'r> ObjectIters<'r> for Handle { } } -impl<'r> ObjectIters<'r> for GlobalVertex { +impl<'r> ObjectIters<'r> for Handle { fn referenced_objects(&'r self) -> Vec<&'r dyn ObjectIters> { Vec::new() } @@ -460,7 +460,9 @@ mod tests { #[test] fn global_vertex() { - let object = GlobalVertex::from_position([0., 0., 0.]); + let stores = Stores::new(); + + let object = GlobalVertex::from_position([0., 0., 0.], &stores); assert_eq!(0, object.curve_iter().count()); assert_eq!(0, object.cycle_iter().count()); @@ -586,7 +588,7 @@ mod tests { .with_surface(Some(surface.clone())) .as_u_axis() .build(&stores); - let global_vertex = GlobalVertex::from_position([0., 0., 0.]); + let global_vertex = GlobalVertex::from_position([0., 0., 0.], &stores); let surface_vertex = SurfaceVertex::new([0., 0.], surface, global_vertex); let object = Vertex::new([0.], curve, surface_vertex); diff --git a/crates/fj-kernel/src/objects/edge.rs b/crates/fj-kernel/src/objects/edge.rs index ae1aa8f41..50234a954 100644 --- a/crates/fj-kernel/src/objects/edge.rs +++ b/crates/fj-kernel/src/objects/edge.rs @@ -48,7 +48,7 @@ impl HalfEdge { ); assert_eq!( &normalize_vertex_order( - [&a, &b].map(|vertex| *vertex.global_form()) + [&a, &b].map(|vertex| vertex.global_form().clone()) ), global_form.vertices_in_normalized_order(), "The global forms of a half-edge's vertices must match the \ @@ -104,7 +104,7 @@ impl fmt::Display for HalfEdge { #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct GlobalEdge { curve: HandleWrapper, - vertices: [GlobalVertex; 2], + vertices: [Handle; 2], } impl GlobalEdge { @@ -115,7 +115,7 @@ impl GlobalEdge { /// of `vertices` here. pub fn new( curve: impl Into>, - vertices: [GlobalVertex; 2], + vertices: [Handle; 2], ) -> Self { let curve = curve.into(); let vertices = normalize_vertex_order(vertices); @@ -137,12 +137,14 @@ impl GlobalEdge { /// and might not match the order of the vertices that were passed to /// [`GlobalEdge::new`]. You must not rely on the vertices being in any /// specific order. - pub fn vertices_in_normalized_order(&self) -> &[GlobalVertex; 2] { + pub fn vertices_in_normalized_order(&self) -> &[Handle; 2] { &self.vertices } } -fn normalize_vertex_order([a, b]: [GlobalVertex; 2]) -> [GlobalVertex; 2] { +fn normalize_vertex_order( + [a, b]: [Handle; 2], +) -> [Handle; 2] { if a < b { [a, b] } else { diff --git a/crates/fj-kernel/src/objects/vertex.rs b/crates/fj-kernel/src/objects/vertex.rs index 88f35dee8..501489482 100644 --- a/crates/fj-kernel/src/objects/vertex.rs +++ b/crates/fj-kernel/src/objects/vertex.rs @@ -1,7 +1,7 @@ use fj_math::Point; use pretty_assertions::assert_eq; -use crate::stores::Handle; +use crate::stores::{Handle, Stores}; use super::{Curve, Surface}; @@ -58,7 +58,7 @@ impl Vertex { } /// Access the global form of this vertex - pub fn global_form(&self) -> &GlobalVertex { + pub fn global_form(&self) -> &Handle { self.surface_form.global_form() } } @@ -68,7 +68,7 @@ impl Vertex { pub struct SurfaceVertex { position: Point<2>, surface: Handle, - global_form: GlobalVertex, + global_form: Handle, } impl SurfaceVertex { @@ -76,7 +76,7 @@ impl SurfaceVertex { pub fn new( position: impl Into>, surface: Handle, - global_form: GlobalVertex, + global_form: Handle, ) -> Self { let position = position.into(); Self { @@ -97,7 +97,7 @@ impl SurfaceVertex { } /// Access the global form of this vertex - pub fn global_form(&self) -> &GlobalVertex { + pub fn global_form(&self) -> &Handle { &self.global_form } } @@ -127,9 +127,12 @@ pub struct GlobalVertex { impl GlobalVertex { /// Construct a `GlobalVertex` from a position - pub fn from_position(position: impl Into>) -> Self { + pub fn from_position( + position: impl Into>, + stores: &Stores, + ) -> Handle { let position = position.into(); - Self { position } + stores.global_vertices.insert(Self { position }) } /// Access the position of the vertex diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index fdddd366e..7430f4a0f 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -81,7 +81,7 @@ impl PartialHalfEdge { let [a_curve, b_curve] = [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord])); - let global_vertex = GlobalVertex::partial() + let global_vertex = Handle::::partial() .from_curve_and_position(curve.clone(), a_curve); [a_curve, b_curve].map(|point_curve| { @@ -236,7 +236,7 @@ pub struct PartialGlobalEdge { /// The vertices that bound the [`GlobalEdge`] in the curve /// /// Must be provided before [`PartialGlobalEdge::build`] is called. - pub vertices: Option<[GlobalVertex; 2]>, + pub vertices: Option<[Handle; 2]>, } impl PartialGlobalEdge { @@ -251,7 +251,7 @@ impl PartialGlobalEdge { /// Update the partial global edge with the given vertices pub fn with_vertices( mut self, - vertices: Option<[GlobalVertex; 2]>, + vertices: Option<[Handle; 2]>, ) -> Self { if let Some(vertices) = vertices { self.vertices = Some(vertices); @@ -267,7 +267,7 @@ impl PartialGlobalEdge { ) -> Self { self.with_curve(Some(curve.global_form().clone())) .with_vertices(Some( - vertices.clone().map(|vertex| *vertex.global_form()), + vertices.clone().map(|vertex| vertex.global_form().clone()), )) } @@ -288,7 +288,7 @@ impl From<&GlobalEdge> for PartialGlobalEdge { fn from(global_edge: &GlobalEdge) -> Self { Self { curve: Some(global_edge.curve().clone().into()), - vertices: Some(*global_edge.vertices_in_normalized_order()), + vertices: Some(global_edge.vertices_in_normalized_order().clone()), } } } diff --git a/crates/fj-kernel/src/partial/objects/mod.rs b/crates/fj-kernel/src/partial/objects/mod.rs index 2457d1d7f..15f17bdc7 100644 --- a/crates/fj-kernel/src/partial/objects/mod.rs +++ b/crates/fj-kernel/src/partial/objects/mod.rs @@ -44,7 +44,7 @@ impl_traits!( Handle, PartialCurve; Cycle, PartialCycle; GlobalEdge, PartialGlobalEdge; - GlobalVertex, PartialGlobalVertex; + Handle, PartialGlobalVertex; HalfEdge, PartialHalfEdge; SurfaceVertex, PartialSurfaceVertex; Vertex, PartialVertex; diff --git a/crates/fj-kernel/src/partial/objects/vertex.rs b/crates/fj-kernel/src/partial/objects/vertex.rs index 0c020e390..c15126d2b 100644 --- a/crates/fj-kernel/src/partial/objects/vertex.rs +++ b/crates/fj-kernel/src/partial/objects/vertex.rs @@ -31,7 +31,7 @@ pub struct PartialVertex { /// /// Can be provided, if already available, or acquired through the surface /// form. - pub global_form: Option>, + pub global_form: Option>>, } impl PartialVertex { @@ -71,7 +71,7 @@ impl PartialVertex { /// Provide a global form for the partial vertex pub fn with_global_form( mut self, - global_form: Option>>, + global_form: Option>>>, ) -> Self { if let Some(global_form) = global_form { self.global_form = Some(global_form.into()); @@ -120,7 +120,7 @@ impl From<&Vertex> for PartialVertex { position: Some(vertex.position()), curve: Some(vertex.curve().clone().into()), surface_form: Some(vertex.surface_form().clone().into()), - global_form: Some((*vertex.global_form()).into()), + global_form: Some((vertex.global_form().clone()).into()), } } } @@ -144,7 +144,7 @@ pub struct PartialSurfaceVertex { /// /// Can be provided, if already available, or computed from the position on /// the [`Surface`]. - pub global_form: Option>, + pub global_form: Option>>, } impl PartialSurfaceVertex { @@ -170,7 +170,7 @@ impl PartialSurfaceVertex { /// Provide a global form for the partial surface vertex pub fn with_global_form( mut self, - global_form: Option>>, + global_form: Option>>>, ) -> Self { if let Some(global_form) = global_form { self.global_form = Some(global_form.into()); @@ -196,7 +196,7 @@ impl PartialSurfaceVertex { let global_form = self .global_form .unwrap_or_else(|| { - GlobalVertex::partial() + Handle::::partial() .from_surface_and_position(&surface, position) .into() }) @@ -211,7 +211,7 @@ impl From<&SurfaceVertex> for PartialSurfaceVertex { Self { position: Some(surface_vertex.position()), surface: Some(surface_vertex.surface().clone()), - global_form: Some((*surface_vertex.global_form()).into()), + global_form: Some((surface_vertex.global_form().clone()).into()), } } } @@ -269,17 +269,17 @@ impl PartialGlobalVertex { } /// Build a full [`GlobalVertex`] from the partial global vertex - pub fn build(self, _: &Stores) -> GlobalVertex { + pub fn build(self, stores: &Stores) -> Handle { let position = self .position .expect("Can't build a `GlobalVertex` without a position"); - GlobalVertex::from_position(position) + GlobalVertex::from_position(position, stores) } } -impl From<&GlobalVertex> for PartialGlobalVertex { - fn from(global_vertex: &GlobalVertex) -> Self { +impl From<&Handle> for PartialGlobalVertex { + fn from(global_vertex: &Handle) -> Self { Self { position: Some(global_vertex.position()), }