From c1690cdbdbd478da2e51ddc81ff2a1af7e0ff74c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 28 Feb 2024 13:22:27 +0100 Subject: [PATCH 01/21] Require `&Geometry` in `BoundingVolume::aabb` --- crates/fj-core/src/algorithms/bounding_volume/cycle.rs | 7 ++++--- crates/fj-core/src/algorithms/bounding_volume/edge.rs | 7 +++++-- crates/fj-core/src/algorithms/bounding_volume/face.rs | 9 ++++++--- crates/fj-core/src/algorithms/bounding_volume/mod.rs | 4 +++- crates/fj-core/src/algorithms/bounding_volume/shell.rs | 6 +++--- crates/fj-core/src/algorithms/bounding_volume/solid.rs | 6 +++--- crates/fj/src/instance.rs | 2 +- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/crates/fj-core/src/algorithms/bounding_volume/cycle.rs b/crates/fj-core/src/algorithms/bounding_volume/cycle.rs index e705dc3084..dad4402bc1 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/cycle.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/cycle.rs @@ -1,13 +1,14 @@ use fj_math::Aabb; -use crate::objects::Cycle; +use crate::{geometry::Geometry, objects::Cycle}; impl super::BoundingVolume<2> for Cycle { - fn aabb(&self) -> Option> { + fn aabb(&self, geometry: &Geometry) -> Option> { let mut aabb: Option> = None; for edge in self.half_edges() { - let new_aabb = edge.aabb().expect("`Edge` can always compute AABB"); + let new_aabb = + edge.aabb(geometry).expect("`Edge` can always compute AABB"); aabb = Some(aabb.map_or(new_aabb, |aabb| aabb.merged(&new_aabb))); } diff --git a/crates/fj-core/src/algorithms/bounding_volume/edge.rs b/crates/fj-core/src/algorithms/bounding_volume/edge.rs index 50be621af7..3145c98ff2 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/edge.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/edge.rs @@ -1,9 +1,12 @@ use fj_math::{Aabb, Vector}; -use crate::{geometry::SurfacePath, objects::HalfEdge}; +use crate::{ + geometry::{Geometry, SurfacePath}, + objects::HalfEdge, +}; impl super::BoundingVolume<2> for HalfEdge { - fn aabb(&self) -> Option> { + fn aabb(&self, _: &Geometry) -> Option> { match self.path() { SurfacePath::Circle(circle) => { // Just calculate the AABB of the whole circle. This is not the diff --git a/crates/fj-core/src/algorithms/bounding_volume/face.rs b/crates/fj-core/src/algorithms/bounding_volume/face.rs index 51cd275500..f1b8b6b81b 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/face.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/face.rs @@ -1,10 +1,13 @@ use fj_math::Aabb; -use crate::{geometry::GlobalPath, objects::Face}; +use crate::{ + geometry::{Geometry, GlobalPath}, + objects::Face, +}; impl super::BoundingVolume<3> for Face { - fn aabb(&self) -> Option> { - self.region().exterior().aabb().map(|aabb2| { + fn aabb(&self, geometry: &Geometry) -> Option> { + self.region().exterior().aabb(geometry).map(|aabb2| { let surface = self.surface().geometry(); match surface.u { diff --git a/crates/fj-core/src/algorithms/bounding_volume/mod.rs b/crates/fj-core/src/algorithms/bounding_volume/mod.rs index 34372592c2..748bd534b2 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/mod.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/mod.rs @@ -8,10 +8,12 @@ mod solid; use fj_math::Aabb; +use crate::geometry::Geometry; + /// Compute a bounding volume for an object pub trait BoundingVolume { /// Compute an axis-aligned bounding box (AABB) /// /// Return `None`, if no AABB can be computed (if the object is empty). - fn aabb(&self) -> Option>; + fn aabb(&self, geometry: &Geometry) -> Option>; } diff --git a/crates/fj-core/src/algorithms/bounding_volume/shell.rs b/crates/fj-core/src/algorithms/bounding_volume/shell.rs index 0d889ffaea..d62e636da5 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/shell.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/shell.rs @@ -1,13 +1,13 @@ use fj_math::Aabb; -use crate::objects::Shell; +use crate::{geometry::Geometry, objects::Shell}; impl super::BoundingVolume<3> for Shell { - fn aabb(&self) -> Option> { + fn aabb(&self, geometry: &Geometry) -> Option> { let mut aabb: Option> = None; for face in self.faces() { - let new_aabb = face.aabb(); + let new_aabb = face.aabb(geometry); aabb = aabb.map_or(new_aabb, |aabb| match new_aabb { Some(new_aabb) => Some(aabb.merged(&new_aabb)), None => Some(aabb), diff --git a/crates/fj-core/src/algorithms/bounding_volume/solid.rs b/crates/fj-core/src/algorithms/bounding_volume/solid.rs index 4c6b5a006f..e0483bf658 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/solid.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/solid.rs @@ -1,13 +1,13 @@ use fj_math::Aabb; -use crate::objects::Solid; +use crate::{geometry::Geometry, objects::Solid}; impl super::BoundingVolume<3> for Solid { - fn aabb(&self) -> Option> { + fn aabb(&self, geometry: &Geometry) -> Option> { let mut aabb: Option> = None; for shell in self.shells() { - let new_aabb = shell.aabb(); + let new_aabb = shell.aabb(geometry); aabb = aabb.map_or(new_aabb, |aabb| match new_aabb { Some(new_aabb) => Some(aabb.merged(&new_aabb)), None => Some(aabb), diff --git a/crates/fj/src/instance.rs b/crates/fj/src/instance.rs index 6fbb59d93c..9e01f8eece 100644 --- a/crates/fj/src/instance.rs +++ b/crates/fj/src/instance.rs @@ -60,7 +60,7 @@ impl Instance { self.core.layers.validation.take_errors()?; } - let aabb = model.aabb().unwrap_or(Aabb { + let aabb = model.aabb(&self.core.layers.geometry).unwrap_or(Aabb { min: Point::origin(), max: Point::origin(), }); From 15218b1fcac1278564d74404f1581be3646474dc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 28 Feb 2024 13:34:13 +0100 Subject: [PATCH 02/21] Require `&Geometry` in `Validate` method --- crates/fj-core/src/validate/edge.rs | 2 +- crates/fj-core/src/validate/face.rs | 4 ++-- crates/fj-core/src/validate/mod.rs | 10 ++++++++-- crates/fj-core/src/validate/shell.rs | 12 +++++++++--- crates/fj-core/src/validate/sketch.rs | 8 ++++---- crates/fj-core/src/validate/solid.rs | 8 ++++---- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/crates/fj-core/src/validate/edge.rs b/crates/fj-core/src/validate/edge.rs index 6f1e598b0c..23aea80fea 100644 --- a/crates/fj-core/src/validate/edge.rs +++ b/crates/fj-core/src/validate/edge.rs @@ -95,7 +95,7 @@ mod tests { ) }; - valid.validate_and_return_first_error()?; + valid.validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( invalid, ValidationError::Edge( diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index 0ebe7044ed..5722e64b02 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -123,7 +123,7 @@ mod tests { &mut core, ); - valid.validate_and_return_first_error()?; + valid.validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( invalid, ValidationError::Face(FaceValidationError::MissingBoundary) @@ -181,7 +181,7 @@ mod tests { Face::new(valid.surface().clone(), region) }; - valid.validate_and_return_first_error()?; + valid.validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( invalid, ValidationError::Face( diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 2c3b3d56c3..f2761466a3 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -73,7 +73,10 @@ mod solid; mod surface; mod vertex; -use crate::validation::{ValidationConfig, ValidationError}; +use crate::{ + geometry::Geometry, + validation::{ValidationConfig, ValidationError}, +}; pub use self::{ edge::EdgeValidationError, face::FaceValidationError, @@ -103,7 +106,10 @@ macro_rules! assert_contains_err { pub trait Validate: Sized { /// Validate the object using default config and return on first error #[allow(clippy::result_large_err)] - fn validate_and_return_first_error(&self) -> Result<(), ValidationError> { + fn validate_and_return_first_error( + &self, + _: &Geometry, + ) -> Result<(), ValidationError> { let mut errors = Vec::new(); self.validate(&ValidationConfig::default(), &mut errors); diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 25764efe2e..65817f168c 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -441,7 +441,9 @@ mod tests { &mut core, ); - valid.shell.validate_and_return_first_error()?; + valid + .shell + .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( invalid, ValidationError::Shell( @@ -462,7 +464,9 @@ mod tests { ); let invalid = valid.shell.remove_face(&valid.abc.face); - valid.shell.validate_and_return_first_error()?; + valid + .shell + .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( invalid, ValidationError::Shell( @@ -508,7 +512,9 @@ mod tests { &mut core, ); - valid.shell.validate_and_return_first_error()?; + valid + .shell + .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( invalid, ValidationError::Shell( diff --git a/crates/fj-core/src/validate/sketch.rs b/crates/fj-core/src/validate/sketch.rs index 83c06fb999..43acf35424 100644 --- a/crates/fj-core/src/validate/sketch.rs +++ b/crates/fj-core/src/validate/sketch.rs @@ -130,7 +130,7 @@ mod tests { let region = ::circle([0., 0.], 1., &mut core) .insert(&mut core); let valid_sketch = Sketch::new(vec![region.clone()]).insert(&mut core); - valid_sketch.validate_and_return_first_error()?; + valid_sketch.validate_and_return_first_error(&core.layers.geometry)?; let shared_cycle = region.exterior(); let invalid_sketch = Sketch::new(vec![ @@ -157,7 +157,7 @@ mod tests { ) .insert(&mut core); let valid_sketch = Sketch::new(vec![region.clone()]).insert(&mut core); - valid_sketch.validate_and_return_first_error()?; + valid_sketch.validate_and_return_first_error(&core.layers.geometry)?; let exterior = region.exterior(); let cloned_edges: Vec<_> = @@ -190,7 +190,7 @@ mod tests { Sketch::new(vec![ Region::new(valid_exterior.clone(), vec![]).insert(&mut core) ]); - valid_sketch.validate_and_return_first_error()?; + valid_sketch.validate_and_return_first_error(&core.layers.geometry)?; let invalid_outer_circle = HalfEdge::from_sibling( &valid_outer_circle, @@ -235,7 +235,7 @@ mod tests { vec![valid_interior], ) .insert(&mut core)]); - valid_sketch.validate_and_return_first_error()?; + valid_sketch.validate_and_return_first_error(&core.layers.geometry)?; let invalid_interior = Cycle::new(vec![inner_circle.clone()]).insert(&mut core); diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 83a46a4e5f..724e85405a 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -232,7 +232,7 @@ mod tests { ); let valid_solid = Solid::new(vec![]).insert(&mut core); - valid_solid.validate_and_return_first_error()?; + valid_solid.validate_and_return_first_error(&core.layers.geometry)?; // Ignore remaining validation errors. let _ = core.layers.validation.take_errors(); @@ -284,7 +284,7 @@ mod tests { ); let valid_solid = Solid::new(vec![]).insert(&mut core); - valid_solid.validate_and_return_first_error()?; + valid_solid.validate_and_return_first_error(&core.layers.geometry)?; // Ignore remaining validation errors. let _ = core.layers.validation.take_errors(); @@ -333,7 +333,7 @@ mod tests { ); let valid_solid = Solid::new(vec![]).insert(&mut core); - valid_solid.validate_and_return_first_error()?; + valid_solid.validate_and_return_first_error(&core.layers.geometry)?; // Ignore remaining validation errors. let _ = core.layers.validation.take_errors(); @@ -372,7 +372,7 @@ mod tests { ); let valid_solid = Solid::new(vec![]).insert(&mut core); - valid_solid.validate_and_return_first_error()?; + valid_solid.validate_and_return_first_error(&core.layers.geometry)?; // Ignore remaining validation errors. let _ = core.layers.validation.take_errors(); From 227c0eeb1c76309998a0cd0b461928f7e49833f4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 13:51:56 +0100 Subject: [PATCH 03/21] Make `&Geometry` available in validation layer --- crates/fj-core/src/layers/objects.rs | 3 +++ crates/fj-core/src/layers/validation.rs | 8 ++++++-- crates/fj-core/src/operations/insert/insert_trait.rs | 1 + 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/layers/objects.rs b/crates/fj-core/src/layers/objects.rs index 4aa192b065..8ab8ec5046 100644 --- a/crates/fj-core/src/layers/objects.rs +++ b/crates/fj-core/src/layers/objects.rs @@ -1,6 +1,7 @@ //! Layer infrastructure for [`Objects`] use crate::{ + geometry::Geometry, objects::{AboutToBeStored, AnyObject, Objects}, validation::Validation, }; @@ -14,6 +15,7 @@ impl Layer { pub fn insert( &mut self, object: AnyObject, + geometry: &Geometry, validation: &mut Layer, ) { let mut events = Vec::new(); @@ -22,6 +24,7 @@ impl Layer { for event in events { let event = ValidateObject { object: event.object.into(), + geometry, }; validation.process(event, &mut Vec::new()); } diff --git a/crates/fj-core/src/layers/validation.rs b/crates/fj-core/src/layers/validation.rs index 402be5e47a..383379fbbe 100644 --- a/crates/fj-core/src/layers/validation.rs +++ b/crates/fj-core/src/layers/validation.rs @@ -1,6 +1,7 @@ //! Layer infrastructure for [`Validation`] use crate::{ + geometry::Geometry, objects::{AnyObject, Stored}, validation::{Validation, ValidationError, ValidationErrors}, }; @@ -15,12 +16,15 @@ impl Layer { } /// Validate an object -pub struct ValidateObject { +pub struct ValidateObject<'r> { /// The object to validate pub object: AnyObject, + + /// Reference to `Geometry`, which is required for validation + pub geometry: &'r Geometry, } -impl Command for ValidateObject { +impl Command for ValidateObject<'_> { type Result = (); type Event = ValidationFailed; diff --git a/crates/fj-core/src/operations/insert/insert_trait.rs b/crates/fj-core/src/operations/insert/insert_trait.rs index e23cdff6ef..c92c70a25e 100644 --- a/crates/fj-core/src/operations/insert/insert_trait.rs +++ b/crates/fj-core/src/operations/insert/insert_trait.rs @@ -42,6 +42,7 @@ macro_rules! impl_insert { let object = (handle.clone(), self).into(); core.layers.objects.insert( object, + &core.layers.geometry, &mut core.layers.validation, ); handle From eb1ce01a470c7dcfbf4752824db7013ae0a3e885 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 13:52:36 +0100 Subject: [PATCH 04/21] Require `&Geometry` in `AnyObject::validate` --- crates/fj-core/src/layers/validation.rs | 3 ++- crates/fj-core/src/objects/any_object.rs | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/layers/validation.rs b/crates/fj-core/src/layers/validation.rs index 383379fbbe..4c44003501 100644 --- a/crates/fj-core/src/layers/validation.rs +++ b/crates/fj-core/src/layers/validation.rs @@ -30,7 +30,8 @@ impl Command for ValidateObject<'_> { fn decide(self, state: &Validation, events: &mut Vec) { let mut errors = Vec::new(); - self.object.validate(&state.config, &mut errors); + self.object + .validate(&state.config, &mut errors, self.geometry); for err in errors { events.push(ValidationFailed { diff --git a/crates/fj-core/src/objects/any_object.rs b/crates/fj-core/src/objects/any_object.rs index 989f363f92..cfeab21d5e 100644 --- a/crates/fj-core/src/objects/any_object.rs +++ b/crates/fj-core/src/objects/any_object.rs @@ -1,4 +1,5 @@ use crate::{ + geometry::Geometry, objects::{ Curve, Cycle, Face, HalfEdge, Objects, Region, Shell, Sketch, Solid, Surface, Vertex, @@ -38,6 +39,7 @@ macro_rules! any_object { pub fn validate(&self, config: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { match self { $( From 34e8c4299d9351471ff380a3a8e683a74c0e2fc6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:00:49 +0100 Subject: [PATCH 05/21] Use more suitable type for macro parameter --- crates/fj-core/src/validate/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index f2761466a3..b862a11787 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -88,7 +88,7 @@ pub use self::{ /// pattern. This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order. #[macro_export] macro_rules! assert_contains_err { - ($o:tt,$p:pat) => { + ($o:expr,$p:pat) => { assert!({ let mut errors = Vec::new(); $o.validate( From 957e6690cb7c09c6f0e1973d5e145cfb40266838 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:01:06 +0100 Subject: [PATCH 06/21] Improve formatting --- crates/fj-core/src/validate/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index b862a11787..03ae5a3467 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -88,7 +88,7 @@ pub use self::{ /// pattern. This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order. #[macro_export] macro_rules! assert_contains_err { - ($o:expr,$p:pat) => { + ($o:expr, $p:pat) => { assert!({ let mut errors = Vec::new(); $o.validate( From 4d416fa88d5b044b378e671364527773a7260caa Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:02:16 +0100 Subject: [PATCH 07/21] Pass `core` into `assert_contains_err!` This is preparation for the next step, when it will be used there. --- crates/fj-core/src/validate/edge.rs | 1 + crates/fj-core/src/validate/face.rs | 2 ++ crates/fj-core/src/validate/mod.rs | 2 +- crates/fj-core/src/validate/shell.rs | 3 +++ crates/fj-core/src/validate/sketch.rs | 2 ++ crates/fj-core/src/validate/solid.rs | 4 ++++ 6 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/edge.rs b/crates/fj-core/src/validate/edge.rs index 23aea80fea..eb2a3c8db5 100644 --- a/crates/fj-core/src/validate/edge.rs +++ b/crates/fj-core/src/validate/edge.rs @@ -97,6 +97,7 @@ mod tests { valid.validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( + core, invalid, ValidationError::Edge( EdgeValidationError::VerticesAreCoincident { .. } diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index 5722e64b02..2ceca5754e 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -125,6 +125,7 @@ mod tests { valid.validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( + core, invalid, ValidationError::Face(FaceValidationError::MissingBoundary) ); @@ -183,6 +184,7 @@ mod tests { valid.validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( + core, invalid, ValidationError::Face( FaceValidationError::InvalidInteriorWinding { .. } diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index 03ae5a3467..a49a2729a5 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -88,7 +88,7 @@ pub use self::{ /// pattern. This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order. #[macro_export] macro_rules! assert_contains_err { - ($o:expr, $p:pat) => { + ($core:expr, $o:expr, $p:pat) => { assert!({ let mut errors = Vec::new(); $o.validate( diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 65817f168c..a678461d40 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -445,6 +445,7 @@ mod tests { .shell .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( + core, invalid, ValidationError::Shell( ShellValidationError::CurveCoordinateSystemMismatch(..) @@ -468,6 +469,7 @@ mod tests { .shell .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( + core, invalid, ValidationError::Shell( ShellValidationError::HalfEdgeHasNoSibling { .. } @@ -516,6 +518,7 @@ mod tests { .shell .validate_and_return_first_error(&core.layers.geometry)?; assert_contains_err!( + core, invalid, ValidationError::Shell( ShellValidationError::CoincidentHalfEdgesAreNotSiblings { .. } diff --git a/crates/fj-core/src/validate/sketch.rs b/crates/fj-core/src/validate/sketch.rs index 43acf35424..b9027b1b9c 100644 --- a/crates/fj-core/src/validate/sketch.rs +++ b/crates/fj-core/src/validate/sketch.rs @@ -138,6 +138,7 @@ mod tests { Region::new(shared_cycle.clone(), vec![]).insert(&mut core), ]); assert_contains_err!( + core, invalid_sketch, ValidationError::Sketch(SketchValidationError::MultipleReferences( ReferenceCountError::Cycle { references: _ } @@ -169,6 +170,7 @@ mod tests { Region::new(exterior.clone(), vec![interior]).insert(&mut core) ]); assert_contains_err!( + core, invalid_sketch, ValidationError::Sketch(SketchValidationError::MultipleReferences( ReferenceCountError::HalfEdge { references: _ } diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 724e85405a..776c2f966d 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -225,6 +225,7 @@ mod tests { .insert(&mut core); assert_contains_err!( + core, invalid_solid, ValidationError::Solid(SolidValidationError::MultipleReferences( ReferenceCountError::Face { references: _ } @@ -277,6 +278,7 @@ mod tests { .insert(&mut core); assert_contains_err!( + core, invalid_solid, ValidationError::Solid(SolidValidationError::MultipleReferences( ReferenceCountError::Region { references: _ } @@ -326,6 +328,7 @@ mod tests { .insert(&mut core); assert_contains_err!( + core, invalid_solid, ValidationError::Solid(SolidValidationError::MultipleReferences( ReferenceCountError::Cycle { references: _ } @@ -365,6 +368,7 @@ mod tests { .insert(&mut core); assert_contains_err!( + core, invalid_solid, ValidationError::Solid(SolidValidationError::MultipleReferences( ReferenceCountError::HalfEdge { references: _ } From f5ca4431574080e616cfb54c5d4022788511fbd9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:04:28 +0100 Subject: [PATCH 08/21] Expect `&Geometry` in `Validate::validate` --- crates/fj-core/src/objects/any_object.rs | 8 ++++++-- crates/fj-core/src/validate/curve.rs | 9 ++++++++- crates/fj-core/src/validate/cycle.rs | 2 ++ crates/fj-core/src/validate/edge.rs | 2 ++ crates/fj-core/src/validate/face.rs | 2 ++ crates/fj-core/src/validate/mod.rs | 6 ++++-- crates/fj-core/src/validate/region.rs | 10 ++++++++-- crates/fj-core/src/validate/shell.rs | 3 ++- crates/fj-core/src/validate/sketch.rs | 4 ++++ crates/fj-core/src/validate/solid.rs | 2 ++ crates/fj-core/src/validate/surface.rs | 10 ++++++++-- crates/fj-core/src/validate/vertex.rs | 10 ++++++++-- 12 files changed, 56 insertions(+), 12 deletions(-) diff --git a/crates/fj-core/src/objects/any_object.rs b/crates/fj-core/src/objects/any_object.rs index cfeab21d5e..3734476896 100644 --- a/crates/fj-core/src/objects/any_object.rs +++ b/crates/fj-core/src/objects/any_object.rs @@ -39,11 +39,15 @@ macro_rules! any_object { pub fn validate(&self, config: &ValidationConfig, errors: &mut Vec, - _: &Geometry, + geometry: &Geometry, ) { match self { $( - Self::$ty(object) => object.validate(config, errors), + Self::$ty(object) => object.validate( + config, + errors, + geometry, + ), )* } } diff --git a/crates/fj-core/src/validate/curve.rs b/crates/fj-core/src/validate/curve.rs index 19b910d8de..6574cce35e 100644 --- a/crates/fj-core/src/validate/curve.rs +++ b/crates/fj-core/src/validate/curve.rs @@ -1,4 +1,5 @@ use crate::{ + geometry::Geometry, objects::Curve, validation::{ValidationConfig, ValidationError}, }; @@ -6,5 +7,11 @@ use crate::{ use super::Validate; impl Validate for Curve { - fn validate(&self, _: &ValidationConfig, _: &mut Vec) {} + fn validate( + &self, + _: &ValidationConfig, + _: &mut Vec, + _: &Geometry, + ) { + } } diff --git a/crates/fj-core/src/validate/cycle.rs b/crates/fj-core/src/validate/cycle.rs index dc9b125ec7..017694733f 100644 --- a/crates/fj-core/src/validate/cycle.rs +++ b/crates/fj-core/src/validate/cycle.rs @@ -1,4 +1,5 @@ use crate::{ + geometry::Geometry, objects::Cycle, validation::{ checks::AdjacentHalfEdgesNotConnected, ValidationCheck, @@ -13,6 +14,7 @@ impl Validate for Cycle { &self, config: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { errors.extend( AdjacentHalfEdgesNotConnected::check(self, config).map(Into::into), diff --git a/crates/fj-core/src/validate/edge.rs b/crates/fj-core/src/validate/edge.rs index eb2a3c8db5..994be2fdf1 100644 --- a/crates/fj-core/src/validate/edge.rs +++ b/crates/fj-core/src/validate/edge.rs @@ -1,6 +1,7 @@ use fj_math::{Point, Scalar}; use crate::{ + geometry::Geometry, objects::HalfEdge, validation::{ValidationConfig, ValidationError}, }; @@ -12,6 +13,7 @@ impl Validate for HalfEdge { &self, config: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { EdgeValidationError::check_vertex_coincidence(self, config, errors); } diff --git a/crates/fj-core/src/validate/face.rs b/crates/fj-core/src/validate/face.rs index 2ceca5754e..d1fdb05f23 100644 --- a/crates/fj-core/src/validate/face.rs +++ b/crates/fj-core/src/validate/face.rs @@ -1,6 +1,7 @@ use fj_math::Winding; use crate::{ + geometry::Geometry, objects::Face, validation::{ValidationConfig, ValidationError}, }; @@ -12,6 +13,7 @@ impl Validate for Face { &self, _: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { FaceValidationError::check_boundary(self, errors); FaceValidationError::check_interior_winding(self, errors); diff --git a/crates/fj-core/src/validate/mod.rs b/crates/fj-core/src/validate/mod.rs index a49a2729a5..9f77067a30 100644 --- a/crates/fj-core/src/validate/mod.rs +++ b/crates/fj-core/src/validate/mod.rs @@ -94,6 +94,7 @@ macro_rules! assert_contains_err { $o.validate( &$crate::validation::ValidationConfig::default(), &mut errors, + &$core.layers.geometry, ); errors.iter().any(|e| matches!(e, $p)) }) @@ -108,10 +109,10 @@ pub trait Validate: Sized { #[allow(clippy::result_large_err)] fn validate_and_return_first_error( &self, - _: &Geometry, + geometry: &Geometry, ) -> Result<(), ValidationError> { let mut errors = Vec::new(); - self.validate(&ValidationConfig::default(), &mut errors); + self.validate(&ValidationConfig::default(), &mut errors, geometry); if let Some(err) = errors.into_iter().next() { return Err(err); @@ -125,5 +126,6 @@ pub trait Validate: Sized { &self, config: &ValidationConfig, errors: &mut Vec, + geometry: &Geometry, ); } diff --git a/crates/fj-core/src/validate/region.rs b/crates/fj-core/src/validate/region.rs index d8b6bbf323..7508108b21 100644 --- a/crates/fj-core/src/validate/region.rs +++ b/crates/fj-core/src/validate/region.rs @@ -1,7 +1,13 @@ -use crate::objects::Region; +use crate::{geometry::Geometry, objects::Region}; use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Region { - fn validate(&self, _: &ValidationConfig, _: &mut Vec) {} + fn validate( + &self, + _: &ValidationConfig, + _: &mut Vec, + _: &Geometry, + ) { + } } diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index a678461d40..c648ce8f1f 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -3,7 +3,7 @@ use std::{collections::BTreeMap, fmt}; use fj_math::{Point, Scalar}; use crate::{ - geometry::{CurveBoundary, SurfaceGeometry}, + geometry::{CurveBoundary, Geometry, SurfaceGeometry}, objects::{Curve, HalfEdge, Shell, Vertex}, queries::{ AllHalfEdgesWithSurface, BoundingVerticesOfHalfEdge, SiblingOfHalfEdge, @@ -18,6 +18,7 @@ impl Validate for Shell { &self, config: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { ShellValidationError::check_curve_coordinates(self, config, errors); ShellValidationError::check_half_edge_pairs(self, errors); diff --git a/crates/fj-core/src/validate/sketch.rs b/crates/fj-core/src/validate/sketch.rs index b9027b1b9c..2674a177d3 100644 --- a/crates/fj-core/src/validate/sketch.rs +++ b/crates/fj-core/src/validate/sketch.rs @@ -1,3 +1,4 @@ +use crate::geometry::Geometry; use crate::{objects::Cycle, storage::Handle}; use crate::{objects::Sketch, validate_references}; use fj_math::Winding; @@ -12,6 +13,7 @@ impl Validate for Sketch { &self, config: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { SketchValidationError::check_object_references(self, config, errors); SketchValidationError::check_exterior_cycles(self, config, errors); @@ -206,6 +208,7 @@ mod tests { Region::new(invalid_exterior.clone(), vec![]).insert(&mut core) ]); assert_contains_err!( + core, invalid_sketch, ValidationError::Sketch( SketchValidationError::ClockwiseExteriorCycle { cycle: _ } @@ -247,6 +250,7 @@ mod tests { ) .insert(&mut core)]); assert_contains_err!( + core, invalid_sketch, ValidationError::Sketch( SketchValidationError::CounterClockwiseInteriorCycle { diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 776c2f966d..53a1f3646d 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -1,6 +1,7 @@ use std::iter::repeat; use crate::{ + geometry::Geometry, objects::{Solid, Vertex}, storage::Handle, validate_references, @@ -17,6 +18,7 @@ impl Validate for Solid { &self, config: &ValidationConfig, errors: &mut Vec, + _: &Geometry, ) { SolidValidationError::check_vertices(self, config, errors); SolidValidationError::check_object_references(self, config, errors); diff --git a/crates/fj-core/src/validate/surface.rs b/crates/fj-core/src/validate/surface.rs index 4cb39c1be9..44c148548c 100644 --- a/crates/fj-core/src/validate/surface.rs +++ b/crates/fj-core/src/validate/surface.rs @@ -1,7 +1,13 @@ -use crate::objects::Surface; +use crate::{geometry::Geometry, objects::Surface}; use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Surface { - fn validate(&self, _: &ValidationConfig, _: &mut Vec) {} + fn validate( + &self, + _: &ValidationConfig, + _: &mut Vec, + _: &Geometry, + ) { + } } diff --git a/crates/fj-core/src/validate/vertex.rs b/crates/fj-core/src/validate/vertex.rs index 1e2e7e7000..d08c2ee8ee 100644 --- a/crates/fj-core/src/validate/vertex.rs +++ b/crates/fj-core/src/validate/vertex.rs @@ -1,7 +1,13 @@ -use crate::objects::Vertex; +use crate::{geometry::Geometry, objects::Vertex}; use super::{Validate, ValidationConfig, ValidationError}; impl Validate for Vertex { - fn validate(&self, _: &ValidationConfig, _: &mut Vec) {} + fn validate( + &self, + _: &ValidationConfig, + _: &mut Vec, + _: &Geometry, + ) { + } } From 62a17e66c69ea15a2c8d99ef05a687dcecd1954a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:31:36 +0100 Subject: [PATCH 09/21] Expect `&Geometry` in `check_curve_coordinates` --- crates/fj-core/src/validate/shell.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index c648ce8f1f..87ff4c5621 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -18,9 +18,11 @@ impl Validate for Shell { &self, config: &ValidationConfig, errors: &mut Vec, - _: &Geometry, + geometry: &Geometry, ) { - ShellValidationError::check_curve_coordinates(self, config, errors); + ShellValidationError::check_curve_coordinates( + self, geometry, config, errors, + ); ShellValidationError::check_half_edge_pairs(self, errors); ShellValidationError::check_half_edge_coincidence(self, config, errors); } @@ -76,6 +78,7 @@ impl ShellValidationError { /// Check that local curve definitions that refer to the same curve match fn check_curve_coordinates( shell: &Shell, + _: &Geometry, config: &ValidationConfig, errors: &mut Vec, ) { From ce540712ce8b80f24c9ec604304d94aa93c7206d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:33:00 +0100 Subject: [PATCH 10/21] Pass `&Geometry` to `check_half_edge_coincidence` --- crates/fj-core/src/validate/shell.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 87ff4c5621..6046368c01 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -24,7 +24,9 @@ impl Validate for Shell { self, geometry, config, errors, ); ShellValidationError::check_half_edge_pairs(self, errors); - ShellValidationError::check_half_edge_coincidence(self, config, errors); + ShellValidationError::check_half_edge_coincidence( + self, geometry, config, errors, + ); } } @@ -212,6 +214,7 @@ impl ShellValidationError { /// Check that non-sibling half-edges are not coincident fn check_half_edge_coincidence( shell: &Shell, + _: &Geometry, config: &ValidationConfig, errors: &mut Vec, ) { From e06d7338fa7a350b23280d20aed6d68b00901e85 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:34:30 +0100 Subject: [PATCH 11/21] Require `&Geometry` in `check_vertices` --- crates/fj-core/src/validate/solid.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 53a1f3646d..6425f20245 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -18,9 +18,9 @@ impl Validate for Solid { &self, config: &ValidationConfig, errors: &mut Vec, - _: &Geometry, + geometry: &Geometry, ) { - SolidValidationError::check_vertices(self, config, errors); + SolidValidationError::check_vertices(self, geometry, config, errors); SolidValidationError::check_object_references(self, config, errors); } } @@ -76,6 +76,7 @@ pub enum SolidValidationError { impl SolidValidationError { fn check_vertices( solid: &Solid, + _: &Geometry, config: &ValidationConfig, errors: &mut Vec, ) { From e24de225bbd0410250ddf3860a0cec04d6e40b16 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:38:54 +0100 Subject: [PATCH 12/21] Read surface geometry from geometry layer --- crates/fj-core/src/algorithms/approx/face.rs | 13 ++-- .../src/algorithms/bounding_volume/face.rs | 2 +- .../fj-core/src/algorithms/triangulate/mod.rs | 66 +++++++++++++++---- crates/fj-core/src/operations/holes.rs | 7 +- crates/fj-core/src/operations/sweep/region.rs | 4 +- crates/fj-core/src/operations/sweep/sketch.rs | 4 +- .../src/operations/transform/surface.rs | 3 +- crates/fj-core/src/validate/shell.rs | 16 ++--- crates/fj-core/src/validate/solid.rs | 4 +- 9 files changed, 84 insertions(+), 35 deletions(-) diff --git a/crates/fj-core/src/algorithms/approx/face.rs b/crates/fj-core/src/algorithms/approx/face.rs index fa07879b1f..1047efd530 100644 --- a/crates/fj-core/src/algorithms/approx/face.rs +++ b/crates/fj-core/src/algorithms/approx/face.rs @@ -90,13 +90,18 @@ impl Approx for &Face { // would need to provide its own approximation, as the edges that bound // it have nothing to do with its curvature. - let exterior = - (self.region().exterior().deref(), &self.surface().geometry()) - .approx_with_cache(tolerance, cache, core); + let exterior = ( + self.region().exterior().deref(), + &core.layers.geometry.of_surface(self.surface()), + ) + .approx_with_cache(tolerance, cache, core); let mut interiors = BTreeSet::new(); for cycle in self.region().interiors() { - let cycle = (cycle.deref(), &self.surface().geometry()) + let cycle = ( + cycle.deref(), + &core.layers.geometry.of_surface(self.surface()), + ) .approx_with_cache(tolerance, cache, core); interiors.insert(cycle); } diff --git a/crates/fj-core/src/algorithms/bounding_volume/face.rs b/crates/fj-core/src/algorithms/bounding_volume/face.rs index f1b8b6b81b..00f5eec256 100644 --- a/crates/fj-core/src/algorithms/bounding_volume/face.rs +++ b/crates/fj-core/src/algorithms/bounding_volume/face.rs @@ -8,7 +8,7 @@ use crate::{ impl super::BoundingVolume<3> for Face { fn aabb(&self, geometry: &Geometry) -> Option> { self.region().exterior().aabb(geometry).map(|aabb2| { - let surface = self.surface().geometry(); + let surface = geometry.of_surface(self.surface()); match surface.u { GlobalPath::Circle(circle) => { diff --git a/crates/fj-core/src/algorithms/triangulate/mod.rs b/crates/fj-core/src/algorithms/triangulate/mod.rs index 0ec1e5ab71..5e76058ab5 100644 --- a/crates/fj-core/src/algorithms/triangulate/mod.rs +++ b/crates/fj-core/src/algorithms/triangulate/mod.rs @@ -161,12 +161,36 @@ mod tests { let triangles = triangulate(face, &mut core)?; - let a = surface.geometry().point_from_surface_coords(a); - let b = surface.geometry().point_from_surface_coords(b); - let e = surface.geometry().point_from_surface_coords(e); - let f = surface.geometry().point_from_surface_coords(f); - let g = surface.geometry().point_from_surface_coords(g); - let h = surface.geometry().point_from_surface_coords(h); + let a = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(a); + let b = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(b); + let e = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(e); + let f = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(f); + let g = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(g); + let h = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(h); // Let's test that some correct triangles are present. We don't need to // test them all. @@ -224,11 +248,31 @@ mod tests { let triangles = triangulate(face, &mut core)?; - let a = surface.geometry().point_from_surface_coords(a); - let b = surface.geometry().point_from_surface_coords(b); - let c = surface.geometry().point_from_surface_coords(c); - let d = surface.geometry().point_from_surface_coords(d); - let e = surface.geometry().point_from_surface_coords(e); + let a = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(a); + let b = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(b); + let c = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(c); + let d = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(d); + let e = core + .layers + .geometry + .of_surface(&surface) + .point_from_surface_coords(e); assert!(triangles.contains_triangle([a, b, d])); assert!(triangles.contains_triangle([a, d, e])); diff --git a/crates/fj-core/src/operations/holes.rs b/crates/fj-core/src/operations/holes.rs index fafe7203ed..d8d296cf8a 100644 --- a/crates/fj-core/src/operations/holes.rs +++ b/crates/fj-core/src/operations/holes.rs @@ -99,10 +99,9 @@ impl AddHole for Shell { let path = { let point = |location: &HoleLocation| { - location - .face - .surface() - .geometry() + core.layers + .geometry + .of_surface(location.face.surface()) .point_from_surface_coords(location.position) }; diff --git a/crates/fj-core/src/operations/sweep/region.rs b/crates/fj-core/src/operations/sweep/region.rs index d8b632854f..50f25867a7 100644 --- a/crates/fj-core/src/operations/sweep/region.rs +++ b/crates/fj-core/src/operations/sweep/region.rs @@ -55,7 +55,7 @@ impl SweepRegion for Region { let top_exterior = sweep_cycle( self.exterior(), - &surface.geometry(), + &core.layers.geometry.of_surface(surface), color, &mut faces, path, @@ -69,7 +69,7 @@ impl SweepRegion for Region { .map(|bottom_cycle| { sweep_cycle( bottom_cycle, - &surface.geometry(), + &core.layers.geometry.of_surface(surface), color, &mut faces, path, diff --git a/crates/fj-core/src/operations/sweep/sketch.rs b/crates/fj-core/src/operations/sweep/sketch.rs index 320ccedd87..087ad51a62 100644 --- a/crates/fj-core/src/operations/sweep/sketch.rs +++ b/crates/fj-core/src/operations/sweep/sketch.rs @@ -43,14 +43,14 @@ impl SweepSketch for Sketch { assert!(region.exterior().winding().is_ccw()); let is_negative_sweep = { - let u = match surface.geometry().u { + let u = match core.layers.geometry.of_surface(&surface).u { GlobalPath::Circle(_) => todo!( "Sweeping sketch from a rounded surfaces is not \ supported" ), GlobalPath::Line(line) => line.direction(), }; - let v = surface.geometry().v; + let v = core.layers.geometry.of_surface(&surface).v; let normal = u.cross(&v); diff --git a/crates/fj-core/src/operations/transform/surface.rs b/crates/fj-core/src/operations/transform/surface.rs index c46b51c5c3..135d15022c 100644 --- a/crates/fj-core/src/operations/transform/surface.rs +++ b/crates/fj-core/src/operations/transform/surface.rs @@ -16,7 +16,8 @@ impl TransformObject for Handle { cache .entry(self) .or_insert_with(|| { - let geometry = self.geometry().transform(transform); + let geometry = + core.layers.geometry.of_surface(self).transform(transform); let surface = Surface::new(geometry).insert(core); core.layers diff --git a/crates/fj-core/src/validate/shell.rs b/crates/fj-core/src/validate/shell.rs index 6046368c01..2628a30f79 100644 --- a/crates/fj-core/src/validate/shell.rs +++ b/crates/fj-core/src/validate/shell.rs @@ -80,7 +80,7 @@ impl ShellValidationError { /// Check that local curve definitions that refer to the same curve match fn check_curve_coordinates( shell: &Shell, - _: &Geometry, + geometry: &Geometry, config: &ValidationConfig, errors: &mut Vec, ) { @@ -145,17 +145,17 @@ impl ShellValidationError { compare_curve_coords( edge_a, - &surface_a.geometry(), + &geometry.of_surface(surface_a), edge_b, - &surface_b.geometry(), + &geometry.of_surface(surface_b), config, &mut mismatches, ); compare_curve_coords( edge_b, - &surface_b.geometry(), + &geometry.of_surface(surface_b), edge_a, - &surface_a.geometry(), + &geometry.of_surface(surface_a), config, &mut mismatches, ); @@ -214,7 +214,7 @@ impl ShellValidationError { /// Check that non-sibling half-edges are not coincident fn check_half_edge_coincidence( shell: &Shell, - _: &Geometry, + geometry: &Geometry, config: &ValidationConfig, errors: &mut Vec, ) { @@ -242,9 +242,9 @@ impl ShellValidationError { // `distinct_min_distance`, that's a problem. if distances( half_edge_a.clone(), - &surface_a.geometry(), + &geometry.of_surface(surface_a), half_edge_b.clone(), - &surface_b.geometry(), + &geometry.of_surface(surface_b), ) .all(|d| d < config.distinct_min_distance) { diff --git a/crates/fj-core/src/validate/solid.rs b/crates/fj-core/src/validate/solid.rs index 6425f20245..60df5825bf 100644 --- a/crates/fj-core/src/validate/solid.rs +++ b/crates/fj-core/src/validate/solid.rs @@ -76,7 +76,7 @@ pub enum SolidValidationError { impl SolidValidationError { fn check_vertices( solid: &Solid, - _: &Geometry, + geometry: &Geometry, config: &ValidationConfig, errors: &mut Vec, ) { @@ -88,7 +88,7 @@ impl SolidValidationError { face.region() .all_cycles() .flat_map(|cycle| cycle.half_edges().iter().cloned()) - .zip(repeat(face.surface().geometry())) + .zip(repeat(geometry.of_surface(face.surface()))) }) .map(|(h, s)| { ( From 7d0270a7dd8f8df3e994e2388e6c9ef0fd783632 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:43:33 +0100 Subject: [PATCH 13/21] Remove redundant surface geometry from `Surface` --- crates/fj-core/src/objects/kinds/surface.rs | 17 +++------- crates/fj-core/src/objects/stores.rs | 31 +++---------------- .../fj-core/src/operations/build/surface.rs | 2 +- .../src/operations/transform/surface.rs | 2 +- 4 files changed, 10 insertions(+), 42 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/surface.rs b/crates/fj-core/src/objects/kinds/surface.rs index 591b7f0ab7..468c91e402 100644 --- a/crates/fj-core/src/objects/kinds/surface.rs +++ b/crates/fj-core/src/objects/kinds/surface.rs @@ -1,19 +1,10 @@ -use crate::geometry::SurfaceGeometry; - /// A two-dimensional shape -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Surface { - geometry: SurfaceGeometry, -} +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +pub struct Surface {} impl Surface { /// Construct an instance of `Surface` - pub fn new(geometry: SurfaceGeometry) -> Self { - Self { geometry } - } - - /// Access the surface's geometry - pub fn geometry(&self) -> SurfaceGeometry { - self.geometry + pub fn new() -> Self { + Self {} } } diff --git a/crates/fj-core/src/objects/stores.rs b/crates/fj-core/src/objects/stores.rs index c9811c6a3e..04d28bb5e8 100644 --- a/crates/fj-core/src/objects/stores.rs +++ b/crates/fj-core/src/objects/stores.rs @@ -1,9 +1,4 @@ -use fj_math::Vector; - -use crate::{ - geometry::{GlobalPath, SurfaceGeometry}, - storage::{Handle, Store}, -}; +use crate::storage::{Handle, Store}; use super::{ Curve, Cycle, Face, HalfEdge, Region, Shell, Sketch, Solid, Surface, Vertex, @@ -92,30 +87,12 @@ impl Default for Surfaces { let mut store: Store = Store::new(); let xy_plane = store.reserve(); - store.insert( - xy_plane.clone(), - Surface::new(SurfaceGeometry { - u: GlobalPath::x_axis(), - v: Vector::unit_y(), - }), - ); + store.insert(xy_plane.clone(), Surface::new()); let xz_plane = store.reserve(); - store.insert( - xz_plane.clone(), - Surface::new(SurfaceGeometry { - u: GlobalPath::x_axis(), - v: Vector::unit_z(), - }), - ); + store.insert(xz_plane.clone(), Surface::new()); let yz_plane = store.reserve(); - store.insert( - yz_plane.clone(), - Surface::new(SurfaceGeometry { - u: GlobalPath::y_axis(), - v: Vector::unit_z(), - }), - ); + store.insert(yz_plane.clone(), Surface::new()); Self { store, diff --git a/crates/fj-core/src/operations/build/surface.rs b/crates/fj-core/src/operations/build/surface.rs index aeeeaaf53b..ec76916eee 100644 --- a/crates/fj-core/src/operations/build/surface.rs +++ b/crates/fj-core/src/operations/build/surface.rs @@ -47,7 +47,7 @@ pub trait BuildSurface { u: u.into(), v: v.into(), }; - let surface = Surface::new(geometry).insert(core); + let surface = Surface::new().insert(core); core.layers .geometry diff --git a/crates/fj-core/src/operations/transform/surface.rs b/crates/fj-core/src/operations/transform/surface.rs index 135d15022c..9afde5d984 100644 --- a/crates/fj-core/src/operations/transform/surface.rs +++ b/crates/fj-core/src/operations/transform/surface.rs @@ -18,7 +18,7 @@ impl TransformObject for Handle { .or_insert_with(|| { let geometry = core.layers.geometry.of_surface(self).transform(transform); - let surface = Surface::new(geometry).insert(core); + let surface = Surface::new().insert(core); core.layers .geometry From 721bc014226bf5265d41c928357edbf99d6d7735 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:43:58 +0100 Subject: [PATCH 14/21] Refactor --- crates/fj-core/src/objects/kinds/surface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/objects/kinds/surface.rs b/crates/fj-core/src/objects/kinds/surface.rs index 468c91e402..c872c26ea8 100644 --- a/crates/fj-core/src/objects/kinds/surface.rs +++ b/crates/fj-core/src/objects/kinds/surface.rs @@ -5,6 +5,6 @@ pub struct Surface {} impl Surface { /// Construct an instance of `Surface` pub fn new() -> Self { - Self {} + Self::default() } } From 3cc898307f7d6e032ea443926c901e366621b469 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:44:47 +0100 Subject: [PATCH 15/21] Refer to `Surface` through `HandleWrapper` --- crates/fj-core/src/objects/kinds/face.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/fj-core/src/objects/kinds/face.rs b/crates/fj-core/src/objects/kinds/face.rs index 1c6428dd44..5237ac07fc 100644 --- a/crates/fj-core/src/objects/kinds/face.rs +++ b/crates/fj-core/src/objects/kinds/face.rs @@ -2,7 +2,7 @@ use fj_math::Winding; use crate::{ objects::{Region, Surface}, - storage::Handle, + storage::{Handle, HandleWrapper}, }; /// A face of a shape @@ -31,14 +31,17 @@ use crate::{ /// [`Shell`]: crate::objects::Shell #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Face { - surface: Handle, + surface: HandleWrapper, region: Handle, } impl Face { /// Construct an instance of `Face` pub fn new(surface: Handle, region: Handle) -> Self { - Self { surface, region } + Self { + surface: surface.into(), + region, + } } /// Access the surface of the face From 24d9d9714c212b4f1d161fc0ef45355367aaa2db Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:46:03 +0100 Subject: [PATCH 16/21] Un-derive `Eq` from `Surface` --- crates/fj-core/src/objects/kinds/surface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/objects/kinds/surface.rs b/crates/fj-core/src/objects/kinds/surface.rs index c872c26ea8..86c8c7f3e8 100644 --- a/crates/fj-core/src/objects/kinds/surface.rs +++ b/crates/fj-core/src/objects/kinds/surface.rs @@ -1,5 +1,5 @@ /// A two-dimensional shape -#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +#[derive(Clone, Copy, Debug, Default, Hash)] pub struct Surface {} impl Surface { From f7d98b60043eed8f6406dc38c0da64465ca78bd2 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:47:54 +0100 Subject: [PATCH 17/21] Improve wording in doc comment --- crates/fj-core/src/objects/kinds/vertex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-core/src/objects/kinds/vertex.rs b/crates/fj-core/src/objects/kinds/vertex.rs index 4d9884ed95..a25f771da7 100644 --- a/crates/fj-core/src/objects/kinds/vertex.rs +++ b/crates/fj-core/src/objects/kinds/vertex.rs @@ -68,7 +68,7 @@ /// ## Equality /// /// `Vertex` contains no data and exists purely to be referenced via a `Handle`, -/// where `Handle::id` can be used to compare different instances of `Vertex`. +/// where `Handle::id` can be used to compare different instances of it. /// /// If `Vertex` had `Eq`/`PartialEq` implementations, it containing no data /// would mean that all instances of `Vertex` would be considered equal. This From 5b66aa7862600bb58bab5b3b9f0571626c0162d1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:48:04 +0100 Subject: [PATCH 18/21] Update documentation of `Surface` --- crates/fj-core/src/objects/kinds/surface.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/fj-core/src/objects/kinds/surface.rs b/crates/fj-core/src/objects/kinds/surface.rs index 86c8c7f3e8..06b95e3165 100644 --- a/crates/fj-core/src/objects/kinds/surface.rs +++ b/crates/fj-core/src/objects/kinds/surface.rs @@ -1,4 +1,19 @@ /// A two-dimensional shape +/// +/// +/// ## Equality +/// +/// `Surface` contains no data and exists purely to be referenced via a +/// `Handle`, where `Handle::id` can be used to compare different instances of +/// it. +/// +/// If `Surface` had `Eq`/`PartialEq` implementations, it containing no data +/// would mean that all instances of `Surface` would be considered equal. This +/// would be very error-prone. +/// +/// If you need to reference a `Surface` from a struct that needs to derive +/// `Eq`/`Ord`/..., you can use `HandleWrapper` to do that. It will +/// use `Handle::id` to provide those `Eq`/`Ord`/... implementations. #[derive(Clone, Copy, Debug, Default, Hash)] pub struct Surface {} From a4db923c6e3e354c6520763bf1efd19495302864 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:48:29 +0100 Subject: [PATCH 19/21] Improve formatting --- crates/fj-core/src/objects/stores.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-core/src/objects/stores.rs b/crates/fj-core/src/objects/stores.rs index 04d28bb5e8..2e8f5e9f28 100644 --- a/crates/fj-core/src/objects/stores.rs +++ b/crates/fj-core/src/objects/stores.rs @@ -91,6 +91,7 @@ impl Default for Surfaces { let xz_plane = store.reserve(); store.insert(xz_plane.clone(), Surface::new()); + let yz_plane = store.reserve(); store.insert(yz_plane.clone(), Surface::new()); From 4047ba6b63869dd15eaa4c85f070256d9d28bac3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:49:12 +0100 Subject: [PATCH 20/21] Inline redundant variable --- crates/fj-core/src/operations/build/surface.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-core/src/operations/build/surface.rs b/crates/fj-core/src/operations/build/surface.rs index ec76916eee..36a6748cf1 100644 --- a/crates/fj-core/src/operations/build/surface.rs +++ b/crates/fj-core/src/operations/build/surface.rs @@ -43,15 +43,15 @@ pub trait BuildSurface { v: impl Into>, core: &mut Core, ) -> Handle { - let geometry = SurfaceGeometry { - u: u.into(), - v: v.into(), - }; let surface = Surface::new().insert(core); - core.layers - .geometry - .define_surface(surface.clone(), geometry); + core.layers.geometry.define_surface( + surface.clone(), + SurfaceGeometry { + u: u.into(), + v: v.into(), + }, + ); surface } From aa665fb4c3a4d87fd55103b8ca0af34f0c02c79d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 13 Mar 2024 14:50:46 +0100 Subject: [PATCH 21/21] Refactor to improve clarity --- crates/fj-core/src/operations/transform/surface.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-core/src/operations/transform/surface.rs b/crates/fj-core/src/operations/transform/surface.rs index 9afde5d984..2205330447 100644 --- a/crates/fj-core/src/operations/transform/surface.rs +++ b/crates/fj-core/src/operations/transform/surface.rs @@ -16,10 +16,10 @@ impl TransformObject for Handle { cache .entry(self) .or_insert_with(|| { - let geometry = - core.layers.geometry.of_surface(self).transform(transform); let surface = Surface::new().insert(core); + let geometry = + core.layers.geometry.of_surface(self).transform(transform); core.layers .geometry .define_surface(surface.clone(), geometry);