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.

Sign up for GitHub

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

Add winding validation for sketches #2256

Merged
merged 3 commits into from
Mar 13, 2024
Merged

Add winding validation for sketches #2256

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0aa266e
Refactor multiple_references tests
emka Mar 11, 2024
9794df8
Add winding validation for sketches (#2158)
emka Mar 10, 2024
9f6bb58
Refactor SketchValidationErrors
emka Mar 11, 2024
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
183 changes: 154 additions & 29 deletions crates/fj-core/src/validate/sketch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::{objects::Cycle, storage::Handle};
use crate::{objects::Sketch, validate_references};
use fj_math::Winding;

use super::{
references::{ReferenceCountError, ReferenceCounter},
Expand All @@ -12,6 +14,8 @@ impl Validate for Sketch {
errors: &mut Vec<ValidationError>,
) {
SketchValidationError::check_object_references(self, config, errors);
SketchValidationError::check_exterior_cycles(self, config, errors);
SketchValidationError::check_interior_cycles(self, config, errors);
}
}

Expand All @@ -21,6 +25,24 @@ pub enum SketchValidationError {
/// Object within sketch referenced by more than one other object
#[error("Object within sketch referenced by more than one other Object")]
MultipleReferences(#[from] ReferenceCountError),
/// Region within sketch has exterior cycle with clockwise winding
#[error(
"Exterior cycle within sketch region has clockwise winding\n
Cycle: {cycle:#?}"
)]
ClockwiseExteriorCycle {
/// Cycle with clockwise winding
cycle: Handle<Cycle>,
},
/// Region within sketch has interior cycle with counter-clockwise winding
#[error(
"Interior cycle within sketch region has counter-clockwise winding\n
Cycle: {cycle:#?}"
)]
CounterClockwiseInteriorCycle {
/// Cycle with counter-clockwise winding
cycle: Handle<Cycle>,
},
}

impl SketchValidationError {
Expand All @@ -47,14 +69,53 @@ impl SketchValidationError {
referenced_cycles, Cycle;
);
}

fn check_exterior_cycles(
sketch: &Sketch,
_config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
) {
sketch.regions().iter().for_each(|region| {
let cycle = region.exterior();
if cycle.winding() == Winding::Cw {
errors.push(ValidationError::Sketch(
SketchValidationError::ClockwiseExteriorCycle {
cycle: cycle.clone(),
},
))
}
});
}

fn check_interior_cycles(
sketch: &Sketch,
_config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
) {
sketch.regions().iter().for_each(|region| {
region
.interiors()
.iter()
.filter(|interior| interior.winding() == Winding::Ccw)
.for_each(|cycle| {
errors.push(ValidationError::Sketch(
SketchValidationError::CounterClockwiseInteriorCycle {
cycle: cycle.clone(),
},
));
})
});
}
}

#[cfg(test)]
mod tests {
use crate::{
assert_contains_err,
objects::{Cycle, HalfEdge, Region, Sketch, Vertex},
operations::{build::BuildHalfEdge, insert::Insert},
operations::{
build::BuildHalfEdge, build::BuildRegion, insert::Insert,
},
validate::{
references::ReferenceCountError, SketchValidationError, Validate,
ValidationError,
Expand All @@ -66,15 +127,15 @@ mod tests {
fn should_find_cycle_multiple_references() -> anyhow::Result<()> {
let mut core = Core::new();

let shared_cycle = Cycle::new(vec![]).insert(&mut core);
let region = <Region as BuildRegion>::circle([0., 0.], 1., &mut core)
.insert(&mut core);
let valid_sketch = Sketch::new(vec![region.clone()]).insert(&mut core);
valid_sketch.validate_and_return_first_error()?;

let shared_cycle = region.exterior();
let invalid_sketch = Sketch::new(vec![
Region::new(
Cycle::new(vec![]).insert(&mut core),
vec![shared_cycle.clone()],
)
.insert(&mut core),
Region::new(shared_cycle, vec![]).insert(&mut core),
Region::new(shared_cycle.clone(), vec![]).insert(&mut core),
Region::new(shared_cycle.clone(), vec![]).insert(&mut core),
]);
assert_contains_err!(
invalid_sketch,
Expand All @@ -83,34 +144,25 @@ mod tests {
))
);

let valid_sketch = Sketch::new(vec![Region::new(
Cycle::new(vec![]).insert(&mut core),
vec![],
)
.insert(&mut core)]);
valid_sketch.validate_and_return_first_error()?;

Ok(())
}

#[test]
fn should_find_half_edge_multiple_references() -> anyhow::Result<()> {
let mut core = Core::new();

let half_edge =
HalfEdge::line_segment([[0., 0.], [1., 0.]], None, &mut core)
.insert(&mut core);
let sibling_edge =
HalfEdge::from_sibling(&half_edge, Vertex::new().insert(&mut core))
.insert(&mut core);

let exterior =
Cycle::new(vec![half_edge.clone(), sibling_edge.clone()])
.insert(&mut core);
let region = <Region as BuildRegion>::polygon(
[[0., 0.], [1., 1.], [0., 1.]],
&mut core,
)
.insert(&mut core);
let valid_sketch = Sketch::new(vec![region.clone()]).insert(&mut core);
valid_sketch.validate_and_return_first_error()?;

let interior =
Cycle::new(vec![half_edge.clone(), sibling_edge.clone()])
.insert(&mut core);
let exterior = region.exterior();
let cloned_edges: Vec<_> =
exterior.half_edges().iter().map(|e| e.clone()).collect();
let interior = Cycle::new(cloned_edges).insert(&mut core);

let invalid_sketch =
Sketch::new(vec![
Expand All @@ -123,10 +175,83 @@ mod tests {
))
);

Ok(())
}

#[test]
fn should_find_clockwise_exterior_cycle() -> anyhow::Result<()> {
let mut core = Core::new();

let valid_outer_circle =
HalfEdge::circle([0., 0.], 1., &mut core).insert(&mut core);
let valid_exterior =
Cycle::new(vec![valid_outer_circle.clone()]).insert(&mut core);
let valid_sketch =
Sketch::new(vec![Region::new(exterior, vec![]).insert(&mut core)]);
Sketch::new(vec![
Region::new(valid_exterior.clone(), vec![]).insert(&mut core)
]);
valid_sketch.validate_and_return_first_error()?;

let invalid_outer_circle = HalfEdge::from_sibling(
&valid_outer_circle,
Vertex::new().insert(&mut core),
)
.insert(&mut core);
let invalid_exterior =
Cycle::new(vec![invalid_outer_circle.clone()]).insert(&mut core);
let invalid_sketch =
Sketch::new(vec![
Region::new(invalid_exterior.clone(), vec![]).insert(&mut core)
]);
Comment on lines +195 to +205
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be shortened by updating the valid sketch. Something like this:

let invalid_sketch = valid_sketch.update_region(valid_sketch.regions().first(), |region|
    region.update_exterior(|cycle| cycle.reverse())
);

I didn't test this, so this might not work precisely like this. But something along those lines should be possible.

assert_contains_err!(
invalid_sketch,
ValidationError::Sketch(
SketchValidationError::ClockwiseExteriorCycle { cycle: _ }
)
);

Ok(())
}

#[test]
fn should_find_counterclockwise_interior_cycle() -> anyhow::Result<()> {
let mut core = Core::new();

let outer_circle =
HalfEdge::circle([0., 0.], 2., &mut core).insert(&mut core);
let inner_circle =
HalfEdge::circle([0., 0.], 1., &mut core).insert(&mut core);
let cw_inner_circle = HalfEdge::from_sibling(
&inner_circle,
Vertex::new().insert(&mut core),
)
.insert(&mut core);
let exterior = Cycle::new(vec![outer_circle.clone()]).insert(&mut core);

let valid_interior =
Cycle::new(vec![cw_inner_circle.clone()]).insert(&mut core);
let valid_sketch = Sketch::new(vec![Region::new(
exterior.clone(),
vec![valid_interior],
)
.insert(&mut core)]);
Comment on lines +220 to +237
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can probably be shortened by using available APIs. Maybe (attention, pseudo-code) Region::circle().add_interior(invalid_circle).

valid_sketch.validate_and_return_first_error()?;

let invalid_interior =
Cycle::new(vec![inner_circle.clone()]).insert(&mut core);
let invalid_sketch = Sketch::new(vec![Region::new(
exterior.clone(),
vec![invalid_interior],
)
.insert(&mut core)]);
Comment on lines +240 to +246
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can probably be shortened using Sketch::update_region and other operations, as suggested above in the other test.

assert_contains_err!(
invalid_sketch,
ValidationError::Sketch(
SketchValidationError::CounterClockwiseInteriorCycle {
cycle: _
}
)
);
Ok(())
}
}