replace polygon-clipping with polyclip-ts #2729
Conversation
|
Really appreciate you putting this together @cmurphy23. Take there is no workaround for the esModuleInterop issue that might let us get this merged (and those issues resolved) sooner? Also understand your note about the tests. Anything that looks like more than a rounding change, we can go through together, perhaps with one of the other maintainers. |
|
No problem, @smallsaucepan, glad to be able to contribute. The PR for polyclip-ts got merged and a new release was made. This PR now references the new release and should be considered for merging. Thanks! |
|
Hi @cmurphy23. Had a go at building your branch and ran into the following error during in turf-intersect. Are you seeing this too? Running polyclip-ts through arethetypeswrong does suggest there's a problem with its packaging:
We've fixed problems like these within Turf, so I might submit a PR to polyclip-ts. Wanted to confirm first though that I'm not the only one seeing this error. |
|
Created an issue luizbarboza/polyclip-ts#18 to follow up on a possible fix at the polyclip-ts end. |
This commit replaces the polygon-clipping library with polyclip-ts, a goal mentioned in several issues (#2409, #2277, #2048), with even more issues related to polyon-clipping problems that don't mention polyclip-ts (#2705, #2420, #1983, and others).
This PR is a placeholder, dependent on this PR for polyclip-ts so that the polyclip-ts library doesn't fail the
esModuleInteropcheck. Once those changes have been merged and released, the polyclip-ts version just needs to be incremented in the different modules' package.json files, then turf should should build and the tests should pass.The affected packages are: intersect, difference, union, dissolve, and mask
It is worth noting that I am not an expert on these algorithms, and in the cases where the test results differed, I simply updated the tests with the new results. The vast majority of the differences were just floating point rounding differences, but there were different results for two turf-difference tests. For the test "issue-721", two polygons are returned for the difference instead of one, where the first polygon is basically identical to the original answer, but a new very small polygon on the border of the polygons has also been found and returned. For "issue-721-inverse", instead of no difference being found, three very small polygons have been found on the borders. I don't actually know what the correct answers are since the test case is truly an edge case and higher precision might produce a different result. However, based on the issues I've mentioned, polyclip-ts seems like the more reliable repo, turfjs is regularly having issues with these operations using polygon-clipping, and it would be nice if it didn't.
Possibly:
Resolves #2409
Resolves #2277
Resolves #2048
Resolves #2705
Resolves #2420
Resolves #1983
... and others (TBC)
I would argue it's closest to a bug fix.
I followed the steps, just need polyclip-ts to update and release, then this code will work.