-
Notifications
You must be signed in to change notification settings - Fork 70
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
Curve curve intersection tylers #219
base: main
Are you sure you want to change the base?
Changes from 11 commits
6e0ec0f
f52accd
4aeaeeb
d5d0be8
1375912
dc5b41a
f4cf191
84400ef
7398c23
cb5a672
6e993f9
fc0bdaf
1a2cb70
c23c4ef
5bfbf1e
9f1c40f
531d538
5a6ac30
7ddb666
64db1a8
099da3e
ba180f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,11 +6,12 @@ use crate::MAX_EXTREMA; | |
use crate::{Line, QuadSpline, Vec2}; | ||
use arrayvec::ArrayVec; | ||
|
||
use crate::common::solve_quadratic; | ||
use crate::common::GAUSS_LEGENDRE_COEFFS_9; | ||
use crate::common::{solve_quadratic}; | ||
use crate::{ | ||
Affine, Nearest, ParamCurve, ParamCurveArclen, ParamCurveArea, ParamCurveCurvature, | ||
ParamCurveDeriv, ParamCurveExtrema, ParamCurveNearest, PathEl, Point, QuadBez, Rect, Shape, | ||
Affine, Nearest, ParamCurve, ParamCurveArclen, ParamCurveArea, ParamCurveBezierClipping, | ||
ParamCurveCurvature, ParamCurveDeriv, ParamCurveExtrema, ParamCurveNearest, PathEl, Point, | ||
QuadBez, Rect, Shape, | ||
}; | ||
|
||
const MAX_SPLINE_SPLIT: usize = 100; | ||
|
@@ -238,7 +239,9 @@ impl CubicBez { | |
}) | ||
} | ||
|
||
fn parameters(&self) -> (Vec2, Vec2, Vec2, Vec2) { | ||
/// Get the parameters such that the curve can be represented by the following formula: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It bothers me slightly that this is in decreasing exponent, where, for example, solve_cubic is in increasing order. Not a dealbreaker though, if it's documented. (and I realize this is existing code, just thinking about it more carefully when we're making it public) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also bothers me that these functions are inconsistent. Since they're both public though, wouldn't it be an issue to change them? Perhaps it's better to just document the exponent order? |
||
/// B(t) = a*t^3 + b*t^2 + c*t + d | ||
pub fn parameters(&self) -> (Vec2, Vec2, Vec2, Vec2) { | ||
let c = (self.p1 - self.p0) * 3.0; | ||
let b = (self.p2 - self.p1) * 3.0 - c; | ||
let d = self.p0.to_vec2(); | ||
|
@@ -309,6 +312,13 @@ impl CubicBez { | |
pub fn is_nan(&self) -> bool { | ||
self.p0.is_nan() || self.p1.is_nan() || self.p2.is_nan() || self.p3.is_nan() | ||
} | ||
|
||
/// Is this cubic Bezier curve a line? | ||
#[inline] | ||
pub fn is_linear(&self, accuracy: f64) -> bool { | ||
self.baseline().nearest(self.p1, accuracy).distance_sq <= accuracy | ||
&& self.baseline().nearest(self.p2, accuracy).distance_sq <= accuracy | ||
} | ||
} | ||
|
||
/// An iterator for cubic beziers. | ||
|
@@ -598,6 +608,7 @@ mod tests { | |
cubics_to_quadratic_splines, Affine, CubicBez, Nearest, ParamCurve, ParamCurveArclen, | ||
ParamCurveArea, ParamCurveDeriv, ParamCurveExtrema, ParamCurveNearest, Point, QuadBez, | ||
}; | ||
use arrayvec::{Array, ArrayVec}; | ||
|
||
#[test] | ||
fn cubicbez_deriv() { | ||
|
@@ -877,4 +888,28 @@ mod tests { | |
converted[0].points()[2].distance(Point::new(88639.0 / 90.0, 52584.0 / 90.0)) < 0.0001 | ||
); | ||
} | ||
|
||
use crate::param_curve::ParamCurveBezierClipping; | ||
#[test] | ||
fn solve_t_for_xy() { | ||
fn verify<T: Array<Item = f64>>(mut roots: ArrayVec<T>, expected: &[f64]) { | ||
assert_eq!(expected.len(), roots.len()); | ||
let epsilon = 1e-6; | ||
roots.sort_by(|a, b| a.partial_cmp(b).unwrap()); | ||
|
||
for i in 0..expected.len() { | ||
assert!((roots[i] - expected[i]).abs() < epsilon); | ||
} | ||
} | ||
|
||
let curve = CubicBez::new((0.0, 0.0), (0.0, 8.0), (10.0, 8.0), (10.0, 0.0)); | ||
verify(curve.solve_t_for_x(5.0), &[0.5]); | ||
verify(curve.solve_t_for_y(6.0), &[0.5]); | ||
|
||
{ | ||
let curve = CubicBez::new((0.0, 10.0), (0.0, 10.0), (10.0, 10.0), (10.0, 10.0)); | ||
|
||
verify(curve.solve_t_for_y(10.0), &[]); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type is slightly odd. Is there a particular reason you didn't use
Option<f64>
? I'm sympathetic to the desire to be more parallel to the other cases.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have it like this so that it's compatible with the return types in
solve_t_for_x
andsolve_t_for_y
. It also makes it so that it follows the same format assolve_quadratic
andsolve_cubic
. I don't have any issues with changing it to useOption<f64>
, but I'd like your thoughts on my reasoning first.