-
Notifications
You must be signed in to change notification settings - Fork 369
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
Make line intersection computation more robust #937
Conversation
MULTIPOLYGON (((0 25.950779066386122, 0 26.860827855779647, 0 29.83879230192533, 0 35.73921397193218, 60 13.183894681853793, 60 10.39852836763784, 60 7.802134559422377, 60 6.657099879646016, 60 6.510515132098641, 60 0, 55.31611974965083 0, 50.00444661661829 0, 43.31611974965083 0, 0 0, 0 14.034613342373582, 0 17.92266612923108, 0 21.587486526024364, 0 25.950779066386122), | ||
(0 21.587486526024364, 34.025852439655786 6.898140262297274, 34.02585243965579 6.898140262297273, 0 21.587486526024364)), | ||
((13.44557253233479 36, 60 36, 60 16.794451829809802, 60 16.36440115550932, 60 14.043996030454757, 2.9187843276549756 36, 11.8945390820011 36, 13.44557253233479 36))) | ||
MULTIPOLYGON (((0 26.860827855779647, 0 29.83879230192533, 0 35.73921397193218, 60 13.183894681853793, 60 10.39852836763784, 60 7.802134559422377, 60 6.657099879646016, 60 6.510515132098641, 60 0, 55.31611974965083 0, 50.00444661661829 0, 43.31611974965083 0, 0 0, 0 14.034613342373582, 0 17.92266612923108, 0 21.587486526024364, 0 25.950779066386122, 0 26.860827855779647)), |
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.
First coordinate in ring is different? Seems odd?
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.
Might just be a bit of non-determinism. Will investigate.
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 think there was a report that overlay would "rotate" results by a single coordinate over repeated operations. Perhaps this is the same phenomenon at work?
@@ -452,7 +445,7 @@ void object::test<10> | |||
"LINESTRING (-58.00593335955 -1.43739086465, -513.86101637525 -457.29247388035)", | |||
"LINESTRING (-215.22279674875 -158.65425425385, -218.1208801283 -160.68343590235)", | |||
1, | |||
"POINT ( -215.22279674875 -158.65425425385 )", | |||
"POINT ( -215.22279674875003 -158.65425425385004 )", |
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.
Feels like a case for rounding? I guess not until we have some other platform that's got a slightly different answer...?
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.
Agreed, this would be more robust if the test specified a precision for the result. More so in GEOS than JTS, since there is more chance of platform-related discrepancy.
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.
Fixed by adding a distance tolerance.
Unfortunately this does not fix #740. Perhaps the larger size of the ordinate values defeats the greater accuracy of the DD intersection computation. |
[Geos switched algorithms](libgeos/geos#937) and [the rounding code](libgeos/geos@bf18ba8) is slightly different.
This does not fix #766. |
Changes the computation of line intersections to use DoubleDouble arithmetic, for improved accuracy. This replaces the previous floating-point implementation.
The main reason for doing this is to fix some failures in
relate
computation (such as #933). These are cases where both inputs contain the same line segment, but input A has a self-intersection in the line segment. Therelate
algorithm computes a new node point at the self-intersection. The FP intersection algorithm computes an intersection point which falls slightly off the parent line segment. The noded input A then no longer matches input B exactly, resulting in Exterior(A) / Interior(B) = 1 (rather then the expected result of F). The DD intersection computation is accurate enough to avoid this problem.CGAlgorithmsDD.intersection
is slower than the original FP implementation. But in typical use intersection calculation is a small fraction of overall processing, so there is minor performance impact in practice.This changes the results of some overlay cases by a small amount. A few unit tests had to be changed to reflect this.
Fixes #933
See also locationtech/jts#396 (https://trac.osgeo.org/geos/ticket/572) for another relate case which is fixed by this change.