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

Fix inconsistency in curve approximation #1056

Merged
merged 5 commits into from
Sep 8, 2022
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Simplify code
This version is simpler, because it doesn't rely on the behavior of
`Range::new`. In fact, this behavior is going to change, which is the
reason for this commit.
hannobraun committed Sep 8, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 5992e3ae49b67a93980399d2d42852a31697edab
8 changes: 4 additions & 4 deletions crates/fj-kernel/src/algorithms/approx/edge.rs
Original file line number Diff line number Diff line change
@@ -19,7 +19,7 @@ impl Approx for &Edge {

fn approx(self, tolerance: super::Tolerance) -> Self::Approximation {
// The range is only used for circles right now.
let boundary = match self.vertices().get() {
let [a, b] = match self.vertices().get() {
Some(vertices) => vertices.map(|&vertex| vertex),
None => {
// Creating vertices from nothing, just for the sake of
@@ -72,11 +72,11 @@ impl Approx for &Edge {
}
};

let range = RangeOnCurve::new(boundary);
let range = RangeOnCurve::new([a, b]);

let first = ApproxPoint::new(
range.start().surface_form().position(),
range.start().global_form().position(),
a.surface_form().position(),
a.global_form().position(),
);
let curve_approx = (self.curve(), range).approx(tolerance);