From ee266a7da2cb5243f47392fb5c5d8443674772f9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Nov 2022 12:00:02 +0100 Subject: [PATCH 1/2] Don't use old validation infrastructure in `Shape` It doesn't do anything useful anymore, and has been replaced by the new validation infrastructure, which validates automatically on insertion. --- crates/fj-operations/src/difference_2d.rs | 6 +++--- crates/fj-operations/src/group.rs | 10 +++++----- crates/fj-operations/src/lib.rs | 20 ++++++++------------ crates/fj-operations/src/shape_processor.rs | 2 +- crates/fj-operations/src/sketch.rs | 8 ++++---- crates/fj-operations/src/sweep.rs | 8 ++++---- crates/fj-operations/src/transform.rs | 7 +++---- 7 files changed, 28 insertions(+), 33 deletions(-) diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index ea27b25fa..4c6493793 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -5,7 +5,7 @@ use fj_kernel::{ algorithms::reverse::Reverse, iter::ObjectIters, objects::{Face, Objects, Sketch}, - validate::{Validate, Validated, ValidationConfig, ValidationError}, + validate::{ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -19,7 +19,7 @@ impl Shape for fj::Difference2d { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { // This method assumes that `b` is fully contained within `a`: // https://github.com/hannobraun/Fornjot/issues/92 @@ -87,7 +87,7 @@ impl Shape for fj::Difference2d { } let difference = Sketch::builder(objects).with_faces(faces).build(); - difference.deref().clone().validate_with_config(config) + Ok(difference.deref().clone()) } fn bounding_volume(&self) -> Aabb<3> { diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index 37a2e40b4..271754e9d 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -1,7 +1,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ objects::{FaceSet, Objects}, - validate::{Validate, Validated, ValidationConfig, ValidationError}, + validate::{ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -15,16 +15,16 @@ impl Shape for fj::Group { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { let mut faces = FaceSet::new(); let a = self.a.compute_brep(config, objects, debug_info)?; let b = self.b.compute_brep(config, objects, debug_info)?; - faces.extend(a.into_inner()); - faces.extend(b.into_inner()); + faces.extend(a); + faces.extend(b); - faces.validate_with_config(config) + Ok(faces) } fn bounding_volume(&self) -> Aabb<3> { diff --git a/crates/fj-operations/src/lib.rs b/crates/fj-operations/src/lib.rs index 33d0d94f5..dae5496d9 100644 --- a/crates/fj-operations/src/lib.rs +++ b/crates/fj-operations/src/lib.rs @@ -27,7 +27,7 @@ mod transform; use fj_interop::debug::DebugInfo; use fj_kernel::{ objects::{FaceSet, Objects, Sketch}, - validate::{Validate, Validated, ValidationConfig, ValidationError}, + validate::{ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -42,7 +42,7 @@ pub trait Shape { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError>; + ) -> Result; /// Access the axis-aligned bounding box of a shape /// @@ -59,28 +59,24 @@ impl Shape for fj::Shape { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { match self { - Self::Shape2d(shape) => shape + Self::Shape2d(shape) => Ok(shape .compute_brep(config, objects, debug_info)? - .into_inner() .faces() - .clone() - .validate_with_config(config), + .clone()), Self::Group(shape) => { shape.compute_brep(config, objects, debug_info) } - Self::Sweep(shape) => shape + Self::Sweep(shape) => Ok(shape .compute_brep(config, objects, debug_info)? - .into_inner() .shells() .map(|shell| shell.faces().clone()) .reduce(|mut a, b| { a.extend(b); a }) - .unwrap_or_default() - .validate_with_config(config), + .unwrap_or_default()), Self::Transform(shape) => { shape.compute_brep(config, objects, debug_info) } @@ -105,7 +101,7 @@ impl Shape for fj::Shape2d { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { match self { Self::Difference(shape) => { shape.compute_brep(config, objects, debug_info) diff --git a/crates/fj-operations/src/shape_processor.rs b/crates/fj-operations/src/shape_processor.rs index 557a02d72..6b32cef48 100644 --- a/crates/fj-operations/src/shape_processor.rs +++ b/crates/fj-operations/src/shape_processor.rs @@ -46,7 +46,7 @@ impl ShapeProcessor { let objects = Objects::new(); let mut debug_info = DebugInfo::new(); let shape = shape.compute_brep(&config, &objects, &mut debug_info)?; - let mesh = (&shape.into_inner(), tolerance).triangulate(); + let mesh = (&shape, tolerance).triangulate(); Ok(ProcessedShape { aabb, diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 8e838ec2b..0d786633b 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -5,7 +5,7 @@ use fj_kernel::{ builder::HalfEdgeBuilder, objects::{Cycle, Face, HalfEdge, Objects, Sketch}, partial::HasPartial, - validate::{Validate, Validated, ValidationConfig, ValidationError}, + validate::{ValidationConfig, ValidationError}, }; use fj_math::{Aabb, Point}; @@ -16,10 +16,10 @@ impl Shape for fj::Sketch { fn compute_brep( &self, - config: &ValidationConfig, + _: &ValidationConfig, objects: &Objects, _: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { let surface = objects.surfaces.xy_plane(); let face = match self.chain() { @@ -51,7 +51,7 @@ impl Shape for fj::Sketch { }; let sketch = Sketch::builder(objects).with_faces([face]).build(); - sketch.deref().clone().validate_with_config(config) + Ok(sketch.deref().clone()) } fn bounding_volume(&self) -> Aabb<3> { diff --git a/crates/fj-operations/src/sweep.rs b/crates/fj-operations/src/sweep.rs index 55f25f201..28b7b1d70 100644 --- a/crates/fj-operations/src/sweep.rs +++ b/crates/fj-operations/src/sweep.rs @@ -4,7 +4,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::sweep::Sweep, objects::{Objects, Solid}, - validate::{Validate, Validated, ValidationConfig, ValidationError}, + validate::{ValidationConfig, ValidationError}, }; use fj_math::{Aabb, Vector}; @@ -18,14 +18,14 @@ impl Shape for fj::Sweep { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { let sketch = self.shape().compute_brep(config, objects, debug_info)?; - let sketch = objects.sketches.insert(sketch.into_inner())?; + let sketch = objects.sketches.insert(sketch)?; let path = Vector::from(self.path()); let solid = sketch.sweep(path, objects)?; - solid.deref().clone().validate_with_config(config) + Ok(solid.deref().clone()) } fn bounding_volume(&self) -> Aabb<3> { diff --git a/crates/fj-operations/src/transform.rs b/crates/fj-operations/src/transform.rs index d84626cf7..8e18db426 100644 --- a/crates/fj-operations/src/transform.rs +++ b/crates/fj-operations/src/transform.rs @@ -2,7 +2,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::transform::TransformObject, objects::{FaceSet, Objects}, - validate::{Validate, Validated, ValidationConfig, ValidationError}, + validate::{ValidationConfig, ValidationError}, }; use fj_math::{Aabb, Transform, Vector}; @@ -16,14 +16,13 @@ impl Shape for fj::Transform { config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, - ) -> Result, ValidationError> { + ) -> Result { let faces = self .shape .compute_brep(config, objects, debug_info)? - .into_inner() .transform(&make_transform(self), objects)?; - faces.validate_with_config(config) + Ok(faces) } fn bounding_volume(&self) -> Aabb<3> { From 66fb1934e39f5815e98ed7bb470f5b8db1bfa101 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Nov 2022 12:03:24 +0100 Subject: [PATCH 2/2] Remove unused argument from trait method --- crates/fj-operations/src/difference_2d.rs | 10 ++++---- crates/fj-operations/src/group.rs | 7 +++--- crates/fj-operations/src/lib.rs | 28 ++++++--------------- crates/fj-operations/src/shape_processor.rs | 5 ++-- crates/fj-operations/src/sketch.rs | 3 +-- crates/fj-operations/src/sweep.rs | 5 ++-- crates/fj-operations/src/transform.rs | 5 ++-- 7 files changed, 23 insertions(+), 40 deletions(-) diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 4c6493793..c5c5e9e30 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -5,7 +5,7 @@ use fj_kernel::{ algorithms::reverse::Reverse, iter::ObjectIters, objects::{Face, Objects, Sketch}, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::Aabb; @@ -16,7 +16,6 @@ impl Shape for fj::Difference2d { fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result { @@ -28,9 +27,10 @@ impl Shape for fj::Difference2d { let mut exteriors = Vec::new(); let mut interiors = Vec::new(); - let [a, b] = self.shapes().each_ref_ext().try_map_ext(|shape| { - shape.compute_brep(config, objects, debug_info) - })?; + let [a, b] = self + .shapes() + .each_ref_ext() + .try_map_ext(|shape| shape.compute_brep(objects, debug_info))?; if let Some(face) = a.face_iter().next() { // If there's at least one face to subtract from, we can proceed. diff --git a/crates/fj-operations/src/group.rs b/crates/fj-operations/src/group.rs index 271754e9d..f34be965f 100644 --- a/crates/fj-operations/src/group.rs +++ b/crates/fj-operations/src/group.rs @@ -1,7 +1,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ objects::{FaceSet, Objects}, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::Aabb; @@ -12,14 +12,13 @@ impl Shape for fj::Group { fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result { let mut faces = FaceSet::new(); - let a = self.a.compute_brep(config, objects, debug_info)?; - let b = self.b.compute_brep(config, objects, debug_info)?; + let a = self.a.compute_brep(objects, debug_info)?; + let b = self.b.compute_brep(objects, debug_info)?; faces.extend(a); faces.extend(b); diff --git a/crates/fj-operations/src/lib.rs b/crates/fj-operations/src/lib.rs index dae5496d9..a74e6107a 100644 --- a/crates/fj-operations/src/lib.rs +++ b/crates/fj-operations/src/lib.rs @@ -27,7 +27,7 @@ mod transform; use fj_interop::debug::DebugInfo; use fj_kernel::{ objects::{FaceSet, Objects, Sketch}, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::Aabb; @@ -39,7 +39,6 @@ pub trait Shape { /// Compute the boundary representation of the shape fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result; @@ -56,20 +55,16 @@ impl Shape for fj::Shape { fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result { match self { - Self::Shape2d(shape) => Ok(shape - .compute_brep(config, objects, debug_info)? - .faces() - .clone()), - Self::Group(shape) => { - shape.compute_brep(config, objects, debug_info) + Self::Shape2d(shape) => { + Ok(shape.compute_brep(objects, debug_info)?.faces().clone()) } + Self::Group(shape) => shape.compute_brep(objects, debug_info), Self::Sweep(shape) => Ok(shape - .compute_brep(config, objects, debug_info)? + .compute_brep(objects, debug_info)? .shells() .map(|shell| shell.faces().clone()) .reduce(|mut a, b| { @@ -77,9 +72,7 @@ impl Shape for fj::Shape { a }) .unwrap_or_default()), - Self::Transform(shape) => { - shape.compute_brep(config, objects, debug_info) - } + Self::Transform(shape) => shape.compute_brep(objects, debug_info), } } @@ -98,17 +91,12 @@ impl Shape for fj::Shape2d { fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result { match self { - Self::Difference(shape) => { - shape.compute_brep(config, objects, debug_info) - } - Self::Sketch(shape) => { - shape.compute_brep(config, objects, debug_info) - } + Self::Difference(shape) => shape.compute_brep(objects, debug_info), + Self::Sketch(shape) => shape.compute_brep(objects, debug_info), } } diff --git a/crates/fj-operations/src/shape_processor.rs b/crates/fj-operations/src/shape_processor.rs index 6b32cef48..87c17fd21 100644 --- a/crates/fj-operations/src/shape_processor.rs +++ b/crates/fj-operations/src/shape_processor.rs @@ -7,7 +7,7 @@ use fj_kernel::{ triangulate::Triangulate, }, objects::Objects, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::Scalar; @@ -42,10 +42,9 @@ impl ShapeProcessor { Some(user_defined_tolerance) => user_defined_tolerance, }; - let config = ValidationConfig::default(); let objects = Objects::new(); let mut debug_info = DebugInfo::new(); - let shape = shape.compute_brep(&config, &objects, &mut debug_info)?; + let shape = shape.compute_brep(&objects, &mut debug_info)?; let mesh = (&shape, tolerance).triangulate(); Ok(ProcessedShape { diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 0d786633b..3b0573d76 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -5,7 +5,7 @@ use fj_kernel::{ builder::HalfEdgeBuilder, objects::{Cycle, Face, HalfEdge, Objects, Sketch}, partial::HasPartial, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::{Aabb, Point}; @@ -16,7 +16,6 @@ impl Shape for fj::Sketch { fn compute_brep( &self, - _: &ValidationConfig, objects: &Objects, _: &mut DebugInfo, ) -> Result { diff --git a/crates/fj-operations/src/sweep.rs b/crates/fj-operations/src/sweep.rs index 28b7b1d70..9286c3d49 100644 --- a/crates/fj-operations/src/sweep.rs +++ b/crates/fj-operations/src/sweep.rs @@ -4,7 +4,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::sweep::Sweep, objects::{Objects, Solid}, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::{Aabb, Vector}; @@ -15,11 +15,10 @@ impl Shape for fj::Sweep { fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result { - let sketch = self.shape().compute_brep(config, objects, debug_info)?; + let sketch = self.shape().compute_brep(objects, debug_info)?; let sketch = objects.sketches.insert(sketch)?; let path = Vector::from(self.path()); diff --git a/crates/fj-operations/src/transform.rs b/crates/fj-operations/src/transform.rs index 8e18db426..5cde79679 100644 --- a/crates/fj-operations/src/transform.rs +++ b/crates/fj-operations/src/transform.rs @@ -2,7 +2,7 @@ use fj_interop::debug::DebugInfo; use fj_kernel::{ algorithms::transform::TransformObject, objects::{FaceSet, Objects}, - validate::{ValidationConfig, ValidationError}, + validate::ValidationError, }; use fj_math::{Aabb, Transform, Vector}; @@ -13,13 +13,12 @@ impl Shape for fj::Transform { fn compute_brep( &self, - config: &ValidationConfig, objects: &Objects, debug_info: &mut DebugInfo, ) -> Result { let faces = self .shape - .compute_brep(config, objects, debug_info)? + .compute_brep(objects, debug_info)? .transform(&make_transform(self), objects)?; Ok(faces)