Skip to content

Commit

Permalink
Fix inconsistency in curve approximation
Browse files Browse the repository at this point in the history
Curve approximation could result in slightly different points, depending
on the direction of the range.
  • Loading branch information
hannobraun committed Sep 8, 2022
1 parent 5992e3a commit f462a3e
Showing 1 changed file with 34 additions and 2 deletions.
36 changes: 34 additions & 2 deletions crates/fj-kernel/src/algorithms/approx/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ fn approx_circle(
points.push(ApproxPoint::new(point_curve, point_global));
}

if range.is_reversed() {
points.reverse();
}

points
}

Expand All @@ -102,12 +106,40 @@ fn number_of_vertices_for_circle(
#[derive(Clone, Copy, Debug)]
pub struct RangeOnCurve {
boundary: [Vertex; 2],
is_reversed: bool,
}

impl RangeOnCurve {
/// Construct an instance of `RangeOnCurve`
pub fn new(boundary: [Vertex; 2]) -> Self {
Self { boundary }
///
/// Ranges are normalized on construction, meaning that the order of
/// vertices passed to this constructor does not influence the range that is
/// constructed.
///
/// This is done to prevent bugs during mesh construction: The curve
/// approximation code is regularly faced with ranges that are reversed
/// versions of each other. This can lead to slightly different
/// approximations, which in turn leads to the aforementioned invalid
/// meshes.
///
/// The caller can use `is_reversed` to determine, if the range was reversed
/// during normalization, to adjust the approximation accordingly.
pub fn new([a, b]: [Vertex; 2]) -> Self {
let (boundary, is_reversed) = if a < b {
([a, b], false)
} else {
([b, a], true)
};

Self {
boundary,
is_reversed,
}
}

/// Indicate whether the range was reversed during normalization
pub fn is_reversed(&self) -> bool {
self.is_reversed
}

/// Access the start of the range
Expand Down

0 comments on commit f462a3e

Please sign in to comment.