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

Improve some validation error messages and validation test output #1486

Merged
merged 5 commits into from
Jan 6, 2023
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
1 change: 0 additions & 1 deletion crates/fj-kernel/src/algorithms/intersect/face_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ impl Intersect for (&Handle<Face>, &Point<2>) {
}

/// The intersection between a face and a point
#[allow(clippy::large_enum_variant)]
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub enum FacePointIntersection {
/// The point is inside of the face
Expand Down
1 change: 0 additions & 1 deletion crates/fj-kernel/src/algorithms/intersect/ray_face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ impl Intersect for (&HorizontalRayToTheRight<3>, &Handle<Face>) {

/// A hit between a ray and a face
#[derive(Clone, Debug, Eq, PartialEq)]
#[allow(clippy::large_enum_variant)]
pub enum RayFaceIntersection {
/// The ray hits the face itself
RayHitsFace,
Expand Down
1 change: 0 additions & 1 deletion crates/fj-kernel/src/partial/objects/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ impl PartialObject for PartialHalfEdge {
impl Default for PartialHalfEdge {
fn default() -> Self {
let curve = Partial::<Curve>::new();

let vertices = array::from_fn(|_| {
Partial::from_partial(PartialVertex {
curve: curve.clone(),
Expand Down
6 changes: 4 additions & 2 deletions crates/fj-kernel/src/validate/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ mod tests {
};

#[test]
fn cycle_half_edge_connections() {
fn cycle_half_edge_connections() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
Expand Down Expand Up @@ -109,7 +109,9 @@ mod tests {
Cycle::new(half_edges)
};

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}
}
54 changes: 41 additions & 13 deletions crates/fj-kernel/src/validate/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,29 +51,37 @@ pub enum HalfEdgeValidationError {
#[error(
"`HalfEdge` vertices are not defined on the same `Curve`\n\
- `Curve` of back vertex: {back_curve:#?}\n\
- `Curve` of front vertex: {front_curve:#?}"
- `Curve` of front vertex: {front_curve:#?}\n\
- `HalfEdge`: {half_edge:#?}"
)]
CurveMismatch {
/// The curve of the [`HalfEdge`]'s back vertex
back_curve: Handle<Curve>,

/// The curve of the [`HalfEdge`]'s front vertex
front_curve: Handle<Curve>,

/// The half-edge
half_edge: HalfEdge,
},

/// [`HalfEdge`]'s [`GlobalCurve`]s do not match
#[error(
"Global form of `HalfEdge`'s `Curve` does not match `GlobalCurve` of \n\
the `HalfEdge`'s `GlobalEdge`\n\
- `GlobalCurve` from `Curve`: {global_curve_from_curve:#?}\n\
- `GlobalCurve` from `GlobalEdge`: {global_curve_from_global_form:#?}",
- `GlobalCurve` from `GlobalEdge`: {global_curve_from_global_form:#?}\n\
- `HalfEdge`: {half_edge:#?}",
)]
GlobalCurveMismatch {
/// The [`GlobalCurve`] from the [`HalfEdge`]'s [`Curve`]
global_curve_from_curve: Handle<GlobalCurve>,

/// The [`GlobalCurve`] from the [`HalfEdge`]'s global form
global_curve_from_global_form: Handle<GlobalCurve>,

/// The half-edge
half_edge: HalfEdge,
},

/// [`HalfEdge`]'s [`GlobalVertex`] objects do not match
Expand All @@ -83,21 +91,26 @@ pub enum HalfEdgeValidationError {
- `GlobalVertex` objects from `Vertex` objects: \
{global_vertices_from_vertices:#?}\n\
- `GlobalVertex` objects from `GlobalEdge`: \
{global_vertices_from_global_form:#?}"
{global_vertices_from_global_form:#?}\n\
- `HalfEdge`: {half_edge:#?}"
)]
GlobalVertexMismatch {
/// The [`GlobalVertex`] from the [`HalfEdge`]'s vertices
global_vertices_from_vertices: [Handle<GlobalVertex>; 2],

/// The [`GlobalCurve`] from the [`HalfEdge`]'s global form
global_vertices_from_global_form: [Handle<GlobalVertex>; 2],

/// The half-edge
half_edge: HalfEdge,
},

/// [`HalfEdge`]'s vertices are coincident
#[error(
"Vertices on curve are coincident\n\
"Vertices of `HalfEdge` on curve are coincident\n\
- Position of back vertex: {back_position:?}\n\
- Position of front vertex: {front_position:?}"
- Position of front vertex: {front_position:?}\n\
- `HalfEdge`: {half_edge:#?}"
)]
VerticesAreCoincident {
/// The position of the back vertex
Expand All @@ -108,6 +121,9 @@ pub enum HalfEdgeValidationError {

/// The distance between the two vertices
distance: Scalar,

/// The half-edge
half_edge: HalfEdge,
},
}

Expand All @@ -120,6 +136,7 @@ impl HalfEdgeValidationError {
return Err(Self::CurveMismatch {
back_curve: back_curve.clone(),
front_curve: front_curve.clone(),
half_edge: half_edge.clone(),
});
}

Expand All @@ -135,6 +152,7 @@ impl HalfEdgeValidationError {
global_curve_from_curve: global_curve_from_curve.clone(),
global_curve_from_global_form: global_curve_from_global_form
.clone(),
half_edge: half_edge.clone(),
});
}

Expand Down Expand Up @@ -168,6 +186,7 @@ impl HalfEdgeValidationError {
return Err(Self::GlobalVertexMismatch {
global_vertices_from_vertices,
global_vertices_from_global_form,
half_edge: half_edge.clone(),
});
}

Expand All @@ -188,6 +207,7 @@ impl HalfEdgeValidationError {
back_position,
front_position,
distance,
half_edge: half_edge.clone(),
});
}

Expand All @@ -210,7 +230,7 @@ mod tests {
};

#[test]
fn half_edge_curve_mismatch() {
fn half_edge_curve_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
Expand All @@ -234,12 +254,14 @@ mod tests {
HalfEdge::new(vertices, valid.global_form().clone())
};

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}

#[test]
fn half_edge_global_curve_mismatch() {
fn half_edge_global_curve_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
Expand All @@ -258,12 +280,14 @@ mod tests {
tmp.build(&mut services.objects)
});

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}

#[test]
fn half_edge_global_vertex_mismatch() {
fn half_edge_global_vertex_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
Expand All @@ -288,12 +312,14 @@ mod tests {
tmp.build(&mut services.objects)
});

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}

#[test]
fn half_edge_vertices_are_coincident() {
fn half_edge_vertices_are_coincident() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
Expand Down Expand Up @@ -323,7 +349,9 @@ mod tests {
valid.global_form().clone(),
);

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}
}
12 changes: 8 additions & 4 deletions crates/fj-kernel/src/validate/face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ mod tests {
};

#[test]
fn face_surface_mismatch() {
fn face_surface_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let surface = services.objects.surfaces.xy_plane();
Expand Down Expand Up @@ -149,12 +149,14 @@ mod tests {
Face::new(valid.exterior().clone(), interiors, valid.color())
};

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}

#[test]
fn face_invalid_interior_winding() {
fn face_invalid_interior_winding() -> anyhow::Result<()> {
let mut services = Services::new();

let surface = services.objects.surfaces.xy_plane();
Expand Down Expand Up @@ -184,7 +186,9 @@ mod tests {
Face::new(valid.exterior().clone(), interiors, valid.color())
};

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}
}
1 change: 0 additions & 1 deletion crates/fj-kernel/src/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ impl Default for ValidationConfig {
}

/// An error that can occur during a validation
#[allow(clippy::large_enum_variant)]
#[derive(Clone, Debug, thiserror::Error)]
pub enum ValidationError {
/// `Cycle` validation error
Expand Down
18 changes: 12 additions & 6 deletions crates/fj-kernel/src/validate/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ mod tests {
};

#[test]
fn vertex_surface_mismatch() {
fn vertex_surface_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let surface = Partial::from(services.objects.surfaces.xy_plane());
Expand Down Expand Up @@ -220,12 +220,14 @@ mod tests {
Vertex::new(valid.position(), valid.curve().clone(), surface_form)
};

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}

#[test]
fn vertex_position_mismatch() {
fn vertex_position_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = {
Expand Down Expand Up @@ -255,12 +257,14 @@ mod tests {
Vertex::new(valid.position(), valid.curve().clone(), surface_form)
};

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}

#[test]
fn surface_vertex_position_mismatch() {
fn surface_vertex_position_mismatch() -> anyhow::Result<()> {
let mut services = Services::new();

let valid = PartialSurfaceVertex {
Expand All @@ -275,7 +279,9 @@ mod tests {
GlobalVertex::new([1., 0., 0.]).insert(&mut services.objects),
);

assert!(valid.validate().is_ok());
valid.validate()?;
assert!(invalid.validate().is_err());

Ok(())
}
}