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

Validate Cycle on construction #1023

Merged
merged 3 commits into from
Sep 1, 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
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/algorithms/reverse/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ impl Reverse for Cycle {

edges.reverse();

Cycle::new(surface).with_edges(edges)
Cycle::new(surface, edges)
}
}
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/algorithms/reverse/face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fn reverse_local_coordinates_in_cycle<'r>(
Edge::from_curve_and_vertices(curve, vertices)
});

Cycle::new(surface).with_edges(edges)
Cycle::new(surface, edges)
})
}

Expand Down
4 changes: 2 additions & 2 deletions crates/fj-kernel/src/algorithms/sweep/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ fn create_non_continuous_side_face(
edges.push(edge);
}

Cycle::new(surface).with_edges(edges)
Cycle::new(surface, edges)
};

Face::new(surface).with_exteriors([cycle]).with_color(color)
Expand All @@ -118,7 +118,7 @@ fn create_continuous_side_face(
// won't start to matter at some point either.
let placeholder = Surface::xy_plane();

let cycle = Cycle::new(placeholder).with_edges([edge]);
let cycle = Cycle::new(placeholder, [edge]);
let approx = cycle.approx(tolerance, ());

let mut quads = Vec::new();
Expand Down
6 changes: 4 additions & 2 deletions crates/fj-kernel/src/algorithms/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ impl TransformObject for Curve {

impl TransformObject for Cycle {
fn transform(self, transform: &Transform) -> Self {
Self::new(self.surface().transform(transform))
.with_edges(self.into_edges().map(|edge| edge.transform(transform)))
Self::new(
self.surface().transform(transform),
self.into_edges().map(|edge| edge.transform(transform)),
)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/builder/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ impl CycleBuilder {
);
}

Cycle::new(self.surface).with_edges(edges)
Cycle::new(self.surface, edges)
}
}
64 changes: 49 additions & 15 deletions crates/fj-kernel/src/objects/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ use crate::builder::CycleBuilder;
use super::{Edge, Surface};

/// A cycle of connected edges
///
/// The end of each edge in the cycle must connect to the beginning of the next
/// edge. The end of the last edge must connect to the beginning of the first
/// one.
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct Cycle {
surface: Surface,
Expand All @@ -20,19 +16,57 @@ impl Cycle {
}

/// Create a new cycle
pub fn new(surface: Surface) -> Self {
Self {
surface,
edges: Vec::new(),
///
/// # Panics
///
/// Panic, if the end of each edge does not connect to the beginning of the
/// next edge.
pub fn new(
surface: Surface,
edges: impl IntoIterator<Item = Edge>,
) -> Self {
let edges = edges.into_iter().collect::<Vec<_>>();

if edges.len() != 1 {
// If the length is one, we might have a cycle made up of just one
// circle. If that isn't the case, we are dealing with line segments
// and can be sure that the following `get_or_panic` calls won't
// panic.

// Verify that all edges connect.
for edges in edges.windows(2) {
// Can't panic, as we passed `2` to `windows`.
//
// Can be cleaned up, once `array_windows` is stable"
// https://doc.rust-lang.org/std/primitive.slice.html#method.array_windows
let [a, b] = [&edges[0], &edges[1]];

let [_, prev] = a.vertices().get_or_panic();
let [next, _] = b.vertices().get_or_panic();

assert_eq!(
prev.global(),
next.global(),
"Edges in cycle do not connect"
);
}

// Verify that the edges form a cycle
if let Some(first) = edges.first() {
if let Some(last) = edges.last() {
let [first, _] = first.vertices().get_or_panic();
let [_, last] = last.vertices().get_or_panic();

assert_eq!(
first.global(),
last.global(),
"Edges do not form a cycle"
);
}
}
}
}

/// Add edges to the cycle
///
/// Consumes the cycle and returns the updated instance.
pub fn with_edges(mut self, edges: impl IntoIterator<Item = Edge>) -> Self {
self.edges.extend(edges);
self
Self { surface, edges }
}

/// Access the surface that this cycle is in
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-operations/src/sketch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl Shape for fj::Sketch {

let edge = Edge::build(surface)
.circle_from_radius(Scalar::from_f64(circle.radius()));
let cycle = Cycle::new(surface).with_edges([edge]);
let cycle = Cycle::new(surface, [edge]);

Face::new(surface)
.with_exteriors([cycle])
Expand Down