From b0dd12a3a1c8c4a9063036849ee671c0bb8269d9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 31 Mar 2022 15:12:55 +0200 Subject: [PATCH] Extract validation code into dedicated trait This makes the various `add_*` methods more uniform, opening the door towards further clean-ups, and also might make it easier to test validation. --- fj-kernel/src/shape/topology.rs | 89 ++------------------- fj-kernel/src/shape/validate.rs | 132 +++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 85 deletions(-) diff --git a/fj-kernel/src/shape/topology.rs b/fj-kernel/src/shape/topology.rs index 79ff92179..3cad423ae 100644 --- a/fj-kernel/src/shape/topology.rs +++ b/fj-kernel/src/shape/topology.rs @@ -1,12 +1,10 @@ -use std::{collections::HashSet, marker::PhantomData}; +use std::marker::PhantomData; use fj_math::Scalar; use crate::topology::{Cycle, Edge, Face, Vertex}; -use super::{ - stores::Stores, Iter, StructuralIssues, ValidationError, ValidationResult, -}; +use super::{stores::Stores, validate::Validate as _, Iter, ValidationResult}; /// The vertices of a shape pub struct Topology<'r> { @@ -39,18 +37,7 @@ impl Topology<'_> { /// In the future, this method is likely to validate more than it already /// does. See documentation of [`crate::kernel`] for some context on that. pub fn add_vertex(&mut self, vertex: Vertex) -> ValidationResult { - if !self.stores.points.contains(&vertex.point) { - return Err(StructuralIssues::default().into()); - } - for existing in self.stores.vertices.iter() { - let distance = - (existing.get().point() - vertex.point()).magnitude(); - - if distance < self.min_distance { - return Err(ValidationError::Uniqueness); - } - } - + vertex.validate(self.min_distance, &self.stores)?; let handle = self.stores.vertices.insert(vertex); Ok(handle) } @@ -70,29 +57,7 @@ impl Topology<'_> { /// converted into curve coordinates, which is likely not the caller's /// intention. pub fn add_edge(&mut self, edge: Edge) -> ValidationResult { - let mut missing_curve = None; - let mut missing_vertices = HashSet::new(); - - if !self.stores.curves.contains(&edge.curve) { - missing_curve = Some(edge.curve.clone()); - } - for vertices in &edge.vertices { - for vertex in vertices { - if !self.stores.vertices.contains(vertex) { - missing_vertices.insert(vertex.clone()); - } - } - } - - if missing_curve.is_some() || !missing_vertices.is_empty() { - return Err(StructuralIssues { - missing_curve, - missing_vertices, - ..StructuralIssues::default() - } - .into()); - } - + edge.validate(self.min_distance, &self.stores)?; let handle = self.stores.edges.insert(edge); Ok(handle) } @@ -109,21 +74,7 @@ impl Topology<'_> { /// - That the cycle is not self-overlapping. /// - That there exists no duplicate cycle, with the same edges. pub fn add_cycle(&mut self, cycle: Cycle) -> ValidationResult { - let mut missing_edges = HashSet::new(); - for edge in &cycle.edges { - if !self.stores.edges.contains(edge) { - missing_edges.insert(edge.clone()); - } - } - - if !missing_edges.is_empty() { - return Err(StructuralIssues { - missing_edges, - ..StructuralIssues::default() - } - .into()); - } - + cycle.validate(self.min_distance, &self.stores)?; let handle = self.stores.cycles.insert(cycle); Ok(handle) } @@ -134,35 +85,7 @@ impl Topology<'_> { /// cycles it refers to are part of the shape). Returns an error, if that is /// not the case. pub fn add_face(&mut self, face: Face) -> ValidationResult { - if let Face::Face { - surface, - exteriors, - interiors, - .. - } = &face - { - let mut missing_surface = None; - let mut missing_cycles = HashSet::new(); - - if !self.stores.surfaces.contains(surface) { - missing_surface = Some(surface.clone()); - } - for cycle in exteriors.iter().chain(interiors) { - if !self.stores.cycles.contains(cycle) { - missing_cycles.insert(cycle.clone()); - } - } - - if missing_surface.is_some() || !missing_cycles.is_empty() { - return Err(StructuralIssues { - missing_surface, - missing_cycles, - ..StructuralIssues::default() - } - .into()); - } - } - + face.validate(self.min_distance, &self.stores)?; let handle = self.stores.faces.insert(face); Ok(handle) } diff --git a/fj-kernel/src/shape/validate.rs b/fj-kernel/src/shape/validate.rs index 3739ca0c2..492cb07fd 100644 --- a/fj-kernel/src/shape/validate.rs +++ b/fj-kernel/src/shape/validate.rs @@ -1,11 +1,139 @@ use std::collections::HashSet; +use fj_math::Scalar; + use crate::{ geometry::{Curve, Surface}, - topology::{Cycle, Edge, Vertex}, + topology::{Cycle, Edge, Face, Vertex}, }; -use super::Handle; +use super::{stores::Stores, Handle}; + +pub trait Validate { + fn validate( + &self, + min_distance: Scalar, + stores: &Stores, + ) -> Result<(), ValidationError>; +} + +impl Validate for Vertex { + fn validate( + &self, + min_distance: Scalar, + stores: &Stores, + ) -> Result<(), ValidationError> { + if !stores.points.contains(&self.point) { + return Err(StructuralIssues::default().into()); + } + for existing in stores.vertices.iter() { + let distance = (existing.get().point() - self.point()).magnitude(); + + if distance < min_distance { + return Err(ValidationError::Uniqueness); + } + } + + Ok(()) + } +} + +impl Validate for Edge { + fn validate( + &self, + _: Scalar, + stores: &Stores, + ) -> Result<(), ValidationError> { + let mut missing_curve = None; + let mut missing_vertices = HashSet::new(); + + if !stores.curves.contains(&self.curve) { + missing_curve = Some(self.curve.clone()); + } + for vertices in &self.vertices { + for vertex in vertices { + if !stores.vertices.contains(vertex) { + missing_vertices.insert(vertex.clone()); + } + } + } + + if missing_curve.is_some() || !missing_vertices.is_empty() { + return Err(StructuralIssues { + missing_curve, + missing_vertices, + ..StructuralIssues::default() + } + .into()); + } + + Ok(()) + } +} + +impl Validate for Cycle { + fn validate( + &self, + _: Scalar, + stores: &Stores, + ) -> Result<(), ValidationError> { + let mut missing_edges = HashSet::new(); + for edge in &self.edges { + if !stores.edges.contains(edge) { + missing_edges.insert(edge.clone()); + } + } + + if !missing_edges.is_empty() { + return Err(StructuralIssues { + missing_edges, + ..StructuralIssues::default() + } + .into()); + } + + Ok(()) + } +} + +impl Validate for Face { + fn validate( + &self, + _: Scalar, + stores: &Stores, + ) -> Result<(), ValidationError> { + if let Face::Face { + surface, + exteriors, + interiors, + .. + } = self + { + let mut missing_surface = None; + let mut missing_cycles = HashSet::new(); + + if !stores.surfaces.contains(surface) { + missing_surface = Some(surface.clone()); + } + for cycle in exteriors.iter().chain(interiors) { + if !stores.cycles.contains(cycle) { + missing_cycles.insert(cycle.clone()); + } + } + + if missing_surface.is_some() || !missing_cycles.is_empty() { + return Err(StructuralIssues { + missing_surface, + missing_cycles, + ..StructuralIssues::default() + } + .into()); + } + } + + Ok(()) + } +} /// Returned by the various `add_` methods of the [`Shape`] API pub type ValidationResult = Result, ValidationError>;