-
Notifications
You must be signed in to change notification settings - Fork 199
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
WIP line-line intersection #122
Conversation
When `ParialEq` is derived for `Line`, `Line{a, b} != Line{b, a}`. I think this is wrong as line segments aren't inherently directed even though it's sometimes convenient to think of them that way. This commit implements `PartialEq` for `Line` such that `Line{a, b} == Line{b, a}`. On a practical level, when a line segment is returned as a result of an intersection operation, the order of the endpoints is an implementation detail. With derived equality, the intersection of Line `l` with itself is not always equal to `l`.
Total ordering on Point is a requirement of the line sweep algorithm that will form the basis of further intersection, union, and difference implementations. It is also very useful for the edge case of line-line intersection where the two lines are co-linear. Once the four endpoints of the two lines can be ordered, it's simple to determine whether the middle two points consitute a gap between the two lines or the (potentially degenerate) overlap between the two lines.
Being able to identify the intersection of two lines is a requirement of the line sweep algorithm at the core of further intersection, union, and difference algorithms. This implementation raises some questions: - What is the output type of the intersection operation? Two line segments can share nothing, a single point, or a line segment between them. Here, I've chosen to implement intersection to return `Option<LineIntersection>` where `LineIntersection` is an enum over the set {`Point`, `Line`}. go.geo side-steps this by returning [`NewPoint(math.Inf(1), math.Inf(1))`](https://github.com/paulmach/go.geo/blob/7c91620e5d753c84d6149b42d39ba6556e1a5f2f/line.go#L166) when the two lines are co-linear. I don't like this approach becuase I think this concept is much more easily expressed by the type system than by what amounts to the `NaN` of points. That said, I'm not wholly satisfied with `Option<LineIntersection>`: it's cumbersome to write and read, and we'd need one for almost every type. I considered defining `LineIntersection` like `{Empty, Point(Point), Line(Line)}`, but that doesn't change much. ArcGIS solves this problem by having separate [intersect and overlap](https://gis.stackexchange.com/questions/130034/what-is-the-difference-between-intersect-overlap-in-arcgis-server) methods. - Is there a better way to deal with degenerate geometries? A `Valid` trait? Validating constructors? Should intersection return a `Result` or panic instead of handling various degeneracies or invalidities? Should we have a separate `Degeneracy` trait that repairs degeneracies (e.g. converts a degenerate `Line` to `Point`)?
6de94bc
to
ef87c5a
Compare
Revise test cases to cover each of the branches in the code: - degenerate, degenerate - non degenerate, degenerate - degenerate, non degenerate - non degenerate, non degenerate - co-linear - intersect at endpoints - overlap - disjoint - parallel, not co-linear - non-parallel
I've opened #123 to hive off discussion about validity. |
fn intersection(&self, rhs: &Rhs) -> Option<Self::OutputGeometry>; | ||
} | ||
|
||
#[derive(PartialEq, Debug)] |
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 can also derive Copy
{ | ||
fn eq(&self, other: &Self) -> bool { | ||
(self.start == other.start && self.end == other.end) || | ||
(self.start == other.end && self.end == other.start) |
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.
hmmm. thinking out loud, i'm wondering if we should avoid start
and end
as terms for Line
. 'start' and 'end' seem to imply that a Line
is directed, even though it's not.
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 agree. a
and b
?
/// Construct the intersections of geometries. | ||
pub mod intersection; | ||
/// Implements total ordering on points | ||
pub mod point_ord; |
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.
where does point_ord
get used in the intersection
impl
? i'm a little confused how both of these modules relate to each other
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.
It currently is used here to make computing the intersection of two parallel, but potentially overlapping line segments easier. The basic idea is that if you sort the endpoints while keeping track of the line segment to which each endpoint belongs, you can figure out the intersection by looking at their order. For example if the sorted endpoints look like [l1a, l2b, l1b, l2a]
then you know (provided none of them are equal) that there is a line segment overlap from l2b
to l1b
.
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.
It will also appear again in the more general intersection algorithm which requires the endpoints to be stored in a binary search tree.
opened a tracking issue for this #288 |
This PR continues the work towards intersection operators for basic types (#80).
I'm marking this as WIP since I'm not wholly satisfied by a few of the details and would appreciate some feedback.
The return type of
Intersection
Two line segments can share nothing, a single point, or a line segment between them. Here, I've chosen to implement intersection to return
Option<LineIntersection>
whereLineIntersection
is an enum over the set {Point
,Line
}.go.geo side-steps this by returning
NewPoint(math.Inf(1), math.Inf(1))
when the two lines are co-linear. I don't like this approach because I think this concept is much more easily expressed by the type system than by what amounts to theNaN
of points. That said, I'm not wholly satisfied withOption<LineIntersection>
: it's cumbersome to write and read, and we'd need one for almost every type.I considered defining
LineIntersection
like{Empty, Point(Point), Line(Line)}
, but that doesn't change much.I also considered using the
Geometry
enum instead of the specializedLineIntersection
, this is basically how PostGIS handles intersection, but I decided against that first because of #119, and second because it over-specifies the types that can actually be returned.ArcGIS solves this problem by having separate intersect and overlap methods.
Handling degenerate lines
A
Valid
trait? Validating constructors? Should intersection return aResult
or panic instead of handling various degeneracies or invalidities? Should we have a separateDegeneracy
trait that repairs degeneracies (e.g. converts a degenerateLine
toPoint
)?Project structure
For now, I added the point ordering as module directly under
algorithm
, but as I add more helper functions to support the line sweep it might make more sense to gather all of that code (trait, impls, and support) in a subdirectory just for intersection.