Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand validation code #1031

Merged
merged 5 commits into from
Sep 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 84 additions & 7 deletions crates/fj-kernel/src/algorithms/validate/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,43 @@ use std::fmt;

use fj_math::{Point, Scalar};

use crate::objects::Vertex;
use crate::objects::{Curve, Vertex};

pub fn validate_curve(
curve: &Curve,
max_distance: impl Into<Scalar>,
) -> Result<(), CoherenceIssues> {
let max_distance = max_distance.into();

let points_curve = [-2., -1., 0., 1., 2.].map(|point| Point::from([point]));

for point_curve in points_curve {
let point_surface = curve.kind().point_from_curve_coords(point_curve);
let point_surface_as_global =
curve.surface().point_from_surface_coords(point_surface);
let point_global =
curve.global().kind().point_from_curve_coords(point_curve);

let distance = (point_surface_as_global - point_global).magnitude();

if distance > max_distance {
Err(CurveCoherenceMismatch {
point_curve,
point_surface,
point_surface_as_global,
point_global,
curve: *curve,
})?
}
}

Ok(())
}

pub fn validate_vertex(
vertex: &Vertex,
max_distance: impl Into<Scalar>,
) -> Result<(), CoherenceMismatch> {
) -> Result<(), CoherenceIssues> {
let max_distance = max_distance.into();

// Validate that the local and global forms of the vertex match. As a side
Expand All @@ -24,19 +55,65 @@ pub fn validate_vertex(
let distance = (local_as_global - global).magnitude();

if distance > max_distance {
return Err(CoherenceMismatch {
Err(VertexCoherenceMismatch {
local,
local_as_global,
global,
});
})?
}

Ok(())
}

/// A mismatch between the local and global forms of an object
/// Issues in coherence validation
#[allow(clippy::large_enum_variant)]
#[derive(Debug, thiserror::Error)]
pub enum CoherenceIssues {
/// Mismatch between the surface and global forms of a curve
#[error("Mismatch between surface and global forms of curve")]
Curve(#[from] CurveCoherenceMismatch),

/// Mismatch between the local and global coordinates of a vertex
#[error("Mismatch between local and global coordinates of vertex")]
Vertex(#[from] VertexCoherenceMismatch),
}

/// A mismatch between the surface and global forms of a curve
///
/// Used in [`CoherenceIssues`].
#[derive(Debug, thiserror::Error)]
pub struct CurveCoherenceMismatch {
/// The curve coordinate for which a mismatch was found
pub point_curve: Point<1>,

/// The curve coordinate, converted to surface coordinates
pub point_surface: Point<2>,

/// The surface coordinates, converted to global coordinates
pub point_surface_as_global: Point<3>,

/// The curve coordinate, converted to global coordinates
pub point_global: Point<3>,

/// The incoherent curve
pub curve: Curve,
}

impl fmt::Display for CurveCoherenceMismatch {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"local: {:?} (converted to surface: {:?}; to global: {:?}), global: {:?},",
self.point_curve, self.point_surface, self.point_surface_as_global, self.point_global,
)
}
}

/// A mismatch between the local and global forms of a vertex
///
/// Used in [`CoherenceIssues`].
#[derive(Debug, Default, thiserror::Error)]
pub struct CoherenceMismatch {
pub struct VertexCoherenceMismatch {
/// The local form of the object
pub local: Point<1>,

Expand All @@ -47,7 +124,7 @@ pub struct CoherenceMismatch {
pub global: Point<3>,
}

impl fmt::Display for CoherenceMismatch {
impl fmt::Display for VertexCoherenceMismatch {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
Expand Down
29 changes: 26 additions & 3 deletions crates/fj-kernel/src/algorithms/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
mod coherence;
mod uniqueness;

pub use self::{coherence::CoherenceMismatch, uniqueness::UniquenessIssues};
pub use self::{
coherence::{CoherenceIssues, VertexCoherenceMismatch},
uniqueness::UniquenessIssues,
};

use std::{collections::HashSet, ops::Deref};

Expand Down Expand Up @@ -60,6 +63,10 @@ where
) -> Result<Validated<Self>, ValidationError> {
let mut global_vertices = HashSet::new();

for curve in self.curve_iter() {
coherence::validate_curve(curve, config.identical_max_distance)?;
}

for global_vertex in self.global_vertex_iter() {
uniqueness::validate_vertex(
global_vertex,
Expand Down Expand Up @@ -136,7 +143,7 @@ impl<T> Deref for Validated<T> {
pub enum ValidationError {
/// Coherence validation failed
#[error("Coherence validation failed")]
Coherence(#[from] CoherenceMismatch),
Coherence(#[from] CoherenceIssues),

/// Geometric validation failed
#[error("Geometric validation failed")]
Expand All @@ -149,7 +156,7 @@ pub enum ValidationError {

#[cfg(test)]
mod tests {
use fj_math::{Point, Scalar};
use fj_math::{Line, Point, Scalar};

use crate::{
algorithms::validate::{Validate, ValidationConfig, ValidationError},
Expand All @@ -159,6 +166,22 @@ mod tests {
},
};

#[test]
fn coherence_curve() {
let line_global = Line::from_points([[0., 0., 0.], [1., 0., 0.]]);
let global_curve = GlobalCurve::from_kind(CurveKind::Line(line_global));

let line_surface = Line::from_points([[0., 0.], [2., 0.]]);
let curve = Curve::new(
Surface::xy_plane(),
CurveKind::Line(line_surface),
global_curve,
);

let result = curve.validate();
assert!(result.is_err());
}

#[test]
fn coherence_edge() {
let a = Point::from([0., 0., 0.]);
Expand Down
14 changes: 14 additions & 0 deletions crates/fj-kernel/src/objects/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ impl Edge {
assert_eq!(curve.global(), global.curve());
assert_eq!(&vertices.to_global(), global.vertices());

// Make sure that the edge vertices are not coincident on the curve. If
// they were, the edge would have no length, and not be valid.
//
// It is perfectly fine for global forms of the the vertices to be
// coincident (in 3D space). That would just mean, that ends of the edge
// connect to each other.
if let Some([a, b]) = vertices.get() {
assert_ne!(
a.position(),
b.position(),
"Vertices of an edge must not be coincident on curve"
);
}

Self {
curve,
vertices,
Expand Down