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

Match @turf/line-overlap based on proximity distance #902

Merged
merged 5 commits into from
Aug 17, 2017

Conversation

DenisCarriere
Copy link
Member

@DenisCarriere DenisCarriere commented Aug 15, 2017

Match @turf/line-overlap based on proximity distance

Primarily a fix for issue #901.

@turf/line-overlap can now match lines within a proximity distance (in kilometers).

JSDocs

/**
 * Takes any LineString or Polygon and returns the overlapping lines between both features.
 *
 * @name lineOverlap
 * @param {Geometry|Feature<LineString|MultiLineString|Polygon|MultiPolygon>} line1 any LineString or Polygon
 * @param {Geometry|Feature<LineString|MultiLineString|Polygon|MultiPolygon>} line2 any LineString or Polygon
 * @param {number} [proximity=0] Proximity distance to match overlapping line segments (in kilometers)
 * @returns {FeatureCollection<LineString>} lines(s) that are overlapping between both features
 * @example
 * var line1 = turf.lineString([[115, -35], [125, -30], [135, -30], [145, -35]]);
 * var line2 = turf.lineString([[115, -25], [125, -30], [135, -30], [145, -25]]);
 *
 * var overlapping = turf.lineOverlap(line1, line2);
 *
 * //addToMap
 * var addToMap = [line1, line2, overlapping]
 */
module.exports = function (line1, line2, proximity) {

To-Do's

  • Add test case for Issue lineOverlap on two polygons giving no output #901
  • Support line segments which overlap which don't exactly share nodes.
  • Add segmentEach to iterate over line segments
  • Add unary-union to both source & target temporary fix using point-on-line has been added until unary-union is completed

Simplified example (no sharing nodes)

  • Red: Source Line
  • Blue: Target Line
  • Green: Overlapping lines (Results)

image

@DenisCarriere DenisCarriere changed the title Fix for lineOverlap #901 Match @turf/line-overlap based on proximity distance Aug 15, 2017
@DenisCarriere
Copy link
Member Author

DenisCarriere commented Aug 15, 2017

@rowanwins @tremby Have a look at this PR, this seem to have solved both the simplified & the initial issue.

Since the line segments might not be exactly on the target line, I added a proximity parameter (in kilometers) used to improve/reduce the matches.

@tremby I used proximity=0.05 (50 meters) for yours and it seems to have worked very well.

Green: Results => overlapping lines

image

@DenisCarriere
Copy link
Member Author

DenisCarriere commented Aug 15, 2017

@rowanwins Tried to use @turf/boolean-point-on-line, it worked for the simplified test case, however it wasn't able to match the other since it has to be exactly on the line to return true.

Is there anyway to add a distance parameter in your boolean module? Is that even possible? Nothing too complicated but having this would help a lot for this type of scenario.

👍

@tremby
Copy link

tremby commented Aug 15, 2017

Looks like it does the trick; thank you! Lovely fast progress.

@DenisCarriere
Copy link
Member Author

Lovely fast progress.

You caught me at a "bored" moment, usually it's not this fast.

Strange that the Travis CI keeps failing... 🤔 works fine locally.

@tremby
Copy link

tremby commented Aug 15, 2017

Questions about this fix:

  • With proximity at 0 does it work on the simplified test case?
  • By extension, is the reason it does not work on my sample data something to do with floating point error? As explained in the issue, the red shape (in your last screenshot) is the result of an intersection with the blue, so barring floating point error or a bug in either this method or intersect I don't see why they wouldn't still be on exactly the same path.

@DenisCarriere
Copy link
Member Author

DenisCarriere commented Aug 15, 2017

With proximity at 0 does it work on the simplified test case?

Yes with 0 it does work on the simplified case (added @turf/boolean-point-on-line).

is the reason it does not work on my sample data something to do with floating point error

Pretty sure it's something like that, floating points, and when you dive down real close you can see that the points don't actually overlap but they cross each other, hence why we needed to add the proximity parameter to make that work.

It's a great example, I'm glad you brought it up!

Strange that the Travis CI keeps failing... 🤔 works fine locally.

Ops, this broke something in @turf/boolean-point-on-line (fixed now, the default proximity wasn't set to 0).
@rowanwins Might worth removing dependencies in your boolean modules, that way we don't have any circling dependencies (dependencies that depend on each other).

@rowanwins
Copy link
Member

rowanwins commented Aug 15, 2017

Gday @DenisCarriere

Yep we should be able to add some sort of proximity parameter (I'd perhaps call it tolerance) similar to any of the other distance style params that we offer for other modules.

point-on-line doesn't have any dependencies, although some of the other boolean ones were probably calling line-overlaps I think.

Will see if I can bash it out quickly this evening.

@DenisCarriere
Copy link
Member Author

@rowanwins 👍

tolerance is a better param name.

point-on-line doesn't have any dependencies, although some of the other boolean ones were probably calling line-overlaps I think.

Most of the "core" functionality can be copy-pasted from the internal methods, they aren't very big modules. Close to zero dependencies for the boolean modules would be ideal, obviously there's some that can't which is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants