Linesegment orthogonal point

[RichMacDonald]

Added an "orthogonal point" to LineSegment. Equivalent to closestPoint when it exists in the interior of the LineSegment. Null when the orthogonal point would be outside the LineSegment, i.e., projectionFactor < 0 or > 1.

[dr-jts]

This PR seems to eliminate the usage of LineSegment.project(p, pf) introduced in #924. Should it be removed?

[RichMacDonald]

This PR seems to eliminate the usage of LineSegment.project(p, pf) introduced in #924. Should it be removed?

This PR was developed separately from $924, because I was not sure about pull request order. And there was a conflict between my pull requests.

I have merged the two together and pushed again. This branch should contain both changes.

[dr-jts]

Is there a use case for this method?

[RichMacDonald]

See the caller in the pull request. For the DistanceOp point-linesegment calculation, each linesegment shares an endpoint. The existing code checks each linesegment in sequence and possibly both endpoints of the linesegment. So the shared endpoints could be checked twice, unnecessarily.

[dr-jts]

How much of a performance improvement does this create?

[dr-jts]

It's probably better to localize this method to the DistanceOp class

[RichMacDonald]

It's probably better to localize this method to the DistanceOp class

There is your use case for orthogonalPoint.

[dr-jts]

What is the use case for orthogonalPoint?

[dr-jts]

This PR introduces another method overload of closestPoint. That should be mentioned in the description.

And is there really a use case for that method?

[dr-jts]

I'm not keen on introducing new methods which don't seem to have a broadly applicable semantics or use case. If you need them for you own code you can easily subclass LineSegment, or make some external functions.

[RichMacDonald]

What is the use case for orthogonalPoint?

Checking the line when the endpoints have already been checked by some other means. In geospatial applications endpoint checking is relatively straightforward, while lines require map-projections (distortions) which are a lot slower.

[dr-jts]

If you're looking to scavenge performance, perhaps this functionality should be provided as a static function? Probably on LineSegment. I've been thinking that many of the LineSegment methods could also be provided as static functions, for one-off use cases.

It really needs some performance testing, though - too easy to prematurely optimize.

[RichMacDonald]

If you have 10 LineSegments, each getting closer to the point, then the current code does 10 point-to-line and 11+10=19 point-to-point calculations. Each intermediate point is calculated twice. There should only be 10 point-to-line and 11 point-to-point calculations.

Pertinent to your other comment, I agree the skip0 boolean is an obscure solution. Better for the DistanceOp to explicitly do the point-to-line and point-to-point calculations separately, with point-to-line being of the form orthgonalPoint-to-line.

And static functions also make sense. You don't need to instantiate a LineSegment just to do a calculation then throw it away. Bad for the garbage-collector. Now if LineSegments and their attributes were declared final, then maybe they would be comparable, but that isn't currently an option.

The code below is problematic as well, because it first does the calculation without instances in Distance.pointToSegment() and then repeats the calculation with instances seg.closestPoint. If you separate point-to-point and orthogonalPoint-to-line, I think you can eliminate the duplication.

double dist = Distance.pointToSegment( coord, coord0[i], coord0[i + 1] );
if (dist < minDistance) {
minDistance = dist;
LineSegment seg = new LineSegment(coord0[i], coord0[i + 1]);
Coordinate segClosestPoint = seg.closestPoint(coord); //This is the same calculation as Distance.pointToSegment!
}