-
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
Add tolerance parmeters to Shape methods #293
base: main
Are you sure you want to change the base?
Conversation
src/param_curve.rs
Outdated
/// A parametrized curve (or a section of one) that can have its signed area measured. | ||
pub trait ParamCurveArea { | ||
/// Compute the signed area under the curve. | ||
/// Compute the signed (counterclockwise) area between the curve and the origin. | ||
/// Equivalently (using Green's theorem), | ||
/// this is integral of the form `(x*dy - y*dx)/2` along the curve. | ||
/// | ||
/// For closed curves, this is the curve's area. | ||
/// For open curves, this is the the area of the resulting shape that would be created if | ||
/// the curve was closed with two line segments between the endpoints and the origin. | ||
/// This allows the area of a piecewise curve to be computed by adding the areas of its segments, | ||
/// generalizing the "shoelace formula." | ||
/// | ||
/// For an open curve with endpoints `(x0, y0)` and `(x1, y1)`, this value | ||
/// is also equivalent to `-intgral(y*dx) - (x0*y0 + x1*y1)/2`. | ||
/// | ||
/// For a closed path, the signed area of the path is the sum of signed | ||
/// areas of the segments. This is a variant of the "shoelace formula." | ||
/// See: | ||
/// <https://github.com/Pomax/bezierinfo/issues/44> and | ||
/// <http://ich.deanmcnamee.com/graphics/2016/03/30/CurveArea.html> | ||
/// | ||
/// This can be computed exactly for Béziers thanks to Green's theorem, | ||
/// and also for simple curves such as circular arcs. For more exotic | ||
/// curves, it's probably best to subdivide to cubics. We leave that | ||
/// to the caller, which is why we don't give an accuracy param here. | ||
/// This can be computed exactly for Béziers, | ||
/// and also for simple curves such as circular arcs. | ||
/// For more exotic curves, it's probably best to subdivide to cubics. | ||
/// We leave that to the caller, which is why we don't give an accuracy param here. | ||
fn signed_area(&self) -> f64; | ||
} |
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.
Updating the documentation as the current implementations were measuring this value instead of the area under the curve (integral of y*dx
).
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.
Overall this looks good. Any special reason why this is still marked as draft?
src/param_curve.rs
Outdated
pub trait ParamCurveArea { | ||
/// Compute the signed area under the curve. | ||
/// Compute the signed (counterclockwise) area between the curve and the origin. |
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 like that we're being specific about the area, but the convention is not to say things like "counterclockwise" without also specifying whether coordinates y-up or y-down.
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.
changed to "counterclockwise from +x to +y"
ea4b5fa
to
23f40d9
Compare
Because this is a breaking change, I want to also get |
Note that clippy is currently failing due to the state of |
This is fixed now on |
A gentle ping, we're planning an 0.10 release which will be a semver bump, and it would be great to get this in. But there will be other chances if not. |
Allows the lesser-used
Shape
methods to be implemented in terms ofpath_elements
and provides default implementations.Resolves #263