New module @turf/polygon-slice #2782
Replies: 40 comments
-
Not a fan of bringing yet another computational geometry library as a dependency... |
Beta Was this translation helpful? Give feedback.
-
@mourner Thanks for your input, I totally agree with your comment. Recently I've sent over many commits to the library (https://github.com/MartyWallace/PolyK/commits/master) before including it as a dependency. Mourner you are the expert in Javascript computational geo libraries (big fan). I'm willing to "Turfify" this library to Zero dependency (only Turf dependencies). @mourner Can you review it after it's done (~1-2 weeks). |
Beta Was this translation helpful? Give feedback.
-
@DenisCarriere oh, don't take my comment as the final word — it's just my first thought. Worth discussing. Additional questions that come to mind are:
@morganherlocker @tcql thoughts? |
Beta Was this translation helpful? Give feedback.
-
Same questions as @mourner for me, and one more. I definitely think this could be useful as a core module. What should the behavior be when a line partially slices a polygon, but does not fully cleave it into 2+ parts? I ask because this is a bit ambiguous and one seemingly intuitive behavior (it inserts a new self-touching edge(s)) would lead to invalid geometries. |
Beta Was this translation helpful? Give feedback.
-
Ok I've got a first draft (80% done), I really like the idea of having this as a core module without even the JSTS dependency. I've built the TurfJS dependencies
Issues with TurfJS dependencies
Next steps
Performance
Debugging I've got some "fancy" looking debug geojson results. https://github.com/Turfjs/turf/tree/slice/packages/turf-polygon-slice/test/out |
Beta Was this translation helpful? Give feedback.
-
Nice one @DenisCarriere - looks like you're making good progress! |
Beta Was this translation helpful? Give feedback.
-
:) thanks, it's definitely shaping up, I might need to take a step back and fix/improve/test some of the turfjs modules I'm using as dependencies. |
Beta Was this translation helpful? Give feedback.
-
@rowanwins Ok this module is long overdue... I'm going to work on this today/week. A few modules I've needed before to make this possible:
|
Beta Was this translation helpful? Give feedback.
-
@DenisCarriere I just got done doing the "new project" setup stuff, and am digging into polygonSlice(). A thing that confuses me with the current implementation is that it returns a FeatureCollection of Linestrings. I thought it was supposed to return sliced polygons. Is that just "work in progress" status of the polygonSlice() function? Or maybe I'm confused about how the function is intended to work. |
Beta Was this translation helpful? Give feedback.
-
Polygon Slice should return a FeatureCollection of Polygons, it might of still been in Debugging mode. In reality Polygons are simply just a collection of connected lines, only difference is the Inner & Outer rings which can be determined by the Winding. The Ref: |
Beta Was this translation helpful? Give feedback.
-
Okay, thanks, that makes sense. I just wanted to make sure I wasn't missing something in what the function was supposed to do. |
Beta Was this translation helpful? Give feedback.
-
Just as an update note, |
Beta Was this translation helpful? Give feedback.
-
Hey @DenisCarriere, I'm looking at this today, as I did have it working, but the So, I wonder if you have any thoughts about a way to go about re-uniting the lines that are output currently into polygon features? Another thing, I think it should work for multi-polygon features. Not sure the best way to do that, but though having another path with some recursion would sort it out? |
Beta Was this translation helpful? Give feedback.
-
Hey @DenisCarriere One more note. I realised that the issue causing the polygonize to fail is in line splitting. In the Evidence 1 - this is the difference between the split points: Evidence 2 - this is the output of polygonize with the bad topology: Evidence 3 - this is the output of polygonize after doing a In running through the tests, I think my fix is a bad workaround to what is perhaps an issue in the line-splitter. I might put together a test case and a bug report. |
Beta Was this translation helpful? Give feedback.
-
@alexgleith Great job!
🤔 That's annoying and good at the same time...
👍 Agreed, however getting Polygon to work first would be the first step.
🤔 Definitely submit a test case for the line splitter, we can look into fixing that first. |
Beta Was this translation helpful? Give feedback.
-
PS also dropping a note here about this blog post and associated repo PPS which also reminds me that it would be cool to have a |
Beta Was this translation helpful? Give feedback.
-
Hey @rowanwins My changes aren't significant, but here they are: https://github.com/alexgleith/turf/tree/slice This commit captures it: alexgleith@1a47118 I really do think that this issue is part of the cause... I don't know how to fix it! Maybe implementing a more complex algorithm (as you've linked to) is the way to go. |
Beta Was this translation helpful? Give feedback.
-
Thanks for those links @alexgleith and your work on the module, will try and take a look this evening and see if I can find a clever way around it. |
Beta Was this translation helpful? Give feedback.
-
@alexgleith I downloaded from your repo but I don't know how to build it. Sorry, I'm a newb to this stuff. |
Beta Was this translation helpful? Give feedback.
-
You might be able to download @alexgleith repo - then
This should generate a |
Beta Was this translation helpful? Give feedback.
-
FYI I did look into this a little while back and the impression I got was that there would probably be benefit in moving to a more known algorithm like the one suggested above |
Beta Was this translation helpful? Give feedback.
-
Thank you for your speedy response. Are you referring to the algorithm by @skaletech? |
Beta Was this translation helpful? Give feedback.
-
Hey @daudihusbands, the code I supplied does not work in all cases, so beware! It does work for most of my cases, though, and I'm using it in production. |
Beta Was this translation helpful? Give feedback.
-
Thanks for the heads up @alexgleith. I am testing it out now. I'll post feedback if I run into any issues. Really appreciate your help. BTW, I didn't get rollup to work. I did notice that the config file is referring to index.js but I didn't find that file. No worries though, I copied the your code into my file. |
Beta Was this translation helpful? Give feedback.
-
When is this module updated? Is there a better solution? This method is urgently needed now。 |
Beta Was this translation helpful? Give feedback.
-
Any timeline in when will this feature gets released ? |
Beta Was this translation helpful? Give feedback.
-
I am not aware of anyone working on this algorithm at the moment. The proposed solutions so far do not appear robust enough to make it to production. I would suggest using a lower level module or writing one yourself, and if that module works well, we can add it as a dependency in turf with a normalized interface. |
Beta Was this translation helpful? Give feedback.
-
Correct me if I'm wrong, but it seems like this feature was never added to Turf. We're simple in our Turf use so we updated the apparently abandoned Turf/turf-cut to work with the latest version of Turf and converted the module into a plain function. It's less elegant than @skaletech's solution above which you should review as well. If it helps anyone who ends up here like we did, here's a link: Since it's a hack I assume we should stop there. Please let me know if otherwise and we can try jumping the hoops to add it as a Turf module. |
Beta Was this translation helpful? Give feedback.
-
I too would like to see a supported way in turf to split a polygon instead of relying on another library. Any chance this could happen? |
Beta Was this translation helpful? Give feedback.
-
Still nothing i suppose? Looks like this feature is more complex than I thought. |
Beta Was this translation helpful? Give feedback.
-
New module
@turf/slice
@turf/polygon-slice
, currently onslice
branchContributions and comments are welcomed before we publish to TurfJS.
Example
Current fallbacks of
PolyK
JSDocs
Input
Output
Benchmark
CC: @Turfjs/ownership
Beta Was this translation helpful? Give feedback.
All reactions