OpenMap-java / openmap

OpenMap is an Open Source JavaBeans-based programmer's toolkit. Using OpenMap, you can quickly build applications and applets that access data from legacy databases and applications.
http://openmap-java.org
Other
73 stars 43 forks source link

Bugs in Geo.intersect and GeoArray.Adapter.distance calculations #38

Open klesniewski opened 7 years ago

klesniewski commented 7 years ago

Hello!

Thank you for all the effort to develop the library :)

I encountered two issues when using GeoArray.Adapter.distance method, which calculates distance from point to polygon. The problems I found are:

  1. Geo.intersect method returns weird intersection points, and because of that GeoArray.Adapter.distance always returns positive infinity in my cases.
  2. GeoArray.Adapter.distance does not take into account cases, where the closes point of polygon is a vertex, and not an edge. In those cases it returns positive infinity even with corrected Geo.intersect implementation.
  3. Method implementation has System.out.println calls.

Ad 1. I corrected the implementation. I didn't dive deep into the maths behind it - I just went for a simple solution to find intersection:

static Geo simpleIntersect(Geo p, Geo q, Geo r) {
    // Find vector of great circle between p and q
    Geo greatCircle1 = p.cross(q);

    // Find vector of great circle, which is perpendicular to vectors of {great circle of p and q} AND {r}
    Geo greatCircle2 = greatCircle1.cross(r);

    // Find two points where these great circles intersect
    // Given the two plane normals, their cross product defines their intersecting line
    Geo intersection = greatCircle1.cross(greatCircle2);
    Geo intersectionAntipode = intersection.antipode();

    // Return intersection closer to r
    return r.distance(intersection) <= r.distance(intersectionAntipode) ? intersection : intersectionAntipode;
}

I added a test to compare it with current implementation and it yields totally different results, whereas the simple implementation seems to be the one, who returns the right result.

@Test
void intersect() {
    Geo p = new Geo(48.0, 9.0);
    Geo q = new Geo(48.0, 8.0);
    Geo r = new Geo(47.995197, 8.018186); // Distance to line around: 0.54365 km
    Geo simpleIntersection = simpleIntersect(p, q, r);
    Geo originalIntersection = p.intersect(q, r);
    assert simpleIntersection.equals(originalIntersection);
}

Result:

Assertion failed: 

assert simpleIntersection.equals(originalIntersection)
       |                  |      |
       |                  false  Geo[0.24139032739860927,98.2826576417656]
       Geo[48.00007729394193,8.018140694114365]

Having the intersect implementation corrected, the GeoArray.Adapter.distance implementation calculates valid results.

Ad 2. I corrected implementation by first iterating over vertices and calculating distance to each. Then I take the minimum distance as the initial ret value, so that if distance to edge is shorter, the new minimum will be found, but if the distance is not shorter, the shortest distance to any of vertices will be used.

double minDistance = Double.POSITIVE_INFINITY;

int size = polygon.getSize();
for (int i = 0; i < size; i++) {
    Geo vertex = polygon.get(i);
    double distance = vertex.distance(point);
    if (distance < minDistance)
        minDistance = distance;
}

ret = minDistance

// From here the GeoArray.Adapter.distance implementation follows

Affected OpenMap version: 5.1.15