abstreet icon indicating copy to clipboard operation
abstreet copied to clipboard

Handle line segments intersecting at their endpoints. The algorithm I

Open dabreegster opened this issue 5 years ago • 4 comments

ported ages ago doesn't handle this, and somehow I didn't notice until b2519e3050a1b90da12f7d5509e453cf8378b928.

@michaelkirk, any thoughts on this incremental improvement? I wanted to take this opportunity to rely on a better tested library for line segment intersection, but all I found was https://crates.io/crates/line_intersection. I gave it a shot at https://github.com/dabreegster/abstreet/blob/21eb14ba52afd549f0d6d0732ec3fe405a19869e/geom/src/line.rs#L75, but it said the line segments in the unit test were co-linear, which doesn't seem correct to me.

I'll keep looking around for ways to reduce the core code in geom. Ideally it'd mostly just wrap other crates that're better tested.

dabreegster avatar Nov 03 '20 20:11 dabreegster

but it said the line segments in the unit test were co-linear, which doesn't seem correct to me.

Oh, hmm... you mean the lines (0.0, 0.0) -> (1.0, 1.0) and (2.0, 2.0) -> (1.0, 1.0)?

Those indeed are collinear. Does their code not work with collinear lines or something?

michaelkirk avatar Nov 04 '20 00:11 michaelkirk

Those indeed are collinear. Does their code not work with collinear lines or something?

It returns collinear in this case, but I'd expect just the endpoint (1, 1). Maybe it depends if the line segment endpoints are open or closed; the docs use [0, 1] notation, so I'm expecting closed.

It seems like in the case that their endpoints overlap we have clear behavior, but what if their overlap is more than a point?

I'll add some more tests and figure out how to implement. I would expect None as the result. But honestly, it's maybe time to audit all of the callers of this method and either return the more nuanced enum of different types of intersections, or indeed keep the 0 or 1 point result.

dabreegster avatar Nov 04 '20 00:11 dabreegster

I mentioned this in Slack, but depending on how urgent this is I'm hoping the geo crate will have a line intersection API merged "soon" (1-2 weeks?).

michaelkirk avatar Nov 19 '20 19:11 michaelkirk

I might hold off on this, then -- thanks! No rush; not planning to work on intersection geometry anytime soon again

dabreegster avatar Nov 19 '20 19:11 dabreegster