Fix Floating-point instability in distance test #139
Conversation
This is a more robust way to calculate distance from a point to a line-segment. It's a direct port from GEOS: https://github.com/OSGeo/geos/blob/master/src/algorithm/CGAlgorithms.cpp#L191 Fixes georust#138
|
PR also includes a test with the values that caused the bug found in #138 |
src/algorithm/distance.rs
Outdated
| } | ||
| let r = ((point.x() - start.x()) * (end.x() - start.x()) + (point.y() - start.y()) * (end.y() - start.y())) / | ||
| ((end.x() - start.x()) * (end.x() - start.x()) + (end.y() - start.y()) * (end.y() - start.y())); | ||
| if r <= T::zero() { |
There was a problem hiding this comment.
I think this would be easier to read as a match statement on r.partial_cmp(T::zero()), but that's just my personal preference so feel free to ignore.
There was a problem hiding this comment.
Not sure I follow. Can you sketch it out?
There was a problem hiding this comment.
Sorry nevermind. I misread L83 my first time through. This is good the way it is.
src/algorithm/distance.rs
Outdated
| if r >= T::one() { | ||
| return point.distance(end); | ||
| } | ||
| let s = ((start.y() - point.y()) * (end.x() - start.x()) - (start.x() - point.x()) * (end.y() - start.y())) / |
There was a problem hiding this comment.
defining dx = end.x() - start.x() and dy = end.y() - start.y() (or similar) would make these three lines easier to read IMO and I don't think would change the computation.
|
@urschrei do you think this PR could help with possible intersection (polygon to polygon) bug? |
|
frewsxcv r+ |
|
bors r+ |
139: Fix Floating-point instability in distance test r=frewsxcv This is a more robust way to calculate distance from a point to a line-segment. It's a direct port from GEOS: https://github.com/OSGeo/geos/blob/master/src/algorithm/CGAlgorithms.cpp#L191 Note the small difference between the test distances in `point_linestring_distance_test`, which match Shapely (GEOS) for the same polygon and point. Old: 1.1313708498984758 New 1.1313708498984762 Fixes #138
|
thanks for fixing this @urschrei! |
|
np! (I'm the one who caused it!) |
Build succeeded |
This is a more robust way to calculate distance from a point to a
line-segment. It's a direct port from GEOS:
https://github.com/OSGeo/geos/blob/master/src/algorithm/CGAlgorithms.cpp#L191
Note the small difference between the test distances in
point_linestring_distance_test, which match Shapely (GEOS) for the same polygon and point.Old: 1.1313708498984758
New 1.1313708498984762
Fixes #138