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.

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

Add Segment Intersection Method and Sweepline Intersector Class #262

Merged
merged 3 commits into from Jul 6, 2021

Conversation

keithdoggett
Copy link
Member

@keithdoggett keithdoggett commented Jun 30, 2021

This is the first of three cherry-picked commits from the valid-op branch.

Summary

  • Adds a segment_intersection method to determine the intersection point between two segments.
  • Adds a SweeplineIntersector class that can be used to determine the intersections between a list of segments.

Other Information

I'm using a modified version of this algorithm for the SweeplineIntersector. Instead of using a BST for the status object, I'm just keeping all of the current segments in an array and comparing them all against each other. This means the worst case is O(n^2), but the average case is a lot better.

Below is the results of a test for finding the intersections in a convex linear ring with various number of vertices vs. the current is_simple? algorithm.

Current Algorithm:

       user     system      total        real
100 Vertices  0.002848   0.000000   0.002848 (  0.002845)
200 Vertices  0.011324   0.000000   0.011324 (  0.011344)
300 Vertices  0.025661   0.000173   0.025834 (  0.025841)
400 Vertices  0.045257   0.000000   0.045257 (  0.045267)
500 Vertices  0.070586   0.000000   0.070586 (  0.070602)
600 Vertices  0.101751   0.000000   0.101751 (  0.101772)
700 Vertices  0.137849   0.000000   0.137849 (  0.137879)
800 Vertices  0.177336   0.000000   0.177336 (  0.177373)
900 Vertices  0.225233   0.000000   0.225233 (  0.225284)
1000 Vertices  0.278194   0.000000   0.278194 (  0.278251)
2500 Vertices  1.741052   0.000000   1.741052 (  1.741438)

SweeplineIntersector:

       user     system      total        real
100 Vertices  0.002291   0.000000   0.002291 (  0.002304)
200 Vertices  0.004537   0.000169   0.004706 (  0.004704)
300 Vertices  0.007165   0.000114   0.007279 (  0.007281)
400 Vertices  0.009682   0.000000   0.009682 (  0.009687)
500 Vertices  0.012650   0.000000   0.012650 (  0.012669)
600 Vertices  0.015084   0.000000   0.015084 (  0.015090)
700 Vertices  0.017796   0.000000   0.017796 (  0.017803)
800 Vertices  0.020277   0.000000   0.020277 (  0.020283)
900 Vertices  0.022950   0.000000   0.022950 (  0.022955)
1000 Vertices  0.025088   0.000000   0.025088 (  0.025092)
2500 Vertices  0.067948   0.000000   0.067948 (  0.067964)

Future Work

Eventually, we may want to modify the segment_intersection method to return some type of struct that indicates the intersection type as well. Colinear intersections just return one point from the intersection, but in the future, we may want to get the subsegment from the intersection. This could cause issues with validity checking in a linear ring like this: LINESTRING(0 0, 1 0, 1 1, 0.5 1, 0.5 1.5, 0.5 1, 0 1, 0 0)
image

Where the segments in the spike might not be included in the proper_intersections array, but don't intersect in a valid way.

…created SweeplineIntersector class to compute intersections on groups of segments.

Segment#segment_intersection sligtly modifies the code from intersects_segment? and actually computes the intersection point (or nil if none).
Edge cases for colinear segments are handled by return a single point from the intersection.
intersects_segment? was modified to just check if segment_intersection is not nil.

A SweeplineIntersector class was added that uses a basic sweepline algorithm to compute the intersections in a set of segments.
This should be faster or as fast as the current validate_geometry intersection algorithm being used (more testing needed), since
it only compares segments where their y-range includes the y value of the event the sweepline is handling.
There are options to return all intersections or proper intersections that will filter out the connections in LineStrings.

In the future, it is possible to make a Sweepline class and have the SweeplineIntersector inherit from it. This would allow
us to use the sweepline more generically in other applications (some polygon clipping algos use a sweepline).
Copy link
Member

@BuonOmo BuonOmo left a comment

Choose a reason for hiding this comment

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

Really cool, thank you for this implementation!

I feel like it could be in a separated file. But that is just IMHO.

I've made a few reviews, but everything feels great globally, it is just about tiny fixes.

On the giving segments versus points, I think even GEOS gives only points, hence I'm ensure we need to go that far!

lib/rgeo/cartesian/calculations.rb Outdated Show resolved Hide resolved
lib/rgeo/cartesian/calculations.rb Outdated Show resolved Hide resolved
lib/rgeo/cartesian/calculations.rb Outdated Show resolved Hide resolved
lib/rgeo/cartesian/calculations.rb Outdated Show resolved Hide resolved
# a new line, it will check if it intersects with any of the segments
# the line currently intersects at that y value.
# This is a more simplistic implementation that uses an array to hold
# observed segments instead of a sorted BST, so performance may be significantly
Copy link
Member

Choose a reason for hiding this comment

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

I am a bit unsure of where it would be more appropriate, could you indicate me ?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

LINESTRING(1 1, 0 1, 0 0, 0.125 0.5, 0.25 0, 0.375 0.5, 0.5 0, 0.625 0.5, 0.75 0, 0.875 0.5, 1 0, 1 1)

In this case, since we are comparing all observed_segments against each other, this would be about O(n^2) since pretty much every segment is being observed at once here. If we were using a BST instead of an array or set, we would sort it so that only adjacent segments are compared to each other, but it's a lot more work to implement the tree and keep track of switches in order.

In this case,

image

LINESTRING (2.0 0.0, 1.847759065 -0.7653668647, 1.4142135624 -1.4142135624, 0.7653668647 -1.847759065, 0.0 -2.0, -0.7653668647 -1.847759065, -1.4142135624 -1.4142135624, -1.847759065 -0.7653668647, -2.0 0.0, -1.847759065 0.7653668647, -1.4142135624 1.4142135624, -0.7653668647 1.847759065, 0.0 2.0, 0.7653668647 1.847759065, 1.4142135624 1.4142135624, 1.847759065 0.7653668647, 2.0 0.0)

There are only ever 2 observed_segments at once so it will run closer to O(nlogn).

Copy link
Member Author

Choose a reason for hiding this comment

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

There's also another strategy that GEOS uses where they break the segments up into monotone pieces and run the intersection on each piece then combine the results.

I don't think it's really necessary we do either of these. If performance is that big of a concern you should use GEOS instead of the simpler Ruby implementation.

Copy link
Member

Choose a reason for hiding this comment

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

@keithdoggett I definitely agree, thanks for the pointer though!

…rved_segments with Set, condense event creation branching.
Copy link
Member

@BuonOmo BuonOmo left a comment

Choose a reason for hiding this comment

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

One last review, otherwise I think we're ready to ship, thanks!

lib/rgeo/cartesian/sweepline_intersector.rb Outdated Show resolved Hide resolved
@keithdoggett keithdoggett assigned BuonOmo and unassigned BuonOmo Jul 6, 2021
@keithdoggett keithdoggett merged commit 02c80c0 into validity-handling Jul 6, 2021
@keithdoggett keithdoggett deleted the sweepline-intersector branch July 6, 2021 12:45
@BuonOmo BuonOmo mentioned this pull request Jul 6, 2021
12 tasks
@keithdoggett keithdoggett mentioned this pull request Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants