Skip to content
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

Jump to bottom

DNM: Updates to Segment.intersection Method #6

Closed
wants to merge 4 commits into from
Closed

DNM: Updates to Segment.intersection Method #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4bfe608
Update segment.intersection
Apr 19, 2019
914cd05
Code clean up
Apr 19, 2019
946d0ea
Update logic
Jul 29, 2019
61b33d3
Resolve merge conflict
Jul 29, 2019
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
46 changes: 43 additions & 3 deletions src/main/java/org/openstreetmap/atlas/geography/Segment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public Optional<Heading> heading()
* @return The intersection point if any, null otherwise
* @see "http://stackoverflow.com/a/1968345/1558687"
*/
public Location intersection(final Segment that)
private Location intersectionUsingDouble(final Segment that)
{
final double p0X = this.start().getLongitude().asDegrees();
final double p0Y = this.start().getLatitude().asDegrees();
Expand All @@ -129,15 +129,55 @@ public Location intersection(final Segment that)

final double sValue;
final double tValue;
sValue = (-s1Y * (p0X - p2X) + s1X * (p0Y - p2Y)) / (-s2X * s1Y + s1X * s2Y);
tValue = (s2X * (p0Y - p2Y) - s2Y * (p0X - p2X)) / (-s2X * s1Y + s1X * s2Y);
final double commonDenominator = -s2X * s1Y + s1X * s2Y;
sValue = (-s1Y * (p0X - p2X) + s1X * (p0Y - p2Y)) / commonDenominator;
tValue = (s2X * (p0Y - p2Y) - s2Y * (p0X - p2X)) / commonDenominator;

if (sValue >= 0 && sValue <= 1 && tValue >= 0 && tValue <= 1)
{
// Collision detected
return new Location(Latitude.degrees(p0Y + tValue * s1Y),
Longitude.degrees(p0X + tValue * s1X));
}
return null;
}

/**
* Implements the same function as intersection, but with long to get the intersection at end of two segments
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there some reason we would not always want to use this version of the function?

Copy link
Owner Author

@sayas01 sayas01 Apr 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I had tested it out on a few cases and the result with degrees returns the most accurate intersection than with long. Like I mentioned in the PR description, returning the end point would be ideal instead of actually calling this function, but there may be some cases I am not aware of.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It sounds like the next step should be to confer with Jack, then.

*
* @param that
* The segment to intersect
* @return The intersection point if any, null otherwise
*/
public Location intersection(final Segment that)
{
final long p0X = this.start().getLongitude().asDm7();
final long p0Y = this.start().getLatitude().asDm7();
final long p1X = this.end().getLongitude().asDm7();
final long p1Y = this.end().getLatitude().asDm7();
final long p2X = that.start().getLongitude().asDm7();
final long p2Y = that.start().getLatitude().asDm7();
final long p3X = that.end().getLongitude().asDm7();
final long p3Y = that.end().getLatitude().asDm7();
final long s1X = p1X - p0X;
final long s1Y = p1Y - p0Y;
final long s2X = p3X - p2X;
final long s2Y = p3Y - p2Y;
final long commonDenominator = -s2X * s1Y + s1X * s2Y;
final long sValue = (-s1Y * (p0X - p2X) + s1X * (p0Y - p2Y)) / commonDenominator;
final long tValue = (s2X * (p0Y - p2Y) - s2Y * (p0X - p2X)) / commonDenominator;
// Checks for intersection
if (sValue >= 0 && sValue <= 1 && tValue >= 0 && tValue <= 1)
{
// Intersections that occur at points other than end points are more accurate using
// double and degrees calculations
final Location intersectionLocationUsingDouble = this.intersectionUsingDouble(that);
// Collision detected. If intersection calculated using double is null,intersection is at end point and so,
// return the location calculated using long and dm7.
return intersectionLocationUsingDouble != null? intersectionLocationUsingDouble :
new Location(Latitude.dm7(p0Y + tValue * s1Y),
Longitude.dm7(p0X + tValue * s1X));
}
// No collision
return null;
}
Expand Down
24 changes: 17 additions & 7 deletions src/test/java/org/openstreetmap/atlas/geography/SegmentTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public class SegmentTest
.wkt("LINESTRING (112.9699474 -84.7999528, 112.9699948 -84.7999669)").segments().get(0);
private static Segment SEGMENT_SAME_END_2 = PolyLine
.wkt("LINESTRING (112.9650809 -84.7999622, 112.9699948 -84.7999669)").segments().get(0);
private static final Location INTERSECTION_LOCATION = Location
.forString("-84.7999669,112.9699948");

@Test
public void testAntimeridianNoIntersection()
Expand All @@ -33,17 +35,16 @@ public void testAntimeridianNoIntersection()
@Test
public void testIntersection()
{
Assert.assertEquals(null, new Segment(Location.TEST_1, Location.TEST_2)
Assert.assertNull(new Segment(Location.TEST_1, Location.TEST_2)
.intersection(new Segment(Location.TEST_4, Location.TEST_3)));
Assert.assertEquals(false, new Segment(Location.TEST_1, Location.TEST_2)
Assert.assertFalse(new Segment(Location.TEST_1, Location.TEST_2)
.intersects(new Segment(Location.TEST_4, Location.TEST_3)));

Assert.assertTrue(new Segment(Location.TEST_1, Location.TEST_3)
.intersects(new Segment(Location.TEST_4, Location.TEST_2)));
Assert.assertEquals(
new Location(Latitude.degrees(37.3273389), Longitude.degrees(-122.0287109)),
new Segment(Location.TEST_1, Location.TEST_3)
new Segment(new Location(Latitude.degrees(37.3273389), Longitude.degrees(-122.0287109)), Location.TEST_3)
.intersection(new Segment(Location.TEST_4, Location.TEST_2)));
Assert.assertEquals(true, new Segment(Location.TEST_1, Location.TEST_3)
.intersects(new Segment(Location.TEST_4, Location.TEST_2)));
}

@Test
Expand All @@ -58,7 +59,8 @@ public void testIntersectionOverflow()

Assert.assertTrue("Overflow issue", maxXandYMovement.intersects(maxXandNegativeYMovement));
Assert.assertTrue("Overflow issue", maxXandNegativeYMovement.intersects(maxXandYMovement));

Assert.assertNull(maxXandYMovement.intersection(maxXandNegativeYMovement));
Assert.assertNull(maxXandNegativeYMovement.intersection(maxXandYMovement));
}

@Test
Expand All @@ -70,5 +72,13 @@ public void testIntersectionWithSameEnd()
SEGMENT_SAME_END_2.reversed().intersects(SEGMENT_SAME_END_1.reversed()));
Assert.assertTrue("Same End is broken", SEGMENT_SAME_END_1.intersects(SEGMENT_SAME_END_2));
Assert.assertTrue("Same End is broken", SEGMENT_SAME_END_2.intersects(SEGMENT_SAME_END_1));
Assert.assertEquals(INTERSECTION_LOCATION,
SEGMENT_SAME_END_1.reversed().intersection(SEGMENT_SAME_END_2.reversed()));
Assert.assertEquals(INTERSECTION_LOCATION,
SEGMENT_SAME_END_2.reversed().intersection(SEGMENT_SAME_END_1.reversed()));
Assert.assertEquals(INTERSECTION_LOCATION,
SEGMENT_SAME_END_1.intersection(SEGMENT_SAME_END_2));
Assert.assertEquals(INTERSECTION_LOCATION,
SEGMENT_SAME_END_2.intersection(SEGMENT_SAME_END_1));
}
}