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 support for partial overlaps and multiple overlaps to @turf/line-overlap #2349

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Rinze-Smits
Copy link

@Rinze-Smits Rinze-Smits commented Oct 12, 2022

There are a number of open issues for @turf/line-overlap: #1890, #1670 , #901 . These are all fixed. It also fixes issues with polygons with holes for which no issue was filed. These are all resolved with this patch.

The module now also finds overlaps where only part of a line segment overlaps only a part of another line segment.
Multiple overlaps had been partially covered in a previous patch, but not for all cases, this is now fixed.
A small refactor was done to reduce duplicated code.
The module puts a bit more effort into making linestrings from found matches.

The test result for #901 was changed due to different handling of duplicates (the testcase has a larger tolerance than the length of some matching line segments, which causes them to be matched multiple times).

Resolves #1890
Resolves #1670
Resolves #901
Resolves #2580

* partial overlaps are those where only part of a line segment overlaps
  only a part of another line segment.
* multiple overlaps on a line segment from the second feature are now
  allowed
@rowanwins
Copy link
Member

I've had a bit of a look - the output from issue-#901.geojson looks a little strange as it's including duplicate coordinates, like it's zigzagging backwards and forwards. So I think we still need some extra work on this @Rinze-Smits , but thank you for looking into things!

Comment on lines +40 to 42
[-113.3384152645109, 53.52244409344282],
[-113.33832502043951, 53.52244398828247],
[-113.3384152645109, 53.52244409344282],
Copy link
Member

Choose a reason for hiding this comment

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

We're getting some duplicate coords here, eg 0 & 2 are the same, 1 & 3 are the same...

@Rinze-Smits
Copy link
Author

Thank you for reviewing! I agree that this is still an issue, but in the current master branch the output of this testcase also contains duplicate coords and in addition to that the current master duplicates those linestrings. So I do not consider this to be a regression but a slight improvement. As I built this patch mainly to fix bugs where output was missing entirely, I do not think completely solving the existing duplicate coordinate issues in the output belongs here, but should rather go in a new patch. I'm willing to create that as well in the near future.

@mfedderly
Copy link
Collaborator

@Rinze-Smits I'd like to merge this if we can. Progress over perfection. Can you reserve the merge conflicts please?
Thanks!

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