From dd9baa253ee7a988e7125dce88e6a24d271ff044 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 14:31:16 +0100 Subject: [PATCH 1/8] Move `Approx` to dedicated module I plan to move the rest of the approximation code there. --- src/kernel/approximation.rs | 9 +++++++++ src/kernel/mod.rs | 1 + src/kernel/topology/edges.rs | 13 +++++-------- 3 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 src/kernel/approximation.rs diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs new file mode 100644 index 000000000..273f1655f --- /dev/null +++ b/src/kernel/approximation.rs @@ -0,0 +1,9 @@ +use parry3d_f64::shape::Segment; + +use crate::math::Point; + +/// An approximation of one or more edges +pub struct Approx { + pub vertices: Vec>, + pub segments: Vec, +} diff --git a/src/kernel/mod.rs b/src/kernel/mod.rs index 45dd12d7b..80f61944e 100644 --- a/src/kernel/mod.rs +++ b/src/kernel/mod.rs @@ -1,3 +1,4 @@ +pub mod approximation; pub mod geometry; pub mod shapes; pub mod topology; diff --git a/src/kernel/topology/edges.rs b/src/kernel/topology/edges.rs index 3ca661075..2f023db4c 100644 --- a/src/kernel/topology/edges.rs +++ b/src/kernel/topology/edges.rs @@ -1,8 +1,11 @@ use nalgebra::vector; -use parry3d_f64::{math::Isometry, shape::Segment}; +use parry3d_f64::math::Isometry; use crate::{ - kernel::geometry::{Circle, Curve}, + kernel::{ + approximation::Approx, + geometry::{Circle, Curve}, + }, math::Point, }; @@ -201,9 +204,3 @@ impl Edge { Approx { vertices, segments } } } - -/// An approximation of one or more edges -pub struct Approx { - pub vertices: Vec>, - pub segments: Vec, -} From dceea4aea8332b6baf4c3f9b9af9d4b36cb6dc1b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 14:32:06 +0100 Subject: [PATCH 2/8] Rename `Approx` to `Approximation` It isn't really used often enough to warrant the shortened name. --- src/kernel/approximation.rs | 2 +- src/kernel/topology/edges.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index 273f1655f..f98a58532 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -3,7 +3,7 @@ use parry3d_f64::shape::Segment; use crate::math::Point; /// An approximation of one or more edges -pub struct Approx { +pub struct Approximation { pub vertices: Vec>, pub segments: Vec, } diff --git a/src/kernel/topology/edges.rs b/src/kernel/topology/edges.rs index 2f023db4c..9bfd85c01 100644 --- a/src/kernel/topology/edges.rs +++ b/src/kernel/topology/edges.rs @@ -3,7 +3,7 @@ use parry3d_f64::math::Isometry; use crate::{ kernel::{ - approximation::Approx, + approximation::Approximation, geometry::{Circle, Curve}, }, math::Point, @@ -65,7 +65,7 @@ impl Edges { /// Only approximating an edge once, and then referring to that /// approximation from then on where needed, would take care of these two /// problems. - pub fn approx(&self, tolerance: f64) -> Approx { + pub fn approx(&self, tolerance: f64) -> Approximation { let mut vertices = Vec::new(); let mut segments = Vec::new(); @@ -76,7 +76,7 @@ impl Edges { segments.extend(approx.segments); } - Approx { vertices, segments } + Approximation { vertices, segments } } } @@ -95,7 +95,7 @@ impl Cycle { /// /// `tolerance` defines how far the approximation is allowed to deviate from /// the actual cycle. - pub fn approx(&self, tolerance: f64) -> Approx { + pub fn approx(&self, tolerance: f64) -> Approximation { let mut vertices = Vec::new(); let mut segments = Vec::new(); @@ -110,7 +110,7 @@ impl Cycle { // the first vertex of the next. Let's remove those duplicates. vertices.dedup(); - Approx { vertices, segments } + Approximation { vertices, segments } } } @@ -175,7 +175,7 @@ impl Edge { /// /// `tolerance` defines how far the approximation is allowed to deviate from /// the actual edge. - pub fn approx(&self, tolerance: f64) -> Approx { + pub fn approx(&self, tolerance: f64) -> Approximation { let mut vertices = Vec::new(); self.curve.approx(tolerance, &mut vertices); @@ -201,6 +201,6 @@ impl Edge { segments.push([v0, v1].into()); } - Approx { vertices, segments } + Approximation { vertices, segments } } } From 33e76347eb94be37d6c4152060fad1d27d69a9d3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 14:36:14 +0100 Subject: [PATCH 3/8] Rename struct field Those are just points. They are not necessarily vertices of the model, so the new name should have less potential for confusion. --- src/kernel/approximation.rs | 2 +- src/kernel/topology/edges.rs | 19 ++++++++++++++----- src/kernel/topology/faces.rs | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index f98a58532..b73d07262 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -4,6 +4,6 @@ use crate::math::Point; /// An approximation of one or more edges pub struct Approximation { - pub vertices: Vec>, + pub points: Vec>, pub segments: Vec, } diff --git a/src/kernel/topology/edges.rs b/src/kernel/topology/edges.rs index 9bfd85c01..d9bf407f8 100644 --- a/src/kernel/topology/edges.rs +++ b/src/kernel/topology/edges.rs @@ -72,11 +72,14 @@ impl Edges { for cycle in &self.cycles { let approx = cycle.approx(tolerance); - vertices.extend(approx.vertices); + vertices.extend(approx.points); segments.extend(approx.segments); } - Approximation { vertices, segments } + Approximation { + points: vertices, + segments, + } } } @@ -102,7 +105,7 @@ impl Cycle { for edge in &self.edges { let approx = edge.approx(tolerance); - vertices.extend(approx.vertices); + vertices.extend(approx.points); segments.extend(approx.segments); } @@ -110,7 +113,10 @@ impl Cycle { // the first vertex of the next. Let's remove those duplicates. vertices.dedup(); - Approximation { vertices, segments } + Approximation { + points: vertices, + segments, + } } } @@ -201,6 +207,9 @@ impl Edge { segments.push([v0, v1].into()); } - Approximation { vertices, segments } + Approximation { + points: vertices, + segments, + } } } diff --git a/src/kernel/topology/faces.rs b/src/kernel/topology/faces.rs index 92d4ce764..64270355e 100644 --- a/src/kernel/topology/faces.rs +++ b/src/kernel/topology/faces.rs @@ -111,7 +111,7 @@ impl Face { let approx = edges.approx(tolerance); let vertices: Vec<_> = approx - .vertices + .points .into_iter() .map(|vertex| { // Can't panic, unless the approximation wrongfully From fd6a6d530efebddf00c58df16ab24a5e85cb1723 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 14:37:37 +0100 Subject: [PATCH 4/8] Update variable names --- src/kernel/topology/edges.rs | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/kernel/topology/edges.rs b/src/kernel/topology/edges.rs index d9bf407f8..8f94bd1c0 100644 --- a/src/kernel/topology/edges.rs +++ b/src/kernel/topology/edges.rs @@ -66,20 +66,17 @@ impl Edges { /// approximation from then on where needed, would take care of these two /// problems. pub fn approx(&self, tolerance: f64) -> Approximation { - let mut vertices = Vec::new(); + let mut points = Vec::new(); let mut segments = Vec::new(); for cycle in &self.cycles { let approx = cycle.approx(tolerance); - vertices.extend(approx.points); + points.extend(approx.points); segments.extend(approx.segments); } - Approximation { - points: vertices, - segments, - } + Approximation { points, segments } } } @@ -99,24 +96,21 @@ impl Cycle { /// `tolerance` defines how far the approximation is allowed to deviate from /// the actual cycle. pub fn approx(&self, tolerance: f64) -> Approximation { - let mut vertices = Vec::new(); + let mut points = Vec::new(); let mut segments = Vec::new(); for edge in &self.edges { let approx = edge.approx(tolerance); - vertices.extend(approx.points); + points.extend(approx.points); segments.extend(approx.segments); } // As this is a cycle, the last vertex of an edge could be identical to // the first vertex of the next. Let's remove those duplicates. - vertices.dedup(); + points.dedup(); - Approximation { - points: vertices, - segments, - } + Approximation { points, segments } } } @@ -182,19 +176,19 @@ impl Edge { /// `tolerance` defines how far the approximation is allowed to deviate from /// the actual edge. pub fn approx(&self, tolerance: f64) -> Approximation { - let mut vertices = Vec::new(); - self.curve.approx(tolerance, &mut vertices); + let mut points = Vec::new(); + self.curve.approx(tolerance, &mut points); if self.reverse { - vertices.reverse() + points.reverse() } - let mut segment_vertices = vertices.clone(); + let mut segment_vertices = points.clone(); if self.vertices.is_none() { // The edge has no vertices, which means it connects to itself. We // need to reflect that in the approximation. - if let Some(&vertex) = vertices.first() { + if let Some(&vertex) = points.first() { segment_vertices.push(vertex); } } @@ -207,9 +201,6 @@ impl Edge { segments.push([v0, v1].into()); } - Approximation { - points: vertices, - segments, - } + Approximation { points, segments } } } From 457cddf137496c4a540295bdbd49a296cfd73d5a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 14:38:18 +0100 Subject: [PATCH 5/8] Update variable name --- src/kernel/topology/faces.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/kernel/topology/faces.rs b/src/kernel/topology/faces.rs index 64270355e..a74e4263d 100644 --- a/src/kernel/topology/faces.rs +++ b/src/kernel/topology/faces.rs @@ -110,7 +110,7 @@ impl Face { Self::Face { edges, surface } => { let approx = edges.approx(tolerance); - let vertices: Vec<_> = approx + let points: Vec<_> = approx .points .into_iter() .map(|vertex| { @@ -136,11 +136,11 @@ impl Face { // We're also going to need a point outside of the polygon, for // the point-in-polygon tests. let aabb = AABB::from_points( - vertices.iter().map(|vertex| &vertex.value), + points.iter().map(|vertex| &vertex.value), ); let outside = aabb.maxs * 2.; - let mut triangles = triangulate(vertices); + let mut triangles = triangulate(points); let face_as_polygon = segments; triangles.retain(|t| { From 564e2a4bcc933351861be87452d06de483b5427a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 14:44:21 +0100 Subject: [PATCH 6/8] Update documentation of `Approximation` --- src/kernel/approximation.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index b73d07262..c845f808f 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -2,8 +2,18 @@ use parry3d_f64::shape::Segment; use crate::math::Point; -/// An approximation of one or more edges +/// An approximation of an edge, multiple edges, or a face pub struct Approximation { + /// All points that make up the approximation + /// + /// These could be actual vertices from the model, points that approximate + /// an edge, or points that approximate a face. pub points: Vec>, + + /// Segments that approximate edges + /// + /// Every approximation will involve edges, typically, and these are + /// approximated by these segments. All the points of these segments will + /// also be available in the `points` field of this struct. pub segments: Vec, } From 23bac08cd35e8dfa203feb7014510aaf4d7c5a75 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 15:00:54 +0100 Subject: [PATCH 7/8] Add `Approximation::validate` --- src/kernel/approximation.rs | 148 +++++++++++++++++++++++++++++++++++- 1 file changed, 146 insertions(+), 2 deletions(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index c845f808f..e317cce6b 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -1,3 +1,8 @@ +#[cfg(test)] +use std::collections::HashSet; + +#[cfg(test)] +use decorum::R64; use parry3d_f64::shape::Segment; use crate::math::Point; @@ -13,7 +18,146 @@ pub struct Approximation { /// Segments that approximate edges /// /// Every approximation will involve edges, typically, and these are - /// approximated by these segments. All the points of these segments will - /// also be available in the `points` field of this struct. + /// approximated by these segments. + /// + /// All the points of these segments will also be available in the `points` + /// field of this struct. This can be verified by calling + /// [`Approximation::validate`]. pub segments: Vec, } + +impl Approximation { + /// Validate the approximation + /// + /// Returns an `Err(ValidationError)`, if the validation is not valid. See + /// [`ValidationError`] for the ways that the approximation can be invalid. + #[cfg(test)] + pub fn validate(&self) -> Result<(), ValidationError> { + let mut duplicate_points = Vec::new(); + let mut duplicate_segments = Vec::new(); + let mut invalid_segments = Vec::new(); + let mut segments_with_invalid_points = Vec::new(); + + // Verify that there are no duplicate points + let mut points = HashSet::new(); + for &point in &self.points { + let point_r64 = point_to_r64(point); + + if points.contains(&point_r64) { + duplicate_points.push(point); + } + + points.insert(point_r64); + } + + let mut segments = HashSet::new(); + for &segment @ Segment { a, b } in &self.segments { + // Verify that there are no duplicate segments + let ab = [point_to_r64(a), point_to_r64(b)]; + let ba = [point_to_r64(b), point_to_r64(a)]; + if segments.contains(&ab) { + duplicate_segments.push(segment); + } + segments.insert(ab); + segments.insert(ba); + + // Verify that segments are actually segments + if a == b { + invalid_segments.push(segment); + } + + // Verify that segments refer to valid points + if !(self.points.contains(&a) && self.points.contains(&b)) { + segments_with_invalid_points.push(segment); + } + } + + if !(duplicate_points.is_empty() + && duplicate_segments.is_empty() + && invalid_segments.is_empty() + && segments_with_invalid_points.is_empty()) + { + return Err(ValidationError { + duplicate_points, + duplicate_segments, + invalid_segments, + segments_with_invalid_points, + }); + } + + Ok(()) + } +} + +/// Error returned by [`Approximation::validate`] +#[cfg(test)] +#[derive(Debug)] +pub struct ValidationError { + /// Points that are duplicated + pub duplicate_points: Vec>, + + /// Segments that are duplicated + pub duplicate_segments: Vec, + + /// Segments that have two equal points + pub invalid_segments: Vec, + + /// Segments that do not refer to points from the approximation + pub segments_with_invalid_points: Vec, +} + +#[cfg(test)] +fn point_to_r64(point: Point<3>) -> [R64; 3] { + [point.x.into(), point.y.into(), point.z.into()] +} + +#[cfg(test)] +mod tests { + use nalgebra::point; + use parry3d_f64::shape::Segment; + + use super::Approximation; + + #[test] + fn test_validate() { + let a = point![0., 1., 2.]; + let b = point![1., 2., 3.]; + let c = point![3., 5., 8.]; + + let valid = Approximation { + points: vec![a, b, c], + segments: vec![Segment { a, b }], + }; + assert!(valid.validate().is_ok()); + + let duplicate_points = Approximation { + points: vec![a, b, c, b], + segments: vec![Segment { a, b }], + }; + assert!(duplicate_points.validate().is_err()); + + let duplicate_segments = Approximation { + points: vec![a, b, c], + segments: vec![Segment { a, b }, Segment { a, b }], + }; + assert!(duplicate_segments.validate().is_err()); + + let duplicate_segments_inverted = Approximation { + points: vec![a, b, c], + segments: vec![Segment { a, b }, Segment { a: b, b: a }], + }; + assert!(duplicate_segments_inverted.validate().is_err()); + + let invalid_segment = Approximation { + points: vec![a, b, c], + segments: vec![Segment { a, b: a }], + }; + assert!(invalid_segment.validate().is_err()); + + let segment_with_invalid_point = Approximation { + points: vec![a, c], + segments: vec![Segment { a, b }], + }; + assert!(segment_with_invalid_point.validate().is_err()); + } +} From d378ce81bd842aa293ee6dc4768ccf4677ed3910 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Feb 2022 15:43:55 +0100 Subject: [PATCH 8/8] Validate approximation in triangulation --- src/kernel/approximation.rs | 5 ----- src/kernel/topology/faces.rs | 7 +++++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index e317cce6b..dd677d8e4 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -1,7 +1,5 @@ -#[cfg(test)] use std::collections::HashSet; -#[cfg(test)] use decorum::R64; use parry3d_f64::shape::Segment; @@ -31,7 +29,6 @@ impl Approximation { /// /// Returns an `Err(ValidationError)`, if the validation is not valid. See /// [`ValidationError`] for the ways that the approximation can be invalid. - #[cfg(test)] pub fn validate(&self) -> Result<(), ValidationError> { let mut duplicate_points = Vec::new(); let mut duplicate_segments = Vec::new(); @@ -90,7 +87,6 @@ impl Approximation { } /// Error returned by [`Approximation::validate`] -#[cfg(test)] #[derive(Debug)] pub struct ValidationError { /// Points that are duplicated @@ -106,7 +102,6 @@ pub struct ValidationError { pub segments_with_invalid_points: Vec, } -#[cfg(test)] fn point_to_r64(point: Point<3>) -> [R64; 3] { [point.x.into(), point.y.into(), point.z.into()] } diff --git a/src/kernel/topology/faces.rs b/src/kernel/topology/faces.rs index a74e4263d..f348ac64c 100644 --- a/src/kernel/topology/faces.rs +++ b/src/kernel/topology/faces.rs @@ -11,6 +11,7 @@ use parry3d_f64::{ query::Ray as Ray3, shape::{Segment as Segment3, Triangle}, }; +use tracing::warn; use crate::{ debug::{DebugInfo, TriangleEdgeCheck}, @@ -110,6 +111,12 @@ impl Face { Self::Face { edges, surface } => { let approx = edges.approx(tolerance); + // Can't make this a panic, as the current approximation code + // actually produces invalid approximations. + if let Err(err) = approx.validate() { + warn!("Invalid approximation: {:?}", err); + } + let points: Vec<_> = approx .points .into_iter()