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

Add tolerance parmeters to Shape methods #293

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Conversation

george-steel
Copy link

Allows the lesser-used Shape methods to be implemented in terms of path_elements and provides default implementations.

Resolves #263

Comment on lines 126 to 150
/// 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;
}
Copy link
Author

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).

Copy link
Contributor

@raphlinus raphlinus left a 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/circle.rs Outdated Show resolved Hide resolved
pub trait ParamCurveArea {
/// Compute the signed area under the curve.
/// Compute the signed (counterclockwise) area between the curve and the origin.
Copy link
Contributor

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.

Copy link
Author

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"

src/param_curve.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
@george-steel
Copy link
Author

Overall this looks good. Any special reason why this is still marked as draft?

Because this is a breaking change, I want to also get ParamCurveWinding ready and land both together.

@george-steel
Copy link
Author

Note that clippy is currently failing due to the state of main.

@waywardmonkeys
Copy link
Contributor

Note that clippy is currently failing due to the state of main.

This is fixed now on main.

@raphlinus
Copy link
Contributor

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.

src/bezpath.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In Shape, have default impls for everything except path_elements
4 participants