-
Notifications
You must be signed in to change notification settings - Fork 71
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
Numerically approximate arc perimeter #381
base: main
Are you sure you want to change the base?
Conversation
I've no idea about the math here, but what Raph and I had discussed (somewhat) for #378 is that I would fix the one part that is broken and it could land and then a follow up (like this one?) would replace the current math with better math for the |
This is based on top of your PR, changing The |
59540e4
to
c8fcd34
Compare
Simplified a bit and hopefully all edge cases are now correct.
|
I've optimized a bit, the implementation now requires three evaluations of the elliptic integrals, one of which is actually a complete integral and could be optimized with a quadratically converging approximation (which I suggest we leave for a later PR, that's also relevant for the |
5e722d3
to
920728b
Compare
This uses an approximation of the arc length using beziers as the analytic solution is quite involved.
Also add some more tests
This doesn't (or doesn't easily?) occur for ellipses, as they instead get a rotation when constructed with radius y > radius x.
99a7762
to
9779d4e
Compare
I need to go over the math once more when I'm fresh, but the relatively stringent tests pass, while the number of iterations per incomplete elliptic integral evaluation is around 3 or 4. This does still require #378 to be merged first. |
// The available accuracy (tolerance) is distributed over the calculation of the two | ||
// incomplete and one complete elliptic integrals. |
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 distribution of the accuracy tolerance is not very clever, but I'd rather not spend too many clock cycles on it, when those cycles could be spent iterating the convergence loops.
The worst case is when the number of complete quarter turns, evaluated as complete elliptic integrals, is very high, as the tolerance available to it decreases linearly in the number of quarter turns.
if 1. <= r * a.powi(5) * (1. - e / a) { | ||
break; | ||
} |
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.
This bound checking is a non-trivial amount of the operations per iteration of the Carlson RF and RD implementations, and while I think this is fine, there's an interesting alternative. Iterating to a relative error is cheaper (the formula is in "Numerical computation of real or complex elliptic integrals"), perhaps calculating a conservative upper bound before the loop starts and basing a relative error bound on that is also a possibility.
This is on top of PR #378.
This implementation uses Carlson symmetric forms (https://arxiv.org/abs/math/9409227v1) of incomplete elliptic integrals of the second kind to numerically approximate elliptical arc lengths. These forms have nice computational properties allowing quick convergence of the approximations (each iteration reduces the error bound by 4⁶). Boost and Gnu Scientific Library are some other projects using them.
For the complete elliptic parts of the arc, the existing complete elliptic integral approximation (#383) is used.