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

Port reference counting validation checks for Sketch to new infrastructure #2367

Merged
merged 33 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
bb3cd13
Rename struct to improve clarity
hannobraun May 27, 2024
e87936e
Avoid useless allocation
hannobraun May 27, 2024
21d5be6
Simplify method name
hannobraun May 27, 2024
4798974
Simplify method name
hannobraun May 27, 2024
28cf226
Order variables to reflect order of use
hannobraun May 27, 2024
610c227
Update variable name
hannobraun May 27, 2024
43223ff
Update variable name
hannobraun May 27, 2024
9ebe849
Simplify variable name
hannobraun May 27, 2024
a0fe317
Simplify variable name
hannobraun May 27, 2024
2279854
Simplify variable name
hannobraun May 27, 2024
02504f5
Simplify variable name
hannobraun May 27, 2024
4b1ad78
Simplify variable name
hannobraun May 27, 2024
4f770f9
Simplify variable name
hannobraun May 27, 2024
5a1bc80
Add missing conversions
hannobraun May 27, 2024
097011f
Remove redundant macro parameter
hannobraun May 27, 2024
84de93d
Avoid macro
hannobraun May 27, 2024
c4309bc
Move reference infra to new validation infra
hannobraun May 27, 2024
fa4642f
Prepare for follow-on change
hannobraun May 27, 2024
aa9a44c
Port part of validation check to new infra
hannobraun May 27, 2024
e1902bb
Port rest of validation check to new infra
hannobraun May 27, 2024
f2fb533
Move tests closer to code under test
hannobraun May 27, 2024
b5d10bd
Simplify variable name
hannobraun May 27, 2024
80cb780
Remove redundant code
hannobraun May 27, 2024
1213f4f
Simplify variable name
hannobraun May 27, 2024
0c87cfa
Derive invalid sketch from valid one
hannobraun May 27, 2024
76ef2bc
Simplify sketch construction
hannobraun May 27, 2024
f3ca0c9
Increase consistency
hannobraun May 27, 2024
2614e42
Simplify variable name
hannobraun May 27, 2024
e172c12
Simplify variable name
hannobraun May 27, 2024
ce1d1bb
Derive invalid sketch from valid one
hannobraun May 27, 2024
d26b602
Simplify sketch construction
hannobraun May 27, 2024
5ed1598
Increase consistency
hannobraun May 27, 2024
26d166e
Update test names
hannobraun May 27, 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
6 changes: 1 addition & 5 deletions crates/fj-core/src/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ mod curve;
mod cycle;
mod face;
mod half_edge;
mod references;
mod region;
mod shell;
mod sketch;
Expand All @@ -78,10 +77,7 @@ use crate::{
validation::{ValidationConfig, ValidationError},
};

pub use self::{
references::ObjectNotExclusivelyOwned, sketch::SketchValidationError,
solid::SolidValidationError,
};
pub use self::{sketch::SketchValidationError, solid::SolidValidationError};

/// Assert that some object has a validation error which matches a specific
/// pattern. This is preferred to matching on [`Validate::validate_and_return_first_error`], since usually we don't care about the order.
Expand Down
68 changes: 0 additions & 68 deletions crates/fj-core/src/validate/references.rs

This file was deleted.

123 changes: 19 additions & 104 deletions crates/fj-core/src/validate/sketch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ use fj_math::Winding;
use crate::{
geometry::Geometry,
storage::Handle,
topology::{Cycle, Sketch},
validate_references,
validation::{checks::AdjacentHalfEdgesNotConnected, ValidationCheck},
topology::{Cycle, HalfEdge, Region, Sketch},
validation::{
checks::{AdjacentHalfEdgesNotConnected, MultipleReferencesToObject},
ValidationCheck,
},
};

use super::{
references::ReferenceCounter, Validate, ValidationConfig, ValidationError,
};
use super::{Validate, ValidationConfig, ValidationError};

impl Validate for Sketch {
fn validate(
Expand All @@ -23,7 +23,18 @@ impl Validate for Sketch {
AdjacentHalfEdgesNotConnected::check(self, geometry, config)
.map(Into::into),
);
SketchValidationError::check_object_references(self, config, errors);
errors.extend(
MultipleReferencesToObject::<Cycle, Region>::check(
self, geometry, config,
)
.map(Into::into),
);
errors.extend(
MultipleReferencesToObject::<HalfEdge, Cycle>::check(
self, geometry, config,
)
.map(Into::into),
);
SketchValidationError::check_exterior_cycles(
self, geometry, config, errors,
);
Expand Down Expand Up @@ -58,30 +69,6 @@ pub enum SketchValidationError {
}

impl SketchValidationError {
fn check_object_references(
sketch: &Sketch,
_config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
) {
let mut referenced_edges = ReferenceCounter::new();
let mut referenced_cycles = ReferenceCounter::new();

sketch.regions().iter().for_each(|r| {
r.all_cycles().for_each(|c| {
referenced_cycles.count_reference(c.clone(), r.clone());
c.half_edges().into_iter().for_each(|e| {
referenced_edges.count_reference(e.clone(), c.clone());
})
})
});

validate_references!(
errors;
referenced_edges, MultipleReferencesToHalfEdge;
referenced_cycles, MultipleReferencesToCycle;
);
}

fn check_exterior_cycles(
sketch: &Sketch,
geometry: &Geometry,
Expand Down Expand Up @@ -126,84 +113,12 @@ impl SketchValidationError {
mod tests {
use crate::{
assert_contains_err,
operations::{
build::BuildHalfEdge, build::BuildRegion, insert::Insert,
},
operations::{build::BuildHalfEdge, insert::Insert},
topology::{Cycle, HalfEdge, Region, Sketch, Vertex},
validate::{SketchValidationError, Validate, ValidationError},
Core,
};

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

let surface = core.layers.topology.surfaces.space_2d();

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

let shared_cycle = region.exterior();
let invalid_sketch = Sketch::new(
surface,
vec![
Region::new(shared_cycle.clone(), vec![]).insert(&mut core),
Region::new(shared_cycle.clone(), vec![]).insert(&mut core),
],
);
assert_contains_err!(
core,
invalid_sketch,
ValidationError::MultipleReferencesToCycle(_)
);

Ok(())
}

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

let surface = core.layers.topology.surfaces.space_2d();

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

let exterior = region.exterior();
let cloned_edges: Vec<_> =
exterior.half_edges().iter().cloned().collect();
let interior = Cycle::new(cloned_edges).insert(&mut core);

let invalid_sketch = Sketch::new(
surface,
vec![
Region::new(exterior.clone(), vec![interior]).insert(&mut core)
],
);
assert_contains_err!(
core,
invalid_sketch,
ValidationError::MultipleReferencesToHalfEdge(_)
);

Ok(())
}

#[test]
fn should_find_clockwise_exterior_cycle() -> anyhow::Result<()> {
let mut core = Core::new();
Expand Down
35 changes: 14 additions & 21 deletions crates/fj-core/src/validate/solid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ use crate::{
geometry::Geometry,
storage::Handle,
topology::{Solid, Vertex},
validate_references,
validation::checks::ReferenceCounter,
};
use fj_math::Point;

use super::{
references::ReferenceCounter, Validate, ValidationConfig, ValidationError,
};
use super::{Validate, ValidationConfig, ValidationError};

impl Validate for Solid {
fn validate(
Expand Down Expand Up @@ -142,33 +140,28 @@ impl SolidValidationError {
_config: &ValidationConfig,
errors: &mut Vec<ValidationError>,
) {
let mut referenced_regions = ReferenceCounter::new();
let mut referenced_faces = ReferenceCounter::new();
let mut referenced_edges = ReferenceCounter::new();
let mut referenced_cycles = ReferenceCounter::new();
let mut faces = ReferenceCounter::new();
let mut regions = ReferenceCounter::new();
let mut cycles = ReferenceCounter::new();
let mut half_edges = ReferenceCounter::new();

solid.shells().iter().for_each(|s| {
s.faces().into_iter().for_each(|f| {
referenced_faces.count_reference(f.clone(), s.clone());
referenced_regions
.count_reference(f.region().clone(), f.clone());
faces.count(f.clone(), s.clone());
regions.count(f.region().clone(), f.clone());
f.region().all_cycles().for_each(|c| {
referenced_cycles
.count_reference(c.clone(), f.region().clone());
cycles.count(c.clone(), f.region().clone());
c.half_edges().into_iter().for_each(|e| {
referenced_edges.count_reference(e.clone(), c.clone());
half_edges.count(e.clone(), c.clone());
})
})
})
});

validate_references!(
errors;
referenced_regions, MultipleReferencesToRegion;
referenced_faces, MultipleReferencesToFace;
referenced_edges, MultipleReferencesToHalfEdge;
referenced_cycles, MultipleReferencesToCycle;
);
errors.extend(faces.multiples().map(Into::into));
errors.extend(regions.multiples().map(Into::into));
errors.extend(cycles.multiples().map(Into::into));
errors.extend(half_edges.multiples().map(Into::into));
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/fj-core/src/validation/checks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod face_boundary;
mod face_winding;
mod half_edge_connection;
mod half_edge_has_no_sibling;
mod multiple_references;

pub use self::{
coincident_half_edges_are_not_siblings::CoincidentHalfEdgesAreNotSiblings,
Expand All @@ -16,4 +17,5 @@ pub use self::{
face_winding::InteriorCycleHasInvalidWinding,
half_edge_connection::AdjacentHalfEdgesNotConnected,
half_edge_has_no_sibling::HalfEdgeHasNoSibling,
multiple_references::{MultipleReferencesToObject, ReferenceCounter},
};
Loading