Skip to content

Commit

Permalink
Remove curve geometry mismatch validation check
Browse files Browse the repository at this point in the history
The CI build is currently failing because of that check. This has
started happening due to a change of how circles are represented in the
new geometry system. Parts of the new geometry system are being used by
this check,

The cause of the problem seems to be that the curves that are failing
the check have their geometry defined as a circle on a straight surface,
and a line on a curved surface. The new geometry system, which is
designed to eliminate this kind of redundancy, doesn't deal with that
well, and delivers slightly different results for each case.

This doesn't actually matter though. Only one of those definitions is
actually used. When disabling the check, the affected models look fine
in the viewer, and most importantly, they validate just fine when
exported to 3MF.

I don't think this validation check was ever there to prevent invalid
geometry. Its purpose was to discover bugs, which these kinds of
discrepancies are a sign of. But here, it wasn't detecting a bug (as far
as I can tell, at least). It was detecting a discrepancy due to the
ongoing transition to the new geometry system.

The new geometry system will eliminate the redundancies that this check
was designed to keep in check. It would have become obsolete anyway. In
light of this, I've decided it has already outlived its purpose.

The alternative would be to somehow paper over this discrepancy until
the redundancy can be eliminated. But I don't know how to do that. I
suspect it would have been hard. And I've already spent enough time
tracking this problem down.

I think it's fine taking the easy way out, and retiring this validation
check a bit early.
  • Loading branch information
hannobraun committed Sep 18, 2024
1 parent 33270a0 commit 9ed1a44
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 296 deletions.
9 changes: 1 addition & 8 deletions crates/fj-core/src/validate/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ use crate::{
geometry::Geometry,
topology::Shell,
validation::{
checks::{
CoincidentHalfEdgesAreNotSiblings, CurveGeometryMismatch,
HalfEdgeHasNoSibling,
},
checks::{CoincidentHalfEdgesAreNotSiblings, HalfEdgeHasNoSibling},
ValidationCheck,
},
};
Expand All @@ -19,10 +16,6 @@ impl Validate for Shell {
errors: &mut Vec<ValidationError>,
geometry: &Geometry,
) {
errors.extend(
CurveGeometryMismatch::check(self, geometry, config)
.map(Into::into),
);
errors.extend(
HalfEdgeHasNoSibling::check(self, geometry, config).map(Into::into),
);
Expand Down
280 changes: 0 additions & 280 deletions crates/fj-core/src/validation/checks/curve_geometry_mismatch.rs

This file was deleted.

2 changes: 0 additions & 2 deletions crates/fj-core/src/validation/checks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
//! See documentation of [parent module](super) for more information.

mod coincident_half_edges_are_not_siblings;
mod curve_geometry_mismatch;
mod face_boundary;
mod face_winding;
mod half_edge_connection;
Expand All @@ -12,7 +11,6 @@ mod multiple_references;

pub use self::{
coincident_half_edges_are_not_siblings::CoincidentHalfEdgesAreNotSiblings,
curve_geometry_mismatch::CurveGeometryMismatch,
face_boundary::FaceHasNoBoundary,
face_winding::InteriorCycleHasInvalidWinding,
half_edge_connection::AdjacentHalfEdgesNotConnected,
Expand Down
8 changes: 2 additions & 6 deletions crates/fj-core/src/validation/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{

use super::checks::{
AdjacentHalfEdgesNotConnected, CoincidentHalfEdgesAreNotSiblings,
CurveGeometryMismatch, FaceHasNoBoundary, HalfEdgeHasNoSibling,
InteriorCycleHasInvalidWinding, MultipleReferencesToObject,
FaceHasNoBoundary, HalfEdgeHasNoSibling, InteriorCycleHasInvalidWinding,
MultipleReferencesToObject,
};

/// An error that can occur during a validation
Expand All @@ -24,10 +24,6 @@ pub enum ValidationError {
#[from] CoincidentHalfEdgesAreNotSiblings,
),

/// Curve geometry mismatch
#[error(transparent)]
CurveGeometryMismatch(#[from] CurveGeometryMismatch),

/// Face has no boundary
#[error(transparent)]
FaceHasNoBoundary(#[from] FaceHasNoBoundary),
Expand Down

0 comments on commit 9ed1a44

Please sign in to comment.