From 0d473b577cebefbfec8d75e407bf2a563d19e8b5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Feb 2022 14:19:44 +0100 Subject: [PATCH 1/3] Add `Approximation::for_cycle` This is an improved version of `Cycle::approx` that correctly handles non-neighboring duplicate points. --- src/kernel/approximation.rs | 95 ++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index 886f3f389..954b32f2e 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -1,10 +1,12 @@ -use std::collections::HashSet; +use std::{cmp::Ordering, collections::HashSet}; use decorum::R64; use parry3d_f64::shape::Segment; use crate::math::Point; +#[cfg(test)] +use super::topology::edges::Cycle; use super::topology::edges::Edge; /// An approximation of an edge, multiple edges, or a face @@ -61,6 +63,51 @@ impl Approximation { Self { points, segments } } + /// Compute an approximation for a cycle + /// + /// `tolerance` defines how far the approximation is allowed to deviate from + /// the actual cycle. + #[cfg(test)] + pub fn for_cycle(cycle: &Cycle, tolerance: f64) -> Self { + let mut points = Vec::new(); + let mut segments = Vec::new(); + + for edge in &cycle.edges { + let approx = Self::for_edge(edge, tolerance); + + points.extend(approx.points); + segments.extend(approx.segments); + } + + // As this is a cycle, neighboring edges are going to share vertices. + // Let's remove all those duplicates. + points.sort_by(|a, b| { + if a.x < b.x { + return Ordering::Less; + } + if a.x > b.x { + return Ordering::Greater; + } + if a.y < b.y { + return Ordering::Less; + } + if a.y > b.y { + return Ordering::Greater; + } + if a.z < b.z { + return Ordering::Less; + } + if a.z > b.z { + return Ordering::Greater; + } + + Ordering::Equal + }); + points.dedup(); + + Self { points, segments } + } + /// Validate the approximation /// /// Returns an `Err(ValidationError)`, if the validation is not valid. See @@ -147,7 +194,10 @@ mod tests { use nalgebra::point; use parry3d_f64::shape::Segment; - use crate::kernel::{geometry::Curve, topology::edges::Edge}; + use crate::kernel::{ + geometry::Curve, + topology::edges::{Cycle, Edge}, + }; use super::Approximation; @@ -200,6 +250,47 @@ mod tests { ); } + #[test] + fn test_for_cycle() { + let tolerance = 1.; + + let a = point![1., 2., 3.]; + let b = point![2., 3., 5.]; + let c = point![3., 5., 8.]; + + let ab = Edge { + curve: Curve::Mock { approx: vec![a, b] }, + vertices: Some([(), ()]), + reverse: false, + }; + let bc = Edge { + curve: Curve::Mock { approx: vec![b, c] }, + vertices: Some([(), ()]), + reverse: false, + }; + let ca = Edge { + curve: Curve::Mock { approx: vec![c, a] }, + vertices: Some([(), ()]), + reverse: false, + }; + + let cycle = Cycle { + edges: vec![ab, bc, ca], + }; + + assert_eq!( + Approximation::for_cycle(&cycle, tolerance), + Approximation { + points: vec![a, b, c], + segments: vec![ + Segment { a: a, b: b }, + Segment { a: b, b: c }, + Segment { a: c, b: a }, + ], + } + ); + } + #[test] fn test_validate() { let a = point![0., 1., 2.]; From d42527d21439288675a6a3848c52dc3ffb220dd1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Feb 2022 14:21:59 +0100 Subject: [PATCH 2/3] Fix handling of duplicate points in approximation This is the edge case being tracked in #138. --- src/kernel/approximation.rs | 5 +---- src/kernel/topology/edges.rs | 26 +------------------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/kernel/approximation.rs b/src/kernel/approximation.rs index 954b32f2e..6669bd252 100644 --- a/src/kernel/approximation.rs +++ b/src/kernel/approximation.rs @@ -5,9 +5,7 @@ use parry3d_f64::shape::Segment; use crate::math::Point; -#[cfg(test)] -use super::topology::edges::Cycle; -use super::topology::edges::Edge; +use super::topology::edges::{Cycle, Edge}; /// An approximation of an edge, multiple edges, or a face #[derive(Debug, PartialEq)] @@ -67,7 +65,6 @@ impl Approximation { /// /// `tolerance` defines how far the approximation is allowed to deviate from /// the actual cycle. - #[cfg(test)] pub fn for_cycle(cycle: &Cycle, tolerance: f64) -> Self { let mut points = Vec::new(); let mut segments = Vec::new(); diff --git a/src/kernel/topology/edges.rs b/src/kernel/topology/edges.rs index e49022e6e..b6f3cfb18 100644 --- a/src/kernel/topology/edges.rs +++ b/src/kernel/topology/edges.rs @@ -70,7 +70,7 @@ impl Edges { let mut segments = Vec::new(); for cycle in &self.cycles { - let approx = cycle.approx(tolerance); + let approx = Approximation::for_cycle(cycle, tolerance); points.extend(approx.points); segments.extend(approx.segments); @@ -90,30 +90,6 @@ pub struct Cycle { pub edges: Vec, } -impl Cycle { - /// Compute an approximation of the cycle - /// - /// `tolerance` defines how far the approximation is allowed to deviate from - /// the actual cycle. - pub fn approx(&self, tolerance: f64) -> Approximation { - let mut points = Vec::new(); - let mut segments = Vec::new(); - - for edge in &self.edges { - let approx = Approximation::for_edge(&edge, tolerance); - - 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. - points.dedup(); - - Approximation { points, segments } - } -} - /// An edge of a shape #[derive(Clone, Debug)] pub struct Edge { From 363ddc2dd9336262f5d77ff165288d3a77585a9e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 9 Feb 2022 14:34:41 +0100 Subject: [PATCH 3/3] Convert warning into assertion This is possible now, since handling of duplicate points in the approximation code has been fixed. --- src/kernel/topology/faces.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/kernel/topology/faces.rs b/src/kernel/topology/faces.rs index f348ac64c..e122acfd9 100644 --- a/src/kernel/topology/faces.rs +++ b/src/kernel/topology/faces.rs @@ -11,7 +11,6 @@ use parry3d_f64::{ query::Ray as Ray3, shape::{Segment as Segment3, Triangle}, }; -use tracing::warn; use crate::{ debug::{DebugInfo, TriangleEdgeCheck}, @@ -110,12 +109,7 @@ impl Face { match self { 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); - } + approx.validate().expect("Invalid approximation"); let points: Vec<_> = approx .points