Skip to content

Linesegment orthogonal point #926

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Linesegment orthogonal point #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e01038e
LineSegment.projectionFactor was being recalculated unnecessarily.
RichMacDonald Nov 1, 2022
a9ab268
Added LineSegment.orthogonalPoint(Coordinate)
RichMacDonald Nov 1, 2022
68886f3
Merge branch 'master' into linesegment-orthogonal-point
RichMacDonald Nov 2, 2022
2575d3b
Skip redundant calculation in the DistanceOp point-linesegment
RichMacDonald Nov 7, 2022
beb39fe
Merge branch 'master' into linesegment-orthogonal-point
RichMacDonald Nov 8, 2022
41c59d9
Merge branch 'master' into linesegment-orthogonal-point
RichMacDonald Nov 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 36 additions & 3 deletions modules/core/src/main/java/org/locationtech/jts/geom/LineSegment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,16 +505,49 @@ public Coordinate reflect(Coordinate p) {
*/
public Coordinate closestPoint(Coordinate p)
{
double factor = projectionFactor(p);
if (factor > 0 && factor < 1) {
return project(p, factor);
return closestPoint(p, false);
}

/**
* Computes the closest point on this line segment to another point.
* @param p the point to find the closest point to
* @param skip0 If true, do not check p0. Client will have determined this is redundant.
* @return a Coordinate which is the closest point on the line segment to the point p
*/
public Coordinate closestPoint(Coordinate p, boolean skip0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a use case for this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much of a performance improvement does this create?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

There is your use case for orthogonalPoint.

{
Coordinate orthog = orthogonalPoint(p);
if (orthog != null) {
return orthog;
}
if (skip0) {
return p1;
}
double dist0 = p0.distance(p);
double dist1 = p1.distance(p);
if (dist0 < dist1)
return p0;
return p1;
}

/**
* Computes the orthogonal point on this line segment to another point.
* @param p the point to find the closest point to
* @return a Coordinate which is the orthogonal point on the line segment to the point p. Null if it does not exist.
*/
public Coordinate orthogonalPoint(Coordinate p)
{
double factor = projectionFactor(p);
if (factor == 0.0) {
return p0;
} else if (factor == 1.0) {
return p1;
} else if (factor > 0 && factor < 1) {
return project(p, factor);
} else {
return null;
}
}
/**
* Computes the closest points on two line segments.
*
Expand Down
2 changes: 1 addition & 1 deletion modules/core/src/main/java/org/locationtech/jts/operation/distance/DistanceOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ private void computeMinDistance(LineString line, Point pt,
if (dist < minDistance) {
minDistance = dist;
LineSegment seg = new LineSegment(coord0[i], coord0[i + 1]);
Coordinate segClosestPoint = seg.closestPoint(coord);
Coordinate segClosestPoint = seg.closestPoint(coord, i > 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!
}

locGeom[0] = new GeometryLocation(line, i, segClosestPoint);
locGeom[1] = new GeometryLocation(pt, 0, coord);
}
Expand Down
31 changes: 31 additions & 0 deletions modules/core/src/test/java/org/locationtech/jts/geom/LineSegmentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,37 @@ public void testProjectionFactor()
assertTrue(seg2.projectionFactor(new Coordinate(11, 0)) == 0.1);
}

public void testOrthogonalPoint()
{
Coordinate l0 = new Coordinate(0, 0);
Coordinate l1 = new Coordinate(10, 10);
LineSegment seg = new LineSegment(l0, l1);

Coordinate orth = seg.orthogonalPoint(new Coordinate(0, 0));
assertTrue(orth.equals(l0));

orth = seg.orthogonalPoint(new Coordinate(10, 10));
assertTrue(orth.equals(l1));

orth = seg.orthogonalPoint(new Coordinate(10, 0));
assertTrue(orth.equals(new Coordinate(5, 5)));

orth = seg.orthogonalPoint(new Coordinate(0, 10));
assertTrue(orth.equals(new Coordinate(5, 5)));

orth = seg.orthogonalPoint(new Coordinate(10, -10));
assertTrue(orth.equals(l0));

orth = seg.orthogonalPoint(new Coordinate(-10, 10));
assertTrue(orth.equals(l0));

orth = seg.orthogonalPoint(new Coordinate(11, 10));
assertNull(orth);

orth = seg.orthogonalPoint(new Coordinate(-1, 0));
assertNull(orth);
}

public void testLineIntersection() {
// simple case
checkLineIntersection(
Expand Down