Skip to content

Commit

Permalink
Don't normalize RangeOnPath::new
Browse files Browse the repository at this point in the history
It seems less error-prone to me, to dumb down `RangeOnPath` and handle
range direction in `PathApproxParams::points`.
  • Loading branch information
hannobraun committed Sep 15, 2022
1 parent 64f179a commit c051f4d
Showing 1 changed file with 26 additions and 27 deletions.
53 changes: 26 additions & 27 deletions crates/fj-kernel/src/algorithms/approx/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
use std::iter;

use fj_math::{Circle, Point, Scalar};
use fj_math::{Circle, Point, Scalar, Sign};

use crate::path::GlobalPath;

Expand Down Expand Up @@ -60,7 +60,6 @@ impl Approx for (GlobalPath, RangeOnPath) {
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)]
pub struct RangeOnPath {
boundary: [Point<1>; 2],
is_reversed: bool,
}

impl RangeOnPath {
Expand All @@ -79,23 +78,8 @@ impl RangeOnPath {
/// The caller can use `is_reversed` to determine, if the range was reversed
/// during normalization, to adjust the approximation accordingly.
pub fn new(boundary: [impl Into<Point<1>>; 2]) -> Self {
let [a, b] = boundary.map(Into::into);

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
let boundary = boundary.map(Into::into);
Self { boundary }
}

/// Access the boundary of the range
Expand Down Expand Up @@ -132,10 +116,6 @@ fn approx_circle<const D: usize>(
points.push((point_curve, point_global));
}

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

points
}

Expand Down Expand Up @@ -173,20 +153,32 @@ impl PathApproxParams {
let range = range.into();

let [a, b] = range.boundary.map(|point| point.t / self.increment());
let direction = (b - a).sign();
let [min, max] = if a < b { [a, b] } else { [b, a] };

// We can't generate a point exactly at the boundaries of the range as
// part of the approximation. Make sure we stay inside the range.
let start = a.floor() + 1.;
let end = b.ceil() - 1.;
let min = min.floor() + 1.;
let max = max.ceil() - 1.;

let [start, end] = match direction {
Sign::Negative => [max, min],
Sign::Positive | Sign::Zero => [min, max],
};

let mut i = start;
iter::from_fn(move || {
if i > end {
let is_finished = match direction {
Sign::Negative => i < end,
Sign::Positive | Sign::Zero => i > end,
};

if is_finished {
return None;
}

let t = self.increment() * i;
i += Scalar::ONE;
i += direction.to_scalar();

Some(Point::from([t]))
})
Expand Down Expand Up @@ -241,6 +233,13 @@ mod tests {
test_path([[2.], [TAU]], [2., 3.]);
test_path([[0.], [TAU - 2.]], [1., 2.]);

// And everything again, but in reverse.
test_path([[TAU], [0.]], [3., 2., 1.]);
test_path([[TAU], [1.]], [3., 2., 1.]);
test_path([[TAU - 1.], [0.]], [3., 2., 1.]);
test_path([[TAU], [2.]], [3., 2.]);
test_path([[TAU - 2.], [0.]], [2., 1.]);

fn test_path(
range: impl Into<RangeOnPath>,
expected_coords: impl IntoIterator<Item = impl Into<Scalar>>,
Expand Down

0 comments on commit c051f4d

Please sign in to comment.