From 97ba4dd2c1c82b40ca07c80da4796b51a7a6e34e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Jun 2022 11:30:49 +0200 Subject: [PATCH 1/4] Simplify `StructuralIssues` --- crates/fj-kernel/src/validation/mod.rs | 10 +++---- crates/fj-kernel/src/validation/structural.rs | 30 +++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 9ae69dffc..6ccd12200 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -162,7 +162,7 @@ impl ValidationError { /// Indicate whether validation found a missing curve pub fn missing_curve(&self, curve: &Handle>) -> bool { if let Self::Structural(StructuralIssues { missing_curve, .. }) = self { - return missing_curve.as_ref() == Some(curve); + return missing_curve.as_ref() == Some(&curve.get()); } false @@ -174,7 +174,7 @@ impl ValidationError { missing_vertices, .. }) = self { - return missing_vertices.contains(vertex); + return missing_vertices.contains(&vertex.get()); } false @@ -183,7 +183,7 @@ impl ValidationError { /// Indicate whether validation found a missing edge pub fn missing_edge(&self, edge: &Handle>) -> bool { if let Self::Structural(StructuralIssues { missing_edges, .. }) = self { - return missing_edges.contains(edge); + return missing_edges.contains(&edge.get()); } false @@ -195,7 +195,7 @@ impl ValidationError { missing_surface, .. }) = self { - return missing_surface.as_ref() == Some(surface); + return missing_surface.as_ref() == Some(&surface.get()); } false @@ -205,7 +205,7 @@ impl ValidationError { pub fn missing_cycle(&self, cycle: &Handle>) -> bool { if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self { - return missing_cycles.contains(cycle); + return missing_cycles.contains(&cycle.get()); } false diff --git a/crates/fj-kernel/src/validation/structural.rs b/crates/fj-kernel/src/validation/structural.rs index 596176dc5..dc1ed7456 100644 --- a/crates/fj-kernel/src/validation/structural.rs +++ b/crates/fj-kernel/src/validation/structural.rs @@ -14,11 +14,11 @@ pub fn validate_edge( let mut missing_vertices = HashSet::new(); if !curves.contains(&edge.curve.canonical()) { - missing_curve = Some(edge.curve.canonical()); + missing_curve = Some(edge.curve.canonical().get()); } for vertex in edge.vertices.iter() { if !vertices.contains(&vertex.canonical()) { - missing_vertices.insert(vertex.canonical().clone()); + missing_vertices.insert(vertex.canonical().get()); } } @@ -42,7 +42,7 @@ pub fn validate_cycle( let edge = edge.canonical(); if !edges.contains(&edge) { - missing_edges.insert(edge.clone()); + missing_edges.insert(edge.get()); } } @@ -66,13 +66,13 @@ pub fn validate_face( let mut missing_cycles = HashSet::new(); if !surfaces.contains(&face.surface) { - missing_surface = Some(face.surface.clone()); + missing_surface = Some(face.surface.get()); } for cycle in face.exteriors.as_handle().chain(face.interiors.as_handle()) { if !cycles.contains(&cycle) { - missing_cycles.insert(cycle); + missing_cycles.insert(cycle.get()); } } @@ -94,19 +94,19 @@ pub fn validate_face( #[derive(Debug, Default, thiserror::Error)] pub struct StructuralIssues { /// Missing curve found in edge validation - pub missing_curve: Option>>, + pub missing_curve: Option>, /// Missing vertices found in edge validation - pub missing_vertices: HashSet>, + pub missing_vertices: HashSet, /// Missing edges found in cycle validation - pub missing_edges: HashSet>>, + pub missing_edges: HashSet>, /// Missing surface found in face validation - pub missing_surface: Option>, + pub missing_surface: Option, /// Missing cycles found in face validation - pub missing_cycles: HashSet>>, + pub missing_cycles: HashSet>, } impl fmt::Display for StructuralIssues { @@ -114,30 +114,30 @@ impl fmt::Display for StructuralIssues { writeln!(f, "Structural issues found:")?; if let Some(curve) = &self.missing_curve { - writeln!(f, "- Missing curve: {:?}", curve.get())?; + writeln!(f, "- Missing curve: {:?}", curve)?; } if !self.missing_vertices.is_empty() { writeln!(f, "- Missing vertices:")?; for vertex in &self.missing_vertices { - writeln!(f, " - {:?}", vertex.get())?; + writeln!(f, " - {:?}", vertex)?; } } if !self.missing_edges.is_empty() { writeln!(f, "- Missing edges:")?; for edge in &self.missing_edges { - writeln!(f, " - {}", edge.get())?; + writeln!(f, " - {}", edge)?; } } if let Some(surface) = &self.missing_surface { - writeln!(f, "- Missing surface: {:?}", surface.get())?; + writeln!(f, "- Missing surface: {:?}", surface)?; } if !self.missing_cycles.is_empty() { writeln!(f, "- Missing cycles:")?; for cycle in &self.missing_cycles { - writeln!(f, " - {:?}", cycle.get())?; + writeln!(f, " - {:?}", cycle)?; } } From d254d8be9fdd985d7142a17f4005da52c9c20182 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Jun 2022 11:31:40 +0200 Subject: [PATCH 2/4] Simplify `UniquenessIssues` --- crates/fj-kernel/src/validation/uniqueness.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validation/uniqueness.rs b/crates/fj-kernel/src/validation/uniqueness.rs index 3287ba1f7..e916ae0c7 100644 --- a/crates/fj-kernel/src/validation/uniqueness.rs +++ b/crates/fj-kernel/src/validation/uniqueness.rs @@ -12,7 +12,7 @@ pub fn validate_vertex( for existing in vertices { if &existing.get() == vertex { return Err(UniquenessIssues { - duplicate_vertex: Some(existing.clone()), + duplicate_vertex: Some(existing.get()), ..UniquenessIssues::default() }); } @@ -61,7 +61,7 @@ pub fn validate_edge( #[derive(Debug, Default, thiserror::Error)] pub struct UniquenessIssues { /// Duplicate vertex found - pub duplicate_vertex: Option>, + pub duplicate_vertex: Option, /// Duplicate edge found pub duplicate_edge: Option, From d0fc188ff0c0644f0fabbf444a1e793c8071cf78 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Jun 2022 11:33:36 +0200 Subject: [PATCH 3/4] Simplify API of `ValidationError` --- crates/fj-kernel/src/validation/mod.rs | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 6ccd12200..2c2de20af 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -40,7 +40,7 @@ use fj_math::Scalar; use crate::{ objects::{Curve, Cycle, Edge, Surface, Vertex}, - shape::{Handle, Shape}, + shape::Shape, }; /// Validate the given [`Shape`] @@ -160,52 +160,52 @@ pub enum ValidationError { impl ValidationError { /// Indicate whether validation found a missing curve - pub fn missing_curve(&self, curve: &Handle>) -> bool { + pub fn missing_curve(&self, curve: &Curve<3>) -> bool { if let Self::Structural(StructuralIssues { missing_curve, .. }) = self { - return missing_curve.as_ref() == Some(&curve.get()); + return missing_curve.as_ref() == Some(curve); } false } /// Indicate whether validation found a missing vertex - pub fn missing_vertex(&self, vertex: &Handle) -> bool { + pub fn missing_vertex(&self, vertex: &Vertex) -> bool { if let Self::Structural(StructuralIssues { missing_vertices, .. }) = self { - return missing_vertices.contains(&vertex.get()); + return missing_vertices.contains(vertex); } false } /// Indicate whether validation found a missing edge - pub fn missing_edge(&self, edge: &Handle>) -> bool { + pub fn missing_edge(&self, edge: &Edge<3>) -> bool { if let Self::Structural(StructuralIssues { missing_edges, .. }) = self { - return missing_edges.contains(&edge.get()); + return missing_edges.contains(edge); } false } /// Indicate whether validation found a missing surface - pub fn missing_surface(&self, surface: &Handle) -> bool { + pub fn missing_surface(&self, surface: &Surface) -> bool { if let Self::Structural(StructuralIssues { missing_surface, .. }) = self { - return missing_surface.as_ref() == Some(&surface.get()); + return missing_surface.as_ref() == Some(surface); } false } /// Indicate whether validation found a missing cycle - pub fn missing_cycle(&self, cycle: &Handle>) -> bool { + pub fn missing_cycle(&self, cycle: &Cycle<3>) -> bool { if let Self::Structural(StructuralIssues { missing_cycles, .. }) = self { - return missing_cycles.contains(&cycle.get()); + return missing_cycles.contains(cycle); } false @@ -274,7 +274,7 @@ mod tests { shape.insert(Cycle::new(vec![edge.clone()])); let err = validate(shape.clone(), &ValidationConfig::default()).unwrap_err(); - assert!(err.missing_edge(&edge)); + assert!(err.missing_edge(&edge.get())); // Referring to edge that *is* from the same shape. Should work. let edge = Edge::builder(&mut shape) @@ -301,9 +301,9 @@ mod tests { }); let err = validate(shape.clone(), &ValidationConfig::default()).unwrap_err(); - assert!(err.missing_curve(&curve)); - assert!(err.missing_vertex(&a.canonical())); - assert!(err.missing_vertex(&b.canonical())); + assert!(err.missing_curve(&curve.get())); + assert!(err.missing_vertex(&a.canonical().get())); + assert!(err.missing_vertex(&b.canonical().get())); let curve = shape.insert(Curve::x_axis()); let a = Vertex::builder(&mut shape).build_from_point([1., 0., 0.]); @@ -339,8 +339,8 @@ mod tests { )); let err = validate(shape.clone(), &ValidationConfig::default()).unwrap_err(); - assert!(err.missing_surface(&surface)); - assert!(err.missing_cycle(&cycle.canonical())); + assert!(err.missing_surface(&surface.get())); + assert!(err.missing_cycle(&cycle.canonical().get())); let surface = shape.insert(Surface::xy_plane()); let cycle = From ef7a36e6bc52f556fc590104f8fa65bd47b0a959 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Jun 2022 11:40:32 +0200 Subject: [PATCH 4/4] Don't rely on access to `Handle`s in validation Access to `Handle`s is not required for the validation code to do its job, and not relying on them being available makes some upcoming changes easier. --- crates/fj-kernel/src/validation/mod.rs | 24 ++++++------ crates/fj-kernel/src/validation/structural.rs | 37 ++++++++----------- crates/fj-kernel/src/validation/uniqueness.rs | 17 ++++----- 3 files changed, 34 insertions(+), 44 deletions(-) diff --git a/crates/fj-kernel/src/validation/mod.rs b/crates/fj-kernel/src/validation/mod.rs index 2c2de20af..706a05470 100644 --- a/crates/fj-kernel/src/validation/mod.rs +++ b/crates/fj-kernel/src/validation/mod.rs @@ -54,31 +54,31 @@ pub fn validate( let mut surfaces = HashSet::new(); let mut vertices = HashSet::new(); - for curve in shape.curves() { + for curve in shape.curves().map(|handle| handle.get()) { curves.insert(curve); } - for vertex in shape.vertices() { - uniqueness::validate_vertex(&vertex.get(), &vertices)?; + for vertex in shape.vertices().map(|handle| handle.get()) { + uniqueness::validate_vertex(&vertex, &vertices)?; vertices.insert(vertex); } - for edge in shape.edges() { - coherence::validate_edge(&edge.get(), config.identical_max_distance)?; - structural::validate_edge(&edge.get(), &curves, &vertices)?; - uniqueness::validate_edge(&edge.get(), &edges)?; + for edge in shape.edges().map(|handle| handle.get()) { + coherence::validate_edge(&edge, config.identical_max_distance)?; + structural::validate_edge(&edge, &curves, &vertices)?; + uniqueness::validate_edge(&edge, &edges)?; edges.insert(edge); } - for cycle in shape.cycles() { - structural::validate_cycle(&cycle.get(), &edges)?; + for cycle in shape.cycles().map(|handle| handle.get()) { + structural::validate_cycle(&cycle, &edges)?; cycles.insert(cycle); } - for surface in shape.surfaces() { + for surface in shape.surfaces().map(|handle| handle.get()) { surfaces.insert(surface); } - for face in shape.faces() { - structural::validate_face(&face.get(), &cycles, &surfaces)?; + for face in shape.faces().map(|handle| handle.get()) { + structural::validate_face(&face, &cycles, &surfaces)?; } Ok(Validated(shape)) diff --git a/crates/fj-kernel/src/validation/structural.rs b/crates/fj-kernel/src/validation/structural.rs index dc1ed7456..f6fe0ca4a 100644 --- a/crates/fj-kernel/src/validation/structural.rs +++ b/crates/fj-kernel/src/validation/structural.rs @@ -1,24 +1,21 @@ use std::{collections::HashSet, fmt}; -use crate::{ - objects::{Curve, Cycle, Edge, Face, Surface, Vertex}, - shape::Handle, -}; +use crate::objects::{Curve, Cycle, Edge, Face, Surface, Vertex}; pub fn validate_edge( edge: &Edge<3>, - curves: &HashSet>>, - vertices: &HashSet>, + curves: &HashSet>, + vertices: &HashSet, ) -> Result<(), StructuralIssues> { let mut missing_curve = None; let mut missing_vertices = HashSet::new(); - if !curves.contains(&edge.curve.canonical()) { + if !curves.contains(&edge.curve()) { missing_curve = Some(edge.curve.canonical().get()); } - for vertex in edge.vertices.iter() { - if !vertices.contains(&vertex.canonical()) { - missing_vertices.insert(vertex.canonical().get()); + for vertex in edge.vertices().into_iter().flatten() { + if !vertices.contains(&vertex) { + missing_vertices.insert(vertex); } } @@ -35,14 +32,12 @@ pub fn validate_edge( pub fn validate_cycle( cycle: &Cycle<3>, - edges: &HashSet>>, + edges: &HashSet>, ) -> Result<(), StructuralIssues> { let mut missing_edges = HashSet::new(); - for edge in &cycle.edges { - let edge = edge.canonical(); - + for edge in cycle.edges() { if !edges.contains(&edge) { - missing_edges.insert(edge.get()); + missing_edges.insert(edge); } } @@ -58,21 +53,19 @@ pub fn validate_cycle( pub fn validate_face( face: &Face, - cycles: &HashSet>>, - surfaces: &HashSet>, + cycles: &HashSet>, + surfaces: &HashSet, ) -> Result<(), StructuralIssues> { if let Face::Face(face) = face { let mut missing_surface = None; let mut missing_cycles = HashSet::new(); - if !surfaces.contains(&face.surface) { + if !surfaces.contains(&face.surface()) { missing_surface = Some(face.surface.get()); } - for cycle in - face.exteriors.as_handle().chain(face.interiors.as_handle()) - { + for cycle in face.all_cycles() { if !cycles.contains(&cycle) { - missing_cycles.insert(cycle.get()); + missing_cycles.insert(cycle); } } diff --git a/crates/fj-kernel/src/validation/uniqueness.rs b/crates/fj-kernel/src/validation/uniqueness.rs index e916ae0c7..67c6af6a2 100644 --- a/crates/fj-kernel/src/validation/uniqueness.rs +++ b/crates/fj-kernel/src/validation/uniqueness.rs @@ -1,18 +1,15 @@ use std::{collections::HashSet, fmt}; -use crate::{ - objects::{Edge, Vertex}, - shape::Handle, -}; +use crate::objects::{Edge, Vertex}; pub fn validate_vertex( vertex: &Vertex, - vertices: &HashSet>, + vertices: &HashSet, ) -> Result<(), UniquenessIssues> { for existing in vertices { - if &existing.get() == vertex { + if existing == vertex { return Err(UniquenessIssues { - duplicate_vertex: Some(existing.get()), + duplicate_vertex: Some(*existing), ..UniquenessIssues::default() }); } @@ -30,13 +27,13 @@ pub fn validate_vertex( /// this code will have to be updated. pub fn validate_edge( edge: &Edge<3>, - edges: &HashSet>>, + edges: &HashSet>, ) -> Result<(), UniquenessIssues> { for existing in edges { - if existing.get().vertices.are_same(&edge.vertices) { + if existing.vertices.are_same(&edge.vertices) { return Err(UniquenessIssues { duplicate_edge: Some(DuplicateEdge { - existing: existing.get(), + existing: existing.clone(), new: edge.clone(), }), ..UniquenessIssues::default()