-
Notifications
You must be signed in to change notification settings - Fork 89
Find tangents from point to cubic Bézier #288
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
Conversation
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.
🎉
src/cubicbez.rs
Outdated
| /// | ||
| /// Result is array of t values such that the line from the given point | ||
| /// to the curve evaluated at that value is tangent to the curve. | ||
| pub fn point_tangents(&self, p: Point) -> ArrayVec<f64, 4> { |
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.
Would tangents_though_point be a clearer name?
src/cubicbez.rs
Outdated
| .collect() | ||
| } | ||
|
|
||
| /// Find lines from the point to the curve tangent to the curve. |
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.
On my initial few read-throughs of this sentence I had a hard time groking it. I can also take a shot at wording it, which might also be flawed, but maybe we can meet in the middle with something that's clearer to unbiased readers:
Find points on the curve where each one's tangent line passes through the given point
pin space.
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 made it a little shorter (it should be one line), but agree this is clearer. I think in my mind I was thinking about the line from the point to the curve, but thinking about it the other way is simpler because it's more apparent that you're getting both the tangent and the incidence.
|
@raphlinus i have heard that @Keavon might be interested in this being in a kurbo release… thoughts? |
|
I've fixed the merge conflict. @Keavon can you confirm this is still useful, and that review comments have been addressed to your satisfaction? |
|
@raphlinus This is still useful and the code/comments indeed look good! It would also be useful to have a For some context surrounding Graphite and Bezier-rs: In Graphite, we're preparing to begin replacing some Bezier-rs functionality with Kurbo because it has better robustness, so this will be helpful to have. Our motivation for making Bezier-rs was twofold:
Maintenance and the future of Bezier-rs is still up in the air but I'm envisioning it as a possible wrapper for Kurbo that could use those algorithms that exist and augment it with our additional features that Kurbo currently lacks (or always will), such as the Poisson-disk point sampling linked above. Downside: Bezier-rs would no longer be a zero-dependency crate. Upside: we can reduce our maintenance burden through deleting a lot of our code by wrapping Kurbo's algorithms, provide better robustness and performance, and probably continue to use Bezier-rs as a testbed or incubator for more complex algorithms instead of needing to officially abandon our maintenance for the crate if we fully switched to Kurbo in Graphite. |
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.
(Approving this, as @Keavon doesn't have the rights to do this, but has confirmed usefulness)
|
Unless I hear otherwise or someone else merges this, I will merge this during or after office hours today. |
|
I'm merging it now. Sounds like there's a good discussion to be had about future coevolution of bezier-rs and kurbo. The poisson disk feature looks quite interesting, but as you say it's probably not in scope for kurbo. |
|
I'm very excited about bezier-rs moving some things into Kurbo! |
|
I should go through and create a comparison chart about which algorithms each of our libraries implement in order to focus attention towards those which may be useful in Kurbo. But I need to emphasize that our algorithms are somewhat janky and probably aren't fast or robust enough for the standards of Kurbo. Though it would still be a useful way to evaluate what may be worth reimplementing in Kurbo with better attention towards those fine details. |
No description provided.