Upgrade sweepline-intersections#3070
Draft
mfedderly wants to merge 3 commits into
Draft
Conversation
…rtex Upgrading sweepline-intersections in line-intersect started to cause failures in boolean-crosses. It looks like rowanwins/sweepline-intersections@e7a48be added endpoint intersection detection. When trying to grapple with this back in boolean-crosses, I noticed that the booleanEqual first parameter was a geometry but the second was a feature which was causing it to return false even when the [1, 1] point was used in the coordinate arrays of both. After fixing the doLineStringsCross method, it fixed the test that had been marked as false, but now the true test is failing. From spot-checking against PostGIS I think we're actually supposed to return false if two linestrings only cross at an end point. postgres=# SELECT ST_Crosses( postgres(# 'LINESTRING(-2 1, 4 1)'::geometry, postgres(# 'LINESTRING(1 1, 1 2, 1 3, 1 4)'::geometry postgres(# ); st_crosses ------------ f (1 row) postgres=# SELECT ST_Crosses( 'LINESTRING(-2 2, 1 1)'::geometry, 'LINESTRING(1 1, 1 2, 1 3, 1 4)'::geometry ); st_crosses ------------ f (1 row)
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrades
sweepline-intersectionsto pick up fixes forarethetypeswrong. There is also a change rowanwins/sweepline-intersections@e7a48be where it adds detection for endpoint intersections which previously did not exist.Breaking:
This changes the behavior of @turf/boolean-crosses from returning true to false when the line/line intersection is on an end point. I think this matches the DE9IM intended behavior now. This might be worth just flagging as a breaking behavior change to be safe, so I'll put it in v8.
Todo:
Clean up the transitive dependencies Fix package.json dependencies/devDependencies rowanwins/sweepline-intersections#25(I wound up just vendoring the code directly since the PR hasn't gotten a response yet)Investigate why we had to back this change out last time and address that(I thought there was an issue with these packages, but I incorrectly remembered what packages Reverted backward incompatible fix to nearestPointOnLine #3017 involved. Still it may be reasonable to wait for v8 if we're going to change the results for crosses on endpoints.)Note: tinyqueue@3 (released nearly 2 years ago) breaks node@18 compatibility. So that's another point towards only including this in turf@8.