Skip to content

Commit

Permalink
Remove Edge::reverse
Browse files Browse the repository at this point in the history
This simplifies some code, without any loss in functionality, as far as
I can tell.

Close #151

The issue this commit addresses is nominally blocked. However, given
that #173 exists, this mechanism doesn't have its desired effect anyway.

A proper fix will likely involve a deeper look at surface orientation,
and I don't think this hack will be part of that. Given that, and that
this removal simplifies some ongoing work, I feel good about doing this
now.
  • Loading branch information
hannobraun committed Feb 23, 2022
1 parent 43298d0 commit 892e6ac
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 41 deletions.
18 changes: 0 additions & 18 deletions src/kernel/approximation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ impl Approximation {
let mut points = Vec::new();
edge.curve.approx(tolerance, &mut points);

if edge.reverse {
points.reverse()
}

// Insert the exact vertices of this edge into the approximation. This
// means we don't rely on the curve approximation to deliver accurate
// representations of these vertices, which they might not be able to
Expand Down Expand Up @@ -265,20 +261,6 @@ mod tests {
segments: vec![Segment::from([b, c]), Segment::from([c, b])],
}
);

let mut edge_reversed = Edge::new(curve.clone(), Some([v2, v1]));
edge_reversed.reverse();
assert_eq!(
Approximation::for_edge(&edge_reversed, tolerance),
Approximation {
points: vec![d, c, b, a],
segments: vec![
Segment::from([d, c]),
Segment::from([c, b]),
Segment::from([b, a]),
],
}
);
}

#[test]
Expand Down
6 changes: 1 addition & 5 deletions src/kernel/shapes/difference_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Shape for fj::Difference2d {
let mut a = self.a.edges();
let mut b = self.b.edges();

let (a, mut b) = if a.cycles.len() == 1 && b.cycles.len() == 1 {
let (a, b) = if a.cycles.len() == 1 && b.cycles.len() == 1 {
(a.cycles.pop().unwrap(), b.cycles.pop().unwrap())
} else {
// See issue:
Expand All @@ -86,10 +86,6 @@ impl Shape for fj::Difference2d {
);
};

for edge in &mut b.edges {
edge.reverse();
}

Edges { cycles: vec![a, b] }
}

Expand Down
19 changes: 1 addition & 18 deletions src/kernel/topology/edges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,6 @@ pub struct Edge {
/// If there are no such vertices, that means the edge is connected to
/// itself (like a full circle, for example).
pub vertices: Option<[Vertex<1>; 2]>,

/// Indicates whether the curve's direction is reversed
///
/// Once this struct keeps track of the vertices that bound the edge, this
/// field can probably be made redundant. The order of the bounding points
/// will simply define the direction of the curve.
pub reverse: bool,
}

impl Edge {
Expand All @@ -86,11 +79,7 @@ impl Edge {
let vertices = vertices
.map(|vertices| vertices.map(|vertex| vertex.to_1d(&curve)));

Self {
curve,
vertices,
reverse: false,
}
Self { curve, vertices }
}

/// Create a circle
Expand All @@ -101,15 +90,9 @@ impl Edge {
radius: Vector::from([radius, 0.]),
}),
vertices: None,
reverse: false,
}
}

/// Reverse the edge
pub fn reverse(&mut self) {
self.reverse = !self.reverse;
}

/// Create a transformed edge
///
/// This method constructs new instances of [`Vertex`], by calling
Expand Down

0 comments on commit 892e6ac

Please sign in to comment.